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

clang-tidy: support diagnostic notes #1742

Merged
merged 2 commits into from
Jul 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/**
Expand All @@ -58,44 +59,74 @@ 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
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]
// <path>:<line>:<column>: <level>: <message> [<checkname>]
// 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
// <path>:<line>:<column>: <level>: <txt>
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 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;
}

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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -64,33 +68,100 @@ public void shouldIgnoreIssuesIfResourceNotFound() {
}

@Test
public void shouldReportCorrectViolations() {
public void shouldReportErrors() {
SensorContextTester context = SensorContextTester.create(fs.baseDir());

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());

CxxClangTidySensor sensor = new CxxClangTidySensor(language);
sensor.execute(context);
assertThat(context.allIssues()).hasSize(1);
}

@Test
public void shouldReportWarnings() {
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-warning.txt"
);
context.setSettings(settings);

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

CxxClangTidySensor sensor = new CxxClangTidySensor(language);
sensor.execute(context);
assertThat(context.allIssues()).hasSize(1);
}

@Test
public void shouldReportNodiscard() {
SensorContextTester context = SensorContextTester.create(fs.baseDir());

Configuration settings = Mockito.mock(Configuration.class);
when(settings.getStringArray("sonar.cxx." + CxxClangTidySensor.REPORT_PATH_KEY)).thenReturn(reportName);
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());

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);

Expand All @@ -105,9 +176,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();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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]]
Original file line number Diff line number Diff line change
@@ -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);
^
Original file line number Diff line number Diff line change
@@ -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);
^