Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New rule S4663: Comments should not be empty #6672

Merged
merged 27 commits into from Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d60e343
rspec scaffolding
gregory-paidis-sonarsource Jan 24, 2023
7bc733e
Remove unnecessary usings
gregory-paidis-sonarsource Jan 24, 2023
697938d
Create initial skeleton for logic
gregory-paidis-sonarsource Jan 25, 2023
3b6e421
Implement the simplest case and some testcases, minor refactoring
gregory-paidis-sonarsource Jan 25, 2023
270b503
Implement single-line documentation comments
gregory-paidis-sonarsource Jan 25, 2023
397f3ce
Add Comment SyntaxTrivia Kinds on the Facades
gregory-paidis-sonarsource Jan 25, 2023
c6e1155
Refactor CommentsShouldNotBeEmpty
gregory-paidis-sonarsource Jan 25, 2023
044dc98
Add a lot of testcases for CommentsShouldNotBeEmpty (C#)
gregory-paidis-sonarsource Jan 25, 2023
a79ff10
Minor changes on S4663 C# Testcases
gregory-paidis-sonarsource Jan 26, 2023
de69307
Minor refactoring on S4663 (C#)
gregory-paidis-sonarsource Jan 26, 2023
cafb38d
Add testcases for S4663 VB.NET
gregory-paidis-sonarsource Jan 26, 2023
d235534
Happy New Year for S4663
gregory-paidis-sonarsource Jan 26, 2023
b98189a
Remove empty line
gregory-paidis-sonarsource Jan 26, 2023
aa9eab5
Refactor the logic of S4663
gregory-paidis-sonarsource Jan 26, 2023
d76e0d4
Add a lot of testcases for S4663
gregory-paidis-sonarsource Jan 26, 2023
2337881
Update dotnet ITs expected for Ember
gregory-paidis-sonarsource Jan 27, 2023
0ad4cff
Apply PR Review Changes
gregory-paidis-sonarsource Jan 27, 2023
f8492a9
Appy PR Review changes
gregory-paidis-sonarsource Jan 30, 2023
7e1b658
Move some logic to the base for S4663
gregory-paidis-sonarsource Jan 30, 2023
3430d93
Apply some more PR Review changes
gregory-paidis-sonarsource Jan 30, 2023
80c0476
Move some members of the facades in the correct alphabetical order
gregory-paidis-sonarsource Jan 30, 2023
aed82c6
Refactor partitioner of S4663 and minor refactor on a helper
gregory-paidis-sonarsource Jan 30, 2023
12daa6e
Inline a simple local function
gregory-paidis-sonarsource Jan 30, 2023
61b9263
Final PR Review changes
gregory-paidis-sonarsource Jan 31, 2023
c21c1f2
Add some whitespace that i removed
gregory-paidis-sonarsource Jan 31, 2023
0994b95
Move some members of the VB facade around
gregory-paidis-sonarsource Jan 31, 2023
99f9c80
Move two SyntaxKinds in the correct alphabetical order in the C# Facade
gregory-paidis-sonarsource Feb 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,17 @@
{
"issues": [
{
"id": "S4663",
"message": "Remove this empty comment",
"location": {
"uri": "sources\Ember-MM\EmberAPI\clsAPICommon.vb",
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
"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",
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
"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; }
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
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