Skip to content

Commit

Permalink
New rule S4663: Comments should not be empty (#6672)
Browse files Browse the repository at this point in the history
* rspec scaffolding

* Remove unnecessary usings

* Create initial skeleton for logic

* Implement the simplest case and some testcases, minor refactoring

* Implement single-line documentation comments

* Add Comment SyntaxTrivia Kinds on the Facades

* Refactor CommentsShouldNotBeEmpty

* Add a lot of testcases for CommentsShouldNotBeEmpty (C#)

* Minor changes on S4663 C# Testcases

* Minor refactoring on S4663 (C#)

* Add testcases for S4663 VB.NET

* Happy New Year for S4663

* Remove empty line

* Refactor the logic of S4663

* Add a lot of testcases for S4663

* Update dotnet ITs expected for Ember

* Apply PR Review Changes

* Appy PR Review changes

* Move some logic to the base for S4663

* Apply some more PR Review changes

* Move some members of the facades in the correct alphabetical order

* Refactor partitioner of S4663 and minor refactor on a helper

* Inline a simple local function

* Final PR Review changes

* Add some whitespace that i removed

* Move some members of the VB facade around

* Move two SyntaxKinds in the correct alphabetical order in the C# Facade
  • Loading branch information
gregory-paidis-sonarsource committed Feb 3, 2023
1 parent ae8b656 commit 81d0d19
Show file tree
Hide file tree
Showing 23 changed files with 1,008 additions and 14 deletions.
@@ -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
}
}
}
]
}
@@ -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
}
}
}
]
}
16 changes: 16 additions & 0 deletions analyzers/rspec/cs/S4663_c#.html
@@ -0,0 +1,16 @@
<p>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.</p>
<h2>Noncompliant Code Example</h2>
<pre>
//

///

/**

*/

/*

*/
</pre>

15 changes: 15 additions & 0 deletions 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"
}
1 change: 1 addition & 0 deletions analyzers/rspec/cs/Sonar_way_profile.json
Expand Up @@ -241,6 +241,7 @@
"S4583",
"S4586",
"S4635",
"S4663",
"S4790",
"S4792",
"S4830",
Expand Down
8 changes: 8 additions & 0 deletions analyzers/rspec/vbnet/S4663_vb.net.html
@@ -0,0 +1,8 @@
<p>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.</p>
<h2>Noncompliant Code Example</h2>
<pre>
'

'''
</pre>

15 changes: 15 additions & 0 deletions 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"
}
1 change: 1 addition & 0 deletions analyzers/rspec/vbnet/Sonar_way_profile.json
Expand Up @@ -108,6 +108,7 @@
"S4581",
"S4583",
"S4586",
"S4663",
"S4790",
"S4792",
"S4830",
Expand Down
Expand Up @@ -40,10 +40,14 @@ internal sealed class CSharpSyntaxFacade : SyntaxFacade<SyntaxKind>

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<SyntaxKind> 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<SyntaxNode> ArgumentExpressions(SyntaxNode node) =>
Expand Down
Expand Up @@ -29,6 +29,13 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade<SyntaxKind>
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,
Expand All @@ -41,19 +48,25 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade<SyntaxKind>
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 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;
public SyntaxKind[] StringLiteralExpressions => new[] { SyntaxKind.StringLiteralExpression, SyntaxKindEx.Utf8StringLiteralExpression };
public SyntaxKind StructDeclaration => SyntaxKind.StructDeclaration;
Expand All @@ -66,8 +79,5 @@ internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade<SyntaxKind>
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 WhitespaceTrivia => SyntaxKind.WhitespaceTrivia;
}
@@ -0,0 +1,87 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 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.Text;

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CommentsShouldNotBeEmpty : CommentsShouldNotBeEmptyBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

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),
};

// //
private static string GetSingleLineText(SyntaxTrivia trivia) =>
trivia.ToString().Trim().Substring(2);

// ///
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();
}

// /* */
private static string GetMultiLineText(SyntaxTrivia trivia) =>
ParseMultiLine(trivia.ToString(), 2); // Length of "/*"

// /** */
private static string GetMultiLineDocumentationText(SyntaxTrivia trivia) =>
ParseMultiLine(trivia.ToFullString(), 3); // Length of "/**"

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);
}

var stringBuilder = new StringBuilder();
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();
}
}
12 changes: 8 additions & 4 deletions analyzers/src/SonarAnalyzer.Common/Facade/ISyntaxKindFacade.cs
Expand Up @@ -26,29 +26,33 @@ public interface ISyntaxKindFacade<out TSyntaxKind>
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; }
abstract TSyntaxKind EndOfLineTrivia { get; }
abstract TSyntaxKind EnumDeclaration { get; }
abstract TSyntaxKind FieldDeclaration { get; }
abstract TSyntaxKind IdentifierName { get; }
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; }
abstract TSyntaxKind SimpleCommentTrivia { get; }
abstract TSyntaxKind SimpleMemberAccessExpression { get; }
abstract TSyntaxKind[] StringLiteralExpressions { get; }
abstract TSyntaxKind StructDeclaration { get; }
abstract TSyntaxKind[] TypeDeclaration { get; }
abstract TSyntaxKind LeftShiftExpression { get; }
abstract TSyntaxKind RightShiftExpression { get; }
abstract TSyntaxKind LeftShiftAssignmentStatement { get; }
abstract TSyntaxKind RightShiftAssignmentStatement { get; }
abstract TSyntaxKind WhitespaceTrivia { get; }
}
2 changes: 2 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs
Expand Up @@ -28,8 +28,10 @@ public abstract class SyntaxFacade<TSyntaxKind>
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<TSyntaxKind> syntaxKinds);
public abstract bool IsAnyKind(SyntaxNode node, params TSyntaxKind[] syntaxKinds);
public abstract bool IsAnyKind(SyntaxTrivia trivia, params TSyntaxKind[] syntaxKinds);
public abstract IEnumerable<SyntaxNode> ArgumentExpressions(SyntaxNode node);
public abstract ImmutableArray<SyntaxNode> AssignmentTargets(SyntaxNode assignment);
public abstract SyntaxNode AssignmentLeft(SyntaxNode assignment);
Expand Down

0 comments on commit 81d0d19

Please sign in to comment.