From bf1f009ea1eef1f3b377979be5a96bb25467deee Mon Sep 17 00:00:00 2001 From: Valeri Hristov Date: Tue, 11 Dec 2018 16:10:08 +0100 Subject: [PATCH 1/2] Undocumented public API metric should count only documentation comments --- its/projects/MetricsTest/bar/Class1.cs | 4 +- its/projects/MetricsTest/foo/Class1.cs | 4 +- its/projects/MetricsTest/foo/Class1_copy.cs | 6 +- its/projects/VbMetricsTest/bar/Module1.vb | 3 + its/projects/VbMetricsTest/foo/Module1.vb | 3 + .../VbMetricsTest/foo/Module1_copy.vb | 3 + .../java/com/sonar/it/csharp/MetricsTest.java | 30 ++++---- .../java/com/sonar/it/vbnet/MetricsTest.java | 22 +++--- .../Metrics/CSharpPublicApiMetric.cs | 73 +++++++++++++++++++ .../SonarAnalyzer.CSharp/Metrics/Metrics.cs | 64 ++++++---------- .../Metrics/MetricsBase.cs | 7 +- .../Metrics/Metrics.cs | 21 +++++- ...etric.cs => VisualBasicPublicApiMetric.cs} | 2 +- .../Common/MetricsTest.cs | 23 +++++- 14 files changed, 182 insertions(+), 83 deletions(-) create mode 100644 sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs rename sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/{PublicApiMetric.cs => VisualBasicPublicApiMetric.cs} (98%) diff --git a/its/projects/MetricsTest/bar/Class1.cs b/its/projects/MetricsTest/bar/Class1.cs index 225c2a103c8..bf7ba7fc521 100644 --- a/its/projects/MetricsTest/bar/Class1.cs +++ b/its/projects/MetricsTest/bar/Class1.cs @@ -8,7 +8,9 @@ namespace CSLib.bar { - // Commented public API + /// + /// Documented public API + /// public class MyClazz { public int MyProperty diff --git a/its/projects/MetricsTest/foo/Class1.cs b/its/projects/MetricsTest/foo/Class1.cs index 1e08d13cbf8..479638f268c 100644 --- a/its/projects/MetricsTest/foo/Class1.cs +++ b/its/projects/MetricsTest/foo/Class1.cs @@ -8,9 +8,11 @@ namespace CSLib.foo { - // Commented public API public class MyClazz { + /// + /// Documented public API + /// public int MyProperty { get diff --git a/its/projects/MetricsTest/foo/Class1_copy.cs b/its/projects/MetricsTest/foo/Class1_copy.cs index 3445bd73622..86eaae06b9f 100644 --- a/its/projects/MetricsTest/foo/Class1_copy.cs +++ b/its/projects/MetricsTest/foo/Class1_copy.cs @@ -8,9 +8,11 @@ namespace CSLib.foo2 { - // Commented public API public class MyClazz { + /// + /// Documented public API + /// public int MyProperty { get @@ -25,7 +27,7 @@ class Class1 public void Main() { MyClazz myClazz = new MyClazz(); - Console.WriteLine("Hello, world! " + myClazz.MyProperty); + Console.WriteLine(myClazz.MyProperty); Console.ReadLine(); } } diff --git a/its/projects/VbMetricsTest/bar/Module1.vb b/its/projects/VbMetricsTest/bar/Module1.vb index 0e41d6f3f34..0e756601b83 100644 --- a/its/projects/VbMetricsTest/bar/Module1.vb +++ b/its/projects/VbMetricsTest/bar/Module1.vb @@ -3,6 +3,9 @@ REM This is a comment! Public Module bar + ''' + ''' Documented public API + ''' Public Class MyClazz Public ReadOnly Property MyProperty As Integer Get diff --git a/its/projects/VbMetricsTest/foo/Module1.vb b/its/projects/VbMetricsTest/foo/Module1.vb index 9dafcfaf2e2..30242583775 100644 --- a/its/projects/VbMetricsTest/foo/Module1.vb +++ b/its/projects/VbMetricsTest/foo/Module1.vb @@ -3,6 +3,9 @@ REM This is a comment! Public Module foo + ''' + ''' Documented public API + ''' Public Class MyClazz Public ReadOnly Property MyProperty As Integer Get diff --git a/its/projects/VbMetricsTest/foo/Module1_copy.vb b/its/projects/VbMetricsTest/foo/Module1_copy.vb index f9a555da81a..a4540c86786 100644 --- a/its/projects/VbMetricsTest/foo/Module1_copy.vb +++ b/its/projects/VbMetricsTest/foo/Module1_copy.vb @@ -3,6 +3,9 @@ REM This is a comment! Public Module foo2 + ''' + ''' Documented public API + ''' Public Class MyClazz Public ReadOnly Property MyProperty As Integer Get diff --git a/its/src/test/java/com/sonar/it/csharp/MetricsTest.java b/its/src/test/java/com/sonar/it/csharp/MetricsTest.java index 8be868f7607..d4829a93427 100644 --- a/its/src/test/java/com/sonar/it/csharp/MetricsTest.java +++ b/its/src/test/java/com/sonar/it/csharp/MetricsTest.java @@ -163,17 +163,17 @@ public void complexityInFunctionsAtFileLevel() { @Test public void linesAtProjectLevel() { - assertThat(getProjectMeasureAsInt("lines")).isEqualTo(99); + assertThat(getProjectMeasureAsInt("lines")).isEqualTo(105); } @Test public void linesAtDirectoryLevel() { - assertThat(getDirectoryMeasureAsInt("lines")).isEqualTo(66); + assertThat(getDirectoryMeasureAsInt("lines")).isEqualTo(70); } @Test public void linesAtFileLevel() { - assertThat(getFileMeasureAsInt("lines")).isEqualTo(33); + assertThat(getFileMeasureAsInt("lines")).isEqualTo(35); } /* Lines of code */ @@ -197,17 +197,17 @@ public void linesOfCodeAtFileLevel() { @Test public void commentLinesAtProjectLevel() { - assertThat(getProjectMeasureAsInt("comment_lines")).isEqualTo(6); + assertThat(getProjectMeasureAsInt("comment_lines")).isEqualTo(12); } @Test public void commentLinesAtDirectoryLevel() { - assertThat(getDirectoryMeasureAsInt("comment_lines")).isEqualTo(4); + assertThat(getDirectoryMeasureAsInt("comment_lines")).isEqualTo(8); } @Test public void commentLinesAtFileLevel() { - assertThat(getFileMeasureAsInt("comment_lines")).isEqualTo(2); + assertThat(getFileMeasureAsInt("comment_lines")).isEqualTo(4); } /* Functions */ @@ -296,20 +296,18 @@ public void linesOfCodeByLine() { assertThat(value).contains("9=1"); assertThat(value).contains("10=1"); - + assertThat(value).contains("11=1"); assertThat(value).contains("12=1"); - assertThat(value).contains("13=1"); - assertThat(value).contains("14=1"); - assertThat(value).contains("15=1"); + assertThat(value).contains("16=1"); assertThat(value).contains("17=1"); assertThat(value).contains("18=1"); assertThat(value).contains("19=1"); assertThat(value).contains("20=1"); assertThat(value).contains("21=1"); - + assertThat(value).contains("22=1"); assertThat(value).contains("23=1"); - assertThat(value).contains("24=1"); + assertThat(value).contains("25=1"); assertThat(value).contains("26=1"); assertThat(value).contains("27=1"); @@ -318,6 +316,8 @@ public void linesOfCodeByLine() { assertThat(value).contains("30=1"); assertThat(value).contains("31=1"); assertThat(value).contains("32=1"); + assertThat(value).contains("33=1"); + assertThat(value).contains("34=1"); assertThat(value.length()).isEqualTo(128); // No other line } @@ -329,9 +329,9 @@ public void executableLines() { String value = getFileMeasure("executable_lines_data").getValue(); - assertThat(value).contains("18=1"); - assertThat(value).contains("28=1"); - assertThat(value).contains("29=1"); + assertThat(value).contains("20=1"); + assertThat(value).contains("30=1"); + assertThat(value).contains("31=1"); assertThat(value.length()).isEqualTo(14); // No other lines } diff --git a/its/src/test/java/com/sonar/it/vbnet/MetricsTest.java b/its/src/test/java/com/sonar/it/vbnet/MetricsTest.java index 0bc5b8f4cba..0e89e3e2456 100644 --- a/its/src/test/java/com/sonar/it/vbnet/MetricsTest.java +++ b/its/src/test/java/com/sonar/it/vbnet/MetricsTest.java @@ -160,17 +160,17 @@ public void complexityInFunctionsAtFileLevel() { @Test public void linesAtProjectLevel() { - assertThat(getProjectMeasureAsInt("lines")).isEqualTo(60); + assertThat(getProjectMeasureAsInt("lines")).isEqualTo(69); } @Test public void linesAtDirectoryLevel() { - assertThat(getDirectoryMeasureAsInt("lines")).isEqualTo(40); + assertThat(getDirectoryMeasureAsInt("lines")).isEqualTo(46); } @Test public void linesAtFileLevel() { - assertThat(getFileMeasureAsInt("lines")).isEqualTo(20); + assertThat(getFileMeasureAsInt("lines")).isEqualTo(23); } /* Comment lines */ @@ -194,17 +194,17 @@ public void linesOfCodeAtFileLevel() { @Test public void commentLinesAtProjectLevel() { - assertThat(getProjectMeasureAsInt("comment_lines")).isEqualTo(3); + assertThat(getProjectMeasureAsInt("comment_lines")).isEqualTo(12); } @Test public void commentLinesAtDirectoryLevel() { - assertThat(getDirectoryMeasureAsInt("comment_lines")).isEqualTo(2); + assertThat(getDirectoryMeasureAsInt("comment_lines")).isEqualTo(8); } @Test public void commentLinesAtFileLevel() { - assertThat(getFileMeasureAsInt("comment_lines")).isEqualTo(1); + assertThat(getFileMeasureAsInt("comment_lines")).isEqualTo(4); } /* Classes */ @@ -295,22 +295,22 @@ public void linesOfCodeByLine() { assertThat(value).contains("1=1"); assertThat(value).contains("5=1"); - assertThat(value).contains("6=1"); - assertThat(value).contains("7=1"); - assertThat(value).contains("8=1"); assertThat(value).contains("9=1"); assertThat(value).contains("10=1"); assertThat(value).contains("11=1"); assertThat(value).contains("12=1"); + assertThat(value).contains("13=1"); assertThat(value).contains("14=1"); assertThat(value).contains("15=1"); - assertThat(value).contains("16=1"); assertThat(value).contains("17=1"); assertThat(value).contains("18=1"); assertThat(value).contains("19=1"); + assertThat(value).contains("20=1"); + assertThat(value).contains("21=1"); + assertThat(value).contains("22=1"); - assertThat(value.length()).isEqualTo(68); // No other line + assertThat(value.length()).isEqualTo(71); // No other line } private Measure getFileMeasure(String metricKey) { diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs new file mode 100644 index 00000000000..659cbf60f92 --- /dev/null +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs @@ -0,0 +1,73 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2018 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using SonarAnalyzer.Helpers; + +namespace SonarAnalyzer.Metrics +{ + public static class CSharpPublicApiMetric + { + public static ImmutableArray GetMembers(SyntaxTree syntaxTree) + { + var root = syntaxTree.GetRoot(); + var publicNodes = ImmutableArray.CreateBuilder(); + var toVisit = new Stack(); + + var members = root.ChildNodes() + .Where(childNode => childNode is MemberDeclarationSyntax); + foreach (var member in members) + { + toVisit.Push(member); + } + + while (toVisit.Any()) + { + var member = toVisit.Pop(); + + var isPublic = member.ChildTokens().AnyOfKind(SyntaxKind.PublicKeyword); + if (isPublic) + { + publicNodes.Add(member); + } + + if (!isPublic && + !member.IsKind(SyntaxKind.NamespaceDeclaration)) + { + continue; + } + + members = member.ChildNodes() + .Where(childNode => childNode is MemberDeclarationSyntax); + foreach (var child in members) + { + toVisit.Push(child); + } + } + + return publicNodes.ToImmutable(); + } + } +} diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs index 91e89fa2e03..4106824316c 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs @@ -34,6 +34,7 @@ namespace SonarAnalyzer.Metrics.CSharp public class Metrics : MetricsBase { private readonly SemanticModel semanticModel; + private readonly Lazy> publicApiNodes; public Metrics(SyntaxTree tree, SemanticModel semanticModel) : base(tree) @@ -45,6 +46,8 @@ public Metrics(SyntaxTree tree, SemanticModel semanticModel) } this.semanticModel = semanticModel; + + this.publicApiNodes = new Lazy>(() => CSharpPublicApiMetric.GetMembers(tree)); } public override ICollection ExecutableLines @@ -78,6 +81,23 @@ protected override bool IsCommentTrivia(SyntaxTrivia trivia) } } + protected override bool IsDocumentationCommentTrivia(SyntaxTrivia trivia) + { + switch (trivia.Kind()) + { + case SyntaxKind.SingleLineCommentTrivia: + case SyntaxKind.MultiLineCommentTrivia: + return trivia.ToString().TrimStart().StartsWith("///"); + + case SyntaxKind.SingleLineDocumentationCommentTrivia: + case SyntaxKind.MultiLineDocumentationCommentTrivia: + return true; + + default: + return false; + } + } + protected override bool IsClass(SyntaxNode node) { switch (node.Kind()) @@ -177,48 +197,8 @@ protected override bool IsFunction(SyntaxNode node) } } - protected override IEnumerable PublicApiNodes - { - get - { - var root = this.tree.GetRoot(); - var publicNodes = ImmutableArray.CreateBuilder(); - var toVisit = new Stack(); - - var members = root.ChildNodes() - .Where(childNode => childNode is MemberDeclarationSyntax); - foreach (var member in members) - { - toVisit.Push(member); - } - - while (toVisit.Any()) - { - var member = toVisit.Pop(); - - var isPublic = member.ChildTokens().AnyOfKind(SyntaxKind.PublicKeyword); - if (isPublic) - { - publicNodes.Add(member); - } - - if (!isPublic && - !member.IsKind(SyntaxKind.NamespaceDeclaration)) - { - continue; - } - - members = member.ChildNodes() - .Where(childNode => childNode is MemberDeclarationSyntax); - foreach (var child in members) - { - toVisit.Push(child); - } - } - - return publicNodes.ToImmutable(); - } - } + protected override ImmutableArray PublicApiNodes => + publicApiNodes.Value; public override int GetComplexity(SyntaxNode node) { diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Metrics/MetricsBase.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Metrics/MetricsBase.cs index 3ece7f99ccc..0239d5c080b 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Metrics/MetricsBase.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Metrics/MetricsBase.cs @@ -20,6 +20,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; using SonarAnalyzer.Helpers; @@ -126,6 +127,8 @@ private static void CategorizeLines(string line, int lineNumber, HashSet no protected abstract bool IsCommentTrivia(SyntaxTrivia trivia); + protected abstract bool IsDocumentationCommentTrivia(SyntaxTrivia trivia); + #endregion Comments #region Classes, Accessors, Functions, Statements @@ -152,9 +155,9 @@ private static void CategorizeLines(string line, int lineNumber, HashSet no public int PublicApiCount => PublicApiNodes.Count(); - public int PublicUndocumentedApiCount => PublicApiNodes.Count(n => !n.GetLeadingTrivia().Any(IsCommentTrivia)); + public int PublicUndocumentedApiCount => PublicApiNodes.Count(n => !n.GetLeadingTrivia().Any(IsDocumentationCommentTrivia)); - protected abstract IEnumerable PublicApiNodes { get; } + protected abstract ImmutableArray PublicApiNodes { get; } #endregion PublicApi diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs index e749999874f..86d5dc26e9a 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs @@ -30,7 +30,7 @@ namespace SonarAnalyzer.Common.VisualBasic { public sealed class Metrics : MetricsBase { - private readonly Lazy> publicApis; + private readonly Lazy> publicApiNodes; public Metrics(SyntaxTree tree) : base(tree) @@ -41,7 +41,7 @@ public Metrics(SyntaxTree tree) throw new ArgumentException(InitalizationErrorTextPattern, nameof(tree)); } - publicApis = new Lazy>(() => PublicApiMetric.GetMembers(tree)); + publicApiNodes = new Lazy>(() => VisualBasicPublicApiMetric.GetMembers(tree)); } protected override bool IsEndOfFile(SyntaxToken token) => @@ -64,6 +64,19 @@ protected override bool IsCommentTrivia(SyntaxTrivia trivia) } } + protected override bool IsDocumentationCommentTrivia(SyntaxTrivia trivia) + { + switch (trivia.Kind()) + { + case SyntaxKind.DocumentationCommentExteriorTrivia: + case SyntaxKind.DocumentationCommentTrivia: + return true; + + default: + return false; + } + } + protected override bool IsClass(SyntaxNode node) { switch (node.Kind()) @@ -99,8 +112,8 @@ protected override bool IsFunction(SyntaxNode node) return true; } - protected override IEnumerable PublicApiNodes => - publicApis.Value; + protected override ImmutableArray PublicApiNodes => + publicApiNodes.Value; private bool IsComplexityIncreasingKind(SyntaxNode node) { diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/PublicApiMetric.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/VisualBasicPublicApiMetric.cs similarity index 98% rename from sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/PublicApiMetric.cs rename to sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/VisualBasicPublicApiMetric.cs index fe9eebe09a2..d1d567319f3 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/PublicApiMetric.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/VisualBasicPublicApiMetric.cs @@ -26,7 +26,7 @@ namespace SonarAnalyzer.Common.VisualBasic { - public static class PublicApiMetric + public static class VisualBasicPublicApiMetric { public static ImmutableArray GetMembers(SyntaxTree syntaxTree) { diff --git a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Common/MetricsTest.cs b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Common/MetricsTest.cs index a531cf9d279..189ef773aa5 100644 --- a/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Common/MetricsTest.cs +++ b/sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Common/MetricsTest.cs @@ -415,11 +415,26 @@ public void PublicUndocumentedApi() PublicUndocumentedApi(AnalyzerLanguage.CSharp, "").Should().Be(0); PublicUndocumentedApi(AnalyzerLanguage.CSharp, "class MyClass { }").Should().Be(0); PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { }").Should().Be(1); - PublicUndocumentedApi(AnalyzerLanguage.CSharp, "/* ... */ public class MyClass { }").Should().Be(0); - PublicUndocumentedApi(AnalyzerLanguage.CSharp, "// ... \n public class MyClass { }").Should().Be(0); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "//... \n public class MyClass { }").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "/* ... */ public class MyClass { }").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "///... \n public class MyClass { }").Should().Be(0); // Count regular comments starting with /// as documentation comments + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "/// ... \n public class MyClass { }").Should().Be(0); PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { \n public int MyField; }").Should().Be(2); - PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { \n /* ... */ public int MyField; }").Should().Be(1); - PublicUndocumentedApi(AnalyzerLanguage.CSharp, "/// ... \n public class MyClass { \n /* ... */ public int MyField; }").Should().Be(0); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { \n /*...*/ \n public int MyField; }").Should().Be(2); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { \n //... \n public int MyField; }").Should().Be(2); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { \n /// ... \n public int MyField; }").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "public class MyClass { \n /// ... \n public int MyField; }").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.CSharp, "/// ... \n public class MyClass { \n /// ... \n public int MyField; }").Should().Be(0); + + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "").Should().Be(0); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "Class [MyClass] \n End Class").Should().Be(0); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "Public Class [MyClass] \n End Class").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "'... \n Public Class [MyClass] \n End Class").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "REM... \n Public Class [MyClass] \n End Class").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "'''... \n Public Class [MyClass] \n End Class").Should().Be(0); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "Public Class [MyClass] \n Public MyField As Integer \n End Class").Should().Be(2); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "Public Class [MyClass] \n '''... \n Public MyField As Integer \n End Class").Should().Be(1); + PublicUndocumentedApi(AnalyzerLanguage.VisualBasic, "'''... \n Public Class [MyClass] \n '''... \n Public MyField As Integer \n End Class").Should().Be(0); } private static int PublicUndocumentedApi(AnalyzerLanguage language, string text) => MetricsFor(language, text).PublicUndocumentedApiCount; From e4a31e9ce5c23e3c23ea901212b9c0acfbed28af Mon Sep 17 00:00:00 2001 From: Valeri Hristov Date: Thu, 13 Dec 2018 10:54:19 +0100 Subject: [PATCH 2/2] Addressing review comments --- .../Metrics/CSharpPublicApiMetric.cs | 27 +++++++++---------- .../SonarAnalyzer.CSharp/Metrics/Metrics.cs | 5 +++- .../Metrics/Metrics.cs | 1 + 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs index 659cbf60f92..41c0c343bde 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/CSharpPublicApiMetric.cs @@ -36,12 +36,7 @@ public static ImmutableArray GetMembers(SyntaxTree syntaxTree) var publicNodes = ImmutableArray.CreateBuilder(); var toVisit = new Stack(); - var members = root.ChildNodes() - .Where(childNode => childNode is MemberDeclarationSyntax); - foreach (var member in members) - { - toVisit.Push(member); - } + PushChildMembers(root, toVisit); while (toVisit.Any()) { @@ -52,22 +47,24 @@ public static ImmutableArray GetMembers(SyntaxTree syntaxTree) { publicNodes.Add(member); } - - if (!isPublic && - !member.IsKind(SyntaxKind.NamespaceDeclaration)) + else if (!member.IsKind(SyntaxKind.NamespaceDeclaration)) { continue; } - members = member.ChildNodes() - .Where(childNode => childNode is MemberDeclarationSyntax); - foreach (var child in members) - { - toVisit.Push(child); - } + PushChildMembers(member, toVisit); } return publicNodes.ToImmutable(); } + + private static void PushChildMembers(SyntaxNode syntaxNode, Stack stack) + { + var members = syntaxNode.ChildNodes().OfType(); + foreach (var member in members) + { + stack.Push(member); + } + } } } diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs index 4106824316c..16e9458b58e 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Metrics/Metrics.cs @@ -86,7 +86,10 @@ protected override bool IsDocumentationCommentTrivia(SyntaxTrivia trivia) switch (trivia.Kind()) { case SyntaxKind.SingleLineCommentTrivia: - case SyntaxKind.MultiLineCommentTrivia: + // Roslyn does not recognize the C# "documentation" comment trivia as such + // unless you provide "/p:DocumentationFile=foo.xml" parameter to MSBuild. + // In case the documentation is not generated, we try to guess if some of + // the existing "normal" comments are actually documentation. return trivia.ToString().TrimStart().StartsWith("///"); case SyntaxKind.SingleLineDocumentationCommentTrivia: diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs index 86d5dc26e9a..f8d31c1deb1 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Metrics/Metrics.cs @@ -68,6 +68,7 @@ protected override bool IsDocumentationCommentTrivia(SyntaxTrivia trivia) { switch (trivia.Kind()) { + // Contrary to C#, VB.NET seems to always recognize the documentation comments. case SyntaxKind.DocumentationCommentExteriorTrivia: case SyntaxKind.DocumentationCommentTrivia: return true;