Address PR feedback from John

src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationAnalysisValueProvider.cs

@@ -32,6 +32,7 @@ internal bool TryGetValue(TKey key, out TValue value)
             }
 
             // Ask the core analysis value provider for the value.
+            // We do it outside the lock statement as this may call into user code which can be a long running operation.
             if (!_analysisValueProvider.TryGetValue(key, out value))
             {
                 value = default(TValue);

src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalysisContext.cs

@@ -199,17 +199,18 @@ public virtual void ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags an
         }
 
         /// <summary>
-        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="key"/>.
+        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="text"/>.
+        /// Note that the pair {<paramref name="valueProvider"/>, <paramref name="text"/>} acts as the key.
         /// Reusing the same <paramref name="valueProvider"/> instance across analyzer actions and/or analyzer instances can improve the overall analyzer performance by avoiding recomputation of the values.
         /// </summary>
         /// <typeparam name="TValue">The type of the value associated with the key.</typeparam>
-        /// <param name="key"><see cref="SourceText"/> for which the value is queried.</param>
-        /// <param name="valueProvider">Provider that computes the underlying value associated with the key.</param>
+        /// <param name="text"><see cref="SourceText"/> for which the value is queried.</param>
+        /// <param name="valueProvider">Provider that computes the underlying value.</param>
         /// <param name="value">Value associated with the key.</param>
         /// <returns>Returns true on success, false otherwise.</returns>
-        public bool TryGetValue<TValue>(SourceText key, SourceTextValueProvider<TValue> valueProvider, out TValue value)
+        public bool TryGetValue<TValue>(SourceText text, SourceTextValueProvider<TValue> valueProvider, out TValue value)
         {
-            return TryGetValue(key, valueProvider.CoreValueProvider, out value);
+            return TryGetValue(text, valueProvider.CoreValueProvider, out value);
         }
 
         private bool TryGetValue<TKey, TValue>(TKey key, AnalysisValueProvider<TKey, TValue> valueProvider, out TValue value)
@@ -432,31 +433,33 @@ public virtual void RegisterOperationAction(Action<OperationAnalysisContext> act
         }
 
         /// <summary>
-        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="key"/>.
+        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="text"/>.
+        /// Note that the pair {<paramref name="valueProvider"/>, <paramref name="text"/>} acts as the key.
         /// Reusing the same <paramref name="valueProvider"/> instance across analyzer actions and/or analyzer instances can improve the overall analyzer performance by avoiding recomputation of the values.
         /// </summary>
         /// <typeparam name="TValue">The type of the value associated with the key.</typeparam>
-        /// <param name="key"><see cref="SourceText"/> for which the value is queried.</param>
-        /// <param name="valueProvider">Provider that computes the underlying value associated with the key.</param>
+        /// <param name="text"><see cref="SourceText"/> for which the value is queried.</param>
+        /// <param name="valueProvider">Provider that computes the underlying value.</param>
         /// <param name="value">Value associated with the key.</param>
         /// <returns>Returns true on success, false otherwise.</returns>
-        public bool TryGetValue<TValue>(SourceText key, SourceTextValueProvider<TValue> valueProvider, out TValue value)
+        public bool TryGetValue<TValue>(SourceText text, SourceTextValueProvider<TValue> valueProvider, out TValue value)
         {
-            return TryGetValue(key, valueProvider.CoreValueProvider, out value);
+            return TryGetValue(text, valueProvider.CoreValueProvider, out value);
         }
 
         /// <summary>
-        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="key"/>.
+        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="tree"/>.
+        /// Note that the pair {<paramref name="valueProvider"/>, <paramref name="tree"/>} acts as the key.
         /// Reusing the same <paramref name="valueProvider"/> instance across analyzer actions and/or analyzer instances can improve the overall analyzer performance by avoiding recomputation of the values.
         /// </summary>
         /// <typeparam name="TValue">The type of the value associated with the key.</typeparam>
-        /// <param name="key"><see cref="SyntaxTree"/> instance for which the value is queried.</param>
-        /// <param name="valueProvider">Provider that computes the underlying value associated with the key.</param>
+        /// <param name="tree"><see cref="SyntaxTree"/> instance for which the value is queried.</param>
+        /// <param name="valueProvider">Provider that computes the underlying value.</param>
         /// <param name="value">Value associated with the key.</param>
         /// <returns>Returns true on success, false otherwise.</returns>
-        public bool TryGetValue<TValue>(SyntaxTree key, SyntaxTreeValueProvider<TValue> valueProvider, out TValue value)
+        public bool TryGetValue<TValue>(SyntaxTree tree, SyntaxTreeValueProvider<TValue> valueProvider, out TValue value)
         {
-            return TryGetValue(key, valueProvider.CoreValueProvider, out value);
+            return TryGetValue(tree, valueProvider.CoreValueProvider, out value);
         }
 
         private bool TryGetValue<TKey, TValue>(TKey key, AnalysisValueProvider<TKey, TValue> valueProvider, out TValue value)
@@ -536,31 +539,33 @@ public void ReportDiagnostic(Diagnostic diagnostic)
         }
 
         /// <summary>
-        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="key"/>.
+        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="text"/>.
+        /// Note that the pair {<paramref name="valueProvider"/>, <paramref name="text"/>} acts as the key.
         /// Reusing the same <paramref name="valueProvider"/> instance across analyzer actions and/or analyzer instances can improve the overall analyzer performance by avoiding recomputation of the values.
         /// </summary>
         /// <typeparam name="TValue">The type of the value associated with the key.</typeparam>
-        /// <param name="key"><see cref="SourceText"/> for which the value is queried.</param>
-        /// <param name="valueProvider">Provider that computes the underlying value associated with the key.</param>
+        /// <param name="text"><see cref="SourceText"/> for which the value is queried.</param>
+        /// <param name="valueProvider">Provider that computes the underlying value.</param>
         /// <param name="value">Value associated with the key.</param>
         /// <returns>Returns true on success, false otherwise.</returns>
-        public bool TryGetValue<TValue>(SourceText key, SourceTextValueProvider<TValue> valueProvider, out TValue value)
+        public bool TryGetValue<TValue>(SourceText text, SourceTextValueProvider<TValue> valueProvider, out TValue value)
         {
-            return TryGetValue(key, valueProvider.CoreValueProvider, out value);
+            return TryGetValue(text, valueProvider.CoreValueProvider, out value);
         }
 
         /// <summary>
-        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="key"/>.
+        /// Attempts to compute or get the cached value provided by the given <paramref name="valueProvider"/> for the given <paramref name="tree"/>.
+        /// Note that the pair {<paramref name="valueProvider"/>, <paramref name="tree"/>} acts as the key.
         /// Reusing the same <paramref name="valueProvider"/> instance across analyzer actions and/or analyzer instances can improve the overall analyzer performance by avoiding recomputation of the values.
         /// </summary>
         /// <typeparam name="TValue">The type of the value associated with the key.</typeparam>
-        /// <param name="key"><see cref="SyntaxTree"/> for which the value is queried.</param>
-        /// <param name="valueProvider">Provider that computes the underlying value associated with the key.</param>
+        /// <param name="tree"><see cref="SyntaxTree"/> for which the value is queried.</param>
+        /// <param name="valueProvider">Provider that computes the underlying value.</param>
         /// <param name="value">Value associated with the key.</param>
         /// <returns>Returns true on success, false otherwise.</returns>
-        public bool TryGetValue<TValue>(SyntaxTree key, SyntaxTreeValueProvider<TValue> valueProvider, out TValue value)
+        public bool TryGetValue<TValue>(SyntaxTree tree, SyntaxTreeValueProvider<TValue> valueProvider, out TValue value)
         {
-            return TryGetValue(key, valueProvider.CoreValueProvider, out value);
+            return TryGetValue(tree, valueProvider.CoreValueProvider, out value);
         }
 
         private bool TryGetValue<TKey, TValue>(TKey key, AnalysisValueProvider<TKey, TValue> valueProvider, out TValue value)

src/Compilers/Core/Portable/DiagnosticAnalyzer/SourceTextValueProvider.cs

@@ -7,7 +7,7 @@
 namespace Microsoft.CodeAnalysis.Diagnostics
 {
     /// <summary>
-    /// Provides custom values associated with <see cref="SourceText"/> instances using the given 'computeValue' delegate.
+    /// Provides custom values associated with <see cref="SourceText"/> instances using the given computeValue delegate.
     /// </summary>
     public sealed class SourceTextValueProvider<TValue>
     {
@@ -16,7 +16,7 @@ public sealed class SourceTextValueProvider<TValue>
         /// <summary>
         /// Provides custom values associated with <see cref="SourceText"/> instances using the given <paramref name="computeValue"/>.
         /// </summary>
-        /// <param name="computeValue">Delegate to compute value associated with a given <see cref="SourceText"/> instance.</param>
+        /// <param name="computeValue">Delegate to compute the value associated with a given <see cref="SourceText"/> instance.</param>
         /// <param name="sourceTextComparer">Optional equality comparer to determine equivalent <see cref="SourceText"/> instances that have the same value.
         /// If no comparer is provided, then <see cref="SourceTextComparer"/> is used by default.</param>
         public SourceTextValueProvider(Func<SourceText, TValue> computeValue, IEqualityComparer<SourceText> sourceTextComparer = null)

src/Compilers/Core/Portable/DiagnosticAnalyzer/SyntaxTreeValueProvider.cs

@@ -6,7 +6,7 @@
 namespace Microsoft.CodeAnalysis.Diagnostics
 {
     /// <summary>
-    /// Provides custom values associated with <see cref="SyntaxTree"/> instances using the given 'computeValue' delegate.
+    /// Provides custom values associated with <see cref="SyntaxTree"/> instances using the given computeValue delegate.
     /// </summary>
     public sealed class SyntaxTreeValueProvider<TValue>
     {
@@ -15,7 +15,7 @@ public sealed class SyntaxTreeValueProvider<TValue>
         /// <summary>
         /// Provides values associated with <see cref="SyntaxTree"/> instances using the given <paramref name="computeValue"/>.
         /// </summary>
-        /// <param name="computeValue">Delegate to compute value associated with a given <see cref="SyntaxTree"/> instance.</param>
+        /// <param name="computeValue">Delegate to compute the value associated with a given <see cref="SyntaxTree"/> instance.</param>
         /// <param name="syntaxTreeComparer">Optional equality comparer to determine equivalent <see cref="SyntaxTree"/> instances that have the same value.
         /// If no comparer is provided, then <see cref="SyntaxTreeComparer"/> is used by default.</param>
         public SyntaxTreeValueProvider(Func<SyntaxTree, TValue> computeValue, IEqualityComparer<SyntaxTree> syntaxTreeComparer = null)

src/Compilers/Core/Portable/PublicAPI.Unshipped.txt

@@ -2,12 +2,12 @@ Microsoft.CodeAnalysis.CompilationOptions.PublicSign.get -> bool
 Microsoft.CodeAnalysis.CompilationOptions.WithPublicSign(bool publicSign) -> Microsoft.CodeAnalysis.CompilationOptions
 Microsoft.CodeAnalysis.CompilationReference.Equals(Microsoft.CodeAnalysis.CompilationReference other) -> bool
 Microsoft.CodeAnalysis.Diagnostics.AnalysisContext.RegisterOperationAction(System.Action<Microsoft.CodeAnalysis.Diagnostics.OperationAnalysisContext> action, params Microsoft.CodeAnalysis.Semantics.OperationKind[] operationKinds) -> void
-Microsoft.CodeAnalysis.Diagnostics.AnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.Text.SourceText key, Microsoft.CodeAnalysis.Diagnostics.SourceTextValueProvider<TValue> valueProvider, out TValue value) -> bool
-Microsoft.CodeAnalysis.Diagnostics.CompilationAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.SyntaxTree key, Microsoft.CodeAnalysis.Diagnostics.SyntaxTreeValueProvider<TValue> valueProvider, out TValue value) -> bool
-Microsoft.CodeAnalysis.Diagnostics.CompilationAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.Text.SourceText key, Microsoft.CodeAnalysis.Diagnostics.SourceTextValueProvider<TValue> valueProvider, out TValue value) -> bool
+Microsoft.CodeAnalysis.Diagnostics.AnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.Text.SourceText text, Microsoft.CodeAnalysis.Diagnostics.SourceTextValueProvider<TValue> valueProvider, out TValue value) -> bool
+Microsoft.CodeAnalysis.Diagnostics.CompilationAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.SyntaxTree tree, Microsoft.CodeAnalysis.Diagnostics.SyntaxTreeValueProvider<TValue> valueProvider, out TValue value) -> bool
+Microsoft.CodeAnalysis.Diagnostics.CompilationAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.Text.SourceText text, Microsoft.CodeAnalysis.Diagnostics.SourceTextValueProvider<TValue> valueProvider, out TValue value) -> bool
 Microsoft.CodeAnalysis.Diagnostics.CompilationStartAnalysisContext.RegisterOperationAction(System.Action<Microsoft.CodeAnalysis.Diagnostics.OperationAnalysisContext> action, params Microsoft.CodeAnalysis.Semantics.OperationKind[] operationKinds) -> void
-Microsoft.CodeAnalysis.Diagnostics.CompilationStartAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.SyntaxTree key, Microsoft.CodeAnalysis.Diagnostics.SyntaxTreeValueProvider<TValue> valueProvider, out TValue value) -> bool
-Microsoft.CodeAnalysis.Diagnostics.CompilationStartAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.Text.SourceText key, Microsoft.CodeAnalysis.Diagnostics.SourceTextValueProvider<TValue> valueProvider, out TValue value) -> bool
+Microsoft.CodeAnalysis.Diagnostics.CompilationStartAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.SyntaxTree tree, Microsoft.CodeAnalysis.Diagnostics.SyntaxTreeValueProvider<TValue> valueProvider, out TValue value) -> bool
+Microsoft.CodeAnalysis.Diagnostics.CompilationStartAnalysisContext.TryGetValue<TValue>(Microsoft.CodeAnalysis.Text.SourceText text, Microsoft.CodeAnalysis.Diagnostics.SourceTextValueProvider<TValue> valueProvider, out TValue value) -> bool
 Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzersOptions.AnalyzerExceptionFilter.get -> System.Func<System.Exception, bool>
 Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzersOptions.CompilationWithAnalyzersOptions(Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions options, System.Action<System.Exception, Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis.Diagnostic> onAnalyzerException, System.Func<System.Exception, bool> analyzerExceptionFilter, bool concurrentAnalysis, bool logAnalyzerExecutionTime) -> void
 Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzersOptions.CompilationWithAnalyzersOptions(Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions options, System.Action<System.Exception, Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, Microsoft.CodeAnalysis.Diagnostic> onAnalyzerException, System.Func<System.Exception, bool> analyzerExceptionFilter, bool concurrentAnalysis, bool logAnalyzerExecutionTime, bool reportSuppressedDiagnostics) -> void