Skip to content

Commit

Permalink
Fix partial method doc comments (#56419)
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson authored Sep 21, 2021
1 parent 0f60b3e commit 8cda746
Show file tree
Hide file tree
Showing 5 changed files with 678 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,30 @@ public override void DefaultVisit(Symbol symbol)
return;
}

bool isPartialMethodDefinitionPart = symbol.IsPartialDefinition(); // CONSIDER: ignore this if isForSingleSymbol?
if (isPartialMethodDefinitionPart)
bool shouldSkipPartialDefinitionComments = false;
if (symbol.IsPartialDefinition())
{
MethodSymbol implementationPart = ((MethodSymbol)symbol).PartialImplementationPart;
if ((object)implementationPart != null)
if (symbol is MethodSymbol { PartialImplementationPart: MethodSymbol implementationPart })
{
Visit(implementationPart);

foreach (var trivia in implementationPart.GetNonNullSyntaxNode().GetLeadingTrivia())
{
if (trivia.Kind() is SyntaxKind.SingleLineDocumentationCommentTrivia or SyntaxKind.MultiLineDocumentationCommentTrivia)
{
// If the partial method implementation has doc comments,
// we will not emit any doc comments found on the definition,
// regardless of whether the partial implementation doc comments are valid.
shouldSkipPartialDefinitionComments = true;
break;
}
}
}
else
{
// The partial method has no implementation. Since it won't be present in the
// output assembly, it shouldn't be included in the documentation file.
shouldSkipPartialDefinitionComments = !_isForSingleSymbol;
}
}

Expand All @@ -276,7 +293,13 @@ public override void DefaultVisit(Symbol symbol)
// If there are no doc comments, then no further work is required (other than to report a diagnostic if one is required).
if (docCommentNodes.IsEmpty)
{
if (maxDocumentationMode >= DocumentationMode.Diagnose && RequiresDocumentationComment(symbol))
if (maxDocumentationMode >= DocumentationMode.Diagnose
&& RequiresDocumentationComment(symbol)
// We never give a missing doc comment warning on a partial method
// implementation, and we skip the missing doc comment warning on a partial
// definition whose documentation we were not going to output anyway.
&& !symbol.IsPartialImplementation()
&& !shouldSkipPartialDefinitionComments)
{
// Report the error at a location in the tree that was parsing doc comments.
Location location = GetLocationInTreeReportingDocumentationCommentDiagnostics(symbol);
Expand All @@ -299,7 +322,7 @@ public override void DefaultVisit(Symbol symbol)
ImmutableArray<CSharpSyntaxNode> includeElementNodes;
if (!TryProcessDocumentationCommentTriviaNodes(
symbol,
isPartialMethodDefinitionPart,
shouldSkipPartialDefinitionComments,
docCommentNodes,
reportParameterOrTypeParameterDiagnostics,
out withUnprocessedIncludes,
Expand Down Expand Up @@ -328,11 +351,11 @@ public override void DefaultVisit(Symbol symbol)
// NOTE: we are expanding include elements AFTER formatting the comment, since the included text is pure
// XML, not XML mixed with documentation comment trivia (e.g. ///). If we expanded them before formatting,
// the formatting engine would have trouble determining what prefix to remove from each line.
TextWriter expanderWriter = isPartialMethodDefinitionPart ? null : _writer; // Don't actually write partial method definition parts.
TextWriter expanderWriter = shouldSkipPartialDefinitionComments ? null : _writer; // Don't actually write partial method definition parts.
IncludeElementExpander.ProcessIncludes(withUnprocessedIncludes, symbol, includeElementNodes,
_compilation, ref documentedParameters, ref documentedTypeParameters, ref _includedFileCache, expanderWriter, _diagnostics, _cancellationToken);
}
else if (_writer != null && !isPartialMethodDefinitionPart)
else if (_writer != null && !shouldSkipPartialDefinitionComments)
{
// CONSIDER: The output would look a little different if we ran the XDocument through an XmlWriter. In particular,
// formatting inside tags (e.g. <__tag___attr__=__"value"__>) would be normalized. Whitespace in elements would
Expand Down Expand Up @@ -396,7 +419,7 @@ symbol is SynthesizedSimpleProgramEntryPointSymbol ||
/// <remarks>This was factored out for clarity, not because it's reusable.</remarks>
private bool TryProcessDocumentationCommentTriviaNodes(
Symbol symbol,
bool isPartialMethodDefinitionPart,
bool shouldSkipPartialDefinitionComments,
ImmutableArray<DocumentationCommentTriviaSyntax> docCommentNodes,
bool reportParameterOrTypeParameterDiagnostics,
out string withUnprocessedIncludes,
Expand Down Expand Up @@ -438,7 +461,7 @@ private bool TryProcessDocumentationCommentTriviaNodes(

// We DO want to write out partial method definition parts if we're processing includes
// because we need to have XML to process.
if (!isPartialMethodDefinitionPart || _processIncludes)
if (!shouldSkipPartialDefinitionComments || _processIncludes)
{
WriteLine("<member name=\"{0}\">", symbol.GetDocumentationCommentId());
Indent();
Expand Down Expand Up @@ -468,7 +491,7 @@ private bool TryProcessDocumentationCommentTriviaNodes(
}

// For partial methods, all parts are validated, but only the implementation part is written to the XML stream.
if (!isPartialMethodDefinitionPart || _processIncludes)
if (!shouldSkipPartialDefinitionComments || _processIncludes)
{
// This string already has indentation and line breaks, so don't call WriteLine - just write the text directly.
Write(formattedXml);
Expand All @@ -483,7 +506,7 @@ private bool TryProcessDocumentationCommentTriviaNodes(
return false;
}

if (!isPartialMethodDefinitionPart || _processIncludes)
if (!shouldSkipPartialDefinitionComments || _processIncludes)
{
Unindent();
WriteLine("</member>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ public sealed override bool IsExtern
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(SourcePartialImplementation ?? this, expandIncludes, ref lazyDocComment);
return SourceDocumentationCommentUtils.GetAndCacheDocumentationComment(this, expandIncludes, ref lazyDocComment);
}

protected override SourceMemberMethodSymbol BoundAttributesSource
Expand Down
Loading

0 comments on commit 8cda746

Please sign in to comment.