Address review feedback.

src/EditorFeatures/Core/Implementation/Diagnostics/AbstractDiagnosticsTaggerProvider.cs

@@ -110,7 +110,7 @@ protected override ITaggerEventSource CreateEventSource(ITextView textViewOpt, I
 
         /// <summary>
         /// Get the <see cref="DiagnosticDataLocation"/> that should have the tag applied to it.
-        /// In most cases, this is the <see cref="DiagnosticData.DataLocation"/> but overrides (e.g. unnecessary classifications).
+        /// In most cases, this is the <see cref="DiagnosticData.DataLocation"/> but overrides can change it (e.g. unnecessary classifications).
         /// </summary>
         /// <param name="diagnosticData">the diagnostic containing the location(s).</param>
         /// <returns>an array of locations that should have the tag applied.</returns>

src/EditorFeatures/Core/Implementation/Diagnostics/DiagnosticsClassificationTaggerProvider.cs

@@ -11,16 +11,15 @@
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editor.Shared.Options;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
+using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Options;
-using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Options;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
 using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Text.Editor;
 using Microsoft.VisualStudio.Text.Tagging;
 using Microsoft.VisualStudio.Utilities;
-using Newtonsoft.Json;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics
@@ -67,11 +66,11 @@ internal partial class DiagnosticsClassificationTaggerProvider : AbstractDiagnos
 
         protected internal override ImmutableArray<DiagnosticDataLocation> GetLocationsToTag(DiagnosticData diagnosticData)
         {
-            using var locationsToTagDisposer = ArrayBuilder<DiagnosticDataLocation>.GetInstance(out var locationsToTag);
+            using var locationsToTagDisposer = PooledObjects.ArrayBuilder<DiagnosticDataLocation>.GetInstance(out var locationsToTag);
 
             // If there are 'unnecessary' locations specified in the property bag, use those instead of the main diagnostic location.
-            if (diagnosticData.AdditionalLocations != null
-                && diagnosticData.AdditionalLocations.Count > 0
+            if (diagnosticData.AdditionalLocations?.Count > 0
+                && diagnosticData.Properties != null
                 && diagnosticData.Properties.TryGetValue(WellKnownDiagnosticTags.Unnecessary, out var unnecessaryIndices))
             {
                 var additionalLocations = diagnosticData.AdditionalLocations.ToImmutableArray();
@@ -87,12 +86,17 @@ protected internal override ImmutableArray<DiagnosticDataLocation> GetLocationsT
 
             static IEnumerable<int> GetLocationIndices(string indicesProperty)
             {
-                var stream = new MemoryStream(Encoding.UTF8.GetBytes(indicesProperty));
-                var serializer = new DataContractJsonSerializer(typeof(IEnumerable<int>));
-
-                var result = serializer.ReadObject(stream) as IEnumerable<int>;
-                stream.Close();
-                return result;
+                try
+                {
+                    using var stream = new MemoryStream(Encoding.UTF8.GetBytes(indicesProperty));
+                    var serializer = new DataContractJsonSerializer(typeof(IEnumerable<int>));
+                    var result = serializer.ReadObject(stream) as IEnumerable<int>;
+                    return result;
+                }
+                catch (Exception e) when (FatalError.ReportWithoutCrash(e))
+                {
+                    return ImmutableArray<int>.Empty;
+                }
             }
         }
     }

src/Features/Core/Portable/Diagnostics/DiagnosticHelper.cs

@@ -8,6 +8,7 @@
 using System.Runtime.Serialization.Json;
 using System.Text;
 using Microsoft.CodeAnalysis.PooledObjects;
+using Microsoft.CodeAnalysis.RemoveUnnecessaryParentheses;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Diagnostics
@@ -69,7 +70,10 @@ internal static class DiagnosticHelper
         /// Typically, these are locations of other items referenced in the message.
         /// If null, <see cref="Diagnostic.AdditionalLocations"/> will return an empty list.
         /// </param>
-        /// <param name="tagIndices">a map of location tag to index in additional locations.</param>
+        /// <param name="tagIndices">
+        /// a map of location tag to index in additional locations.
+        /// <see cref="AbstractRemoveUnnecessaryParenthesesDiagnosticAnalyzer{TLanguageKindEnum, TParenthesizedExpressionSyntax}"/> for an example of usage.
+        /// </param>
         /// <param name="messageArgs">Arguments to the message of the diagnostic.</param>
         /// <returns>The <see cref="Diagnostic"/> instance.</returns>
         public static Diagnostic CreateWithLocationTags(
@@ -80,12 +84,18 @@ internal static class DiagnosticHelper
             IDictionary<string, IEnumerable<int>> tagIndices,
             params object[] messageArgs)
         {
-            var properties = tagIndices.Select(kvp => new KeyValuePair<string, string>(kvp.Key, EncodeIndices(kvp.Value))).ToImmutableDictionary();
+            Contract.ThrowIfTrue(additionalLocations.IsEmpty());
+            Contract.ThrowIfTrue(tagIndices.IsEmpty());
+
+            var properties = tagIndices.Select(kvp => new KeyValuePair<string, string>(kvp.Key, EncodeIndices(kvp.Value, additionalLocations.Count()))).ToImmutableDictionary();
 
             return Create(descriptor, location, effectiveSeverity, additionalLocations, properties, messageArgs);
 
-            static string EncodeIndices(IEnumerable<int> indices)
+            static string EncodeIndices(IEnumerable<int> indices, int additionalLocationsLength)
             {
+                // Ensure that the provided tag index is a valid index into additional locations.
+                Contract.ThrowIfFalse(indices.All(idx => idx >= 0 && idx < additionalLocationsLength));
+
                 using var stream = new MemoryStream();
                 var serializer = new DataContractJsonSerializer(typeof(IEnumerable<int>));
                 serializer.WriteObject(stream, indices);

src/Features/Core/Portable/RemoveUnnecessaryParentheses/AbstractRemoveUnnecessaryParenthesesDiagnosticAnalyzer.cs

@@ -5,7 +5,6 @@
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Threading;
-using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.LanguageServices;

src/Workspaces/Core/Portable/Diagnostics/DiagnosticData.cs

@@ -34,7 +34,7 @@ internal sealed class DiagnosticData : IEquatable<DiagnosticData>
 
         public readonly ProjectId? ProjectId;
         public readonly DiagnosticDataLocation? DataLocation;
-        public readonly IReadOnlyCollection<DiagnosticDataLocation>? AdditionalLocations;
+        public readonly IReadOnlyCollection<DiagnosticDataLocation> AdditionalLocations;
 
         /// <summary>
         /// Language name (<see cref="LanguageNames"/>) or null if the diagnostic is not associated with source code.