Some null security in annotation ctors

MDoerner · MDoerner · commit a300d1521e65 · 2018-12-15T15:01:16.000+01:00
Also some minor tweaks based on review remarks for PR #4641
diff --git a/Rubberduck.Parsing/Annotations/AttributeAnnotationBase.cs b/Rubberduck.Parsing/Annotations/AttributeAnnotationBase.cs
@@ -9,7 +9,7 @@ public abstract class AttributeAnnotationBase : AnnotationBase, IAttributeAnnota
         protected AttributeAnnotationBase(AnnotationType annotationType, QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IReadOnlyList<string> attributeValues) 
         :base(annotationType, qualifiedSelection, context)
         {
-            AttributeValues = attributeValues;
+            AttributeValues = attributeValues ?? new List<string>();
         }
 
         public abstract string Attribute { get; }
diff --git a/Rubberduck.Parsing/Annotations/DefaultMemberAnnotation.cs b/Rubberduck.Parsing/Annotations/DefaultMemberAnnotation.cs
@@ -13,7 +13,7 @@ public sealed class DefaultMemberAnnotation : AttributeAnnotationBase
         public DefaultMemberAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)
             : base(AnnotationType.DefaultMember, qualifiedSelection, context, new List<string> { "0" })
         {
-            Description = parameters.FirstOrDefault();
+            Description = parameters?.FirstOrDefault() ?? string.Empty;
         }
 
         public string Description { get; }
diff --git a/Rubberduck.Parsing/Annotations/DescriptionAnnotation.cs b/Rubberduck.Parsing/Annotations/DescriptionAnnotation.cs
@@ -11,7 +11,7 @@ namespace Rubberduck.Parsing.Annotations
     public sealed class DescriptionAnnotation : AttributeAnnotationBase
     {
         public DescriptionAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)
-            : base(AnnotationType.Description, qualifiedSelection, context, parameters.Take(1).ToList())
+            : base(AnnotationType.Description, qualifiedSelection, context, parameters?.Take(1).ToList())
         {
             Description = AttributeValues?.FirstOrDefault();
             if ((Description?.StartsWith("\"") ?? false) && Description.EndsWith("\""))
diff --git a/Rubberduck.Parsing/Annotations/ExposedModuleAnnotation.cs b/Rubberduck.Parsing/Annotations/ExposedModuleAnnotation.cs
@@ -10,7 +10,7 @@ namespace Rubberduck.Parsing.Annotations
     public sealed class ExposedModuleAnnotation : AttributeAnnotationBase
     {
         public ExposedModuleAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)
-            : base(AnnotationType.Exposed, qualifiedSelection, context, new List<string> { "True" })
+            : base(AnnotationType.Exposed, qualifiedSelection, context, new List<string> { Tokens.True })
         {
             
         }
diff --git a/Rubberduck.Parsing/Annotations/MemberAttributeAnnotation.cs b/Rubberduck.Parsing/Annotations/MemberAttributeAnnotation.cs
@@ -8,9 +8,9 @@ namespace Rubberduck.Parsing.Annotations
     public class MemberAttributeAnnotation : AttributeAnnotationBase
     {
         public MemberAttributeAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IReadOnlyList<string> parameters)
-        :base(AnnotationType.MemberAttribute, qualifiedSelection, context, parameters.Skip(1).ToList())
+        :base(AnnotationType.MemberAttribute, qualifiedSelection, context, parameters?.Skip(1).ToList())
         {
-            Attribute = parameters.FirstOrDefault();
+            Attribute = parameters?.FirstOrDefault() ?? string.Empty;
         }
 
         public override string Attribute { get; }
diff --git a/Rubberduck.Parsing/Annotations/ModuleAttributeAnnotation.cs b/Rubberduck.Parsing/Annotations/ModuleAttributeAnnotation.cs
@@ -8,9 +8,9 @@ namespace Rubberduck.Parsing.Annotations
     public class ModuleAttributeAnnotation : AttributeAnnotationBase
     {
         public ModuleAttributeAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IReadOnlyList<string> paramaters) 
-        :base(AnnotationType.ModuleAttribute, qualifiedSelection, context, paramaters.Skip(1).ToList())
+        :base(AnnotationType.ModuleAttribute, qualifiedSelection, context, paramaters?.Skip(1).ToList())
         {
-            Attribute = paramaters.FirstOrDefault();
+            Attribute = paramaters?.FirstOrDefault() ?? string.Empty;
         }
 
         public override string Attribute { get; }
diff --git a/Rubberduck.Parsing/Annotations/ModuleDescriptionAnnotation.cs b/Rubberduck.Parsing/Annotations/ModuleDescriptionAnnotation.cs
@@ -11,7 +11,7 @@ namespace Rubberduck.Parsing.Annotations
     public sealed class ModuleDescriptionAnnotation : AttributeAnnotationBase
     {
         public ModuleDescriptionAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> attributeValues)
-            : base(AnnotationType.ModuleDescription, qualifiedSelection, context, attributeValues.Take(1).ToList())
+            : base(AnnotationType.ModuleDescription, qualifiedSelection, context, attributeValues?.Take(1).ToList())
         {
             Description = AttributeValues?.FirstOrDefault();
             if ((Description?.StartsWith("\"") ?? false) && Description.EndsWith("\""))
diff --git a/Rubberduck.Parsing/Annotations/PredeclaredIdAnnotation.cs b/Rubberduck.Parsing/Annotations/PredeclaredIdAnnotation.cs
@@ -10,7 +10,7 @@ namespace Rubberduck.Parsing.Annotations
     public sealed class PredeclaredIdAnnotation : AttributeAnnotationBase
     {
         public PredeclaredIdAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)
-            : base(AnnotationType.PredeclaredId, qualifiedSelection, context, new List<string>{"True"})
+            : base(AnnotationType.PredeclaredId, qualifiedSelection, context, new List<string>{Tokens.True})
         {}
 
         public override string Attribute => "VB_PredeclaredId";
diff --git a/Rubberduck.Parsing/VBA/AttributesUpdater.cs b/Rubberduck.Parsing/VBA/AttributesUpdater.cs
@@ -63,12 +63,12 @@ public void AddAttribute(IRewriteSession rewriteSession, Declaration declaration
                 if (lastModuleAttribute == null)
                 {
                     //This should never happen for a real module.
-                    var codeToInsert = $"Attribute {attribute} ={AttributeValuesText(values)}{Environment.NewLine}";
+                    var codeToInsert = $"Attribute {attribute} = {AttributeValuesText(values)}{Environment.NewLine}";
                     rewriter.InsertBefore(0, codeToInsert);
                 }
                 else
                 {
-                    var codeToInsert = $"{Environment.NewLine}Attribute {attribute} ={AttributeValuesText(values)}";
+                    var codeToInsert = $"{Environment.NewLine}Attribute {attribute} = {AttributeValuesText(values)}";
                     rewriter.InsertAfter(lastModuleAttribute.stop.TokenIndex, codeToInsert);
                 }  
             }
@@ -78,27 +78,20 @@ public void AddAttribute(IRewriteSession rewriteSession, Declaration declaration
                 var firstEndOfLineInMember = attributesContext.GetDescendent<VBAParser.EndOfLineContext>();
                 if (firstEndOfLineInMember == null)
                 {
-                    var codeToInsert = $"{Environment.NewLine}Attribute {attribute} ={AttributeValuesText(values)}";
+                    var codeToInsert = $"{Environment.NewLine}Attribute {attribute} = {AttributeValuesText(values)}";
                     rewriter.InsertAfter(declaration.AttributesPassContext.Stop.TokenIndex, codeToInsert);
                 }
                 else
                 {
-                    var codeToInsert = $"Attribute {attribute} ={AttributeValuesText(values)}{Environment.NewLine}";
+                    var codeToInsert = $"Attribute {attribute} = {AttributeValuesText(values)}{Environment.NewLine}";
                     rewriter.InsertAfter(firstEndOfLineInMember.Stop.TokenIndex, codeToInsert);
                 }
             }
         }
 
         private static string AttributeValuesText(IEnumerable<string> attributeValues)
         {
-            var builder = new StringBuilder();
-            foreach (var attributeValue in attributeValues)
-            {
-                builder.Append($" {attributeValue},");
-            }
-            //Remove trailing comma.
-            builder.Length--;
-            return builder.ToString();
+            return string.Join(", ", attributeValues);
         }
 
         public void RemoveAttribute(IRewriteSession rewriteSession, Declaration declaration, string attribute, IReadOnlyList<string> values = null)
@@ -168,7 +161,7 @@ private static void UpdateAttributeValues(IModuleRewriter rewriter, AttributeNod
             var statementContext = nodeToUpdate.Context;
             var firstIndexToReplace = statementContext.EQ().Symbol.TokenIndex + 1;
             var lastIndexToReplace = statementContext.stop.TokenIndex;
-            var replacementText = AttributeValuesText(values);
+            var replacementText = $" {AttributeValuesText(values)}";
 
             rewriter.Replace(new Interval(firstIndexToReplace, lastIndexToReplace), replacementText);
         }

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ public abstract class AttributeAnnotationBase : AnnotationBase, IAttributeAnnota`
`9`	`9`	`protected AttributeAnnotationBase(AnnotationType annotationType, QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IReadOnlyList<string> attributeValues)`
`10`	`10`	`:base(annotationType, qualifiedSelection, context)`
`11`	`11`	`{`
`12`		`- AttributeValues = attributeValues;`
	`12`	`+ AttributeValues = attributeValues ?? new List<string>();`
`13`	`13`	`}`
`14`	`14`
`15`	`15`	`public abstract string Attribute { get; }`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ public sealed class DefaultMemberAnnotation : AttributeAnnotationBase`
`13`	`13`	`public DefaultMemberAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)`
`14`	`14`	`: base(AnnotationType.DefaultMember, qualifiedSelection, context, new List<string> { "0" })`
`15`	`15`	`{`
`16`		`- Description = parameters.FirstOrDefault();`
	`16`	`+ Description = parameters?.FirstOrDefault() ?? string.Empty;`
`17`	`17`	`}`
`18`	`18`
`19`	`19`	`public string Description { get; }`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ namespace Rubberduck.Parsing.Annotations`
`11`	`11`	`public sealed class DescriptionAnnotation : AttributeAnnotationBase`
`12`	`12`	`{`
`13`	`13`	`public DescriptionAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)`
`14`		`- : base(AnnotationType.Description, qualifiedSelection, context, parameters.Take(1).ToList())`
	`14`	`+ : base(AnnotationType.Description, qualifiedSelection, context, parameters?.Take(1).ToList())`
`15`	`15`	`{`
`16`	`16`	`Description = AttributeValues?.FirstOrDefault();`
`17`	`17`	`if ((Description?.StartsWith("\"") ?? false) && Description.EndsWith("\""))`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ namespace Rubberduck.Parsing.Annotations`
`10`	`10`	`public sealed class ExposedModuleAnnotation : AttributeAnnotationBase`
`11`	`11`	`{`
`12`	`12`	`public ExposedModuleAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IEnumerable<string> parameters)`
`13`		`- : base(AnnotationType.Exposed, qualifiedSelection, context, new List<string> { "True" })`
	`13`	`+ : base(AnnotationType.Exposed, qualifiedSelection, context, new List<string> { Tokens.True })`
`14`	`14`	`{`
`15`	`15`
`16`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,9 +8,9 @@ namespace Rubberduck.Parsing.Annotations`
`8`	`8`	`public class MemberAttributeAnnotation : AttributeAnnotationBase`
`9`	`9`	`{`
`10`	`10`	`public MemberAttributeAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IReadOnlyList<string> parameters)`
`11`		`- :base(AnnotationType.MemberAttribute, qualifiedSelection, context, parameters.Skip(1).ToList())`
	`11`	`+ :base(AnnotationType.MemberAttribute, qualifiedSelection, context, parameters?.Skip(1).ToList())`
`12`	`12`	`{`
`13`		`- Attribute = parameters.FirstOrDefault();`
	`13`	`+ Attribute = parameters?.FirstOrDefault() ?? string.Empty;`
`14`	`14`	`}`
`15`	`15`
`16`	`16`	`public override string Attribute { get; }`
Original file line number	Diff line number	Diff line change
`@@ -8,9 +8,9 @@ namespace Rubberduck.Parsing.Annotations`
`8`	`8`	`public class ModuleAttributeAnnotation : AttributeAnnotationBase`
`9`	`9`	`{`
`10`	`10`	`public ModuleAttributeAnnotation(QualifiedSelection qualifiedSelection, VBAParser.AnnotationContext context, IReadOnlyList<string> paramaters)`
`11`		`- :base(AnnotationType.ModuleAttribute, qualifiedSelection, context, paramaters.Skip(1).ToList())`
	`11`	`+ :base(AnnotationType.ModuleAttribute, qualifiedSelection, context, paramaters?.Skip(1).ToList())`
`12`	`12`	`{`
`13`		`- Attribute = paramaters.FirstOrDefault();`
	`13`	`+ Attribute = paramaters?.FirstOrDefault() ?? string.Empty;`
`14`	`14`	`}`
`15`	`15`
`16`	`16`	`public override string Attribute { get; }`
Original file line number	Diff line number	Diff line change
`@@ -63,12 +63,12 @@ public void AddAttribute(IRewriteSession rewriteSession, Declaration declaration`
`63`	`63`	`if (lastModuleAttribute == null)`
`64`	`64`	`{`
`65`	`65`	`//This should never happen for a real module.`
`66`		`- var codeToInsert = $"Attribute {attribute} ={AttributeValuesText(values)}{Environment.NewLine}";`
	`66`	`+ var codeToInsert = $"Attribute {attribute} = {AttributeValuesText(values)}{Environment.NewLine}";`
`67`	`67`	`rewriter.InsertBefore(0, codeToInsert);`
`68`	`68`	`}`
`69`	`69`	`else`
`70`	`70`	`{`
`71`		`- var codeToInsert = $"{Environment.NewLine}Attribute {attribute} ={AttributeValuesText(values)}";`
	`71`	`+ var codeToInsert = $"{Environment.NewLine}Attribute {attribute} = {AttributeValuesText(values)}";`
`72`	`72`	`rewriter.InsertAfter(lastModuleAttribute.stop.TokenIndex, codeToInsert);`
`73`	`73`	`}`
`74`	`74`	`}`
`@@ -78,27 +78,20 @@ public void AddAttribute(IRewriteSession rewriteSession, Declaration declaration`
`78`	`78`	`var firstEndOfLineInMember = attributesContext.GetDescendent<VBAParser.EndOfLineContext>();`
`79`	`79`	`if (firstEndOfLineInMember == null)`
`80`	`80`	`{`
`81`		`- var codeToInsert = $"{Environment.NewLine}Attribute {attribute} ={AttributeValuesText(values)}";`
	`81`	`+ var codeToInsert = $"{Environment.NewLine}Attribute {attribute} = {AttributeValuesText(values)}";`
`82`	`82`	`rewriter.InsertAfter(declaration.AttributesPassContext.Stop.TokenIndex, codeToInsert);`
`83`	`83`	`}`
`84`	`84`	`else`
`85`	`85`	`{`
`86`		`- var codeToInsert = $"Attribute {attribute} ={AttributeValuesText(values)}{Environment.NewLine}";`
	`86`	`+ var codeToInsert = $"Attribute {attribute} = {AttributeValuesText(values)}{Environment.NewLine}";`
`87`	`87`	`rewriter.InsertAfter(firstEndOfLineInMember.Stop.TokenIndex, codeToInsert);`
`88`	`88`	`}`
`89`	`89`	`}`
`90`	`90`	`}`
`91`	`91`
`92`	`92`	`private static string AttributeValuesText(IEnumerable<string> attributeValues)`
`93`	`93`	`{`
`94`		`- var builder = new StringBuilder();`
`95`		`- foreach (var attributeValue in attributeValues)`
`96`		`- {`
`97`		`- builder.Append($" {attributeValue},");`
`98`		`- }`
`99`		`- //Remove trailing comma.`
`100`		`- builder.Length--;`
`101`		`- return builder.ToString();`
	`94`	`+ return string.Join(", ", attributeValues);`
`102`	`95`	`}`
`103`	`96`
`104`	`97`	`public void RemoveAttribute(IRewriteSession rewriteSession, Declaration declaration, string attribute, IReadOnlyList<string> values = null)`
`@@ -168,7 +161,7 @@ private static void UpdateAttributeValues(IModuleRewriter rewriter, AttributeNod`
`168`	`161`	`var statementContext = nodeToUpdate.Context;`
`169`	`162`	`var firstIndexToReplace = statementContext.EQ().Symbol.TokenIndex + 1;`
`170`	`163`	`var lastIndexToReplace = statementContext.stop.TokenIndex;`
`171`		`- var replacementText = AttributeValuesText(values);`
	`164`	`+ var replacementText = $" {AttributeValuesText(values)}";`
`172`	`165`
`173`	`166`	`rewriter.Replace(new Interval(firstIndexToReplace, lastIndexToReplace), replacementText);`
`174`	`167`	`}`