Skip to content

Commit a7dad3f

Browse files
maksim-grebeniuk-sonarsourcesonartech
authored andcommitted
SONARPY-3087 Create rule S7632: Warn the user if the syntax of NOSONAR(SXXXX) isn't valid (#383)
GitOrigin-RevId: 1876d3c30e6ab8805a3bffc079d35c60c92529d9
1 parent 5008d65 commit a7dad3f

10 files changed

Lines changed: 316 additions & 14 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.sonar.check.Rule;
20+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
21+
import org.sonar.plugins.python.api.nosonar.NoSonarInfoParser;
22+
import org.sonar.plugins.python.api.tree.Token;
23+
import org.sonar.plugins.python.api.tree.Tree;
24+
import org.sonar.plugins.python.api.tree.Trivia;
25+
26+
@Rule(key = "S7632")
27+
public class NoSonarCommentFormatCheck extends PythonSubscriptionCheck {
28+
29+
private static final String MESSAGE = "Fix the syntax of this issue suppression comment.";
30+
private final NoSonarInfoParser parser;
31+
32+
public NoSonarCommentFormatCheck() {
33+
parser = new NoSonarInfoParser();
34+
}
35+
36+
37+
@Override
38+
public void initialize(Context context) {
39+
context.registerSyntaxNodeConsumer(Tree.Kind.TOKEN, ctx -> {
40+
Token token = (Token) ctx.syntaxNode();
41+
for (Trivia trivia : token.trivia()) {
42+
if (parser.isInvalidIssueSuppressionComment(trivia.token().value())) {
43+
ctx.addIssue(trivia.token(), MESSAGE);
44+
}
45+
}
46+
});
47+
}
48+
}
49+

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ public Stream<Class<?>> getChecks() {
294294
NoPersonReferenceInTodoCheck.class,
295295
NoReRaiseInExitCheck.class,
296296
NoSonarCommentCheck.class,
297+
NoSonarCommentFormatCheck.class,
297298
NotDiscoverableTestMethodCheck.class,
298299
NotImplementedErrorInOperatorMethodsCheck.class,
299300
NumpyWeekMaskValidationCheck.class,
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<p>This rule raises an issue when issue suppression comments have an incorrect format or syntax.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>Issue suppression comments like <code># NOSONAR</code> and <code># noqa</code> are essential tools for controlling code analysis. When these
4+
comments have incorrect syntax, they may not work as expected, leading to confusion about which issues are actually suppressed.</p>
5+
<p>Python code analysis supports two main suppression formats: - <code># NOSONAR</code> - SonarQube’s suppression comment - <code># noqa</code> -
6+
Python’s standard "no quality assurance" comment</p>
7+
<p>Each format has specific syntax rules. When these rules are violated, the suppression might fail silently or behave unexpectedly, making it unclear
8+
whether issues are intentionally ignored or accidentally unsuppressed.</p>
9+
<h3>What is the potential impact?</h3>
10+
<p>Incorrectly formatted suppression comments can lead to unintended code analysis behavior. Issues that developers think are suppressed might still
11+
be reported, while malformed syntax might cause the analyzer to ignore more issues than intended. This creates confusion during code review and
12+
reduces confidence in the analysis results.</p>
13+
<h2>How to fix it</h2>
14+
<p>Fix the syntax of issue suppression comments to follow the correct format.</p>
15+
<p>For <code># NOSONAR</code>: - Use <code># NOSONAR</code> alone to suppress all issues on the line - Use <code># NOSONAR()</code> with empty
16+
parentheses to suppress all issues - Use <code># NOSONAR(rule1, rule2)</code> to suppress specific rules - Don’t use redundant commas in the
17+
parentheses, e.g. <code># NOSONAR(,)</code> - Close all parentheses properly</p>
18+
<p>For <code># noqa</code>: - Use <code># noqa</code> alone to suppress all issues on the line - Use <code># noqa: rule1,rule2</code> to suppress
19+
specific rules (with or without spaces after colon) - Don’t use redundant commas in the comma-separated lists, e.g. <code># noqa: ,rule1</code> -
20+
Don’t forget the colon (<code>:</code>) between <code>noqa</code> and the rule ID, and don’t use other punctuation</p>
21+
<h3>Code examples</h3>
22+
<h4>Noncompliant code example</h4>
23+
<pre data-diff-id="1" data-diff-type="noncompliant">
24+
def example():
25+
x = 1 # NOSONAR( # Noncompliant
26+
y = 2 # NOSONAR(a,) # Noncompliant
27+
z = 3 # NOSONAR)( # Noncompliant
28+
a = 4 # noqa: ,rule1 # Noncompliant
29+
b = 5 # noqa- rule1,rule2 # Noncompliant
30+
</pre>
31+
<h4>Compliant solution</h4>
32+
<pre data-diff-id="1" data-diff-type="compliant">
33+
def example():
34+
x = 1 # NOSONAR
35+
y = 2 # NOSONAR(a)
36+
z = 3 # NOSONAR
37+
a = 4 # noqa: rule1
38+
b = 5 # noqa: rule1,rule2
39+
</pre>
40+
<h2>Resources</h2>
41+
<h3>Documentation</h3>
42+
<ul>
43+
<li> SonarQube documentation - <a href="https://docs.sonarqube.org/latest/user-guide/issues/#header-4">Managing your code issues</a> </li>
44+
<li> Flake8 documentation - <a href="https://flake8.pycqa.org/en/latest/user/violations.html#in-line-ignoring-errors">In-line Ignoring Errors</a>
45+
</li>
46+
</ul>
47+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "Issue suppression comment should have the correct format",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "1min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Major",
11+
"ruleSpecification": "RSPEC-7632",
12+
"sqKey": "S7632",
13+
"scope": "All",
14+
"quickfix": "unknown",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "MEDIUM"
18+
},
19+
"attribute": "CONVENTIONAL"
20+
}
21+
}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@
286286
"S7515",
287287
"S7516",
288288
"S7517",
289-
"S7519"
289+
"S7519",
290+
"S7632"
290291
]
291292
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class NoSonarCommentFormatCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/nosonarCommentFormat.py", new NoSonarCommentFormatCheck());
27+
}
28+
29+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
2+
# NOSONAR
3+
# NOSONAR()
4+
# NOSONAR(a, b)
5+
# NOSONAR with some text
6+
# NOSONAR() with some text
7+
# NOSONAR(a, b) with some text
8+
# NOSONAR ()
9+
# NOSONARa
10+
# noqa
11+
# noqa: a,b
12+
# noqa: a, b
13+
14+
# Noncompliant@+1
15+
# NOSONAR(
16+
# Noncompliant@+1
17+
# NOSONAR)
18+
# Noncompliant@+1
19+
# NOSONAR)(
20+
# Noncompliant@+1
21+
# NOSONAR(,)
22+
# Noncompliant@+1
23+
# NOSONAR(a,)
24+
# Noncompliant@+1
25+
# NOSONAR(a (b))
26+
# Noncompliant@+1
27+
# noqa: a,,c
28+
# Noncompliant@+1
29+
# noqa: a,c,
30+
# Noncompliant@+1
31+
# noqa: a, some text

python-commons/src/test/java/org/sonar/plugins/python/nosonar/NoSonarLineInfoCollectorTest.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,24 @@ private static Stream<Arguments> provideCollectorParameters() {
105105
""
106106
),
107107
Arguments.of("# noqa: some text",
108-
Map.of(1, new NoSonarLineInfo(Set.of("some"))),
108+
Map.of(1, new NoSonarLineInfo(Set.of("some text"))),
109109
Set.of(),
110-
"some"
110+
"some text"
111111
),
112112
Arguments.of("# noqa: a,b",
113113
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"))),
114114
Set.of(),
115115
"a,b"
116116
),
117117
Arguments.of("# noqa: a, b",
118-
Map.of(1, new NoSonarLineInfo(Set.of("a"))),
118+
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"))),
119+
Set.of(),
120+
"a,b"
121+
),
122+
Arguments.of("# noqa: a, b # Some text",
123+
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"))),
119124
Set.of(),
120-
"a"
125+
"a,b"
121126
),
122127
Arguments.of("""
123128
""\"

python-frontend/src/main/java/org/sonar/plugins/python/api/nosonar/NoSonarInfoParser.java

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.sonar.plugins.python.api.nosonar;
1818

1919
import java.util.HashSet;
20+
import java.util.List;
2021
import java.util.Optional;
2122
import java.util.function.Predicate;
2223
import java.util.regex.Matcher;
@@ -27,7 +28,9 @@
2728

2829
public class NoSonarInfoParser {
2930

30-
private static final String NOQA_PATTERN_REGEX = "^#\\s*noqa(?::\\s*([^\\s]+))?($|\\s.*)";
31+
private static final String NOQA_PREFIX_REGEX = "#\\s*noqa([\\s:].*)?";
32+
private static final String NOQA_PATTERN_REGEX = "^#\\s*noqa(?::\\s*(.+))?.*";
33+
private static final String NOSONAR_PREFIX_REGEX = "^#\\s*NOSONAR(\\W.*)?";
3134
private static final String NOSONAR_PATTERN_REGEX = "^#\\s*NOSONAR(?:\\s*\\(([^)]*)\\))?($|\\s.*)";
3235

3336
private final Pattern noSonarPattern;
@@ -38,6 +41,34 @@ public NoSonarInfoParser() {
3841
noQaPattern = Pattern.compile(NOQA_PATTERN_REGEX);
3942
}
4043

44+
public boolean isInvalidIssueSuppressionComment(String commentsLine) {
45+
return splitInlineComments(commentsLine)
46+
.stream()
47+
.anyMatch(comment -> isInvalidNoSonarComment(comment) || isInvalidNoQaComment(comment));
48+
}
49+
50+
private boolean isInvalidNoSonarComment(String comment) {
51+
if (!Pattern.matches(NOSONAR_PREFIX_REGEX, comment)) {
52+
return false;
53+
}
54+
if (!isValidNoSonar(comment)) {
55+
return true;
56+
}
57+
var rules = parseNoSonarRules(comment).toList();
58+
return rules.size() > 1 && rules.stream().anyMatch(r -> r.isBlank() || r.contains(" "));
59+
}
60+
61+
private boolean isInvalidNoQaComment(String comment) {
62+
if (!comment.matches(NOQA_PREFIX_REGEX)) {
63+
return false;
64+
}
65+
if (!isValidNoQa(comment)) {
66+
return true;
67+
}
68+
var rules = parseNoQaRules(comment).toList();
69+
return !rules.isEmpty() && rules.stream().anyMatch(r -> r.isBlank() || r.contains(" "));
70+
}
71+
4172
private static boolean isValidNoSonar(String noSonarCommentLine) {
4273
return noSonarCommentLine.matches(NOSONAR_PATTERN_REGEX);
4374
}
@@ -48,20 +79,43 @@ private static boolean isValidNoQa(String noSonarCommentLine) {
4879

4980
public Optional<NoSonarLineInfo> parse(String commentLine) {
5081
var rules = new HashSet<String>();
51-
if (isValidNoSonar(commentLine)) {
52-
parseNoSonarRules(commentLine)
82+
var comments = splitInlineComments(commentLine);
83+
for (var comment : comments) {
84+
var noSonarLineInfo = parseComment(comment);
85+
86+
if (noSonarLineInfo != null) {
87+
if (noSonarLineInfo.isSuppressedRuleKeysEmpty()) {
88+
return Optional.of(noSonarLineInfo);
89+
}
90+
rules.addAll(noSonarLineInfo.suppressedRuleKeys());
91+
}
92+
}
93+
return Optional.of(rules).filter(Predicate.not(HashSet::isEmpty)).map(NoSonarLineInfo::new);
94+
}
95+
96+
@CheckForNull
97+
private NoSonarLineInfo parseComment(String comment) {
98+
var rules = new HashSet<String>();
99+
if (isValidNoSonar(comment)) {
100+
parseNoSonarRules(comment)
53101
.filter(Predicate.not(String::isEmpty))
54102
.forEach(rules::add);
55-
} else if (isValidNoQa(commentLine)) {
56-
parseNoQaRules(commentLine)
103+
} else if (isValidNoQa(comment)) {
104+
parseNoQaRules(comment)
57105
.filter(Predicate.not(String::isEmpty))
58106
.forEach(rules::add);
59107
} else {
60-
return Optional.empty();
108+
return null;
61109
}
62-
return Optional.of(new NoSonarLineInfo(rules));
110+
return new NoSonarLineInfo(rules);
63111
}
64112

113+
private static List<String> splitInlineComments(String commentsLine) {
114+
return Stream.of(commentsLine.split("#"))
115+
.filter(Predicate.not(String::isBlank))
116+
.map(s -> "#" + s)
117+
.toList();
118+
}
65119

66120
private Stream<String> parseNoSonarRules(String noSonarCommentLine) {
67121
var contentInsideParentheses = getParamsString(noSonarPattern, noSonarCommentLine);

0 commit comments

Comments
 (0)