Skip to content

Commit

Permalink
Merge pull request #3371 from jnm2/allow_full_sentence_links
Browse files Browse the repository at this point in the history
SA1629 should allow full-sentence links instead of forcing the period to glow white
  • Loading branch information
sharwell authored Jun 21, 2023
2 parents a69732f + 0896aa6 commit 14245e2
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,23 @@ public interface ITest
await VerifyCSharpFixAsync(testCode, expected, fixedTestCode, default).ConfigureAwait(false);
}

[Theory]
[InlineData("a", true)]
[InlineData("see", true)]
[InlineData("seealso", false)]
public async Task TestFullSentenceLinkAsync(string tag, bool insideSummary)
{
var surrounding = insideSummary ? (Start: "<summary>", End: "<summary>") : (Start: string.Empty, End: string.Empty);

var testCode = $@"
/// {surrounding.Start}<{tag} href=""someurl"">Periods aren't required to glow white at the end of a full-sentence link.</{tag}>{surrounding.End}
public interface ITest
{{
}}
";
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, default).ConfigureAwait(false);
}

[Theory]
[InlineData(",")]
[InlineData(";")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,7 @@ private static void HandleSectionOrBlockXmlElement(SyntaxNodeAnalysisContext con

if (!string.IsNullOrEmpty(textWithoutTrailingWhitespace))
{
if (!textWithoutTrailingWhitespace.EndsWith(".", StringComparison.Ordinal)
&& !textWithoutTrailingWhitespace.EndsWith(".)", StringComparison.Ordinal)
&& (startingWithFinalParagraph || !textWithoutTrailingWhitespace.EndsWith(":", StringComparison.Ordinal))
&& !textWithoutTrailingWhitespace.EndsWith("-or-", StringComparison.Ordinal))
if (IsMissingRequiredPeriod(textWithoutTrailingWhitespace, startingWithFinalParagraph))
{
int spanStart = textToken.SpanStart + textWithoutTrailingWhitespace.Length;
ImmutableDictionary<string, string> properties = null;
Expand Down Expand Up @@ -164,10 +161,15 @@ void SetReplaceChar()
}
else if (xmlElement.Content[i].IsInlineElement() && !currentParagraphDone)
{
// Treat empty XML elements as a "word not ending with a period"
var location = Location.Create(xmlElement.SyntaxTree, new TextSpan(xmlElement.Content[i].Span.End, 1));
context.ReportDiagnostic(Diagnostic.Create(Descriptor, location));
currentParagraphDone = true;
var lastTextElement = XmlCommentHelper.TryGetLastTextElementWithContent(xmlElement.Content[i]);

if (lastTextElement is null // Treat empty XML elements as a "word not ending with a period"
|| IsMissingRequiredPeriod(lastTextElement.TextTokens.Last().Text.TrimEnd(' ', '\r', '\n'), startingWithFinalParagraph))
{
var location = Location.Create(xmlElement.SyntaxTree, new TextSpan(xmlElement.Content[i].Span.End, 1));
context.ReportDiagnostic(Diagnostic.Create(Descriptor, location));
currentParagraphDone = true;
}
}
else if (xmlElement.Content[i] is XmlElementSyntax childXmlElement)
{
Expand Down Expand Up @@ -200,5 +202,13 @@ void SetReplaceChar()
}
}
}

private static bool IsMissingRequiredPeriod(string textWithoutTrailingWhitespace, bool startingWithFinalParagraph)
{
return !textWithoutTrailingWhitespace.EndsWith(".", StringComparison.Ordinal)
&& !textWithoutTrailingWhitespace.EndsWith(".)", StringComparison.Ordinal)
&& (startingWithFinalParagraph || !textWithoutTrailingWhitespace.EndsWith(":", StringComparison.Ordinal))
&& !textWithoutTrailingWhitespace.EndsWith("-or-", StringComparison.Ordinal);
}
}
}
31 changes: 31 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlCommentHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,37 @@ internal static XmlTextSyntax TryGetFirstTextElementWithContent(XmlNodeSyntax no
return null;
}

/// <summary>
/// Returns the last <see cref="XmlTextSyntax"/> which is not simply empty or whitespace.
/// </summary>
/// <param name="node">The XML content to search.</param>
/// <returns>The last <see cref="XmlTextSyntax"/> which is not simply empty or whitespace, or
/// <see langword="null"/> if no such element exists.</returns>
internal static XmlTextSyntax TryGetLastTextElementWithContent(XmlNodeSyntax node)
{
if (node is XmlEmptyElementSyntax)
{
return null;
}
else if (node is XmlTextSyntax xmlText)
{
return !IsConsideredEmpty(node) ? xmlText : null;
}
else if (node is XmlElementSyntax xmlElement)
{
for (var i = xmlElement.Content.Count - 1; i >= 0; i--)
{
var nestedContent = TryGetFirstTextElementWithContent(xmlElement.Content[i]);
if (nestedContent != null)
{
return nestedContent;
}
}
}

return null;
}

/// <summary>
/// Checks if a <see cref="SyntaxTrivia"/> contains a <see cref="DocumentationCommentTriviaSyntax"/> and returns
/// <see langword="true"/> if it is considered empty.
Expand Down

0 comments on commit 14245e2

Please sign in to comment.