Merge pull request #36021 from sharwell/comment-caching

🐛 Fix incorrect caching of XML documentation comments with/without expandIncludes

src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs

@@ -30,6 +30,7 @@ internal abstract class SourceEventSymbol : EventSymbol, IAttributeTargetSymbol
         private SymbolCompletionState _state;
         private CustomAttributesBag<CSharpAttributeData> _lazyCustomAttributesBag;
         private string _lazyDocComment;
+        private string _lazyExpandedDocComment;
         private OverriddenOrHiddenMembersResult _lazyOverriddenOrHiddenMembers;
         private ThreeState _lazyIsWindowsRuntimeEvent = ThreeState.Unknown;
 
@@ -587,7 +588,8 @@ protected void CheckModifiersAndType(DiagnosticBag diagnostics)
 
         public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
-            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref _lazyDocComment);
+            ref var lazyDocComment = ref expandIncludes ? ref _lazyExpandedDocComment : ref _lazyDocComment;
+            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment);
         }
 
         protected static void CopyEventCustomModifiers(EventSymbol eventWithCustomModifiers, ref TypeWithAnnotations type, AssemblySymbol containingAssembly)

src/Compilers/CSharp/Portable/Symbols/Source/SourceFieldSymbol.cs

@@ -150,6 +150,7 @@ internal abstract class SourceFieldSymbolWithSyntaxReference : SourceFieldSymbol
         private readonly SyntaxReference _syntaxReference;
 
         private string _lazyDocComment;
+        private string _lazyExpandedDocComment;
         private ConstantValue _lazyConstantEarlyDecodingValue = Microsoft.CodeAnalysis.ConstantValue.Unset;
         private ConstantValue _lazyConstantValue = Microsoft.CodeAnalysis.ConstantValue.Unset;
 
@@ -221,7 +222,8 @@ public sealed override ImmutableArray<SyntaxReference> DeclaringSyntaxReferences
 
         public sealed override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
-            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref _lazyDocComment);
+            ref var lazyDocComment = ref expandIncludes ? ref _lazyExpandedDocComment : ref _lazyDocComment;
+            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment);
         }
 
         internal sealed override ConstantValue GetConstantValue(ConstantFieldsInProgress inProgress, bool earlyDecodingWellKnownAttributes)

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs

@@ -164,6 +164,7 @@ public void EnsureMetadataVirtual()
 
         protected ImmutableArray<Location> locations;
         protected string lazyDocComment;
+        protected string lazyExpandedDocComment;
 
         //null if has never been computed. Initial binding diagnostics
         //are stashed here in service of API usage patterns
@@ -674,6 +675,7 @@ public sealed override ImmutableArray<SyntaxReference> DeclaringSyntaxReferences
 
         public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
+            ref var lazyDocComment = ref expandIncludes ? ref this.lazyExpandedDocComment : ref this.lazyDocComment;
             return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment);
         }
 

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs

@@ -30,6 +30,7 @@ internal sealed partial class SourceNamedTypeSymbol : SourceMemberContainerTypeS
         private CustomAttributesBag<CSharpAttributeData> _lazyCustomAttributesBag;
 
         private string _lazyDocComment;
+        private string _lazyExpandedDocComment;
 
         private ThreeState _lazyIsExplicitDefinitionOfNoPiaLocalType = ThreeState.Unknown;
 
@@ -109,7 +110,8 @@ private static SyntaxToken GetName(CSharpSyntaxNode node)
 
         public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
-            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref _lazyDocComment);
+            ref var lazyDocComment = ref expandIncludes ? ref _lazyExpandedDocComment : ref _lazyDocComment;
+            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment);
         }
 
         #endregion

src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs

@@ -751,7 +751,8 @@ public override MethodSymbol PartialImplementationPart
 
         public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
-            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this.SourcePartialImplementation ?? this, expandIncludes, ref lazyDocComment);
+            ref var lazyDocComment = ref expandIncludes ? ref this.lazyExpandedDocComment : ref this.lazyDocComment;
+            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(SourcePartialImplementation ?? this, expandIncludes, ref lazyDocComment);
         }
 
         internal override bool IsExplicitInterfaceImplementation

src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs

@@ -45,6 +45,7 @@ internal sealed class SourcePropertySymbol : PropertySymbol, IAttributeTargetSym
         private readonly string _sourceName;
 
         private string _lazyDocComment;
+        private string _lazyExpandedDocComment;
         private OverriddenOrHiddenMembersResult _lazyOverriddenOrHiddenMembers;
         private SynthesizedSealedPropertyAccessor _lazySynthesizedSealedAccessor;
         private CustomAttributesBag<CSharpAttributeData> _lazyCustomAttributesBag;
@@ -1051,7 +1052,8 @@ private static void CheckAbstractPropertyAccessorNotPrivate(SourcePropertyAccess
 
         public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
-            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref _lazyDocComment);
+            ref var lazyDocComment = ref expandIncludes ? ref _lazyExpandedDocComment : ref _lazyDocComment;
+            return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment);
         }
 
         // Separate these checks out of FindExplicitlyImplementedProperty because they depend on the accessor symbols,

src/Compilers/CSharp/Test/Symbol/DocumentationComments/IncludeTests.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System.Linq;
+using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
+using Roslyn.Test.Utilities;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.CSharp.UnitTests
+{
+    public class IncludeTests : CSharpTestBase
+    {
+        [Theory]
+        [InlineData("Field", "F:Acme.Widget.Field")]
+        [InlineData(WellKnownMemberNames.StaticConstructorName, "M:Acme.Widget.#cctor")]
+        [InlineData("Event", "E:Acme.Widget.Event")]
+        [InlineData("Property", "P:Acme.Widget.Property")]
+        [InlineData("Method", "M:Acme.Widget.Method")]
+        [InlineData("NamedType", "T:Acme.Widget.NamedType")]
+        public void TestDocumentationCaching(string symbolName, string documentationId)
+        {
+            using var _ = new EnsureEnglishUICulture();
+
+            var compilation = CreateCompilationWithMscorlib40AndDocumentationComments(@"namespace Acme
+{
+    class Widget
+    {
+        /// <include file=""NonExistent.xml"" />
+        int Field;
+
+        /// <include file=""NonExistent.xml"" />
+        static Widget() { }
+
+        /// <include file=""NonExistent.xml"" />
+        event EventHandler Event;
+
+        /// <include file=""NonExistent.xml"" />
+        int Property { get; }
+
+        /// <include file=""NonExistent.xml"" />
+        void Method() { }
+
+        /// <include file=""NonExistent.xml"" />
+        class NamedType { }
+    }
+}
+");
+
+            var acmeNamespace = (NamespaceSymbol)compilation.GlobalNamespace.GetMembers("Acme").Single();
+            var widgetClass = acmeNamespace.GetTypeMembers("Widget").Single();
+
+            var symbol = widgetClass.GetMembers(symbolName).Single();
+            Assert.Equal(documentationId, symbol.GetDocumentationCommentId());
+            Assert.Equal(
+$@"<member name=""{documentationId}"">
+    <!-- Include tag is invalid --><include file=""NonExistent.xml"" />
+</member>
+", symbol.GetDocumentationCommentXml(expandIncludes: true));
+            Assert.Equal(
+$@"<member name=""{documentationId}"">
+    <include file=""NonExistent.xml"" />
+</member>
+", symbol.GetDocumentationCommentXml(expandIncludes: false));
+            Assert.Equal(
+$@"<member name=""{documentationId}"">
+    <!-- Include tag is invalid --><include file=""NonExistent.xml"" />
+</member>
+", symbol.GetDocumentationCommentXml(expandIncludes: true));
+            Assert.Equal(
+$@"<member name=""{documentationId}"">
+    <include file=""NonExistent.xml"" />
+</member>
+", symbol.GetDocumentationCommentXml(expandIncludes: false));
+        }
+    }
+}

src/Compilers/VisualBasic/Portable/DocumentationComments/SourceDocumentationCommentUtils.vb

 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-Imports System.Text
-Imports System.Threading
 Imports System.Globalization
-Imports Microsoft.CodeAnalysis.Text
-Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
-Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
+Imports System.Threading
 
 Namespace Microsoft.CodeAnalysis.VisualBasic
     Friend Module SourceDocumentationCommentUtils
+        Friend Function GetAndCacheDocumentationComment(
+            symbol As Symbol,
+            preferredCulture As CultureInfo,
+            expandIncludes As Boolean,
+            ByRef lazyXmlText As String,
+            cancellationToken As CancellationToken
+        ) As String
+
+            If lazyXmlText Is Nothing Then
+                Dim xmlText = GetDocumentationCommentForSymbol(symbol, preferredCulture, expandIncludes, cancellationToken)
+                Interlocked.CompareExchange(lazyXmlText, xmlText, Nothing)
+            End If
+
+            Return lazyXmlText
+        End Function
 
         ''' <summary>
         ''' Returns documentation comment for a type, field, property, event or method, 

src/Compilers/VisualBasic/Portable/Symbols/Source/SourceEventSymbol.vb

@@ -43,6 +43,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         Private _lazyImplementedEvents As ImmutableArray(Of EventSymbol)
         Private _lazyDelegateParameters As ImmutableArray(Of ParameterSymbol)
         Private _lazyDocComment As String
+        Private _lazyExpandedDocComment As String
 
         ' Attributes on event. Set once after construction. IsNull means not set. 
         Private _lazyCustomAttributesBag As CustomAttributesBag(Of VisualBasicAttributeData)
@@ -690,13 +691,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         Public Overrides Function GetDocumentationCommentXml(Optional preferredCulture As CultureInfo = Nothing,
                                                              Optional expandIncludes As Boolean = False,
                                                              Optional cancellationToken As CancellationToken = Nothing) As String
-            If _lazyDocComment Is Nothing Then
-                ' NOTE: replace Nothing with empty comment
-                Interlocked.CompareExchange(
-                    _lazyDocComment, GetDocumentationCommentForSymbol(Me, preferredCulture, expandIncludes, cancellationToken), Nothing)
+            If expandIncludes Then
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyExpandedDocComment, cancellationToken)
+            Else
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyDocComment, cancellationToken)
             End If
-
-            Return _lazyDocComment
         End Function
 
         Friend Shared Function DecodeModifiers(modifiers As SyntaxTokenList,

src/Compilers/VisualBasic/Portable/Symbols/Source/SourceFieldSymbol.vb

@@ -26,6 +26,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         Private ReadOnly _syntaxRef As SyntaxReference
 
         Private _lazyDocComment As String
+        Private _lazyExpandedDocComment As String
         Private _lazyCustomAttributesBag As CustomAttributesBag(Of VisualBasicAttributeData)
 
         ' Set to 1 when the compilation event has been produced
@@ -112,13 +113,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         End Property
 
         Public Overrides Function GetDocumentationCommentXml(Optional preferredCulture As CultureInfo = Nothing, Optional expandIncludes As Boolean = False, Optional cancellationToken As CancellationToken = Nothing) As String
-            If _lazyDocComment Is Nothing Then
-                ' NOTE: replace Nothing with empty comment
-                Interlocked.CompareExchange(
-                    _lazyDocComment, GetDocumentationCommentForSymbol(Me, preferredCulture, expandIncludes, cancellationToken), Nothing)
+            If expandIncludes Then
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyExpandedDocComment, cancellationToken)
+            Else
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyDocComment, cancellationToken)
             End If
-
-            Return _lazyDocComment
         End Function
 
         ''' <summary>

src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMethodSymbol.vb

@@ -45,6 +45,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         Private _lazyLocations As ImmutableArray(Of Location)
 
         Private _lazyDocComment As String
+        Private _lazyExpandedDocComment As String
 
         'Nothing if diags have never been computed. Initial binding diagnostics
         'are stashed here to optimize API usage patterns
@@ -806,13 +807,11 @@ lReportErrorOnTwoTokens:
         End Function
 
         Public NotOverridable Overrides Function GetDocumentationCommentXml(Optional preferredCulture As CultureInfo = Nothing, Optional expandIncludes As Boolean = False, Optional cancellationToken As CancellationToken = Nothing) As String
-            If _lazyDocComment Is Nothing Then
-                ' NOTE: replace Nothing with empty comment
-                Interlocked.CompareExchange(
-                    _lazyDocComment, GetDocumentationCommentForSymbol(Me, preferredCulture, expandIncludes, cancellationToken), Nothing)
+            If expandIncludes Then
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyExpandedDocComment, cancellationToken)
+            Else
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyDocComment, cancellationToken)
             End If
-
-            Return _lazyDocComment
         End Function
 
         ''' <summary>

src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol.vb

@@ -30,6 +30,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         Private ReadOnly _corTypeId As SpecialType
 
         Private _lazyDocComment As String
+        Private _lazyExpandedDocComment As String
         Private _lazyEnumUnderlyingType As NamedTypeSymbol
 
         ' Stores symbols for overriding WithEvents properties if we have such
@@ -138,12 +139,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         End Function
 
         Public Overrides Function GetDocumentationCommentXml(Optional preferredCulture As CultureInfo = Nothing, Optional expandIncludes As Boolean = False, Optional cancellationToken As CancellationToken = Nothing) As String
-            If _lazyDocComment Is Nothing Then
-                ' NOTE: replace Nothing with empty comment
-                Interlocked.CompareExchange(
-                    _lazyDocComment, GetDocumentationCommentForSymbol(Me, preferredCulture, expandIncludes, cancellationToken), Nothing)
+            If expandIncludes Then
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyExpandedDocComment, cancellationToken)
+            Else
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyDocComment, cancellationToken)
             End If
-            Return _lazyDocComment
         End Function
 
         ' Create a LocationSpecificBinder for the type. This is a binder that wraps the

src/Compilers/VisualBasic/Portable/Symbols/Source/SourcePropertySymbol.vb

@@ -32,6 +32,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         Private _setMethod As MethodSymbol
         Private _backingField As FieldSymbol
         Private _lazyDocComment As String
+        Private _lazyExpandedDocComment As String
         Private _lazyMeParameter As ParameterSymbol
 
         ' Attributes on property. Set once after construction. IsNull means not set. 
@@ -1051,13 +1052,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
         End Function
 
         Public Overrides Function GetDocumentationCommentXml(Optional preferredCulture As CultureInfo = Nothing, Optional expandIncludes As Boolean = False, Optional cancellationToken As CancellationToken = Nothing) As String
-            If _lazyDocComment Is Nothing Then
-                ' NOTE: replace Nothing with empty comment
-                Interlocked.CompareExchange(
-                    _lazyDocComment, GetDocumentationCommentForSymbol(Me, preferredCulture, expandIncludes, cancellationToken), Nothing)
+            If expandIncludes Then
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyExpandedDocComment, cancellationToken)
+            Else
+                Return GetAndCacheDocumentationComment(Me, preferredCulture, expandIncludes, _lazyDocComment, cancellationToken)
             End If
-
-            Return _lazyDocComment
         End Function
 
         Private Shared Function DecodeModifiers(modifiers As SyntaxTokenList,

src/Compilers/VisualBasic/Test/Symbol/DocumentationComments/IncludeTests.vb

+' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
+Imports Roslyn.Test.Utilities
+
+Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests
+    Public Class IncludeTests
+        <Theory>
+        <InlineData("F", "F:Acme.Widget.F")>
+        <InlineData(WellKnownMemberNames.StaticConstructorName, "M:Acme.Widget.#cctor")>
+        <InlineData("E", "E:Acme.Widget.E")>
+        <InlineData("P", "P:Acme.Widget.P")>
+        <InlineData("M", "M:Acme.Widget.M")>
+        <InlineData("NamedType", "T:Acme.Widget.NamedType")>
+        Public Sub TestDocumentationCaching(symbolName As String, documentationId As String)
+            Using New EnsureEnglishUICulture()
+                Dim compilation = CreateCompilationWithMscorlib40(
+                <compilation name="ConstructorDocumentationCommentTests">
+                    <file name="a.vb">
+                    Namespace Acme
+                        Class Widget
+                            ''' &lt;include file="NonExistent.xml" /&gt;
+                            Dim F As Integer
+
+                            ''' &lt;include file="NonExistent.xml" /&gt;
+                            Shared Sub New()
+                            End Sub
+
+                            ''' &lt;include file="NonExistent.xml" /&gt;
+                            Event E As EventHandler
+
+                            ''' &lt;include file="NonExistent.xml" /&gt;
+                            Property P As Integer
+
+                            ''' &lt;include file="NonExistent.xml" /&gt;
+                            Sub M()
+                            End Sub
+
+                            ''' &lt;include file="NonExistent.xml" /&gt;
+                            Class NamedType
+                            End Class
+                        End Class
+                    End Namespace
+                    </file>
+                </compilation>)
+
+                Dim acmeNamespace = DirectCast(compilation.GlobalNamespace.GetMembers("Acme").Single(), NamespaceSymbol)
+                Dim widgetClass = acmeNamespace.GetTypeMembers("Widget").Single()
+
+                Dim symbol = widgetClass.GetMembers(symbolName).Single()
+                Assert.Equal(documentationId, symbol.GetDocumentationCommentId())
+                Assert.Equal(
+$"<member name=""{documentationId}"">
+ <!--warning BC42310: XML comment tag 'include' must have a 'path' attribute. XML comment will be ignored.-->
+</member>", symbol.GetDocumentationCommentXml(expandIncludes:=True))
+                Assert.Equal(
+$"<member name=""{documentationId}"">
+ <include file=""NonExistent.xml"" />
+</member>", symbol.GetDocumentationCommentXml(expandIncludes:=False))
+                Assert.Equal(
+$"<member name=""{documentationId}"">
+ <!--warning BC42310: XML comment tag 'include' must have a 'path' attribute. XML comment will be ignored.-->
+</member>", symbol.GetDocumentationCommentXml(expandIncludes:=True))
+                Assert.Equal(
+$"<member name=""{documentationId}"">
+ <include file=""NonExistent.xml"" />
+</member>", symbol.GetDocumentationCommentXml(expandIncludes:=False))
+            End Using
+        End Sub
+    End Class
+End Namespace