From d60e34351ae37e034eea1c2eb527e80ce74792e7 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 24 Jan 2023 13:35:30 +0100 Subject: [PATCH 01/27] rspec scaffolding --- analyzers/rspec/cs/S4663_c#.html | 16 +++++++ analyzers/rspec/cs/S4663_c#.json | 15 ++++++ analyzers/rspec/cs/Sonar_way_profile.json | 1 + analyzers/rspec/vbnet/S4663_vb.net.html | 8 ++++ analyzers/rspec/vbnet/S4663_vb.net.json | 15 ++++++ analyzers/rspec/vbnet/Sonar_way_profile.json | 1 + .../Rules/CommentsShouldNotBeEmpty.cs | 48 +++++++++++++++++++ .../Rules/CommentsShouldNotBeEmptyBase.cs | 36 ++++++++++++++ .../Rules/CommentsShouldNotBeEmpty.cs | 48 +++++++++++++++++++ .../PackagingTests/RuleTypeMappingCS.cs | 2 +- .../PackagingTests/RuleTypeMappingVB.cs | 2 +- .../Rules/CommentsShouldNotBeEmptyTest.cs | 41 ++++++++++++++++ .../TestCases/CommentsShouldNotBeEmpty.cs | 5 ++ .../TestCases/CommentsShouldNotBeEmpty.vb | 7 +++ 14 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 analyzers/rspec/cs/S4663_c#.html create mode 100644 analyzers/rspec/cs/S4663_c#.json create mode 100644 analyzers/rspec/vbnet/S4663_vb.net.html create mode 100644 analyzers/rspec/vbnet/S4663_vb.net.json create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs create mode 100644 analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs create mode 100644 analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb diff --git a/analyzers/rspec/cs/S4663_c#.html b/analyzers/rspec/cs/S4663_c#.html new file mode 100644 index 00000000000..a250d368623 --- /dev/null +++ b/analyzers/rspec/cs/S4663_c#.html @@ -0,0 +1,16 @@ +

An empty comment is likely to be a mistake and doesn’t help to improve the readability of the code. For these reasons, it should be removed.

+

Noncompliant Code Example

+
+//
+
+///
+
+/**
+
+ */
+
+/*
+
+ */
+
+ diff --git a/analyzers/rspec/cs/S4663_c#.json b/analyzers/rspec/cs/S4663_c#.json new file mode 100644 index 00000000000..05e82bc91e7 --- /dev/null +++ b/analyzers/rspec/cs/S4663_c#.json @@ -0,0 +1,15 @@ +{ + "title": "Comments should not be empty", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "1min" + }, + "tags": [], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-4663", + "sqKey": "S4663", + "scope": "Main", + "quickfix": "unknown" +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index 444cf4210f6..5879a1c7933 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -241,6 +241,7 @@ "S4583", "S4586", "S4635", + "S4663", "S4790", "S4792", "S4830", diff --git a/analyzers/rspec/vbnet/S4663_vb.net.html b/analyzers/rspec/vbnet/S4663_vb.net.html new file mode 100644 index 00000000000..eac2d8bb6d4 --- /dev/null +++ b/analyzers/rspec/vbnet/S4663_vb.net.html @@ -0,0 +1,8 @@ +

An empty comment is likely to be a mistake and doesn’t help to improve the readability of the code. For these reasons, it should be removed.

+

Noncompliant Code Example

+
+'
+
+'''
+
+ diff --git a/analyzers/rspec/vbnet/S4663_vb.net.json b/analyzers/rspec/vbnet/S4663_vb.net.json new file mode 100644 index 00000000000..05e82bc91e7 --- /dev/null +++ b/analyzers/rspec/vbnet/S4663_vb.net.json @@ -0,0 +1,15 @@ +{ + "title": "Comments should not be empty", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "1min" + }, + "tags": [], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-4663", + "sqKey": "S4663", + "scope": "Main", + "quickfix": "unknown" +} diff --git a/analyzers/rspec/vbnet/Sonar_way_profile.json b/analyzers/rspec/vbnet/Sonar_way_profile.json index 11efa1e3eb7..46bcb99963e 100644 --- a/analyzers/rspec/vbnet/Sonar_way_profile.json +++ b/analyzers/rspec/vbnet/Sonar_way_profile.json @@ -108,6 +108,7 @@ "S4581", "S4583", "S4586", + "S4663", "S4790", "S4792", "S4830", diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs new file mode 100644 index 00000000000..aff2cc33707 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -0,0 +1,48 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 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.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using SonarAnalyzer.Common; +using SonarAnalyzer.Helpers; + +namespace SonarAnalyzer.Rules.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase +{ + + protected override ILanguageFacade Language => CSharpFacade.Instance; + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterSyntaxNodeActionInNonGenerated(c => + { + var node = c.Node; + if (true) + { + c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation())); + } + }, + SyntaxKind.InvocationExpression); +} diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs new file mode 100644 index 00000000000..c75b05d2f95 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -0,0 +1,36 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 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.Immutable; +using Microsoft.CodeAnalysis; +using SonarAnalyzer.Common; +using SonarAnalyzer.Helpers; + +namespace SonarAnalyzer.Rules; + +public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosticAnalyzer + where TSyntaxKind : struct +{ + private const string DiagnosticId = "S4663"; + + protected override string MessageFormat => "FIXME"; + + protected CommentsShouldNotBeEmptyBase() : base(DiagnosticId) { } +} diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs new file mode 100644 index 00000000000..8e8ec8e5086 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -0,0 +1,48 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 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.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.VisualBasic; +using Microsoft.CodeAnalysis.VisualBasic.Syntax; +using SonarAnalyzer.Common; +using SonarAnalyzer.Helpers; + +namespace SonarAnalyzer.Rules.VisualBasic; + +[DiagnosticAnalyzer(LanguageNames.VisualBasic)] +public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase +{ + + protected override ILanguageFacade Language => VisualBasicFacade.Instance; + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterSyntaxNodeActionInNonGenerated(c => + { + var node = c.Node; + if (true) + { + c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation())); + } + }, + SyntaxKind.InvocationExpression); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs index 0cb1259d864..1f44dcb4ef6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs @@ -4587,7 +4587,7 @@ internal static class RuleTypeMappingCS // ["S4660"], // ["S4661"], // ["S4662"], - // ["S4663"], + ["S4663"] = "CODE_SMELL", // ["S4664"], // ["S4665"], // ["S4666"], diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingVB.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingVB.cs index 755f2ef4aa9..54584469d62 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingVB.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingVB.cs @@ -4587,7 +4587,7 @@ internal static class RuleTypeMappingVB // ["S4660"], // ["S4661"], // ["S4662"], - // ["S4663"], + ["S4663"] = "CODE_SMELL", // ["S4664"], // ["S4665"], // ["S4666"], diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs new file mode 100644 index 00000000000..3730eeed145 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs @@ -0,0 +1,41 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 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 CS = SonarAnalyzer.Rules.CSharp; +using VB = SonarAnalyzer.Rules.VisualBasic; + +namespace SonarAnalyzer.UnitTest.Rules; + +[TestClass] +public class CommentsShouldNotBeEmptyTest +{ + private readonly VerifierBuilder builderCS = new VerifierBuilder(); + + [TestMethod] + public void CommentsShouldNotBeEmpty_CS() => + builderCS.AddPaths("CommentsShouldNotBeEmpty.cs").Verify(); + + private readonly VerifierBuilder builderVB = new VerifierBuilder(); // FIXME: Move this up + + [TestMethod] + public void CommentsShouldNotBeEmpty_VB() => + builderVB.AddPaths("CommentsShouldNotBeEmpty.vb").Verify(); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs new file mode 100644 index 00000000000..a790d6d1f92 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -0,0 +1,5 @@ +using System; + +public class Program +{ +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb new file mode 100644 index 00000000000..f5b9e9276c6 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb @@ -0,0 +1,7 @@ +Public Class Program + + Public Sub Test() + + End Sub + +End Class From 7bc733eae30246eab47e298212b25e3b446feb90 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 24 Jan 2023 14:12:15 +0100 Subject: [PATCH 02/27] Remove unnecessary usings --- .../Rules/CommentsShouldNotBeEmpty.cs | 11 +---------- .../Rules/CommentsShouldNotBeEmptyBase.cs | 5 ----- .../Rules/CommentsShouldNotBeEmpty.cs | 11 +---------- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index aff2cc33707..2a19544af35 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -18,15 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Collections.Immutable; -using System.Linq; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; -using SonarAnalyzer.Common; -using SonarAnalyzer.Helpers; - namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -36,7 +27,7 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => CSharpFacade.Instance; protected override void Initialize(SonarAnalysisContext context) => - context.RegisterSyntaxNodeActionInNonGenerated(c => + context.RegisterNodeAction(c => { var node = c.Node; if (true) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index c75b05d2f95..d02d836c767 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -18,11 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Collections.Immutable; -using Microsoft.CodeAnalysis; -using SonarAnalyzer.Common; -using SonarAnalyzer.Helpers; - namespace SonarAnalyzer.Rules; public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosticAnalyzer diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 8e8ec8e5086..e8287b91f12 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -18,15 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Collections.Immutable; -using System.Linq; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.VisualBasic; -using Microsoft.CodeAnalysis.VisualBasic.Syntax; -using SonarAnalyzer.Common; -using SonarAnalyzer.Helpers; - namespace SonarAnalyzer.Rules.VisualBasic; [DiagnosticAnalyzer(LanguageNames.VisualBasic)] @@ -36,7 +27,7 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => VisualBasicFacade.Instance; protected override void Initialize(SonarAnalysisContext context) => - context.RegisterSyntaxNodeActionInNonGenerated(c => + context.RegisterNodeAction(c => { var node = c.Node; if (true) From 697938d87dbed025872a09b356dbf6b878516bc9 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 25 Jan 2023 09:37:49 +0100 Subject: [PATCH 03/27] Create initial skeleton for logic --- .../Rules/CommentsShouldNotBeEmpty.cs | 39 ++++++++++++++----- .../Rules/CommentsShouldNotBeEmptyBase.cs | 13 ++++++- .../Rules/CommentsShouldNotBeEmpty.cs | 25 +++++++----- .../Rules/CommentsShouldNotBeEmptyTest.cs | 3 +- .../TestCases/CommentsShouldNotBeEmpty.cs | 6 ++- 5 files changed, 62 insertions(+), 24 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index 2a19544af35..4c266ec59f8 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -18,22 +18,41 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis.CSharp; + namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase { - protected override ILanguageFacade Language => CSharpFacade.Instance; - protected override void Initialize(SonarAnalysisContext context) => - context.RegisterNodeAction(c => + protected override void CheckTrivia(IEnumerable trivia) + { + foreach (var trivium in trivia) + { + switch (trivium.Kind()) { - var node = c.Node; - if (true) - { - c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation())); - } - }, - SyntaxKind.InvocationExpression); + case SyntaxKind.SingleLineCommentTrivia: // usecase: // + var vv1 = trivium.ToFullString(); + break; + case SyntaxKind.MultiLineCommentTrivia: // usecase: /* [many lines...] */ + var vv2 = trivium.ToFullString(); + break; + case SyntaxKind.SingleLineDocumentationCommentTrivia: // usecase: /// + var vv3 = trivium.ToFullString(); + break; + case SyntaxKind.MultiLineDocumentationCommentTrivia: // usecase: /** + var vv4 = trivium.ToFullString(); + break; + default: + break; + } + } + } + + void DoWork() + { + + } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index d02d836c767..37f92d7454f 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -24,8 +24,19 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti where TSyntaxKind : struct { private const string DiagnosticId = "S4663"; - protected override string MessageFormat => "FIXME"; protected CommentsShouldNotBeEmptyBase() : base(DiagnosticId) { } + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterTreeAction(Language.GeneratedCodeRecognizer, c => + { + foreach (var token in c.Tree.GetRoot().DescendantTokens()) + { + CheckTrivia(token.LeadingTrivia); + CheckTrivia(token.TrailingTrivia); + } + }); + + protected abstract void CheckTrivia(IEnumerable trivia); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index e8287b91f12..2740ffc1b8b 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -23,17 +23,22 @@ namespace SonarAnalyzer.Rules.VisualBasic; [DiagnosticAnalyzer(LanguageNames.VisualBasic)] public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase { - protected override ILanguageFacade Language => VisualBasicFacade.Instance; - protected override void Initialize(SonarAnalysisContext context) => - context.RegisterNodeAction(c => + protected override void CheckTrivia(IEnumerable trivia) + { + // FIXME + foreach (var trivium in trivia) + { + switch (trivium.Kind()) { - var node = c.Node; - if (true) - { - c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation())); - } - }, - SyntaxKind.InvocationExpression); + case SyntaxKind.CommentTrivia: // usecase: ' + var x = 1; + break; + case SyntaxKind.DocumentationCommentTrivia: // usecase: ''' + default: + break; + } + } + } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs index 3730eeed145..c0d93b894be 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs @@ -28,13 +28,12 @@ namespace SonarAnalyzer.UnitTest.Rules; public class CommentsShouldNotBeEmptyTest { private readonly VerifierBuilder builderCS = new VerifierBuilder(); + private readonly VerifierBuilder builderVB = new VerifierBuilder(); [TestMethod] public void CommentsShouldNotBeEmpty_CS() => builderCS.AddPaths("CommentsShouldNotBeEmpty.cs").Verify(); - private readonly VerifierBuilder builderVB = new VerifierBuilder(); // FIXME: Move this up - [TestMethod] public void CommentsShouldNotBeEmpty_VB() => builderVB.AddPaths("CommentsShouldNotBeEmpty.vb").Verify(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index a790d6d1f92..259a3b3651b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -1,5 +1,9 @@ using System; -public class Program +// +// +// a +// +public class SingleLineComment { } From 3b6e421718cec8764fe1613446dfb579a78ee88b Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 25 Jan 2023 10:03:02 +0100 Subject: [PATCH 04/27] Implement the simplest case and some testcases, minor refactoring --- .../Rules/CommentsShouldNotBeEmpty.cs | 10 ++++++--- .../Rules/CommentsShouldNotBeEmptyBase.cs | 8 +++---- .../Rules/CommentsShouldNotBeEmpty.cs | 2 +- .../TestCases/CommentsShouldNotBeEmpty.cs | 22 ++++++++++++++----- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index 4c266ec59f8..79547f00df4 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -27,14 +27,18 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => CSharpFacade.Instance; - protected override void CheckTrivia(IEnumerable trivia) + protected override void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { foreach (var trivium in trivia) { switch (trivium.Kind()) { - case SyntaxKind.SingleLineCommentTrivia: // usecase: // - var vv1 = trivium.ToFullString(); + case SyntaxKind.SingleLineCommentTrivia: + var vv1 = trivium.ToFullString().Trim(); + if (string.CompareOrdinal(vv1, "//") == 0) + { + context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); + } break; case SyntaxKind.MultiLineCommentTrivia: // usecase: /* [many lines...] */ var vv2 = trivium.ToFullString(); diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 37f92d7454f..9addfce0441 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -24,7 +24,7 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti where TSyntaxKind : struct { private const string DiagnosticId = "S4663"; - protected override string MessageFormat => "FIXME"; + protected override string MessageFormat => "Remove this empty comment"; protected CommentsShouldNotBeEmptyBase() : base(DiagnosticId) { } @@ -33,10 +33,10 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti { foreach (var token in c.Tree.GetRoot().DescendantTokens()) { - CheckTrivia(token.LeadingTrivia); - CheckTrivia(token.TrailingTrivia); + CheckTrivia(c, token.LeadingTrivia); + CheckTrivia(c, token.TrailingTrivia); } }); - protected abstract void CheckTrivia(IEnumerable trivia); + protected abstract void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 2740ffc1b8b..741da9e7710 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -25,7 +25,7 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => VisualBasicFacade.Instance; - protected override void CheckTrivia(IEnumerable trivia) + protected override void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { // FIXME foreach (var trivium in trivia) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index 259a3b3651b..df15049272e 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -1,9 +1,21 @@ using System; -// -// -// a -// -public class SingleLineComment +[Obsolete] // Ipsem Lorum +public class SimpleLineComment // +// Noncompliant@-1 (inline comment) { + // +// +// Noncompliant@-2 (a lot of whitespace before) +// Noncompliant@-2 (a lot of whitespace after) + +// * +// Ipsem Lorum +// \n +// +// Noncompliant @-1 + +// + +// Noncompliant @-2 } From 270b503d2f5a505a165d1e4131f88a4334955104 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 25 Jan 2023 11:03:01 +0100 Subject: [PATCH 05/27] Implement single-line documentation comments --- .../Rules/CommentsShouldNotBeEmpty.cs | 42 ++++++++++---- .../TestCases/CommentsShouldNotBeEmpty.cs | 56 ++++++++++++++++++- 2 files changed, 87 insertions(+), 11 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index 79547f00df4..1b3b1db4842 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -34,20 +34,16 @@ protected override void CheckTrivia(SonarSyntaxTreeReportingContext context, IEn switch (trivium.Kind()) { case SyntaxKind.SingleLineCommentTrivia: - var vv1 = trivium.ToFullString().Trim(); - if (string.CompareOrdinal(vv1, "//") == 0) - { - context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); - } + CheckSingleLineTrivia(context, trivium); break; case SyntaxKind.MultiLineCommentTrivia: // usecase: /* [many lines...] */ - var vv2 = trivium.ToFullString(); + CheckMultiLineTrivia(context, trivium); break; - case SyntaxKind.SingleLineDocumentationCommentTrivia: // usecase: /// - var vv3 = trivium.ToFullString(); + case SyntaxKind.SingleLineDocumentationCommentTrivia: + CheckSingleLineDocumentationTrivia(context, trivium); break; case SyntaxKind.MultiLineDocumentationCommentTrivia: // usecase: /** - var vv4 = trivium.ToFullString(); + CheckMultiLineDocumentationTrivia(context, trivium); break; default: break; @@ -55,7 +51,33 @@ protected override void CheckTrivia(SonarSyntaxTreeReportingContext context, IEn } } - void DoWork() + private void CheckSingleLineTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) + { + if (string.CompareOrdinal(trivium.ToFullString().Trim(), "//") == 0) + { + context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); + } + } + + private void CheckMultiLineTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) + { + + } + + private void CheckSingleLineDocumentationTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) + { + var lines = trivium.ToFullString().Split(MetricsBase.LineTerminators, StringSplitOptions.None); + foreach (var line in lines.Take(lines.Length - 1)) // last entry is empty string after newline + { + if (string.CompareOrdinal(line.Trim(), "///") != 0) + { + return; + } + } + context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); + } + + private void CheckMultiLineDocumentationTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) { } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index df15049272e..2f7498fc25f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -1,7 +1,7 @@ using System; [Obsolete] // Ipsem Lorum -public class SimpleLineComment // +public class SingleLineComment // // Noncompliant@-1 (inline comment) { // @@ -18,4 +18,58 @@ public class SimpleLineComment // // // Noncompliant @-2 + +// /**/ + +// /// + +// // + +// /** */ +} + +[Obsolete] /// Ipsem Lorum +public class SingleLineDocumentationComment /// +// Noncompliant@-1 (inline comment) +{ + /// + // Noncompliant@-1 (a lot of whitespace before) + + /// + // Noncompliant@-1 (a lot of whitespace after) + + /// + /// * + /// Ipsem Lorum + /// \\n + /// + + /// + /// hey there + + /// hey there + /// + + /// /// + + /// // + + /// /* */ + + /// /** */ + + /// \r + + /// \n + + /// \r\n + + /// \t + + /// z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ + + /// + /// + /// + // Noncompliant @-3 } From 397f3ce5f8a562c7599e4ffa85a5603bb9fae3ac Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 25 Jan 2023 15:05:23 +0100 Subject: [PATCH 06/27] Add Comment SyntaxTrivia Kinds on the Facades --- .../SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs | 7 +++++++ .../src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs | 1 + .../Facade/VisualBasicSyntaxKindFacade.cs | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs index 0a97a8d62a8..970eb33bbf2 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs @@ -29,6 +29,13 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade SyntaxKindEx.RecordClassDeclaration, }; public SyntaxKind ClassDeclaration => SyntaxKind.ClassDeclaration; + public SyntaxKind[] CommentTrivia => new[] + { + SyntaxKind.SingleLineCommentTrivia, + SyntaxKind.MultiLineCommentTrivia, + SyntaxKind.SingleLineDocumentationCommentTrivia, + SyntaxKind.MultiLineDocumentationCommentTrivia, + }; public SyntaxKind[] ComparisonKinds => new[] { SyntaxKind.GreaterThanExpression, diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs index 3d8a0052fc4..b64ade32ed1 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs @@ -26,6 +26,7 @@ public interface ISyntaxKindFacade abstract TSyntaxKind Attribute { get; } abstract TSyntaxKind[] ClassAndRecordDeclaration { get; } abstract TSyntaxKind ClassDeclaration { get; } + abstract TSyntaxKind[] CommentTrivia { get; } abstract TSyntaxKind[] ComparisonKinds { get; } abstract TSyntaxKind ConstructorDeclaration { get; } abstract TSyntaxKind[] DefaultExpressions { get; } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs index 602fd70132b..e961c8afaf3 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs @@ -25,6 +25,11 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.Attribute; public SyntaxKind[] ClassAndRecordDeclaration => new[] { SyntaxKind.ClassBlock }; public SyntaxKind ClassDeclaration => SyntaxKind.ClassBlock; + public SyntaxKind[] CommentTrivia => new[] + { + SyntaxKind.CommentTrivia, + SyntaxKind.DocumentationCommentTrivia, + }; public SyntaxKind[] ComparisonKinds => new[] { SyntaxKind.GreaterThanExpression, From c6e115593f82b72f8fd53946e10c9ebb0a87ad08 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 25 Jan 2023 15:10:46 +0100 Subject: [PATCH 07/27] Refactor CommentsShouldNotBeEmpty --- .../Rules/CommentsShouldNotBeEmpty.cs | 96 ++++++++++++------- .../Rules/CommentsShouldNotBeEmptyBase.cs | 15 ++- .../Rules/CommentsShouldNotBeEmpty.cs | 40 +++++--- 3 files changed, 104 insertions(+), 47 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index 1b3b1db4842..d1f9ef374bf 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -18,7 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.CodeAnalysis.CSharp; +using System.Text; namespace SonarAnalyzer.Rules.CSharp; @@ -27,58 +27,86 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => CSharpFacade.Instance; - protected override void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) + protected override string GetCommentText(SyntaxTrivia trivia) + => trivia.Kind() switch + { + SyntaxKind.SingleLineCommentTrivia => GetSingleLineText(trivia), + SyntaxKind.MultiLineCommentTrivia => GetMultiLineText(trivia), + SyntaxKind.SingleLineDocumentationCommentTrivia => GetSingleLineDocumentationText(trivia), + SyntaxKind.MultiLineDocumentationCommentTrivia => GetMultiLineDocumentationText(trivia), + }; + + protected override bool IsValidTriviaType(SyntaxTrivia trivia) + => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + + // // + private static string GetSingleLineText(SyntaxTrivia trivia) + => trivia.ToString().Trim().Substring(2); + + // /* */ + private static string GetMultiLineText(SyntaxTrivia trivia) { - foreach (var trivium in trivia) + var stringBuilder = new StringBuilder(); + var commentText = trivia.ToString().Trim().Substring(2); + + if (commentText.EndsWith("*/", StringComparison.Ordinal)) // Might be unclosed, still reported { - switch (trivium.Kind()) + commentText = commentText.Substring(0, commentText.Length - 2); + } + + foreach (var line in commentText.Split(MetricsBase.LineTerminators, StringSplitOptions.None)) + { + var trimmedLine = line.TrimStart(null); + if (trimmedLine.StartsWith("*", StringComparison.Ordinal)) { - case SyntaxKind.SingleLineCommentTrivia: - CheckSingleLineTrivia(context, trivium); - break; - case SyntaxKind.MultiLineCommentTrivia: // usecase: /* [many lines...] */ - CheckMultiLineTrivia(context, trivium); - break; - case SyntaxKind.SingleLineDocumentationCommentTrivia: - CheckSingleLineDocumentationTrivia(context, trivium); - break; - case SyntaxKind.MultiLineDocumentationCommentTrivia: // usecase: /** - CheckMultiLineDocumentationTrivia(context, trivium); - break; - default: - break; + trimmedLine = trimmedLine.TrimStart('*'); } + + stringBuilder.Append(trimmedLine.Trim()); } + + return stringBuilder.ToString(); } - private void CheckSingleLineTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) + // /// + private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) { - if (string.CompareOrdinal(trivium.ToFullString().Trim(), "//") == 0) + var stringBuilder = new StringBuilder(); + + foreach (var line in trivia.ToFullString().Split(MetricsBase.LineTerminators, StringSplitOptions.None)) { - context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); + var trimmedLine = line.TrimStart(null); + trimmedLine = trimmedLine.StartsWith("///") + ? trimmedLine.Substring(3).Trim() + : trimmedLine.TrimEnd(null); + + stringBuilder.Append(trimmedLine); } + return stringBuilder.ToString(); } - private void CheckMultiLineTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) + // /** */ + private static string GetMultiLineDocumentationText(SyntaxTrivia trivia) { + var stringBuilder = new StringBuilder(); + var commentText = trivia.ToFullString().Trim().Substring(3); - } + if (commentText.EndsWith("*/", StringComparison.Ordinal)) // Might be unclosed, still reported + { + commentText = commentText.Substring(0, commentText.Length - 2); + } - private void CheckSingleLineDocumentationTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) - { - var lines = trivium.ToFullString().Split(MetricsBase.LineTerminators, StringSplitOptions.None); - foreach (var line in lines.Take(lines.Length - 1)) // last entry is empty string after newline + foreach (var line in commentText.Split(MetricsBase.LineTerminators, StringSplitOptions.None)) { - if (string.CompareOrdinal(line.Trim(), "///") != 0) + var trimmedLine = line.TrimStart(null); + if (trimmedLine.StartsWith("*", StringComparison.Ordinal)) { - return; + trimmedLine = trimmedLine.TrimStart('*'); } - } - context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); - } - private void CheckMultiLineDocumentationTrivia(SonarSyntaxTreeReportingContext context, SyntaxTrivia trivium) - { + stringBuilder.Append(trimmedLine.Trim()); + } + return stringBuilder.ToString(); } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 9addfce0441..c490ab963a0 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -24,6 +24,10 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti where TSyntaxKind : struct { private const string DiagnosticId = "S4663"; + + protected abstract bool IsValidTriviaType(SyntaxTrivia trivia); + protected abstract string GetCommentText(SyntaxTrivia trivia); + protected override string MessageFormat => "Remove this empty comment"; protected CommentsShouldNotBeEmptyBase() : base(DiagnosticId) { } @@ -38,5 +42,14 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti } }); - protected abstract void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia); + protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) + { + foreach (var trivium in trivia.Where(ShouldReport)) + { + context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); + } + + bool ShouldReport(SyntaxTrivia trivia) + => IsValidTriviaType(trivia) && string.IsNullOrEmpty(GetCommentText(trivia)); + } } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 741da9e7710..2eed2c7026d 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.Text; + namespace SonarAnalyzer.Rules.VisualBasic; [DiagnosticAnalyzer(LanguageNames.VisualBasic)] @@ -25,20 +27,34 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => VisualBasicFacade.Instance; - protected override void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) + protected override string GetCommentText(SyntaxTrivia trivia) + => trivia.Kind() switch + { + SyntaxKind.CommentTrivia => GetText(trivia), + SyntaxKind.DocumentationCommentTrivia => GetDocumentationText(trivia), + }; + + protected override bool IsValidTriviaType(SyntaxTrivia trivia) + => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + + // ' + private static string GetText(SyntaxTrivia trivia) + => trivia.ToString().Trim().Substring(1); + + // ''' + private static string GetDocumentationText(SyntaxTrivia trivia) { - // FIXME - foreach (var trivium in trivia) + var stringBuilder = new StringBuilder(); + + foreach (var line in trivia.ToFullString().Split(MetricsBase.LineTerminators, StringSplitOptions.None)) { - switch (trivium.Kind()) - { - case SyntaxKind.CommentTrivia: // usecase: ' - var x = 1; - break; - case SyntaxKind.DocumentationCommentTrivia: // usecase: ''' - default: - break; - } + var trimmedLine = line.TrimStart(null); + trimmedLine = trimmedLine.StartsWith("'''") + ? trimmedLine.Substring(3).Trim() + : trimmedLine.TrimEnd(null); + + stringBuilder.Append(trimmedLine); } + return stringBuilder.ToString(); } } From 044dc9839fc0806426007fd60f9fc8766ec0d8ec Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Wed, 25 Jan 2023 15:11:06 +0100 Subject: [PATCH 08/27] Add a lot of testcases for CommentsShouldNotBeEmpty (C#) --- .../TestCases/CommentsShouldNotBeEmpty.cs | 242 ++++++++++++++++-- 1 file changed, 224 insertions(+), 18 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index 2f7498fc25f..ea86567764d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -1,38 +1,48 @@ using System; [Obsolete] // Ipsem Lorum -public class SingleLineComment // +public class SingleLine // // Noncompliant@-1 (inline comment) { - // -// -// Noncompliant@-2 (a lot of whitespace before) -// Noncompliant@-2 (a lot of whitespace after) + // + // Noncompliant@-1 (a lot of whitespace before) + + // + // Noncompliant@-1 (a lot of whitespace after) + + // * + // Ipsem Lorum + // + // Noncompliant @-1 + + // -// * -// Ipsem Lorum -// \n -// -// Noncompliant @-1 + // Noncompliant @-2 -// + // \r -// Noncompliant @-2 + // \n -// /**/ + // \r\n -// /// + // \t -// // + // z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ -// /** */ + // /**/ + + // /// + + // // + + // /** */ } [Obsolete] /// Ipsem Lorum -public class SingleLineDocumentationComment /// +public class SingleLineDocumentation /// // Noncompliant@-1 (inline comment) { - /// + /// // Noncompliant@-1 (a lot of whitespace before) /// @@ -73,3 +83,199 @@ public class SingleLineDocumentationComment /// /// // Noncompliant @-3 } + +[Obsolete] /* Ipsem Lorum */ +[Serializable] /**/ +// Noncompliant @-1 (inline comment) +public class MultiLine /* */ +// Noncompliant @-1 (inline comment) +{ + /* */ + // Noncompliant@-1 (a lot of whitespace before) + + /* */ + // Noncompliant@-1 (a lot of whitespace inside) + + /**/ + // Noncompliant@-1 (a lot of whitespace after) + + // Noncompliant@+1 + /* + * + */ + + // Noncompliant@+1 + /* + * + */ + + // Noncompliant@+1 + /* + * + * + * + */ + + + // Noncompliant@+1 + /* + + */ + + // Noncompliant@+1 + /* + + + + */ + + /* hey + */ + + /* + there */ + + /* hey + * + */ + + /* + * + there */ + + /* + + Ipsem Lorum + \n + */ + + /* + hey there + */ + + /* + // + */ + + /* + /// + */ + + /* + \r + */ + + /* + \n + */ + + /* + \r\n + */ + + /* + \t + */ + + /* + z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ + */ +} + +[Obsolete] /** Ipsem Lorum */ +[Serializable] /***/ +// Noncompliant @-1 (inline comment) +public class MultiLineDocumentation /** */ +// Noncompliant @-1 (inline comment) +{ + /** */ + // Noncompliant@-1 (a lot of whitespace before) + + /** */ + // Noncompliant@-1 (a lot of whitespace inside) + + /***/ + // Noncompliant@-1 (a lot of whitespace after) + + // Noncompliant@+1 + /** + * + */ + + // Noncompliant@+1 + /** + * + */ + + // Noncompliant@+1 + /** + * + * + * + */ + + + // Noncompliant@+1 + /** + + */ + + // Noncompliant@+1 + /** + + + + */ + + /** hey + */ + + /** + there */ + + /** hey + * + */ + + /** + * + there */ + + /** + + Ipsem Lorum + \n + */ + + /** + hey there + */ + + /** + // + */ + + /** + /// + */ + + /** + \r + */ + + /** + \n + */ + + /** + \r\n + */ + + /** + \t + */ + + /** + z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ + */ +} From a79ff10d180d6a3b3f4e26f48204bcb35b781b23 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 10:56:25 +0100 Subject: [PATCH 09/27] Minor changes on S4663 C# Testcases --- .../TestCases/CommentsShouldNotBeEmpty.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index ea86567764d..32884440c2d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -96,7 +96,7 @@ public class MultiLine /* */ /* */ // Noncompliant@-1 (a lot of whitespace inside) - /**/ + /**/ // Noncompliant@-1 (a lot of whitespace after) // Noncompliant@+1 @@ -194,7 +194,7 @@ public class MultiLineDocumentation /** */ /** */ // Noncompliant@-1 (a lot of whitespace inside) - /***/ + /***/ // Noncompliant@-1 (a lot of whitespace after) // Noncompliant@+1 @@ -202,11 +202,6 @@ public class MultiLineDocumentation /** */ * */ - // Noncompliant@+1 - /** - * - */ - // Noncompliant@+1 /** * From de693073834a0b8da69876b64a8c898769fbd852 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 10:57:08 +0100 Subject: [PATCH 10/27] Minor refactoring on S4663 (C#) --- .../Rules/CommentsShouldNotBeEmpty.cs | 34 +++++-------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index d1f9ef374bf..7b6a88cfc14 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -44,29 +44,8 @@ private static string GetSingleLineText(SyntaxTrivia trivia) => trivia.ToString().Trim().Substring(2); // /* */ - private static string GetMultiLineText(SyntaxTrivia trivia) - { - var stringBuilder = new StringBuilder(); - var commentText = trivia.ToString().Trim().Substring(2); - - if (commentText.EndsWith("*/", StringComparison.Ordinal)) // Might be unclosed, still reported - { - commentText = commentText.Substring(0, commentText.Length - 2); - } - - foreach (var line in commentText.Split(MetricsBase.LineTerminators, StringSplitOptions.None)) - { - var trimmedLine = line.TrimStart(null); - if (trimmedLine.StartsWith("*", StringComparison.Ordinal)) - { - trimmedLine = trimmedLine.TrimStart('*'); - } - - stringBuilder.Append(trimmedLine.Trim()); - } - - return stringBuilder.ToString(); - } + private static string GetMultiLineText(SyntaxTrivia trivia) => + ParseMultiLine(trivia.ToString(), 2); // Length of "/*" // /// private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) @@ -86,16 +65,19 @@ private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) } // /** */ - private static string GetMultiLineDocumentationText(SyntaxTrivia trivia) + private static string GetMultiLineDocumentationText(SyntaxTrivia trivia) => + ParseMultiLine(trivia.ToFullString(), 3); // Length of "/**" + + private static string ParseMultiLine(string commentText, int initialTrimSize) { - var stringBuilder = new StringBuilder(); - var commentText = trivia.ToFullString().Trim().Substring(3); + commentText = commentText.Trim().Substring(initialTrimSize); if (commentText.EndsWith("*/", StringComparison.Ordinal)) // Might be unclosed, still reported { commentText = commentText.Substring(0, commentText.Length - 2); } + var stringBuilder = new StringBuilder(); foreach (var line in commentText.Split(MetricsBase.LineTerminators, StringSplitOptions.None)) { var trimmedLine = line.TrimStart(null); From cafb38d489b5008189aacf631b36458e1ed76578 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 11:14:35 +0100 Subject: [PATCH 11/27] Add testcases for S4663 VB.NET --- .../TestCases/CommentsShouldNotBeEmpty.vb | 82 ++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb index f5b9e9276c6..20201da2e88 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb @@ -1,7 +1,85 @@ -Public Class Program +Public Class Testcases - Public Sub Test() + Public Sub Comment() ' + ' Noncompliant@-1 (inline comment) + Dim x = 42 'Ipsem Lorum + Dim y = 42 ' Ipsem Lorum + Dim z = 42 ' + ' Noncompliant@-1 (whitespace) + + Dim a = 42 '''Ipsem Lorum + Dim b = 42 ''' Ipsem Lorum + Dim c = 42 ''' + + ' + ' Noncompliant@-1 (whitespace) + + ' * + ' Ipsem Lorum + 'Ipsem Lorum + ' + ' Noncompliant @-1 + + ' + + ' Noncompliant @-2 + + ' \r + + ' \n + + ' \r\n + + ' \t + + ' z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ + + ' ''' + + '''' + + ' ' + + '' + + '''''' + End Sub + + Public Sub DocumentationComment() + ''' + ' Noncompliant@-1 (whitespace) + + ''' * + ''' Ipsem Lorum + '''Ipsem Lorum + ''' + + ''' Ipsem Lorum + + '''Ipsem Lorum + + ''' + ' Noncompliant @-1 + + ' Noncompliant @+1 + ''' + ''' + ''' + + ''' \r + + ''' \n + + ''' \r\n + + ''' \t + + ''' z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ + + ''' ''' + + ''' ' End Sub End Class From d23553427a82eebb4bade7950477f546325e388c Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 11:18:48 +0100 Subject: [PATCH 12/27] Happy New Year for S4663 --- .../src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs | 2 +- .../SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs | 2 +- .../SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs | 2 +- .../Rules/CommentsShouldNotBeEmptyTest.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index 7b6a88cfc14..6766005767a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -1,6 +1,6 @@ /* * SonarAnalyzer for .NET - * Copyright (C) 2015-2022 SonarSource SA + * Copyright (C) 2015-2023 SonarSource SA * mailto: contact AT sonarsource DOT com * * This program is free software; you can redistribute it and/or diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index c490ab963a0..97e8bdb7547 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -1,6 +1,6 @@ /* * SonarAnalyzer for .NET - * Copyright (C) 2015-2022 SonarSource SA + * Copyright (C) 2015-2023 SonarSource SA * mailto: contact AT sonarsource DOT com * * This program is free software; you can redistribute it and/or diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 2eed2c7026d..802c0525cc6 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -1,6 +1,6 @@ /* * SonarAnalyzer for .NET - * Copyright (C) 2015-2022 SonarSource SA + * Copyright (C) 2015-2023 SonarSource SA * mailto: contact AT sonarsource DOT com * * This program is free software; you can redistribute it and/or diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs index c0d93b894be..0049e293fce 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs @@ -1,6 +1,6 @@ /* * SonarAnalyzer for .NET - * Copyright (C) 2015-2022 SonarSource SA + * Copyright (C) 2015-2023 SonarSource SA * mailto: contact AT sonarsource DOT com * * This program is free software; you can redistribute it and/or From b98189a65d86df2a45eddfb8ee3633bb9dae0638 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 11:19:48 +0100 Subject: [PATCH 13/27] Remove empty line --- .../SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs index 0049e293fce..6dea699cc0a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/CommentsShouldNotBeEmptyTest.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - using CS = SonarAnalyzer.Rules.CSharp; using VB = SonarAnalyzer.Rules.VisualBasic; From aa9eab54cbb4c1387121a77a391052e4ea0ba5b0 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 17:49:51 +0100 Subject: [PATCH 14/27] Refactor the logic of S4663 --- .../Rules/CommentsShouldNotBeEmpty.cs | 18 +++-- .../Rules/CommentsShouldNotBeEmptyBase.cs | 81 +++++++++++++++++-- .../Rules/CommentsShouldNotBeEmpty.cs | 10 ++- 3 files changed, 94 insertions(+), 15 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index 6766005767a..ff1aae93e38 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -27,6 +27,13 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => CSharpFacade.Instance; + protected override bool IsValidTriviaType(SyntaxTrivia trivia) + => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + + protected override bool IsSimpleComment(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.SingleLineCommentTrivia); + protected override bool IsEndOfLine(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.EndOfLineTrivia); + protected override bool IsWhitespace(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.WhitespaceTrivia); + protected override string GetCommentText(SyntaxTrivia trivia) => trivia.Kind() switch { @@ -36,17 +43,10 @@ protected override string GetCommentText(SyntaxTrivia trivia) SyntaxKind.MultiLineDocumentationCommentTrivia => GetMultiLineDocumentationText(trivia), }; - protected override bool IsValidTriviaType(SyntaxTrivia trivia) - => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); - // // private static string GetSingleLineText(SyntaxTrivia trivia) => trivia.ToString().Trim().Substring(2); - // /* */ - private static string GetMultiLineText(SyntaxTrivia trivia) => - ParseMultiLine(trivia.ToString(), 2); // Length of "/*" - // /// private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) { @@ -64,6 +64,10 @@ private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) return stringBuilder.ToString(); } + // /* */ + private static string GetMultiLineText(SyntaxTrivia trivia) => + ParseMultiLine(trivia.ToString(), 2); // Length of "/*" + // /** */ private static string GetMultiLineDocumentationText(SyntaxTrivia trivia) => ParseMultiLine(trivia.ToFullString(), 3); // Length of "/**" diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 97e8bdb7547..9c2dda36e0f 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis.Text; + namespace SonarAnalyzer.Rules; public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosticAnalyzer @@ -25,8 +27,12 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti { private const string DiagnosticId = "S4663"; - protected abstract bool IsValidTriviaType(SyntaxTrivia trivia); protected abstract string GetCommentText(SyntaxTrivia trivia); + protected abstract bool IsValidTriviaType(SyntaxTrivia trivia); + + protected abstract bool IsSimpleComment(SyntaxTrivia trivia); + protected abstract bool IsEndOfLine(SyntaxTrivia trivia); + protected abstract bool IsWhitespace(SyntaxTrivia trivia); protected override string MessageFormat => "Remove this empty comment"; @@ -44,12 +50,77 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { - foreach (var trivium in trivia.Where(ShouldReport)) + foreach (var partition in Partition(trivia).Where(ShouldReport)) + { + var start = partition.First().GetLocation().SourceSpan.Start; + var end = partition.Last().GetLocation().SourceSpan.End; + + var location = Location.Create(context.Tree, TextSpan.FromBounds(start, end)); + context.ReportIssue(Diagnostic.Create(Rule, location)); + } + } + + protected IEnumerable> Partition(IEnumerable trivia) + { + var res = new List>(); + + var current = new List(); + var endOfLineFound = false; + + foreach (var trivium in trivia) { - context.ReportIssue(Diagnostic.Create(Rule, trivium.GetLocation())); + if (IsWhitespace(trivium)) + { + continue; + } + + if (IsSimpleComment(trivium)) // put it on the current block of "//" + { + current.Add(trivium); + endOfLineFound = false; + continue; + } + + // This is for the case, of two different comment types, for example: + // // + // /// + if (IsValidTriviaType(trivium)) // valid but not "//", because of the upper if + { + AddCurrent(); + res.Add(new List { trivium }); + } + else if (IsEndOfLine(trivium)) + { + // This is for the case, of an empty line in between, for example: + // // + // + // // + if (endOfLineFound) + { + AddCurrent(); + } + else + { + endOfLineFound = true; + } + } + else + { + AddCurrent(); + } } - bool ShouldReport(SyntaxTrivia trivia) - => IsValidTriviaType(trivia) && string.IsNullOrEmpty(GetCommentText(trivia)); + res.Add(current); + return res; + + void AddCurrent() + { + res.Add(current); + current = new(); + endOfLineFound = false; + } } + + protected bool ShouldReport(IEnumerable trivia) => + trivia.Any() && trivia.All(x => GetCommentText(x) == string.Empty); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 802c0525cc6..92a702d62a7 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -27,6 +27,13 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => VisualBasicFacade.Instance; + protected override bool IsValidTriviaType(SyntaxTrivia trivia) + => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + + protected override bool IsSimpleComment(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.CommentTrivia); + protected override bool IsEndOfLine(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.EndOfLineTrivia); + protected override bool IsWhitespace(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.WhitespaceTrivia); + protected override string GetCommentText(SyntaxTrivia trivia) => trivia.Kind() switch { @@ -34,9 +41,6 @@ protected override string GetCommentText(SyntaxTrivia trivia) SyntaxKind.DocumentationCommentTrivia => GetDocumentationText(trivia), }; - protected override bool IsValidTriviaType(SyntaxTrivia trivia) - => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); - // ' private static string GetText(SyntaxTrivia trivia) => trivia.ToString().Trim().Substring(1); From d76e0d4bca1f84362bf3b14ada2bab4a06298af6 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 26 Jan 2023 17:50:14 +0100 Subject: [PATCH 15/27] Add a lot of testcases for S4663 --- .../TestCases/CommentsShouldNotBeEmpty.cs | 102 +++++++++++++++++- .../TestCases/CommentsShouldNotBeEmpty.vb | 44 +++++++- 2 files changed, 137 insertions(+), 9 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index 32884440c2d..74f22be8f1f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -2,18 +2,42 @@ [Obsolete] // Ipsem Lorum public class SingleLine // -// Noncompliant@-1 (inline comment) -{ +// hey + +// Noncompliant@-3 (inline comment) +{ // Ipsem Lorum // - // Noncompliant@-1 (a lot of whitespace before) + + // Noncompliant@-2 (a lot of whitespace before) // - // Noncompliant@-1 (a lot of whitespace after) + + // Noncompliant@-2 (a lot of whitespace after) // * // Ipsem Lorum // - // Noncompliant @-1 + + // hey + // + // + // + + // + // + // hey + // + // + // + + // + // + // + // hey + // + // + // + // @@ -36,6 +60,73 @@ public class SingleLine // // // // /** */ + + // + // + // hey + // + // + // there + // + // + + // + // + // + /// text + // Noncompliant@-4 + + // + // + // + /* + * text + */ + // Noncompliant@-6 + + // + // + // + /** + * text + */ + // Noncompliant@-6 + + void Method() + { + // Noncompliant@+2 + + // + // + // + var x = 42; // + // + // + // + + // Noncompliant@-5 + // Noncompliant@-5 + + // + // + // hello + // + // + var y = 42; // + // + // + // + // there + + // Noncompliant@-6 + + // + } // + // + + // Noncompliant@-4 + // Noncompliant@-4 + // Noncompliant@-4 } [Obsolete] /// Ipsem Lorum @@ -58,6 +149,7 @@ public class SingleLineDocumentation /// /// hey there /// hey there + /// text /// /// /// diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb index 20201da2e88..66af30b900a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb @@ -1,25 +1,55 @@ Public Class Testcases Public Sub Comment() ' - ' Noncompliant@-1 (inline comment) + + ' Noncompliant@-2 (inline comment) Dim x = 42 'Ipsem Lorum Dim y = 42 ' Ipsem Lorum Dim z = 42 ' - ' Noncompliant@-1 (whitespace) + + ' Noncompliant@-2 Dim a = 42 '''Ipsem Lorum Dim b = 42 ''' Ipsem Lorum Dim c = 42 ''' ' - ' Noncompliant@-1 (whitespace) + + ' Noncompliant@-2 (whitespace) + + + ' + ' + ' + ' hey + + ' hey + ' + ' + ' + + ' Noncompliant@+2 + + ' + ' + ' + ' + + ' Noncompliant@+2 + + ' + + ' + ' + ' + + ' Noncompliant@-4 ' * ' Ipsem Lorum 'Ipsem Lorum ' - ' Noncompliant @-1 ' @@ -44,6 +74,12 @@ '' '''''' + + ' + ' + ' + ''' Ipsem Lorum + ' Noncompliant@-4 End Sub Public Sub DocumentationComment() From 2337881ce8597d2f69536a808946c1b8a622d287 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Fri, 27 Jan 2023 09:54:58 +0100 Subject: [PATCH 16/27] Update dotnet ITs expected for Ember --- ...A35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S4663.json | 17 +++++++++++++++++ ...550E-DD76-4F4D-8250-8598140F828B}-S4663.json | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 analyzers/its/expected/Ember-MM/EmberAPI-{208AA35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S4663.json create mode 100644 analyzers/its/expected/Ember-MM/scraper.EmberCore-{EF6A550E-DD76-4F4D-8250-8598140F828B}-S4663.json diff --git a/analyzers/its/expected/Ember-MM/EmberAPI-{208AA35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S4663.json b/analyzers/its/expected/Ember-MM/EmberAPI-{208AA35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S4663.json new file mode 100644 index 00000000000..936136f3497 --- /dev/null +++ b/analyzers/its/expected/Ember-MM/EmberAPI-{208AA35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S4663.json @@ -0,0 +1,17 @@ +{ +"issues": [ +{ +"id": "S4663", +"message": "Remove this empty comment", +"location": { +"uri": "sources\Ember-MM\EmberAPI\clsAPICommon.vb", +"region": { +"startLine": 577, +"startColumn": 19, +"endLine": 577, +"endColumn": 21 +} +} +} +] +} diff --git a/analyzers/its/expected/Ember-MM/scraper.EmberCore-{EF6A550E-DD76-4F4D-8250-8598140F828B}-S4663.json b/analyzers/its/expected/Ember-MM/scraper.EmberCore-{EF6A550E-DD76-4F4D-8250-8598140F828B}-S4663.json new file mode 100644 index 00000000000..dac912b5aff --- /dev/null +++ b/analyzers/its/expected/Ember-MM/scraper.EmberCore-{EF6A550E-DD76-4F4D-8250-8598140F828B}-S4663.json @@ -0,0 +1,17 @@ +{ +"issues": [ +{ +"id": "S4663", +"message": "Remove this empty comment", +"location": { +"uri": "sources\Ember-MM\Addons\scraper.EmberCore\scraperMovieNativeModule.vb", +"region": { +"startLine": 364, +"startColumn": 104, +"endLine": 364, +"endColumn": 105 +} +} +} +] +} From 0ad4cff5fb12a31a23309cd51c014985e1fb2c3e Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Fri, 27 Jan 2023 17:07:31 +0100 Subject: [PATCH 17/27] Apply PR Review Changes --- .../Rules/CommentsShouldNotBeEmpty.cs | 4 --- .../Rules/CommentsShouldNotBeEmptyBase.cs | 34 +++++++++++-------- .../Rules/CommentsShouldNotBeEmpty.cs | 2 -- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index ff1aae93e38..c4dbc5cc569 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -51,14 +51,12 @@ private static string GetSingleLineText(SyntaxTrivia trivia) private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) { var stringBuilder = new StringBuilder(); - foreach (var line in trivia.ToFullString().Split(MetricsBase.LineTerminators, StringSplitOptions.None)) { var trimmedLine = line.TrimStart(null); trimmedLine = trimmedLine.StartsWith("///") ? trimmedLine.Substring(3).Trim() : trimmedLine.TrimEnd(null); - stringBuilder.Append(trimmedLine); } return stringBuilder.ToString(); @@ -75,7 +73,6 @@ private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) private static string ParseMultiLine(string commentText, int initialTrimSize) { commentText = commentText.Trim().Substring(initialTrimSize); - if (commentText.EndsWith("*/", StringComparison.Ordinal)) // Might be unclosed, still reported { commentText = commentText.Substring(0, commentText.Length - 2); @@ -92,7 +89,6 @@ private static string ParseMultiLine(string commentText, int initialTrimSize) stringBuilder.Append(trimmedLine.Trim()); } - return stringBuilder.ToString(); } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 9c2dda36e0f..9c9f0de4a52 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -60,12 +60,11 @@ protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable< } } - protected IEnumerable> Partition(IEnumerable trivia) + protected List> Partition(IEnumerable trivia) { var res = new List>(); - var current = new List(); - var endOfLineFound = false; + var firstEndOfLineFound = false; foreach (var trivium in trivia) { @@ -77,7 +76,7 @@ protected IEnumerable> Partition(IEnumerable> Partition(IEnumerable { trivium }); + CloseCurrentPartition(); + // all comments except single-line comments are parsed as a block already. + current.Add(trivium); + CloseCurrentPartition(); } else if (IsEndOfLine(trivium)) { @@ -95,32 +96,35 @@ protected IEnumerable> Partition(IEnumerable 0) + { + res.Add(current); + current = new(); + } + firstEndOfLineFound = false; } } - protected bool ShouldReport(IEnumerable trivia) => + private bool ShouldReport(IEnumerable trivia) => trivia.Any() && trivia.All(x => GetCommentText(x) == string.Empty); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 92a702d62a7..2c4b949624f 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -49,14 +49,12 @@ private static string GetText(SyntaxTrivia trivia) private static string GetDocumentationText(SyntaxTrivia trivia) { var stringBuilder = new StringBuilder(); - foreach (var line in trivia.ToFullString().Split(MetricsBase.LineTerminators, StringSplitOptions.None)) { var trimmedLine = line.TrimStart(null); trimmedLine = trimmedLine.StartsWith("'''") ? trimmedLine.Substring(3).Trim() : trimmedLine.TrimEnd(null); - stringBuilder.Append(trimmedLine); } return stringBuilder.ToString(); From f8492a970c0217d52d71b422c3ff365b64d7eb10 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 30 Jan 2023 10:24:57 +0100 Subject: [PATCH 18/27] Appy PR Review changes --- .../Rules/CommentsShouldNotBeEmpty.cs | 23 ++++++++----- .../Rules/CommentsShouldNotBeEmptyBase.cs | 33 ++++++++++--------- .../Rules/CommentsShouldNotBeEmpty.cs | 19 +++++++---- 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index c4dbc5cc569..b684b3a4ce2 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -27,15 +27,20 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => CSharpFacade.Instance; - protected override bool IsValidTriviaType(SyntaxTrivia trivia) - => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + protected override bool IsSimpleComment(SyntaxTrivia trivia) => + trivia.IsKind(SyntaxKind.SingleLineCommentTrivia); - protected override bool IsSimpleComment(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.SingleLineCommentTrivia); - protected override bool IsEndOfLine(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.EndOfLineTrivia); - protected override bool IsWhitespace(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.WhitespaceTrivia); + protected override bool IsEndOfLine(SyntaxTrivia trivia) => + trivia.IsKind(SyntaxKind.EndOfLineTrivia); - protected override string GetCommentText(SyntaxTrivia trivia) - => trivia.Kind() switch + protected override bool IsWhitespace(SyntaxTrivia trivia) => + trivia.IsKind(SyntaxKind.WhitespaceTrivia); + + protected override bool IsValidTriviaType(SyntaxTrivia trivia) => + Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + + protected override string GetCommentText(SyntaxTrivia trivia) => + trivia.Kind() switch { SyntaxKind.SingleLineCommentTrivia => GetSingleLineText(trivia), SyntaxKind.MultiLineCommentTrivia => GetMultiLineText(trivia), @@ -44,8 +49,8 @@ protected override string GetCommentText(SyntaxTrivia trivia) }; // // - private static string GetSingleLineText(SyntaxTrivia trivia) - => trivia.ToString().Trim().Substring(2); + private static string GetSingleLineText(SyntaxTrivia trivia) => + trivia.ToString().Trim().Substring(2); // /// private static string GetSingleLineDocumentationText(SyntaxTrivia trivia) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 9c9f0de4a52..04fb0fd3f70 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -27,8 +27,8 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti { private const string DiagnosticId = "S4663"; - protected abstract string GetCommentText(SyntaxTrivia trivia); protected abstract bool IsValidTriviaType(SyntaxTrivia trivia); + protected abstract string GetCommentText(SyntaxTrivia trivia); protected abstract bool IsSimpleComment(SyntaxTrivia trivia); protected abstract bool IsEndOfLine(SyntaxTrivia trivia); @@ -50,7 +50,7 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { - foreach (var partition in Partition(trivia).Where(ShouldReport)) + foreach (var partition in Partition(trivia)?.Where(ShouldReport)) { var start = partition.First().GetLocation().SourceSpan.Start; var end = partition.Last().GetLocation().SourceSpan.End; @@ -60,10 +60,10 @@ protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable< } } - protected List> Partition(IEnumerable trivia) + protected List>? Partition(IEnumerable trivia) { - var res = new List>(); - var current = new List(); + var res = new Lazy>>(); + var current = new Lazy>(); var firstEndOfLineFound = false; foreach (var trivium in trivia) @@ -75,7 +75,7 @@ protected List> Partition(IEnumerable trivia) if (IsSimpleComment(trivium)) // put it on the current block of "//" { - current.Add(trivium); + current.Value.Add(trivium); firstEndOfLineFound = false; continue; } @@ -87,15 +87,16 @@ protected List> Partition(IEnumerable trivia) { CloseCurrentPartition(); // all comments except single-line comments are parsed as a block already. - current.Add(trivium); + current.Value.Add(trivium); CloseCurrentPartition(); } + // This handles an empty line, for example: + // // some comment \n <- EOL found, set to true + // // \n <- EOL is set to false at CommentTrivia, set to true after it + // // some other comment <- EOL is set to false at CommentTrivia, set to true after it + // \n <- EOL found, is already true, closes current partition else if (IsEndOfLine(trivium)) { - // This is for the case, of an empty line in between, for example: - // // - // - // // if (firstEndOfLineFound) { CloseCurrentPartition(); @@ -111,14 +112,14 @@ protected List> Partition(IEnumerable trivia) } } - res.Add(current); - return res; + res.Value.Add(current.Value); + return res.Value; void CloseCurrentPartition() { - if (current.Count > 0) + if (current.Value.Count > 0) { - res.Add(current); + res.Value.Add(current.Value); current = new(); } firstEndOfLineFound = false; @@ -126,5 +127,5 @@ void CloseCurrentPartition() } private bool ShouldReport(IEnumerable trivia) => - trivia.Any() && trivia.All(x => GetCommentText(x) == string.Empty); + trivia.Any() && trivia.All(x => string.IsNullOrWhiteSpace(GetCommentText(x))); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index 2c4b949624f..e6e5eb08f5f 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -27,15 +27,20 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => VisualBasicFacade.Instance; - protected override bool IsValidTriviaType(SyntaxTrivia trivia) - => Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + protected override bool IsSimpleComment(SyntaxTrivia trivia) => + trivia.IsKind(SyntaxKind.CommentTrivia); - protected override bool IsSimpleComment(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.CommentTrivia); - protected override bool IsEndOfLine(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.EndOfLineTrivia); - protected override bool IsWhitespace(SyntaxTrivia trivia) => trivia.IsKind(SyntaxKind.WhitespaceTrivia); + protected override bool IsEndOfLine(SyntaxTrivia trivia) => + trivia.IsKind(SyntaxKind.EndOfLineTrivia); - protected override string GetCommentText(SyntaxTrivia trivia) - => trivia.Kind() switch + protected override bool IsWhitespace(SyntaxTrivia trivia) => + trivia.IsKind(SyntaxKind.WhitespaceTrivia); + + protected override bool IsValidTriviaType(SyntaxTrivia trivia) => + Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); + + protected override string GetCommentText(SyntaxTrivia trivia) => + trivia.Kind() switch { SyntaxKind.CommentTrivia => GetText(trivia), SyntaxKind.DocumentationCommentTrivia => GetDocumentationText(trivia), From 7e1b6583a20fff24d5b1bd386c5e4dd36dbe3859 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 30 Jan 2023 11:06:32 +0100 Subject: [PATCH 19/27] Move some logic to the base for S4663 --- .../Facade/CSharpSyntaxKindFacade.cs | 17 ++++++----- .../Rules/CommentsShouldNotBeEmpty.cs | 12 -------- .../Facade/ISyntaxKindFacade.cs | 3 ++ .../Rules/CommentsShouldNotBeEmptyBase.cs | 28 +++++++++++++------ .../Facade/VisualBasicSyntaxKindFacade.cs | 13 +++++---- .../Rules/CommentsShouldNotBeEmpty.cs | 12 -------- 6 files changed, 40 insertions(+), 45 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs index 970eb33bbf2..cb7ab181ac7 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs @@ -29,13 +29,6 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade SyntaxKindEx.RecordClassDeclaration, }; public SyntaxKind ClassDeclaration => SyntaxKind.ClassDeclaration; - public SyntaxKind[] CommentTrivia => new[] - { - SyntaxKind.SingleLineCommentTrivia, - SyntaxKind.MultiLineCommentTrivia, - SyntaxKind.SingleLineDocumentationCommentTrivia, - SyntaxKind.MultiLineDocumentationCommentTrivia, - }; public SyntaxKind[] ComparisonKinds => new[] { SyntaxKind.GreaterThanExpression, @@ -77,4 +70,14 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentExpression; public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentExpression; + public SyntaxKind[] CommentTrivia => new[] + { + SyntaxKind.SingleLineCommentTrivia, + SyntaxKind.MultiLineCommentTrivia, + SyntaxKind.SingleLineDocumentationCommentTrivia, + SyntaxKind.MultiLineDocumentationCommentTrivia, + }; + public SyntaxKind SimpleCommentTrivia => SyntaxKind.SingleLineCommentTrivia; + public SyntaxKind WhitespaceTrivia => SyntaxKind.WhitespaceTrivia; + public SyntaxKind EndOfLineTrivia => SyntaxKind.EndOfLineTrivia; } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs index b684b3a4ce2..4339b95afae 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/CommentsShouldNotBeEmpty.cs @@ -27,18 +27,6 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => CSharpFacade.Instance; - protected override bool IsSimpleComment(SyntaxTrivia trivia) => - trivia.IsKind(SyntaxKind.SingleLineCommentTrivia); - - protected override bool IsEndOfLine(SyntaxTrivia trivia) => - trivia.IsKind(SyntaxKind.EndOfLineTrivia); - - protected override bool IsWhitespace(SyntaxTrivia trivia) => - trivia.IsKind(SyntaxKind.WhitespaceTrivia); - - protected override bool IsValidTriviaType(SyntaxTrivia trivia) => - Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); - protected override string GetCommentText(SyntaxTrivia trivia) => trivia.Kind() switch { diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs index b64ade32ed1..305d18bb345 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs @@ -30,6 +30,7 @@ public interface ISyntaxKindFacade abstract TSyntaxKind[] ComparisonKinds { get; } abstract TSyntaxKind ConstructorDeclaration { get; } abstract TSyntaxKind[] DefaultExpressions { get; } + abstract TSyntaxKind EndOfLineTrivia { get; } abstract TSyntaxKind EnumDeclaration { get; } abstract TSyntaxKind FieldDeclaration { get; } abstract TSyntaxKind IdentifierName { get; } @@ -44,10 +45,12 @@ public interface ISyntaxKindFacade abstract TSyntaxKind ParameterList { get; } abstract TSyntaxKind ReturnStatement { get; } abstract TSyntaxKind SimpleAssignment { get; } + abstract TSyntaxKind SimpleCommentTrivia { get; } abstract TSyntaxKind SimpleMemberAccessExpression { get; } abstract TSyntaxKind[] StringLiteralExpressions { get; } abstract TSyntaxKind StructDeclaration { get; } abstract TSyntaxKind[] TypeDeclaration { get; } + abstract TSyntaxKind WhitespaceTrivia { get; } abstract TSyntaxKind LeftShiftExpression { get; } abstract TSyntaxKind RightShiftExpression { get; } abstract TSyntaxKind LeftShiftAssignmentStatement { get; } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 04fb0fd3f70..6431422ec39 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -27,13 +27,8 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti { private const string DiagnosticId = "S4663"; - protected abstract bool IsValidTriviaType(SyntaxTrivia trivia); protected abstract string GetCommentText(SyntaxTrivia trivia); - protected abstract bool IsSimpleComment(SyntaxTrivia trivia); - protected abstract bool IsEndOfLine(SyntaxTrivia trivia); - protected abstract bool IsWhitespace(SyntaxTrivia trivia); - protected override string MessageFormat => "Remove this empty comment"; protected CommentsShouldNotBeEmptyBase() : base(DiagnosticId) { } @@ -48,7 +43,7 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti } }); - protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) + private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { foreach (var partition in Partition(trivia)?.Where(ShouldReport)) { @@ -58,9 +53,12 @@ protected void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable< var location = Location.Create(context.Tree, TextSpan.FromBounds(start, end)); context.ReportIssue(Diagnostic.Create(Rule, location)); } + + bool ShouldReport(IEnumerable trivia) => + trivia.Any() && trivia.All(x => string.IsNullOrWhiteSpace(GetCommentText(x))); } - protected List>? Partition(IEnumerable trivia) + private List>? Partition(IEnumerable trivia) { var res = new Lazy>>(); var current = new Lazy>(); @@ -126,6 +124,18 @@ void CloseCurrentPartition() } } - private bool ShouldReport(IEnumerable trivia) => - trivia.Any() && trivia.All(x => string.IsNullOrWhiteSpace(GetCommentText(x))); + private bool IsValidTriviaType(SyntaxTrivia trivia) => + Language.SyntaxKind.CommentTrivia.Contains((TSyntaxKind)Enum.ToObject(typeof(TSyntaxKind), trivia.RawKind)); + + private bool IsSimpleComment(SyntaxTrivia trivia) => + IsKind(trivia, Language.SyntaxKind.SimpleCommentTrivia); + + private bool IsEndOfLine(SyntaxTrivia trivia) => + IsKind(trivia, Language.SyntaxKind.EndOfLineTrivia); + + private bool IsWhitespace(SyntaxTrivia trivia) => + IsKind(trivia, Language.SyntaxKind.WhitespaceTrivia); + + private bool IsKind(SyntaxTrivia trivia, TSyntaxKind kind) => + kind.Equals(Enum.ToObject(typeof(TSyntaxKind), trivia.RawKind)); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs index e961c8afaf3..45d41778a50 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs @@ -25,11 +25,6 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.Attribute; public SyntaxKind[] ClassAndRecordDeclaration => new[] { SyntaxKind.ClassBlock }; public SyntaxKind ClassDeclaration => SyntaxKind.ClassBlock; - public SyntaxKind[] CommentTrivia => new[] - { - SyntaxKind.CommentTrivia, - SyntaxKind.DocumentationCommentTrivia, - }; public SyntaxKind[] ComparisonKinds => new[] { SyntaxKind.GreaterThanExpression, @@ -63,4 +58,12 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.RightShiftExpression; public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentStatement; public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentStatement; + public SyntaxKind[] CommentTrivia => new[] + { + SyntaxKind.CommentTrivia, + SyntaxKind.DocumentationCommentTrivia, + }; + public SyntaxKind SimpleCommentTrivia => SyntaxKind.CommentTrivia; + public SyntaxKind WhitespaceTrivia => SyntaxKind.WhitespaceTrivia; + public SyntaxKind EndOfLineTrivia => SyntaxKind.EndOfLineTrivia; } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs index e6e5eb08f5f..773f222a0cd 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/CommentsShouldNotBeEmpty.cs @@ -27,18 +27,6 @@ public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase Language => VisualBasicFacade.Instance; - protected override bool IsSimpleComment(SyntaxTrivia trivia) => - trivia.IsKind(SyntaxKind.CommentTrivia); - - protected override bool IsEndOfLine(SyntaxTrivia trivia) => - trivia.IsKind(SyntaxKind.EndOfLineTrivia); - - protected override bool IsWhitespace(SyntaxTrivia trivia) => - trivia.IsKind(SyntaxKind.WhitespaceTrivia); - - protected override bool IsValidTriviaType(SyntaxTrivia trivia) => - Language.SyntaxKind.CommentTrivia.Contains(trivia.Kind()); - protected override string GetCommentText(SyntaxTrivia trivia) => trivia.Kind() switch { From 3430d93380cc46bafa689432f22abf5a22d027c3 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 30 Jan 2023 14:14:55 +0100 Subject: [PATCH 20/27] Apply some more PR Review changes --- .../Facade/CSharpSyntaxFacade.cs | 4 ++ .../Facade/SyntaxFacade.cs | 2 + .../Rules/CommentsShouldNotBeEmptyBase.cs | 60 ++++++++++--------- .../Facade/VisualBasicSyntaxFacade.cs | 4 ++ .../Helpers/VisualBasicSyntaxHelper.cs | 3 + 5 files changed, 45 insertions(+), 28 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index c57298bf631..c26432354f0 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -40,10 +40,14 @@ internal sealed class CSharpSyntaxFacade : SyntaxFacade public override bool IsKind(SyntaxToken token, SyntaxKind kind) => token.IsKind(kind); + public override bool IsKind(SyntaxTrivia trivia, SyntaxKind kind) => trivia.IsKind(kind); + public override bool IsAnyKind(SyntaxNode node, ISet syntaxKinds) => node.IsAnyKind(syntaxKinds); public override bool IsAnyKind(SyntaxNode node, params SyntaxKind[] syntaxKinds) => node.IsAnyKind(syntaxKinds); + public override bool IsAnyKind(SyntaxTrivia trivia, params SyntaxKind[] syntaxKinds) => trivia.IsAnyKind(syntaxKinds); + public override bool IsNullLiteral(SyntaxNode node) => node.IsNullLiteral(); public override IEnumerable ArgumentExpressions(SyntaxNode node) => diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs index d97f5cfa666..9144a08af14 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs @@ -28,8 +28,10 @@ public abstract class SyntaxFacade public abstract bool IsNullLiteral(SyntaxNode node); public abstract bool IsKind(SyntaxNode node, TSyntaxKind kind); public abstract bool IsKind(SyntaxToken token, TSyntaxKind kind); + public abstract bool IsKind(SyntaxTrivia trivia, TSyntaxKind kind); public abstract bool IsAnyKind(SyntaxNode node, ISet syntaxKinds); public abstract bool IsAnyKind(SyntaxNode node, params TSyntaxKind[] syntaxKinds); + public abstract bool IsAnyKind(SyntaxTrivia trivia, params TSyntaxKind[] syntaxKinds); public abstract IEnumerable ArgumentExpressions(SyntaxNode node); public abstract ImmutableArray AssignmentTargets(SyntaxNode assignment); public abstract SyntaxNode AssignmentLeft(SyntaxNode assignment); diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 6431422ec39..323d4c71bbd 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -45,11 +45,16 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { - foreach (var partition in Partition(trivia)?.Where(ShouldReport)) + var partitions = Partition(trivia); + if (partitions is null) + { + return; + } + + foreach (var partition in partitions.Where(ShouldReport)) { var start = partition.First().GetLocation().SourceSpan.Start; var end = partition.Last().GetLocation().SourceSpan.End; - var location = Location.Create(context.Tree, TextSpan.FromBounds(start, end)); context.ReportIssue(Diagnostic.Create(Rule, location)); } @@ -58,34 +63,30 @@ private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable string.IsNullOrWhiteSpace(GetCommentText(x))); } - private List>? Partition(IEnumerable trivia) + private List> Partition(IEnumerable trivia) { - var res = new Lazy>>(); - var current = new Lazy>(); + // hot path: avoid unnecessary allocations + List> res = null; + List current = null; var firstEndOfLineFound = false; foreach (var trivium in trivia) { - if (IsWhitespace(trivium)) - { - continue; - } - if (IsSimpleComment(trivium)) // put it on the current block of "//" { - current.Value.Add(trivium); + current ??= new(); + current.Add(trivium); firstEndOfLineFound = false; - continue; } - // This is for the case, of two different comment types, for example: // // // /// - if (IsValidTriviaType(trivium)) // valid but not "//", because of the upper if + else if (IsValidTriviaType(trivium)) // valid but not "//", because of the upper if { CloseCurrentPartition(); // all comments except single-line comments are parsed as a block already. - current.Value.Add(trivium); + current ??= new(); + current.Add(trivium); CloseCurrentPartition(); } // This handles an empty line, for example: @@ -104,38 +105,41 @@ private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable 0) + if (current is { Count: > 0 }) { - res.Value.Add(current.Value); - current = new(); + res ??= new(); + res.Add(current); + current = null; } firstEndOfLineFound = false; } } private bool IsValidTriviaType(SyntaxTrivia trivia) => - Language.SyntaxKind.CommentTrivia.Contains((TSyntaxKind)Enum.ToObject(typeof(TSyntaxKind), trivia.RawKind)); + Language.Syntax.IsAnyKind(trivia, Language.SyntaxKind.CommentTrivia); private bool IsSimpleComment(SyntaxTrivia trivia) => - IsKind(trivia, Language.SyntaxKind.SimpleCommentTrivia); + Language.Syntax.IsKind(trivia, Language.SyntaxKind.SimpleCommentTrivia); private bool IsEndOfLine(SyntaxTrivia trivia) => - IsKind(trivia, Language.SyntaxKind.EndOfLineTrivia); + Language.Syntax.IsKind(trivia, Language.SyntaxKind.EndOfLineTrivia); private bool IsWhitespace(SyntaxTrivia trivia) => - IsKind(trivia, Language.SyntaxKind.WhitespaceTrivia); - - private bool IsKind(SyntaxTrivia trivia, TSyntaxKind kind) => - kind.Equals(Enum.ToObject(typeof(TSyntaxKind), trivia.RawKind)); + Language.Syntax.IsKind(trivia, Language.SyntaxKind.WhitespaceTrivia); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs index b49dea3e7b7..c03a1e1f4bd 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs @@ -42,10 +42,14 @@ internal sealed class VisualBasicSyntaxFacade : SyntaxFacade public override bool IsKind(SyntaxToken token, SyntaxKind kind) => token.IsKind(kind); + public override bool IsKind(SyntaxTrivia trivia, SyntaxKind kind) => trivia.IsKind(kind); + public override bool IsAnyKind(SyntaxNode node, ISet syntaxKinds) => node.IsAnyKind(syntaxKinds); public override bool IsAnyKind(SyntaxNode node, params SyntaxKind[] syntaxKinds) => node.IsAnyKind(syntaxKinds); + public override bool IsAnyKind(SyntaxTrivia trivia, params SyntaxKind[] syntaxKinds) => trivia.IsAnyKind(syntaxKinds); + public override IEnumerable ArgumentExpressions(SyntaxNode node) => node switch { diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs index b9ceda82773..39994819bd2 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs @@ -101,6 +101,9 @@ public static StatementSyntax GetSucceedingStatement(this StatementSyntax curren public static bool AnyOfKind(this IEnumerable nodes, SyntaxKind kind) => nodes.Any(n => n.RawKind == (int)kind); + public static bool IsAnyKind(this SyntaxTrivia syntaxTrivia, params SyntaxKind[] syntaxKinds) => + syntaxKinds.Contains((SyntaxKind)syntaxTrivia.RawKind); + public static SyntaxToken? GetMethodCallIdentifier(this InvocationExpressionSyntax invocation) { if (invocation == null || From 80c0476ad2a603c828e9fbd3df1993d0185549fd Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 30 Jan 2023 17:04:25 +0100 Subject: [PATCH 21/27] Move some members of the facades in the correct alphabetical order --- .../Facade/CSharpSyntaxKindFacade.cs | 26 +++++++++---------- .../Facade/ISyntaxKindFacade.cs | 8 +++--- .../Facade/VisualBasicSyntaxKindFacade.cs | 14 +++++----- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs index cb7ab181ac7..f1c5c26f906 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs @@ -29,6 +29,13 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade SyntaxKindEx.RecordClassDeclaration, }; public SyntaxKind ClassDeclaration => SyntaxKind.ClassDeclaration; + public SyntaxKind[] CommentTrivia => new[] + { + SyntaxKind.SingleLineCommentTrivia, + SyntaxKind.MultiLineCommentTrivia, + SyntaxKind.SingleLineDocumentationCommentTrivia, + SyntaxKind.MultiLineDocumentationCommentTrivia, + }; public SyntaxKind[] ComparisonKinds => new[] { SyntaxKind.GreaterThanExpression, @@ -41,19 +48,25 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade public SyntaxKind ConstructorDeclaration => SyntaxKind.ConstructorDeclaration; public SyntaxKind[] DefaultExpressions => new[] { SyntaxKind.DefaultExpression, SyntaxKindEx.DefaultLiteralExpression }; public SyntaxKind EnumDeclaration => SyntaxKind.EnumDeclaration; + public SyntaxKind EndOfLineTrivia => SyntaxKind.EndOfLineTrivia; public SyntaxKind FieldDeclaration => SyntaxKind.FieldDeclaration; public SyntaxKind IdentifierName => SyntaxKind.IdentifierName; public SyntaxKind IdentifierToken => SyntaxKind.IdentifierToken; public SyntaxKind InvocationExpression => SyntaxKind.InvocationExpression; public SyntaxKind InterpolatedStringExpression => SyntaxKind.InterpolatedStringExpression; + public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentExpression; + public SyntaxKind LeftShiftExpression => SyntaxKind.LeftShiftExpression; public SyntaxKind LocalDeclaration => SyntaxKind.LocalDeclarationStatement; public SyntaxKind[] MethodDeclarations => new[] { SyntaxKind.MethodDeclaration }; public SyntaxKind[] ObjectCreationExpressions => new[] { SyntaxKind.ObjectCreationExpression, SyntaxKindEx.ImplicitObjectCreationExpression }; public SyntaxKind Parameter => SyntaxKind.Parameter; public SyntaxKind ParameterList => SyntaxKind.ParameterList; + public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentExpression; + public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; public SyntaxKind RefKeyword => SyntaxKind.RefKeyword; public SyntaxKind ReturnStatement => SyntaxKind.ReturnStatement; public SyntaxKind SimpleAssignment => SyntaxKind.SimpleAssignmentExpression; + public SyntaxKind SimpleCommentTrivia => SyntaxKind.SingleLineCommentTrivia; public SyntaxKind SimpleMemberAccessExpression => SyntaxKind.SimpleMemberAccessExpression; public SyntaxKind[] StringLiteralExpressions => new[] { SyntaxKind.StringLiteralExpression, SyntaxKindEx.Utf8StringLiteralExpression }; public SyntaxKind StructDeclaration => SyntaxKind.StructDeclaration; @@ -66,18 +79,5 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade SyntaxKindEx.RecordClassDeclaration, SyntaxKindEx.RecordStructDeclaration, }; - public SyntaxKind LeftShiftExpression => SyntaxKind.LeftShiftExpression; - public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; - public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentExpression; - public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentExpression; - public SyntaxKind[] CommentTrivia => new[] - { - SyntaxKind.SingleLineCommentTrivia, - SyntaxKind.MultiLineCommentTrivia, - SyntaxKind.SingleLineDocumentationCommentTrivia, - SyntaxKind.MultiLineDocumentationCommentTrivia, - }; - public SyntaxKind SimpleCommentTrivia => SyntaxKind.SingleLineCommentTrivia; public SyntaxKind WhitespaceTrivia => SyntaxKind.WhitespaceTrivia; - public SyntaxKind EndOfLineTrivia => SyntaxKind.EndOfLineTrivia; } diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs index 305d18bb345..9394383d9f8 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs @@ -37,11 +37,15 @@ public interface ISyntaxKindFacade abstract TSyntaxKind IdentifierToken { get; } abstract TSyntaxKind InvocationExpression { get; } abstract TSyntaxKind InterpolatedStringExpression { get; } + abstract TSyntaxKind LeftShiftAssignmentStatement { get; } + abstract TSyntaxKind LeftShiftExpression { get; } abstract TSyntaxKind LocalDeclaration { get; } abstract TSyntaxKind[] MethodDeclarations { get; } abstract TSyntaxKind[] ObjectCreationExpressions { get; } abstract TSyntaxKind Parameter { get; } abstract TSyntaxKind RefKeyword { get; } + abstract TSyntaxKind RightShiftExpression { get; } + abstract TSyntaxKind RightShiftAssignmentStatement { get; } abstract TSyntaxKind ParameterList { get; } abstract TSyntaxKind ReturnStatement { get; } abstract TSyntaxKind SimpleAssignment { get; } @@ -51,8 +55,4 @@ public interface ISyntaxKindFacade abstract TSyntaxKind StructDeclaration { get; } abstract TSyntaxKind[] TypeDeclaration { get; } abstract TSyntaxKind WhitespaceTrivia { get; } - abstract TSyntaxKind LeftShiftExpression { get; } - abstract TSyntaxKind RightShiftExpression { get; } - abstract TSyntaxKind LeftShiftAssignmentStatement { get; } - abstract TSyntaxKind RightShiftAssignmentStatement { get; } } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs index 45d41778a50..a13a63e96e7 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs @@ -25,6 +25,11 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.Attribute; public SyntaxKind[] ClassAndRecordDeclaration => new[] { SyntaxKind.ClassBlock }; public SyntaxKind ClassDeclaration => SyntaxKind.ClassBlock; + public SyntaxKind[] CommentTrivia => new[] + { + SyntaxKind.CommentTrivia, + SyntaxKind.DocumentationCommentTrivia, + }; public SyntaxKind[] ComparisonKinds => new[] { SyntaxKind.GreaterThanExpression, @@ -36,6 +41,7 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.ConstructorBlock; public SyntaxKind[] DefaultExpressions => new[] { SyntaxKind.NothingLiteralExpression }; + public SyntaxKind EndOfLineTrivia => SyntaxKind.EndOfLineTrivia; public SyntaxKind EnumDeclaration => SyntaxKind.EnumStatement; public SyntaxKind FieldDeclaration => SyntaxKind.FieldDeclaration; public SyntaxKind IdentifierName => SyntaxKind.IdentifierName; @@ -50,6 +56,7 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.ByRefKeyword; public SyntaxKind ReturnStatement => SyntaxKind.ReturnStatement; public SyntaxKind SimpleAssignment => SyntaxKind.SimpleAssignmentStatement; + public SyntaxKind SimpleCommentTrivia => SyntaxKind.CommentTrivia; public SyntaxKind SimpleMemberAccessExpression => SyntaxKind.SimpleMemberAccessExpression; public SyntaxKind[] StringLiteralExpressions => new[] { SyntaxKind.StringLiteralExpression }; public SyntaxKind StructDeclaration => SyntaxKind.StructureBlock; @@ -58,12 +65,5 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.RightShiftExpression; public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentStatement; public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentStatement; - public SyntaxKind[] CommentTrivia => new[] - { - SyntaxKind.CommentTrivia, - SyntaxKind.DocumentationCommentTrivia, - }; - public SyntaxKind SimpleCommentTrivia => SyntaxKind.CommentTrivia; public SyntaxKind WhitespaceTrivia => SyntaxKind.WhitespaceTrivia; - public SyntaxKind EndOfLineTrivia => SyntaxKind.EndOfLineTrivia; } From aed82c6bae44aeecc59719a43a5d9b26b2ee2f07 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 30 Jan 2023 17:16:17 +0100 Subject: [PATCH 22/27] Refactor partitioner of S4663 and minor refactor on a helper --- .../Rules/CommentsShouldNotBeEmptyBase.cs | 41 ++++++++++--------- .../Helpers/VisualBasicSyntaxHelper.cs | 6 +-- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 323d4c71bbd..0e0390212c2 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -45,7 +45,7 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia) { - var partitions = Partition(trivia); + var partitions = PartitionComments(trivia); if (partitions is null) { return; @@ -63,10 +63,10 @@ private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable string.IsNullOrWhiteSpace(GetCommentText(x))); } - private List> Partition(IEnumerable trivia) + private List> PartitionComments(IEnumerable trivia) { - // hot path: avoid unnecessary allocations - List> res = null; + // Hotpath: avoid unnecessary allocations + List> partitions = null; List current = null; var firstEndOfLineFound = false; @@ -74,8 +74,7 @@ private List> Partition(IEnumerable trivia) { if (IsSimpleComment(trivium)) // put it on the current block of "//" { - current ??= new(); - current.Add(trivium); + AddTriviaToPartition(ref current, trivium); firstEndOfLineFound = false; } // This is for the case, of two different comment types, for example: @@ -83,11 +82,10 @@ private List> Partition(IEnumerable trivia) // /// else if (IsValidTriviaType(trivium)) // valid but not "//", because of the upper if { - CloseCurrentPartition(); + CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); // all comments except single-line comments are parsed as a block already. - current ??= new(); - current.Add(trivium); - CloseCurrentPartition(); + AddTriviaToPartition(ref current, trivium); + CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); } // This handles an empty line, for example: // // some comment \n <- EOL found, set to true @@ -98,7 +96,7 @@ private List> Partition(IEnumerable trivia) { if (firstEndOfLineFound) { - CloseCurrentPartition(); + CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); } else { @@ -107,26 +105,29 @@ private List> Partition(IEnumerable trivia) } else if (!IsWhitespace(trivium)) { - CloseCurrentPartition(); + CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); } } - if (current is not null) + CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); + return partitions; + + static void AddTriviaToPartition(ref List current, SyntaxTrivia trivia) { - res ??= new(); - res.Add(current); + current ??= new(); + current.Add(trivia); } - return res; - - void CloseCurrentPartition() + // Hotpath: Don't capture variables + static void CloseCurrentPartition(ref List current, ref List> partitions, ref bool firstEndOfLineFound) { if (current is { Count: > 0 }) { - res ??= new(); - res.Add(current); + partitions ??= new(); + partitions.Add(current); current = null; } + firstEndOfLineFound = false; } } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs index 39994819bd2..d392971b41c 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs @@ -98,12 +98,12 @@ public static StatementSyntax GetSucceedingStatement(this StatementSyntax curren public static bool IsAnyKind(this SyntaxToken syntaxToken, params SyntaxKind[] syntaxKinds) => syntaxKinds.Contains((SyntaxKind)syntaxToken.RawKind); - public static bool AnyOfKind(this IEnumerable nodes, SyntaxKind kind) => - nodes.Any(n => n.RawKind == (int)kind); - public static bool IsAnyKind(this SyntaxTrivia syntaxTrivia, params SyntaxKind[] syntaxKinds) => syntaxKinds.Contains((SyntaxKind)syntaxTrivia.RawKind); + public static bool AnyOfKind(this IEnumerable nodes, SyntaxKind kind) => + nodes.Any(n => n.RawKind == (int)kind); + public static SyntaxToken? GetMethodCallIdentifier(this InvocationExpressionSyntax invocation) { if (invocation == null || From 12daa6e77b074c5238fcc45a5f4cc73d019c53d8 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Mon, 30 Jan 2023 17:49:45 +0100 Subject: [PATCH 23/27] Inline a simple local function --- .../Rules/CommentsShouldNotBeEmptyBase.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index 0e0390212c2..f5836aaaef9 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.Linq; using Microsoft.CodeAnalysis.Text; namespace SonarAnalyzer.Rules; @@ -51,16 +52,13 @@ private void CheckTrivia(SonarSyntaxTreeReportingContext context, IEnumerable trivia.Any() && trivia.All(x => string.IsNullOrWhiteSpace(GetCommentText(x))))) { var start = partition.First().GetLocation().SourceSpan.Start; var end = partition.Last().GetLocation().SourceSpan.End; var location = Location.Create(context.Tree, TextSpan.FromBounds(start, end)); context.ReportIssue(Diagnostic.Create(Rule, location)); } - - bool ShouldReport(IEnumerable trivia) => - trivia.Any() && trivia.All(x => string.IsNullOrWhiteSpace(GetCommentText(x))); } private List> PartitionComments(IEnumerable trivia) From 61b9263a7a72b3cb7574c00eff47877c77db9ecf Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 31 Jan 2023 11:01:09 +0100 Subject: [PATCH 24/27] Final PR Review changes --- .../Rules/CommentsShouldNotBeEmptyBase.cs | 26 ++++++---- .../TestCases/CommentsShouldNotBeEmpty.cs | 48 ++++++++++++++----- .../TestCases/CommentsShouldNotBeEmpty.vb | 37 +++++++++++--- 3 files changed, 83 insertions(+), 28 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs index f5836aaaef9..2acea99fd14 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/CommentsShouldNotBeEmptyBase.cs @@ -39,8 +39,16 @@ public abstract class CommentsShouldNotBeEmptyBase : SonarDiagnosti { foreach (var token in c.Tree.GetRoot().DescendantTokens()) { - CheckTrivia(c, token.LeadingTrivia); - CheckTrivia(c, token.TrailingTrivia); + // Hotpath: Don't allocate the trivia enumerable if not needed + if (token.HasLeadingTrivia) + { + CheckTrivia(c, token.LeadingTrivia); + } + + if (token.HasTrailingTrivia) + { + CheckTrivia(c, token.TrailingTrivia); + } } }); @@ -70,10 +78,9 @@ private List> PartitionComments(IEnumerable tri foreach (var trivium in trivia) { - if (IsSimpleComment(trivium)) // put it on the current block of "//" + if (IsSimpleComment(trivium)) { - AddTriviaToPartition(ref current, trivium); - firstEndOfLineFound = false; + AddTriviaToPartition(ref current, trivium, ref firstEndOfLineFound); } // This is for the case, of two different comment types, for example: // // @@ -82,11 +89,11 @@ private List> PartitionComments(IEnumerable tri { CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); // all comments except single-line comments are parsed as a block already. - AddTriviaToPartition(ref current, trivium); + AddTriviaToPartition(ref current, trivium, ref firstEndOfLineFound); CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); } // This handles an empty line, for example: - // // some comment \n <- EOL found, set to true + // // some comment \n <- EOL found, firstEndOfLineFound set to true // // \n <- EOL is set to false at CommentTrivia, set to true after it // // some other comment <- EOL is set to false at CommentTrivia, set to true after it // \n <- EOL found, is already true, closes current partition @@ -110,13 +117,14 @@ private List> PartitionComments(IEnumerable tri CloseCurrentPartition(ref current, ref partitions, ref firstEndOfLineFound); return partitions; - static void AddTriviaToPartition(ref List current, SyntaxTrivia trivia) + // Hotpath: Don't capture variables + static void AddTriviaToPartition(ref List current, SyntaxTrivia trivia, ref bool firstEndOfLineFound) { current ??= new(); current.Add(trivia); + firstEndOfLineFound = false; } - // Hotpath: Don't capture variables static void CloseCurrentPartition(ref List current, ref List> partitions, ref bool firstEndOfLineFound) { if (current is { Count: > 0 }) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index 74f22be8f1f..c6130b84400 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -10,7 +10,7 @@ public class SingleLine // // Noncompliant@-2 (a lot of whitespace before) - // + // // Noncompliant@-2 (a lot of whitespace after) @@ -41,7 +41,7 @@ public class SingleLine // // - // Noncompliant @-2 + // Noncompliant @-2 // \r @@ -136,7 +136,7 @@ public class SingleLineDocumentation /// /// // Noncompliant@-1 (a lot of whitespace before) - /// + /// // Noncompliant@-1 (a lot of whitespace after) /// @@ -188,17 +188,17 @@ public class MultiLine /* */ /* */ // Noncompliant@-1 (a lot of whitespace inside) - /**/ + /**/ // Noncompliant@-1 (a lot of whitespace after) // Noncompliant@+1 /* - * + * */ // Noncompliant@+1 /* - * + * */ // Noncompliant@+1 @@ -228,11 +228,11 @@ public class MultiLine /* */ there */ /* hey - * + * */ /* - * + * there */ /* @@ -286,12 +286,12 @@ public class MultiLineDocumentation /** */ /** */ // Noncompliant@-1 (a lot of whitespace inside) - /***/ + /***/ // Noncompliant@-1 (a lot of whitespace after) // Noncompliant@+1 /** - * + * */ // Noncompliant@+1 @@ -321,11 +321,11 @@ public class MultiLineDocumentation /** */ there */ /** hey - * + * */ /** - * + * there */ /** @@ -366,3 +366,27 @@ hey there z̶̤͚̅̍a̷͈̤̪͌͛̈ļ̷̈͐͝g̸̰̈́͂̆o̴̓̏͜ */ } + +// +// hey +// +// +// there +// + +/// +/// hey +/// +/// there +/// + + +// Noncompliant@+2 + +// +// +// + +// Noncompliant@+1 +/// +/// diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb index 66af30b900a..483e82103c9 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb @@ -1,20 +1,20 @@ Public Class Testcases - Public Sub Comment() ' + Public Sub Comment() ' ' Noncompliant@-2 (inline comment) Dim x = 42 'Ipsem Lorum Dim y = 42 ' Ipsem Lorum - Dim z = 42 ' + Dim z = 42 ' ' Noncompliant@-2 Dim a = 42 '''Ipsem Lorum Dim b = 42 ''' Ipsem Lorum - Dim c = 42 ''' + Dim c = 42 ''' - ' + ' ' Noncompliant@-2 (whitespace) @@ -53,7 +53,7 @@ ' - ' Noncompliant @-2 + ' Noncompliant @-2 ' \r @@ -83,7 +83,7 @@ End Sub Public Sub DocumentationComment() - ''' + ''' ' Noncompliant@-1 (whitespace) ''' * @@ -96,7 +96,7 @@ '''Ipsem Lorum ''' - ' Noncompliant @-1 + ' Noncompliant @-1 ' Noncompliant @+1 ''' @@ -119,3 +119,26 @@ End Sub End Class +' +' hey +' +' +' there +' + +''' +''' hey +''' +''' there +''' + + +' Noncompliant@+2 + +' +' +' + +' Noncompliant@+1 +''' +''' From c21c1f270068d6e8ef543a3839c00cc335331732 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 31 Jan 2023 11:05:58 +0100 Subject: [PATCH 25/27] Add some whitespace that i removed --- .../TestCases/CommentsShouldNotBeEmpty.cs | 8 ++++---- .../TestCases/CommentsShouldNotBeEmpty.vb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs index c6130b84400..095eb9350f8 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.cs @@ -10,7 +10,7 @@ public class SingleLine // // Noncompliant@-2 (a lot of whitespace before) - // + // // Noncompliant@-2 (a lot of whitespace after) @@ -136,7 +136,7 @@ public class SingleLineDocumentation /// /// // Noncompliant@-1 (a lot of whitespace before) - /// + /// // Noncompliant@-1 (a lot of whitespace after) /// @@ -188,7 +188,7 @@ public class MultiLine /* */ /* */ // Noncompliant@-1 (a lot of whitespace inside) - /**/ + /**/ // Noncompliant@-1 (a lot of whitespace after) // Noncompliant@+1 @@ -286,7 +286,7 @@ public class MultiLineDocumentation /** */ /** */ // Noncompliant@-1 (a lot of whitespace inside) - /***/ + /***/ // Noncompliant@-1 (a lot of whitespace after) // Noncompliant@+1 diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb index 483e82103c9..8d5000fd227 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/CommentsShouldNotBeEmpty.vb @@ -14,7 +14,7 @@ Dim b = 42 ''' Ipsem Lorum Dim c = 42 ''' - ' + ' ' Noncompliant@-2 (whitespace) @@ -83,7 +83,7 @@ End Sub Public Sub DocumentationComment() - ''' + ''' ' Noncompliant@-1 (whitespace) ''' * From 0994b95c20a83e58fa9c879dc23d4f7c0100443d Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Tue, 31 Jan 2023 11:07:42 +0100 Subject: [PATCH 26/27] Move some members of the VB facade around --- .../Facade/VisualBasicSyntaxKindFacade.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs index a13a63e96e7..2e855745d74 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxKindFacade.cs @@ -48,6 +48,8 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.IdentifierToken; public SyntaxKind InvocationExpression => SyntaxKind.InvocationExpression; public SyntaxKind InterpolatedStringExpression => SyntaxKind.InterpolatedStringExpression; + public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentStatement; + public SyntaxKind LeftShiftExpression => SyntaxKind.LeftShiftExpression; public SyntaxKind LocalDeclaration => SyntaxKind.LocalDeclarationStatement; public SyntaxKind[] MethodDeclarations => new[] { SyntaxKind.FunctionStatement, SyntaxKind.SubStatement }; public SyntaxKind[] ObjectCreationExpressions => new[] { SyntaxKind.ObjectCreationExpression }; @@ -55,15 +57,13 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade SyntaxKind.ParameterList; public SyntaxKind RefKeyword => SyntaxKind.ByRefKeyword; public SyntaxKind ReturnStatement => SyntaxKind.ReturnStatement; + public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentStatement; + public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; public SyntaxKind SimpleAssignment => SyntaxKind.SimpleAssignmentStatement; public SyntaxKind SimpleCommentTrivia => SyntaxKind.CommentTrivia; public SyntaxKind SimpleMemberAccessExpression => SyntaxKind.SimpleMemberAccessExpression; public SyntaxKind[] StringLiteralExpressions => new[] { SyntaxKind.StringLiteralExpression }; public SyntaxKind StructDeclaration => SyntaxKind.StructureBlock; public SyntaxKind[] TypeDeclaration => new[] { SyntaxKind.ClassBlock, SyntaxKind.StructureBlock, SyntaxKind.InterfaceBlock, SyntaxKind.EnumBlock }; - public SyntaxKind LeftShiftExpression => SyntaxKind.LeftShiftExpression; - public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; - public SyntaxKind LeftShiftAssignmentStatement => SyntaxKind.LeftShiftAssignmentStatement; - public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentStatement; public SyntaxKind WhitespaceTrivia => SyntaxKind.WhitespaceTrivia; } From 99f9c80c2d5384f0e697237a1698b49365ba25f7 Mon Sep 17 00:00:00 2001 From: Gregory Paidis Date: Thu, 2 Feb 2023 17:59:21 +0100 Subject: [PATCH 27/27] Move two SyntaxKinds in the correct alphabetical order in the C# Facade --- .../src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs index f1c5c26f906..83954886377 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxKindFacade.cs @@ -61,10 +61,10 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade public SyntaxKind[] ObjectCreationExpressions => new[] { SyntaxKind.ObjectCreationExpression, SyntaxKindEx.ImplicitObjectCreationExpression }; public SyntaxKind Parameter => SyntaxKind.Parameter; public SyntaxKind ParameterList => SyntaxKind.ParameterList; - public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentExpression; - public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; public SyntaxKind RefKeyword => SyntaxKind.RefKeyword; public SyntaxKind ReturnStatement => SyntaxKind.ReturnStatement; + public SyntaxKind RightShiftAssignmentStatement => SyntaxKind.RightShiftAssignmentExpression; + public SyntaxKind RightShiftExpression => SyntaxKind.RightShiftExpression; public SyntaxKind SimpleAssignment => SyntaxKind.SimpleAssignmentExpression; public SyntaxKind SimpleCommentTrivia => SyntaxKind.SingleLineCommentTrivia; public SyntaxKind SimpleMemberAccessExpression => SyntaxKind.SimpleMemberAccessExpression;