Skip to content

Commit 5d9f4cc

Browse files
guwirthguwirth
authored
Merge pull request #1742 from guwirth/clang-tidy-flow
clang-tidy: support diagnostic notes
2 parents 68a5078 + fc91eb1 commit 5d9f4cc

File tree

6 files changed

+167
-38
lines changed

6 files changed

+167
-38
lines changed

‎cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java‎

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ public class CxxClangTidySensor extends CxxIssuesReportSensor {
4343
public static final String DEFAULT_CHARSET_DEF = "UTF-8";
4444
private static final Logger LOG = Loggers.get(CxxClangTidySensor.class);
4545

46-
private static final String REGEX = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): ([^]]+) \\[([^]]+)\\]";
46+
private static final String REGEX
47+
= "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): (.+)";
4748
private static final Pattern PATTERN = Pattern.compile(REGEX);
4849

4950
/**
@@ -58,44 +59,74 @@ public CxxClangTidySensor(CxxLanguage language) {
5859
@Override
5960
public void describe(SensorDescriptor descriptor) {
6061
descriptor
61-
.name(getLanguage().getName() + " ClangTidySensor")
62-
.onlyOnLanguage(getLanguage().getKey())
63-
.createIssuesForRuleRepository(getRuleRepositoryKey())
64-
.onlyWhenConfiguration(conf -> conf.hasKey(getReportPathKey()));
62+
.name(getLanguage().getName() + " ClangTidySensor")
63+
.onlyOnLanguage(getLanguage().getKey())
64+
.createIssuesForRuleRepository(getRuleRepositoryKey())
65+
.onlyWhenConfiguration(conf -> conf.hasKey(getReportPathKey()));
6566
}
6667

6768
@Override
6869
protected void processReport(final SensorContext context, File report) {
6970
final String reportCharset = getContextStringProperty(context,
70-
getLanguage().getPluginProperty(REPORT_CHARSET_DEF), DEFAULT_CHARSET_DEF);
71+
getLanguage().getPluginProperty(REPORT_CHARSET_DEF), DEFAULT_CHARSET_DEF);
7172
LOG.debug("Parsing 'clang-tidy' report, CharSet= '{}'", reportCharset);
7273

7374
try (Scanner scanner = new Scanner(report, reportCharset)) {
75+
// sample:
7476
// E:\Development\SonarQube\cxx\sonar-cxx\sonar-cxx-plugin\src\test\resources\org\sonar\plugins\cxx\
7577
// reports-project\clang-tidy-reports\..\..\cpd.cc:76:20:
7678
// warning: ISO C++11 does not allow conversion from string literal to 'char *'
7779
// [clang-diagnostic-writable-strings]
78-
// <path>:<line>:<column>: <level>: <message> [<checkname>]
79-
// relative paths
80-
80+
CxxReportIssue issue = null;
8181
while (scanner.hasNextLine()) {
82-
String line = scanner.nextLine();
83-
final Matcher matcher = PATTERN.matcher(line);
82+
String nextLine = scanner.nextLine();
83+
final Matcher matcher = PATTERN.matcher(nextLine);
8484
if (matcher.matches()) {
85+
// group: 1 2 3 4 5
86+
// <path>:<line>:<column>: <level>: <txt>
8587
MatchResult m = matcher.toMatchResult();
86-
String path = m.group(1);
87-
String lineId = m.group(2);
88-
String message = m.group(5);
89-
String check = m.group(6);
88+
String path = m.group(1); // relative paths
89+
String line = m.group(2);
90+
//String column = m.group(3);
91+
String level = m.group(4); // error, warning, note, ...
92+
String txt = m.group(5); // info( [ruleId])?
93+
String info = null;
94+
String ruleId = null;
95+
96+
if (txt.endsWith("]")) { // [ruleId]
97+
for (int i = txt.length() - 2; i >= 0; i--) {
98+
char c = txt.charAt(i);
99+
if (c == '[') {
100+
info = txt.substring(0, i - 1);
101+
ruleId = txt.substring(i + 1, txt.length() - 1);
102+
break;
103+
}
104+
if (!(Character.isLetterOrDigit(c) || c == '-' || c == '.')) {
105+
break;
106+
}
107+
}
108+
}
109+
if (info == null) {
110+
info = txt;
111+
}
90112

91-
CxxReportIssue issue = new CxxReportIssue(check, path, lineId, message);
92-
saveUniqueViolation(context, issue);
113+
if (ruleId != null) {
114+
if (issue != null) {
115+
saveUniqueViolation(context, issue);
116+
}
117+
issue = new CxxReportIssue(ruleId, path, line, info);
118+
} else if ((issue != null) && "note".equals(level)) {
119+
issue.addFlowElement(path, line, info);
120+
}
93121
}
94122
}
123+
if (issue != null) {
124+
saveUniqueViolation(context, issue);
125+
}
95126
} catch (final java.io.FileNotFoundException
96-
| java.lang.IllegalArgumentException
97-
| java.lang.IllegalStateException
98-
| java.util.InputMismatchException e) {
127+
| java.lang.IllegalArgumentException
128+
| java.lang.IllegalStateException
129+
| java.util.InputMismatchException e) {
99130
LOG.error("Failed to parse clang-tidy report: {}", e);
100131
}
101132
}

‎cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java‎

Lines changed: 92 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,11 @@
2424
import org.assertj.core.api.SoftAssertions;
2525
import org.junit.Before;
2626
import org.junit.Test;
27-
import org.mockito.Mockito;
2827
import static org.mockito.Mockito.when;
2928
import org.sonar.api.batch.fs.internal.DefaultFileSystem;
3029
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
3130
import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor;
3231
import org.sonar.api.batch.sensor.internal.SensorContextTester;
33-
import org.sonar.api.config.Configuration;
3432
import org.sonar.api.config.internal.MapSettings;
3533
import org.sonar.cxx.CxxLanguage;
3634
import org.sonar.cxx.sensors.utils.TestUtils;
@@ -40,22 +38,28 @@ public class CxxClangTidySensorTest {
4038
private DefaultFileSystem fs;
4139
private CxxLanguage language;
4240
private final MapSettings settings = new MapSettings();
43-
private final String[] reportName = {"clang-tidy-reports/cpd.report.txt"};
4441

4542
@Before
4643
public void setUp() {
4744
fs = TestUtils.mockFileSystem();
4845
language = TestUtils.mockCxxLanguage();
49-
when(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY)).thenReturn("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY);
50-
when(language.getPluginProperty(CxxClangTidySensor.REPORT_CHARSET_DEF)).thenReturn("UTF-8");
51-
when(language.IsRecoveryEnabled()).thenReturn(Optional.of(Boolean.TRUE));
46+
47+
when(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY)).
48+
thenReturn("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY);
49+
when(language.getPluginProperty(CxxClangTidySensor.REPORT_CHARSET_DEF)).
50+
thenReturn("UTF-8");
51+
when(language.IsRecoveryEnabled()).
52+
thenReturn(Optional.of(Boolean.TRUE));
5253
}
5354

5455
@Test
5556
public void shouldIgnoreIssuesIfResourceNotFound() {
5657
SensorContextTester context = SensorContextTester.create(fs.baseDir());
5758

58-
settings.setProperty(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), reportName[0]);
59+
settings.setProperty(
60+
language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY),
61+
"clang-tidy-reports/cpd.error.txt"
62+
);
5963
context.setSettings(settings);
6064

6165
CxxClangTidySensor sensor = new CxxClangTidySensor(language);
@@ -64,33 +68,100 @@ public void shouldIgnoreIssuesIfResourceNotFound() {
6468
}
6569

6670
@Test
67-
public void shouldReportCorrectViolations() {
71+
public void shouldReportErrors() {
72+
SensorContextTester context = SensorContextTester.create(fs.baseDir());
73+
74+
settings.setProperty(
75+
language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY),
76+
"clang-tidy-reports/cpd.report-error.txt"
77+
);
78+
context.setSettings(settings);
79+
80+
context.fileSystem().add(TestInputFileBuilder
81+
.create("ProjectKey", "sources/utils/code_chunks.cpp")
82+
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n")
83+
.build());
84+
85+
CxxClangTidySensor sensor = new CxxClangTidySensor(language);
86+
sensor.execute(context);
87+
assertThat(context.allIssues()).hasSize(1);
88+
}
89+
90+
@Test
91+
public void shouldReportWarnings() {
6892
SensorContextTester context = SensorContextTester.create(fs.baseDir());
6993

70-
settings.setProperty(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), reportName[0]);
94+
settings.setProperty(
95+
language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY),
96+
"clang-tidy-reports/cpd.report-warning.txt"
97+
);
7198
context.setSettings(settings);
7299

73100
context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp")
74-
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build());
101+
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build());
75102

76103
CxxClangTidySensor sensor = new CxxClangTidySensor(language);
104+
sensor.execute(context);
105+
assertThat(context.allIssues()).hasSize(1);
106+
}
107+
108+
@Test
109+
public void shouldReportNodiscard() {
110+
SensorContextTester context = SensorContextTester.create(fs.baseDir());
77111

78-
Configuration settings = Mockito.mock(Configuration.class);
79-
when(settings.getStringArray("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY)).thenReturn(reportName);
112+
settings.setProperty(
113+
language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY),
114+
"clang-tidy-reports/cpd.report-nodiscard.txt"
115+
);
116+
context.setSettings(settings);
117+
118+
context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp")
119+
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build());
80120

121+
CxxClangTidySensor sensor = new CxxClangTidySensor(language);
81122
sensor.execute(context);
82123
assertThat(context.allIssues()).hasSize(1);
83124
}
84125

126+
@Test
127+
public void shouldReportFlow() {
128+
SensorContextTester context = SensorContextTester.create(fs.baseDir());
129+
130+
settings.setProperty(
131+
language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY),
132+
"clang-tidy-reports/cpd.report-note.txt"
133+
);
134+
context.setSettings(settings);
135+
136+
context.fileSystem().add(TestInputFileBuilder
137+
.create("ProjectKey", "sources/utils/code_chunks.cpp")
138+
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n")
139+
.build());
140+
141+
CxxClangTidySensor sensor = new CxxClangTidySensor(language);
142+
sensor.execute(context);
143+
144+
SoftAssertions softly = new SoftAssertions();
145+
softly.assertThat(context.allIssues()).hasSize(1); // one issue
146+
softly.assertThat(context.allIssues().iterator().next().flows()).hasSize(1); // with one flow
147+
softly.assertThat(context.allIssues().iterator().next().flows().get(0).locations()).hasSize(4); // with four items
148+
softly.assertAll();
149+
}
150+
85151
@Test
86152
public void invalidReportReportsNoIssues() {
87153
SensorContextTester context = SensorContextTester.create(fs.baseDir());
88154

89-
settings.setProperty(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), "clang-tidy-reports/cpd.report-empty.txt");
155+
settings.setProperty(
156+
language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY),
157+
"clang-tidy-reports/cpd.report-empty.txt"
158+
);
90159
context.setSettings(settings);
91160

92-
context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp")
93-
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build());
161+
context.fileSystem().add(TestInputFileBuilder
162+
.create("ProjectKey", "sources/utils/code_chunks.cpp")
163+
.setLanguage("cpp").initMetadata("asd\nasdas\nasda\n")
164+
.build());
94165

95166
CxxClangTidySensor sensor = new CxxClangTidySensor(language);
96167

@@ -105,9 +176,12 @@ public void sensorDescriptor() {
105176
sensor.describe(descriptor);
106177

107178
SoftAssertions softly = new SoftAssertions();
108-
softly.assertThat(descriptor.name()).isEqualTo(language.getName() + " ClangTidySensor");
109-
softly.assertThat(descriptor.languages()).containsOnly(language.getKey());
110-
softly.assertThat(descriptor.ruleRepositories()).containsOnly(CxxClangTidyRuleRepository.getRepositoryKey(language));
179+
softly.assertThat(descriptor.name())
180+
.isEqualTo(language.getName() + " ClangTidySensor");
181+
softly.assertThat(descriptor.languages())
182+
.containsOnly(language.getKey());
183+
softly.assertThat(descriptor.ruleRepositories())
184+
.containsOnly(CxxClangTidyRuleRepository.getRepositoryKey(language));
111185
softly.assertAll();
112186
}
113187

cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report.txt renamed to cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-error.txt

File renamed without changes.

‎cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-nodiscard.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
1 warning generated.
2+
sources\utils\code_chunks.cpp:2:3: warning: function 'empty' should be marked [[nodiscard]] [modernize-use-nodiscard]
3+
bool empty() const;
4+
^
5+
[[nodiscard]]

‎cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-note.txt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
1 warning generated.
2+
sources\utils\code_chunks.cpp:11:18: warning: Dereference of null pointer (loaded from variable 'x') [clang-analyzer-core.NullDereference]
3+
printf("%d", *x);
4+
^
5+
sources\utils\code_chunks.cpp:7:5: note: 'x' initialized to a null pointer value
6+
int* x = nullptr;
7+
^
8+
sources\utils\code_chunks.cpp:8:9: note: Assuming 'argc' is <= 1
9+
if (argc > 1) {
10+
^
11+
sources\utils\code_chunks.cpp:8:5: note: Taking false branch
12+
if (argc > 1) {
13+
^
14+
sources\utils\code_chunks.cpp:11:18: note: Dereference of null pointer (loaded from variable 'x')
15+
printf("%d", *x);
16+
^

‎cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-warning.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
sources\utils\code_chunks.cpp:2:10: warning: function 'A::foo' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
2+
void foo(int bar);
3+
^

0 commit comments

Comments
 (0)