Skip to content

Commit 900d5b4

Browse files
thomas-serre-sonarsourcesonartech
thomas-serre-sonarsource
authored and
sonartech
committed
SONARPY-3119 Send the comments containing NOSONAR through telemetry (#387)
GitOrigin-RevId: 86a554c49b56492ca7971a1caf2082ff312db7a3
1 parent 36d5687 commit 900d5b4

File tree

8 files changed

+174
-39
lines changed

8 files changed

+174
-39
lines changed

‎python-commons/src/main/java/org/sonar/plugins/python/IPynbSensor.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ public void execute(SensorContext context) {
108108
} else {
109109
processNotebooksFiles(pythonFiles, context);
110110
}
111+
111112
sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOSONAR_RULE_ID_KEYS, noSonarLineInfoCollector.getSuppressedRuleIds());
113+
sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOSONAR_COMMENTS_KEYS, noSonarLineInfoCollector.getCommentWithExactlyOneRuleSuppressed());
114+
112115
sensorTelemetryStorage.send(context);
113116
}
114117

‎python-commons/src/main/java/org/sonar/plugins/python/TelemetryMetricKey.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ public enum TelemetryMetricKey {
2727
IPYNB_DATABRICKS_FOUND("python.notebook.databricks.ipynb"),
2828
PYTHON_DEPENDENCIES("python.dependencies"),
2929
PYTHON_DEPENDENCIES_FORMAT_VERSION("python.dependencies.format_version"),
30-
NOSONAR_RULE_ID_KEYS("python.nosonar.rule_ids");
30+
NOSONAR_RULE_ID_KEYS("python.nosonar.rule_ids"),
31+
NOSONAR_COMMENTS_KEYS("python.nosonar.comments");
3132

3233
private final String key;
3334

‎python-commons/src/main/java/org/sonar/plugins/python/nosonar/NoSonarLineInfoCollector.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,4 +142,12 @@ public String getSuppressedRuleIds(){
142142
.sorted()
143143
.collect(Collectors.joining(","));
144144
}
145+
146+
public String getCommentWithExactlyOneRuleSuppressed(){
147+
return componentKeyToNoSonarLineInfoMap.values().stream()
148+
.flatMap(map -> map.values().stream())
149+
.filter(noSonarLineInfo -> noSonarLineInfo.suppressedRuleKeys().size() == 1)
150+
.map(noSonarLineInfo -> noSonarLineInfo.suppressedRuleKeys().stream().findFirst().get() +":"+ noSonarLineInfo.comment())
151+
.collect(Collectors.joining(";;"));
152+
}
145153
}

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

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@ class NoSonarLineInfoCollectorTest {
3131

3232
@ParameterizedTest
3333
@MethodSource("provideCollectorParameters")
34-
void collector_test(String code, Map<Integer, NoSonarLineInfo> expectedLineInfos, Set<Integer> expectedEmptyNoSonarLines, String expectedSuppressedRuleIds) {
34+
void collector_test(
35+
String code,
36+
Map<Integer, NoSonarLineInfo> expectedLineInfos,
37+
Set<Integer> expectedEmptyNoSonarLines,
38+
String expectedSuppressedRuleIds,
39+
String expectedComments
40+
) {
3541
var astNode = PythonParser.create().parse(code);
3642

3743
var fileInput = new PythonTreeMaker().fileInput(astNode);
@@ -41,6 +47,7 @@ void collector_test(String code, Map<Integer, NoSonarLineInfo> expectedLineInfos
4147
Assertions.assertThat(collector.get("foo.py")).isEqualTo(expectedLineInfos);
4248
Assertions.assertThat(collector.getLinesWithEmptyNoSonar("foo.py")).isEqualTo(expectedEmptyNoSonarLines);
4349
Assertions.assertThat(collector.getSuppressedRuleIds()).isEqualTo(expectedSuppressedRuleIds);
50+
Assertions.assertThat(collector.getCommentWithExactlyOneRuleSuppressed()).isEqualTo(expectedComments);
4451
}
4552

4653
private static Stream<Arguments> provideCollectorParameters() {
@@ -50,79 +57,116 @@ private static Stream<Arguments> provideCollectorParameters() {
5057
""",
5158
Map.of(1, new NoSonarLineInfo(Set.of("something"))),
5259
Set.of(),
53-
"something"
60+
"something",
61+
"something:"
5462
),
5563
Arguments.of("""
5664
a = 1 # NOSONAR(one, two) some text
5765
""",
58-
Map.of(1, new NoSonarLineInfo(Set.of("one", "two"))),
66+
Map.of(1, new NoSonarLineInfo(Set.of("one", "two"), "some text")),
5967
Set.of(),
60-
"one,two"
68+
"one,two",
69+
""
6170
),
6271
Arguments.of("""
6372
a = 1 # NOSONAR()
6473
""",
6574
Map.of(1, new NoSonarLineInfo(Set.of())),
6675
Set.of(1),
76+
"",
6777
""
6878
),
6979
Arguments.of("""
7080
a = 1 # NOSONAR(something,)
7181
""",
7282
Map.of(1, new NoSonarLineInfo(Set.of("something"))),
7383
Set.of(),
74-
"something"
84+
"something",
85+
"something:"
7586
),
7687
Arguments.of("""
77-
a = 1 # NOSONAR(something,) adsgvnbdfa;l
88+
a = 1 # NOSONAR(something,) this is a comment on why I suppressed this rule
7889
""",
79-
Map.of(1, new NoSonarLineInfo(Set.of("something"))),
90+
Map.of(1, new NoSonarLineInfo(Set.of("something"), "this is a comment on why I suppressed this rule")),
8091
Set.of(),
81-
"something"
92+
"something",
93+
"something:this is a comment on why I suppressed this rule"
8294
),
8395
Arguments.of("""
8496
a = 1 # NOSONAR
8597
""",
86-
Map.of(1, new NoSonarLineInfo(Set.of())),
98+
Map.of(1, new NoSonarLineInfo(Set.of(), "")),
8799
Set.of(1),
100+
"",
88101
""
89102
),
90103
Arguments.of("""
91104
a = 1 # NOSONAR some text
92105
""",
93-
Map.of(1, new NoSonarLineInfo(Set.of())),
106+
Map.of(1, new NoSonarLineInfo(Set.of(), "some text")),
94107
Set.of(1),
108+
"",
95109
""
96110
),
111+
Arguments.of("""
112+
a = 1 # NOSONAR(aRule) this is a very long comment and I don't want to send all these characters to sonarqube through telemetry
113+
""",
114+
Map.of(1, new NoSonarLineInfo(Set.of("aRule"), "this is a very long comment and I don't want to s")),
115+
Set.of(),
116+
"aRule",
117+
"aRule:this is a very long comment and I don't want to s"
118+
),
119+
Arguments.of("""
120+
a = 1 # NOSONAR some comment
121+
b = 2 # NOSONAR(bRule) b comment
122+
c = 3 # NOSONAR(cRule) c comment
123+
d, e = 4,5 # NOSONAR(dRule, eRule) d and e comment
124+
""",
125+
Map.of(
126+
1, new NoSonarLineInfo(Set.of(), "some comment"),
127+
2, new NoSonarLineInfo(Set.of("bRule"), "b comment"),
128+
3, new NoSonarLineInfo(Set.of("cRule"), "c comment"),
129+
4, new NoSonarLineInfo(Set.of("dRule", "eRule"), "d and e comment")
130+
),
131+
Set.of(1),
132+
"bRule,cRule,dRule,eRule",
133+
"bRule:b comment;;cRule:c comment"
134+
),
97135
Arguments.of("a = 1 # NOSONAR some text",
98-
Map.of(1, new NoSonarLineInfo(Set.of())),
136+
Map.of(1, new NoSonarLineInfo(Set.of(), "some text")),
99137
Set.of(1),
138+
"",
100139
""
101140
),
102141
Arguments.of("# NOSONAR some text",
103-
Map.of(1, new NoSonarLineInfo(Set.of())),
142+
Map.of(1, new NoSonarLineInfo(Set.of(), "some text")),
104143
Set.of(1),
144+
"",
105145
""
106146
),
107147
Arguments.of("# noqa: some text",
108148
Map.of(1, new NoSonarLineInfo(Set.of("some text"))),
109149
Set.of(),
110-
"some text"
150+
"some text",
151+
"some text:"
111152
),
112153
Arguments.of("# noqa: a,b",
113154
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"))),
114155
Set.of(),
115-
"a,b"
156+
"a,b",
157+
""
116158
),
117159
Arguments.of("# noqa: a, b",
118160
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"))),
119161
Set.of(),
120-
"a,b"
162+
"a,b",
163+
""
121164
),
122165
Arguments.of("# noqa: a, b # Some text",
123-
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"))),
166+
Map.of(1, new NoSonarLineInfo(Set.of("a", "b"), "# Some text")),
124167
Set.of(),
125-
"a,b"
168+
"a,b",
169+
""
126170
),
127171
Arguments.of("""
128172
""\"
@@ -139,6 +183,7 @@ private static Stream<Arguments> provideCollectorParameters() {
139183
5, new NoSonarLineInfo(Set.of())
140184
),
141185
Set.of(1, 2, 3, 4, 5),
186+
"",
142187
""
143188
)
144189
);

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

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424
import java.util.regex.Pattern;
2525
import java.util.stream.Stream;
2626
import javax.annotation.CheckForNull;
27-
import javax.annotation.Nullable;
2827

2928
public class NoSonarInfoParser {
3029

30+
private static final Integer MAX_COMMENT_LENGTH = 50;
3131
private static final String NOQA_PREFIX_REGEX = "#\\s*noqa([\\s:].*)?";
3232
private static final String NOQA_PATTERN_REGEX = "^#\\s*noqa(?::\\s*(.+))?.*";
3333
private static final String NOSONAR_PREFIX_REGEX = "^#\\s*NOSONAR(\\W.*)?";
@@ -79,6 +79,7 @@ public static boolean isValidNoQa(String noSonarCommentLine) {
7979

8080
public Optional<NoSonarLineInfo> parse(String commentLine) {
8181
var rules = new HashSet<String>();
82+
StringBuilder concatenatedCommentBuilder = new StringBuilder();
8283
var comments = splitInlineComments(commentLine);
8384
for (var comment : comments) {
8485
var noSonarLineInfo = parseComment(comment);
@@ -88,26 +89,35 @@ public Optional<NoSonarLineInfo> parse(String commentLine) {
8889
return Optional.of(noSonarLineInfo);
8990
}
9091
rules.addAll(noSonarLineInfo.suppressedRuleKeys());
92+
concatenatedCommentBuilder.append(noSonarLineInfo.comment());
93+
} else {
94+
concatenatedCommentBuilder.append(comment.strip());
9195
}
9296
}
93-
return Optional.of(rules).filter(Predicate.not(HashSet::isEmpty)).map(NoSonarLineInfo::new);
97+
if (rules.isEmpty()) {
98+
return Optional.empty();
99+
}
100+
return Optional.of(new NoSonarLineInfo(rules, concatenatedCommentBuilder.toString()));
94101
}
95102

96103
@CheckForNull
97-
private NoSonarLineInfo parseComment(String comment) {
104+
private NoSonarLineInfo parseComment(String commentLine) {
98105
var rules = new HashSet<String>();
99-
if (isValidNoSonar(comment)) {
100-
parseNoSonarRules(comment)
106+
String comment = "";
107+
if (isValidNoSonar(commentLine)) {
108+
parseNoSonarRules(commentLine)
101109
.filter(Predicate.not(String::isEmpty))
102110
.forEach(rules::add);
103-
} else if (isValidNoQa(comment)) {
104-
parseNoQaRules(comment)
111+
comment = parseNoSonarComment(commentLine);
112+
} else if (isValidNoQa(commentLine)) {
113+
parseNoQaRules(commentLine)
105114
.filter(Predicate.not(String::isEmpty))
106115
.forEach(rules::add);
116+
comment = parseNoQaComment(commentLine);
107117
} else {
108118
return null;
109119
}
110-
return new NoSonarLineInfo(rules);
120+
return new NoSonarLineInfo(rules, comment);
111121
}
112122

113123
private static List<String> splitInlineComments(String commentsLine) {
@@ -122,23 +132,40 @@ private Stream<String> parseNoSonarRules(String noSonarCommentLine) {
122132
return parseParamsString(contentInsideParentheses);
123133
}
124134

135+
private String parseNoSonarComment(String noSonarCommentLine) {
136+
return getTruncatedCommentString(noSonarPattern, noSonarCommentLine).strip();
137+
}
125138

126139
private Stream<String> parseNoQaRules(String noSonarCommentLine) {
127140
var contentInsideParentheses = getParamsString(noQaPattern, noSonarCommentLine);
128141
return parseParamsString(contentInsideParentheses);
129142
}
130143

131-
@CheckForNull
144+
private String parseNoQaComment(String noSonarCommentLine) {
145+
return getTruncatedCommentString(noQaPattern, noSonarCommentLine).strip();
146+
}
147+
132148
private static String getParamsString(Pattern pattern, String noSonarCommentLine) {
149+
return getPatternGroup(1, pattern, noSonarCommentLine);
150+
}
151+
152+
private static String getTruncatedCommentString(Pattern pattern, String noSonarCommentLine) {
153+
// Actually, group 1 could be the comment group if no rule is specified but we are only interested in comment with exactly one rule suppressed.
154+
String commentString = getPatternGroup(2, pattern, noSonarCommentLine);
155+
return commentString.length() > MAX_COMMENT_LENGTH ? commentString.substring(0, MAX_COMMENT_LENGTH) : commentString;
156+
}
157+
158+
private static String getPatternGroup(int groupIndex, Pattern pattern, String noSonarCommentLine) {
133159
return Optional.of(noSonarCommentLine)
134160
.map(pattern::matcher)
135161
.filter(Matcher::matches)
136-
.map(m -> m.group(1))
137-
.orElse(null);
162+
.filter(m -> m.groupCount() >= groupIndex)
163+
.map(m -> m.group(groupIndex))
164+
.orElse("");
138165
}
139166

140-
private static Stream<String> parseParamsString(@Nullable String paramsString) {
141-
if (paramsString == null) {
167+
private static Stream<String> parseParamsString(String paramsString) {
168+
if (paramsString.isBlank()) {
142169
return Stream.of();
143170
}
144171

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818

1919
import java.util.Set;
2020

21-
public record NoSonarLineInfo(Set<String> suppressedRuleKeys) {
21+
public record NoSonarLineInfo(Set<String> suppressedRuleKeys, String comment) {
22+
public NoSonarLineInfo(Set<String> suppressedRuleKeys) {
23+
this(suppressedRuleKeys, "");
24+
}
2225

2326
public boolean isSuppressedRuleKeysEmpty() {
2427
return suppressedRuleKeys.isEmpty();

‎python-frontend/src/test/java/org/sonar/plugins/python/api/nosonar/NoSonarInfoParserTest.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ private static Stream<Arguments> provideParserParameters() {
5151
),
5252
Arguments.of(
5353
"# NOSONAR(one, two) some text",
54-
new NoSonarLineInfo(Set.of("one", "two"))
54+
new NoSonarLineInfo(Set.of("one", "two"),"some text")
5555
),
5656
Arguments.of(
5757
"# NOSONAR()",
@@ -63,27 +63,35 @@ private static Stream<Arguments> provideParserParameters() {
6363
),
6464
Arguments.of(
6565
"# NOSONAR(something,) abc",
66-
new NoSonarLineInfo(Set.of("something"))
66+
new NoSonarLineInfo(Set.of("something"),"abc")
6767
),
6868
Arguments.of(
6969
"# NOSONAR",
70-
new NoSonarLineInfo(Set.of())
70+
new NoSonarLineInfo(Set.of(),"")
71+
),
72+
Arguments.of(
73+
"# NOSONAR a very long comment that I don't want to keep that long because there is more than 50 characters",
74+
new NoSonarLineInfo(Set.of(),"a very long comment that I don't want to keep tha")
7175
),
7276
Arguments.of(
7377
"# NOSONAR some text",
74-
new NoSonarLineInfo(Set.of())
78+
new NoSonarLineInfo(Set.of(), "some text")
7579
),
7680
Arguments.of(
7781
"# NOSONAR(a) # NOSONAR(b,c) # Some text # NOSONAR(d,)",
78-
new NoSonarLineInfo(Set.of("a", "b", "c", "d"))
82+
new NoSonarLineInfo(Set.of("a", "b", "c", "d"), "# Some text")
7983
),
8084
Arguments.of(
8185
"# NOSONAR(a) # NOSONAR(b,c) # Some text # NOSONAR",
8286
new NoSonarLineInfo(Set.of())
8387
),
8488
Arguments.of(
8589
"# some text # NOSONAR(a)",
86-
new NoSonarLineInfo(Set.of("a"))
90+
new NoSonarLineInfo(Set.of("a"), "# some text")
91+
),
92+
Arguments.of(
93+
"# NOSONAR(a) # NOSONAR(b,c) # Some text # NOSONAR",
94+
new NoSonarLineInfo(Set.of())
8795
),
8896
Arguments.of(
8997
"# noqa: some text",
@@ -99,7 +107,7 @@ private static Stream<Arguments> provideParserParameters() {
99107
),
100108
Arguments.of(
101109
"# noqa: a, b # noqa: c # some text # noqa: d,e",
102-
new NoSonarLineInfo(Set.of("a", "b", "c", "d", "e"))
110+
new NoSonarLineInfo(Set.of("a", "b", "c", "d", "e"), "# some text")
103111
),
104112
Arguments.of(
105113
"# noqa: a, b # noqa: c # some text # noqa",

0 commit comments

Comments
 (0)