From 588a65f109fa1fb6756aecbca1832867c25d7d18 Mon Sep 17 00:00:00 2001 From: guwirth Date: Mon, 22 Jul 2019 16:27:13 +0200 Subject: [PATCH 1/2] clang-tidy: support diagnostic notes - add error, warning, note samples and tests - add flow support: note items - flow items are without ruleId - refactoring - close #1720 First draft without test in UI. --- .../sensors/clangtidy/CxxClangTidySensor.java | 45 +++++---- .../clangtidy/CxxClangTidySensorTest.java | 94 +++++++++++++++---- .../{cpd.report.txt => cpd.report-error.txt} | 0 .../clang-tidy-reports/cpd.report-note.txt | 16 ++++ .../clang-tidy-reports/cpd.report-warning.txt | 3 + 5 files changed, 123 insertions(+), 35 deletions(-) rename cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/{cpd.report.txt => cpd.report-error.txt} (100%) create mode 100644 cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-note.txt create mode 100644 cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-warning.txt diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java index bf466263aa..da1e59bf42 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java @@ -43,7 +43,8 @@ public class CxxClangTidySensor extends CxxIssuesReportSensor { public static final String DEFAULT_CHARSET_DEF = "UTF-8"; private static final Logger LOG = Loggers.get(CxxClangTidySensor.class); - private static final String REGEX = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): ([^]]+) \\[([^]]+)\\]"; + private static final String REGEX + = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): ([^]]+)( \\[([^]]+)\\])?"; private static final Pattern PATTERN = Pattern.compile(REGEX); /** @@ -67,35 +68,47 @@ public void describe(SensorDescriptor descriptor) { @Override protected void processReport(final SensorContext context, File report) { final String reportCharset = getContextStringProperty(context, - getLanguage().getPluginProperty(REPORT_CHARSET_DEF), DEFAULT_CHARSET_DEF); + getLanguage().getPluginProperty(REPORT_CHARSET_DEF), DEFAULT_CHARSET_DEF); LOG.debug("Parsing 'clang-tidy' report, CharSet= '{}'", reportCharset); try (Scanner scanner = new Scanner(report, reportCharset)) { + // sample: // E:\Development\SonarQube\cxx\sonar-cxx\sonar-cxx-plugin\src\test\resources\org\sonar\plugins\cxx\ // reports-project\clang-tidy-reports\..\..\cpd.cc:76:20: // warning: ISO C++11 does not allow conversion from string literal to 'char *' // [clang-diagnostic-writable-strings] - // ::: : [] - // relative paths - + CxxReportIssue issue = null; while (scanner.hasNextLine()) { - String line = scanner.nextLine(); - final Matcher matcher = PATTERN.matcher(line); + String nextLine = scanner.nextLine(); + final Matcher matcher = PATTERN.matcher(nextLine); if (matcher.matches()) { + // group: 1 2 3 4 5 7 + // ::: : [] MatchResult m = matcher.toMatchResult(); - String path = m.group(1); - String lineId = m.group(2); - String message = m.group(5); - String check = m.group(6); + String path = m.group(1); // relative paths + String line = m.group(2); + //String column = m.group(3); + String level = m.group(4); // error, warning, note, ... + String info = m.group(5); + String ruleId = m.group(7); // optional - CxxReportIssue issue = new CxxReportIssue(check, path, lineId, message); - saveUniqueViolation(context, issue); + if (ruleId != null) { + if (issue != null) { + saveUniqueViolation(context, issue); + } + issue = new CxxReportIssue(ruleId, path, line, info); + } else if ((issue != null) && "note".equals(level)) { + issue.addFlowElement(path, line, info); + } } } + if (issue != null) { + saveUniqueViolation(context, issue); + } } catch (final java.io.FileNotFoundException - | java.lang.IllegalArgumentException - | java.lang.IllegalStateException - | java.util.InputMismatchException e) { + | java.lang.IllegalArgumentException + | java.lang.IllegalStateException + | java.util.InputMismatchException e) { LOG.error("Failed to parse clang-tidy report: {}", e); } } diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java index e6190c5aad..40e29318fd 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java +++ b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java @@ -24,13 +24,11 @@ import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; import static org.mockito.Mockito.when; import org.sonar.api.batch.fs.internal.DefaultFileSystem; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; import org.sonar.api.batch.sensor.internal.SensorContextTester; -import org.sonar.api.config.Configuration; import org.sonar.api.config.internal.MapSettings; import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.sensors.utils.TestUtils; @@ -40,22 +38,28 @@ public class CxxClangTidySensorTest { private DefaultFileSystem fs; private CxxLanguage language; private final MapSettings settings = new MapSettings(); - private final String[] reportName = {"clang-tidy-reports/cpd.report.txt"}; @Before public void setUp() { fs = TestUtils.mockFileSystem(); language = TestUtils.mockCxxLanguage(); - when(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY)).thenReturn("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY); - when(language.getPluginProperty(CxxClangTidySensor.REPORT_CHARSET_DEF)).thenReturn("UTF-8"); - when(language.IsRecoveryEnabled()).thenReturn(Optional.of(Boolean.TRUE)); + + when(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY)). + thenReturn("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY); + when(language.getPluginProperty(CxxClangTidySensor.REPORT_CHARSET_DEF)). + thenReturn("UTF-8"); + when(language.IsRecoveryEnabled()). + thenReturn(Optional.of(Boolean.TRUE)); } @Test public void shouldIgnoreIssuesIfResourceNotFound() { SensorContextTester context = SensorContextTester.create(fs.baseDir()); - settings.setProperty(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), reportName[0]); + settings.setProperty( + language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), + "clang-tidy-reports/cpd.error.txt" + ); context.setSettings(settings); CxxClangTidySensor sensor = new CxxClangTidySensor(language); @@ -64,33 +68,82 @@ public void shouldIgnoreIssuesIfResourceNotFound() { } @Test - public void shouldReportCorrectViolations() { + public void shouldReportErrors() { SensorContextTester context = SensorContextTester.create(fs.baseDir()); - settings.setProperty(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), reportName[0]); + settings.setProperty( + language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), + "clang-tidy-reports/cpd.report-error.txt" + ); context.setSettings(settings); - context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp") - .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build()); + context.fileSystem().add(TestInputFileBuilder + .create("ProjectKey", "sources/utils/code_chunks.cpp") + .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n") + .build()); CxxClangTidySensor sensor = new CxxClangTidySensor(language); + sensor.execute(context); + assertThat(context.allIssues()).hasSize(1); + } - Configuration settings = Mockito.mock(Configuration.class); - when(settings.getStringArray("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY)).thenReturn(reportName); + @Test + public void shouldReportWarnings() { + SensorContextTester context = SensorContextTester.create(fs.baseDir()); + settings.setProperty( + language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), + "clang-tidy-reports/cpd.report-warning.txt" + ); + context.setSettings(settings); + + context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp") + .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build()); + + CxxClangTidySensor sensor = new CxxClangTidySensor(language); sensor.execute(context); assertThat(context.allIssues()).hasSize(1); } + @Test + public void shouldReportFlow() { + SensorContextTester context = SensorContextTester.create(fs.baseDir()); + + settings.setProperty( + language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), + "clang-tidy-reports/cpd.report-note.txt" + ); + context.setSettings(settings); + + context.fileSystem().add(TestInputFileBuilder + .create("ProjectKey", "sources/utils/code_chunks.cpp") + .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n") + .build()); + + CxxClangTidySensor sensor = new CxxClangTidySensor(language); + sensor.execute(context); + + SoftAssertions softly = new SoftAssertions(); + softly.assertThat(context.allIssues()).hasSize(1); // one issue + softly.assertThat(context.allIssues().iterator().next().flows()).hasSize(1); // with one flow + softly.assertThat(context.allIssues().iterator().next().flows().get(0).locations()).hasSize(4); // with four items + softly.assertAll(); + } + @Test public void invalidReportReportsNoIssues() { SensorContextTester context = SensorContextTester.create(fs.baseDir()); - settings.setProperty(language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), "clang-tidy-reports/cpd.report-empty.txt"); + settings.setProperty( + language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), + "clang-tidy-reports/cpd.report-empty.txt" + ); context.setSettings(settings); - context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp") - .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build()); + context.fileSystem().add(TestInputFileBuilder + .create("ProjectKey", "sources/utils/code_chunks.cpp") + .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n") + .build()); CxxClangTidySensor sensor = new CxxClangTidySensor(language); @@ -105,9 +158,12 @@ public void sensorDescriptor() { sensor.describe(descriptor); SoftAssertions softly = new SoftAssertions(); - softly.assertThat(descriptor.name()).isEqualTo(language.getName() + " ClangTidySensor"); - softly.assertThat(descriptor.languages()).containsOnly(language.getKey()); - softly.assertThat(descriptor.ruleRepositories()).containsOnly(CxxClangTidyRuleRepository.getRepositoryKey(language)); + softly.assertThat(descriptor.name()) + .isEqualTo(language.getName() + " ClangTidySensor"); + softly.assertThat(descriptor.languages()) + .containsOnly(language.getKey()); + softly.assertThat(descriptor.ruleRepositories()) + .containsOnly(CxxClangTidyRuleRepository.getRepositoryKey(language)); softly.assertAll(); } diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report.txt b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-error.txt similarity index 100% rename from cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report.txt rename to cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-error.txt diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-note.txt b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-note.txt new file mode 100644 index 0000000000..9394da7752 --- /dev/null +++ b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-note.txt @@ -0,0 +1,16 @@ +1 warning generated. +sources\utils\code_chunks.cpp:11:18: warning: Dereference of null pointer (loaded from variable 'x') [clang-analyzer-core.NullDereference] + printf("%d", *x); + ^ +sources\utils\code_chunks.cpp:7:5: note: 'x' initialized to a null pointer value + int* x = nullptr; + ^ +sources\utils\code_chunks.cpp:8:9: note: Assuming 'argc' is <= 1 + if (argc > 1) { + ^ +sources\utils\code_chunks.cpp:8:5: note: Taking false branch + if (argc > 1) { + ^ +sources\utils\code_chunks.cpp:11:18: note: Dereference of null pointer (loaded from variable 'x') + printf("%d", *x); + ^ diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-warning.txt b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-warning.txt new file mode 100644 index 0000000000..7af2bca249 --- /dev/null +++ b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-warning.txt @@ -0,0 +1,3 @@ +sources\utils\code_chunks.cpp:2:10: warning: function 'A::foo' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name] + void foo(int bar); + ^ From fc91eb1d16723687dd0c9bb6b0c2fab6aa9bedb0 Mon Sep 17 00:00:00 2001 From: guwirth Date: Thu, 25 Jul 2019 16:09:38 +0200 Subject: [PATCH 2/2] fix #1719 --- .../sensors/clangtidy/CxxClangTidySensor.java | 38 ++++++++++++++----- .../clangtidy/CxxClangTidySensorTest.java | 18 +++++++++ .../cpd.report-nodiscard.txt | 5 +++ 3 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-nodiscard.txt diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java index da1e59bf42..03b5b678d8 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java @@ -44,7 +44,7 @@ public class CxxClangTidySensor extends CxxIssuesReportSensor { private static final Logger LOG = Loggers.get(CxxClangTidySensor.class); private static final String REGEX - = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): ([^]]+)( \\[([^]]+)\\])?"; + = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): (.+)"; private static final Pattern PATTERN = Pattern.compile(REGEX); /** @@ -59,10 +59,10 @@ public CxxClangTidySensor(CxxLanguage language) { @Override public void describe(SensorDescriptor descriptor) { descriptor - .name(getLanguage().getName() + " ClangTidySensor") - .onlyOnLanguage(getLanguage().getKey()) - .createIssuesForRuleRepository(getRuleRepositoryKey()) - .onlyWhenConfiguration(conf -> conf.hasKey(getReportPathKey())); + .name(getLanguage().getName() + " ClangTidySensor") + .onlyOnLanguage(getLanguage().getKey()) + .createIssuesForRuleRepository(getRuleRepositoryKey()) + .onlyWhenConfiguration(conf -> conf.hasKey(getReportPathKey())); } @Override @@ -82,15 +82,33 @@ protected void processReport(final SensorContext context, File report) { String nextLine = scanner.nextLine(); final Matcher matcher = PATTERN.matcher(nextLine); if (matcher.matches()) { - // group: 1 2 3 4 5 7 - // ::: : [] + // group: 1 2 3 4 5 + // ::: : MatchResult m = matcher.toMatchResult(); String path = m.group(1); // relative paths String line = m.group(2); //String column = m.group(3); - String level = m.group(4); // error, warning, note, ... - String info = m.group(5); - String ruleId = m.group(7); // optional + String level = m.group(4); // error, warning, note, ... + String txt = m.group(5); // info( [ruleId])? + String info = null; + String ruleId = null; + + if (txt.endsWith("]")) { // [ruleId] + for (int i = txt.length() - 2; i >= 0; i--) { + char c = txt.charAt(i); + if (c == '[') { + info = txt.substring(0, i - 1); + ruleId = txt.substring(i + 1, txt.length() - 1); + break; + } + if (!(Character.isLetterOrDigit(c) || c == '-' || c == '.')) { + break; + } + } + } + if (info == null) { + info = txt; + } if (ruleId != null) { if (issue != null) { diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java index 40e29318fd..121deda4df 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java +++ b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java @@ -105,6 +105,24 @@ public void shouldReportWarnings() { assertThat(context.allIssues()).hasSize(1); } + @Test + public void shouldReportNodiscard() { + SensorContextTester context = SensorContextTester.create(fs.baseDir()); + + settings.setProperty( + language.getPluginProperty(CxxClangTidySensor.REPORT_PATH_KEY), + "clang-tidy-reports/cpd.report-nodiscard.txt" + ); + context.setSettings(settings); + + context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/code_chunks.cpp") + .setLanguage("cpp").initMetadata("asd\nasdas\nasda\n").build()); + + CxxClangTidySensor sensor = new CxxClangTidySensor(language); + sensor.execute(context); + assertThat(context.allIssues()).hasSize(1); + } + @Test public void shouldReportFlow() { SensorContextTester context = SensorContextTester.create(fs.baseDir()); diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-nodiscard.txt b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-nodiscard.txt new file mode 100644 index 0000000000..fa3dbb9c4e --- /dev/null +++ b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/clang-tidy-reports/cpd.report-nodiscard.txt @@ -0,0 +1,5 @@ +1 warning generated. +sources\utils\code_chunks.cpp:2:3: warning: function 'empty' should be marked [[nodiscard]] [modernize-use-nodiscard] + bool empty() const; + ^ + [[nodiscard]]