diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/DocumentationRules/SA1600CSharp11UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/DocumentationRules/SA1600CSharp11UnitTests.cs index 1d1777cda..6f414baf8 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/DocumentationRules/SA1600CSharp11UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/DocumentationRules/SA1600CSharp11UnitTests.cs @@ -3,9 +3,70 @@ namespace StyleCop.Analyzers.Test.CSharp11.DocumentationRules { + using Microsoft.CodeAnalysis.Testing; using StyleCop.Analyzers.Test.CSharp10.DocumentationRules; + using static StyleCop.Analyzers.Test.Verifiers.StyleCopDiagnosticVerifier< + StyleCop.Analyzers.DocumentationRules.SA1600ElementsMustBeDocumented>; public partial class SA1600CSharp11UnitTests : SA1600CSharp10UnitTests { + /* These methods reflect a fix for a Roslyn issue from C# 9 where diagnostics were reported twice. */ + + protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(5,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + + // /0/Test0.cs(5,43): warning SA1600: Elements should be documented + Diagnostic().WithLocation(1), + }; + } + + protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoDocumentation() + { + return new[] + { + // /0/Test0.cs(2,15): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + + // /0/Test0.cs(2,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(1), + }; + } + + protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorPartialParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(6,43): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + }; + } + + protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludePartialParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(3,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + }; + } + + protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludeMissingParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(3,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + + // /0/Test0.cs(3,43): warning SA1600: Elements should be documented + Diagnostic().WithLocation(1), + + // /0/Test0.cs(3,56): warning SA1600: Elements should be documented + Diagnostic().WithLocation(2), + }; + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/DocumentationRules/SA1600CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/DocumentationRules/SA1600CSharp9UnitTests.cs index 42ae8fef9..47daac12c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/DocumentationRules/SA1600CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/DocumentationRules/SA1600CSharp9UnitTests.cs @@ -5,11 +5,194 @@ namespace StyleCop.Analyzers.Test.CSharp9.DocumentationRules { + using System.Linq; + using System.Threading; + using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; using StyleCop.Analyzers.Test.CSharp8.DocumentationRules; + using StyleCop.Analyzers.Test.Helpers; + using Xunit; + using static StyleCop.Analyzers.Test.Verifiers.StyleCopDiagnosticVerifier< + StyleCop.Analyzers.DocumentationRules.SA1600ElementsMustBeDocumented>; public partial class SA1600CSharp9UnitTests : SA1600CSharp8UnitTests { + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorNoParameterDocumentationAsync(string keyword) + { + string testCode = $@" +/// +/// Record. +/// +public {keyword} MyRecord(int {{|#0:Param1|}}, string {{|#1:Param2|}});"; + + DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorNoParameterDocumentation(); + + await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorNoDocumentationAsync(string keyword) + { + string testCode = $@" +public {keyword} {{|#0:MyRecord|}}(int {{|#1:Param1|}});"; + + DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorNoDocumentation(); + + await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorCompleteParameterDocumentationAsync(string keyword) + { + string testCode = $@" +/// +/// Record. +/// +/// Parameter one. +/// Parameter two. +public {keyword} MyRecord(int Param1, string Param2);"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorPartialParameterDocumentationAsync(string keyword) + { + string testCode = $@" +/// +/// Record. +/// +/// Parameter one. +public {keyword} MyRecord(int Param1, string {{|#0:Param2|}});"; + + DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorPartialParameterDocumentation(); + + await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorInheritdocAsync(string keyword) + { + string testCode = $@" +/// +public {keyword} MyRecord(int Param1, string Param2);"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorIncludeParameterDocumentationAsync(string keyword) + { + string testCode = $@" +/// +public {keyword} MyRecord(int Param1, string Param2, bool Param3);"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorIncludeInheritdocAsync(string keyword) + { + string testCode = $@" +/// +public {keyword} MyRecord(int Param1, string Param2, bool Param3);"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorIncludePartialParameterDocumentationAsync(string keyword) + { + string testCode = $@" +/// +public {keyword} MyRecord(int {{|#0:Param1|}}, string Param2, bool Param3);"; + + DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorIncludePartialParameterDocumentation(); + + await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))] + [WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")] + public async Task TestRecordPrimaryConstructorIncludeMissingParameterDocumentationAsync(string keyword) + { + string testCode = $@" +/// +public {keyword} MyRecord(int {{|#0:Param1|}}, string {{|#1:Param2|}}, bool {{|#2:Param3|}});"; + + DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorIncludeMissingParameterDocumentation(); + + await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false); + } + + protected static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[] expected, CancellationToken cancellationToken) + { + string typeWithoutParameterDocumentation = @" + + + Foo + + + "; + string typeWithPartialParameterDocumentation = @" + + + Foo + + Param 2. + Param 3. + + "; + string typeWithParameterDocumentation = @" + + + Foo + + Param 1. + Param 2. + Param 3. + + "; + string typeWithIncludedInheritdoc = @" + + + + "; + + var test = new CSharpTest + { + TestCode = source, + XmlReferences = + { + { "MissingParameterDocumentation.xml", typeWithoutParameterDocumentation }, + { "WithParameterDocumentation.xml", typeWithParameterDocumentation }, + { "WithPartialParameterDocumentation.xml", typeWithPartialParameterDocumentation }, + { "WithInheritdoc.xml", typeWithIncludedInheritdoc }, + }, + }; + + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(cancellationToken); + } + protected override DiagnosticResult[] GetExpectedResultTestRegressionMethodGlobalNamespace(string code) { if (code == "public void {|#0:TestMember|}() { }") @@ -26,5 +209,73 @@ protected override DiagnosticResult[] GetExpectedResultTestRegressionMethodGloba return base.GetExpectedResultTestRegressionMethodGlobalNamespace(code); } + + /* The below result overrides are due to a Roslyn bug causing diagnostics to be reported twice, which was fixed in a later version. */ + + protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(5,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + Diagnostic().WithLocation(0), + + // /0/Test0.cs(5,43): warning SA1600: Elements should be documented + Diagnostic().WithLocation(1), + Diagnostic().WithLocation(1), + }; + } + + protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoDocumentation() + { + return new[] + { + // /0/Test0.cs(2,15): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + Diagnostic().WithLocation(0), + + // /0/Test0.cs(2,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(1), + Diagnostic().WithLocation(1), + }; + } + + protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorPartialParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(6,43): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + Diagnostic().WithLocation(0), + }; + } + + protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludePartialParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(3,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + Diagnostic().WithLocation(0), + }; + } + + protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludeMissingParameterDocumentation() + { + return new[] + { + // /0/Test0.cs(3,28): warning SA1600: Elements should be documented + Diagnostic().WithLocation(0), + Diagnostic().WithLocation(0), + + // /0/Test0.cs(3,43): warning SA1600: Elements should be documented + Diagnostic().WithLocation(1), + Diagnostic().WithLocation(1), + + // /0/Test0.cs(3,56): warning SA1600: Elements should be documented + Diagnostic().WithLocation(2), + Diagnostic().WithLocation(2), + }; + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1600ElementsMustBeDocumented.cs b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1600ElementsMustBeDocumented.cs index f64633941..e7b3f95e9 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1600ElementsMustBeDocumented.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1600ElementsMustBeDocumented.cs @@ -6,8 +6,12 @@ namespace StyleCop.Analyzers.DocumentationRules { using System; + using System.Collections; + using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; + using System.Reflection.Metadata; + using System.Xml.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -143,11 +147,35 @@ public static void HandleBaseTypeDeclaration(SyntaxNodeAnalysisContext context, Accessibility declaredAccessibility = declaration.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken); Accessibility effectiveAccessibility = declaration.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken); - if (NeedsComment(settings.DocumentationRules, declaration.Kind(), declaration.Parent.Kind(), declaredAccessibility, effectiveAccessibility)) + if (!NeedsComment(settings.DocumentationRules, declaration.Kind(), declaration.Parent.Kind(), declaredAccessibility, effectiveAccessibility)) { - if (!XmlCommentHelper.HasDocumentation(declaration)) + return; + } + + XmlComment xmlComment = context.GetParsedXmlComment(context.Node); + + if (xmlComment.IsMissing) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, declaration.Identifier.GetLocation())); + } + + if (xmlComment.HasInheritdoc) + { + return; + } + + if (context.Node is TypeDeclarationSyntax typeDeclaration && RecordDeclarationSyntaxWrapper.IsInstance(typeDeclaration)) + { + RecordDeclarationSyntaxWrapper recordDeclaration = (RecordDeclarationSyntaxWrapper)typeDeclaration; + + IEnumerable parameters = recordDeclaration.ParameterList?.Parameters ?? Enumerable.Empty(); + + foreach (var parameter in parameters) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, declaration.Identifier.GetLocation())); + if (!xmlComment.DocumentedParameterNames.Contains(parameter.Identifier.ValueText)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, parameter.Identifier.GetLocation())); + } } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlComment.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlComment.cs new file mode 100644 index 000000000..50ade7432 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlComment.cs @@ -0,0 +1,29 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.Helpers +{ + using System.Collections.Generic; + using System.Linq; + + internal class XmlComment + { + internal XmlComment(string[] documentedParameterNames, bool hasInheritdoc) + { + this.DocumentedParameterNames = documentedParameterNames; + this.HasInheritdoc = hasInheritdoc; + } + + internal XmlComment() + { + } + + internal static XmlComment MissingSummary => new() { IsMissing = true }; + + internal bool IsMissing { get; private set; } + + internal bool HasInheritdoc { get; private set; } + + internal IEnumerable DocumentedParameterNames { get; private set; } = Enumerable.Empty(); + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlCommentHelper.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlCommentHelper.cs index 24d2728d1..29ff07497 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlCommentHelper.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/XmlCommentHelper.cs @@ -5,11 +5,13 @@ namespace StyleCop.Analyzers.Helpers { + using System.Collections.Generic; using System.Linq; using System.Text; using System.Xml.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; + using Microsoft.CodeAnalysis.Diagnostics; using StyleCop.Analyzers.Helpers.ObjectPools; /// @@ -436,6 +438,63 @@ internal static bool IsBlockElement(this XmlNodeSyntax nodeSyntax) return false; } + /// + /// Parses a object from a . + /// + /// The analysis context that will be checked. + /// The node to parse the documentation for. + /// The parsed . + internal static XmlComment GetParsedXmlComment(this SyntaxNodeAnalysisContext context, SyntaxNode node) + { + var documentation = node.GetDocumentationCommentTriviaSyntax(); + + if (documentation == null || IsMissingOrEmpty(documentation.ParentTrivia)) + { + return XmlComment.MissingSummary; + } + + bool hasInheritdoc = false; + string[] documentedParameterNames; + + if (documentation.Content.GetFirstXmlElement(InheritdocXmlTag) != null) + { + hasInheritdoc = true; + } + + bool hasIncludedDocumentation = documentation.Content.GetFirstXmlElement(IncludeXmlTag) != null; + + if (hasIncludedDocumentation) + { + var declaredSymbol = context.SemanticModel.GetDeclaredSymbol(node, context.CancellationToken); + var rawDocumentation = declaredSymbol?.GetDocumentationCommentXml(expandIncludes: true, cancellationToken: context.CancellationToken); + var includedDocumentation = XElement.Parse(rawDocumentation ?? "", LoadOptions.None); + + if (includedDocumentation.Nodes().OfType().Any(element => element.Name == InheritdocXmlTag)) + { + hasInheritdoc = true; + } + + IEnumerable paramElements = includedDocumentation.Nodes() + .OfType() + .Where(x => x.Name == ParamXmlTag); + + documentedParameterNames = paramElements + .SelectMany(x => x.Attributes().Where(y => y.Name == NameArgumentName)) + .Select(x => x.Value) + .ToArray(); + } + else + { + IEnumerable xmlNodes = documentation.Content.GetXmlElements(ParamXmlTag); + documentedParameterNames = xmlNodes.Select(GetFirstAttributeOrDefault) + .Where(x => x != null) + .Select(x => x.Identifier.Identifier.ValueText) + .ToArray(); + } + + return new XmlComment(documentedParameterNames, hasInheritdoc); + } + private static bool IsInlineElement(string localName) { switch (localName)