Skip to content

Commit

Permalink
Merge pull request #2805 from guwirth/td-15
Browse files Browse the repository at this point in the history 
fix technical debt
  • Loading branch information
@guwirth
guwirth authored Nov 9, 2024
2 parents 174b87b + 4addc84 commit 666c53b
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 105 deletions.
146 changes: 67 additions & 79 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/ClangTidyParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import java.io.File;
import java.io.IOException;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.Map;
import java.util.regex.Pattern;
import org.slf4j.Logger;
Expand All @@ -34,11 +34,17 @@ public class ClangTidyParser {

private static final Logger LOG = LoggerFactory.getLogger(ClangTidyParser.class);

private static final String REGEX = "((?>[a-zA-Z]:[\\\\/])??[^:]++):(\\d{1,5}):(\\d{1,5}): ([^:]++): (.+)";
private static final Pattern PATTERN = Pattern.compile(REGEX);
private static final String LINE_REGEX = "((?>[a-zA-Z]:[\\\\/])??[^:]++):(\\d{1,5}):(\\d{1,5}): ([^:]++): (.+)";
private static final Pattern LINE_PATTERN = Pattern.compile(LINE_REGEX);

private static final Map<String, String> map = Map.of(
"note", "",
"warning", "clang-diagnostic-warning",
"error", "clang-diagnostic-error",
"fatal error", "clang-diagnostic-error"
);

private final CxxIssuesReportSensor sensor;
private Issue issue = null;

public ClangTidyParser(CxxIssuesReportSensor sensor) {
this.sensor = sensor;
Expand All @@ -48,122 +54,104 @@ public void parse(File report, String defaultEncoding) throws IOException {
try (var scanner = new TextScanner(report, defaultEncoding)) {
LOG.debug("Encoding='{}'", scanner.encoding());

CxxReportIssue currentIssue = null;
CxxReportIssue issue = null;
while (scanner.hasNextLine()) {
if (!parseLine(scanner.nextLine())) {
LineData data = parseLine(scanner.nextLine());
if (data == null) {
continue;
}
if ("note".equals(issue.level)) {
if (currentIssue != null) {
currentIssue.addFlowElement(issue.path, issue.line, issue.column, issue.info);
if ("note".equals(data.level)) {
if (issue != null) {
issue.addFlowElement(data.path, data.line, data.column, data.info);
}
} else {
if (currentIssue != null) {
sensor.saveUniqueViolation(currentIssue);
if (issue != null) {
sensor.saveUniqueViolation(issue);
}
currentIssue = new CxxReportIssue(issue.ruleId, issue.path, issue.line, issue.column, issue.info);
for (var aliasRuleId : issue.aliasRuleIds) {
currentIssue.addAliasRuleId(aliasRuleId);
issue = new CxxReportIssue(data.ruleId, data.path, data.line, data.column, data.info);
for (var aliasRuleId : data.aliasRuleIds) {
issue.addAliasRuleId(aliasRuleId);
}
}
}
if (currentIssue != null) {
sensor.saveUniqueViolation(currentIssue);
if (issue != null) {
sensor.saveUniqueViolation(issue);
}
}
}

private boolean parseLine(String data) {
var matcher = PATTERN.matcher(data);
issue = null;
if (matcher.matches()) {
issue = new Issue();
private LineData parseLine(String line) {
var lineMatcher = LINE_PATTERN.matcher(line);
if (lineMatcher.matches()) {
LineData data = new LineData();
// group: 1 2 3 4 5
// <path>:<line>:<column>: <level>: <info> [ruleIds]
// sample:
// c:\a\file.cc:5:20: warning: txt txt [clang-diagnostic-writable-strings]
var m = matcher.toMatchResult();
issue.path = m.group(1); // relative paths
issue.line = m.group(2); // 1...n
issue.column = m.group(3); // 1...n
issue.level = m.group(4); // error, warning, note, ...
issue.info = m.group(5); // info [ruleIds]

// Clang-Tidy column numbers are from 1...n and SQ is using 0...n
var lineMatchResult = lineMatcher.toMatchResult();
data.path = lineMatchResult.group(1); // relative paths
data.line = lineMatchResult.group(2); // 1...n
data.column = lineMatchResult.group(3); // 1...n
data.level = lineMatchResult.group(4); // error, warning, note, ...
data.info = lineMatchResult.group(5); // info [ruleIds]

try {
issue.column = Integer.toString(Integer.parseInt(issue.column) - 1);
// Clang-Tidy column numbers are from 1...n and SQ is using 0...n
data.column = Integer.toString(Integer.parseInt(data.column) - 1);
} catch (java.lang.NumberFormatException e) {
issue.column = "";
data.column = "";
}

splitRuleIds(); // info [ruleId, aliasId, ...]
}

return issue != null;
}

private void splitRuleIds() {
issue.ruleId = getDefaultRuleId();

if (!issue.info.endsWith("]")) { // [...]
return;
}
// info [ruleId, aliasId, ...]
//
if (data.info.endsWith("]")) {
int pos = data.info.lastIndexOf('[');
if (pos != -1) {
for (var ruleId : data.info.substring(pos + 1, data.info.length() - 1).trim().split("\\s*[, ]\\s*")) {
if (data.ruleId == null) {
data.ruleId = ruleId;
} else {
if (!"-warnings-as-errors".equals(ruleId)) {
data.aliasRuleIds.add(ruleId);
}
}
}
data.info = data.info.substring(0, pos - 1);
}
}

var end = issue.info.length() - 1;
for (var start = issue.info.length() - 2; start >= 0; start--) {
var c = issue.info.charAt(start);
if (Character.isLetterOrDigit(c) || c == '-' || c == '.' || c == '_') {
// continue
} else if (c == ',') {
var aliasId = issue.info.substring(start + 1, end);
if (!"-warnings-as-errors".equals(aliasId)) {
issue.aliasRuleIds.addFirst(aliasId);
if (data.ruleId != null) {
// map Clang warning (-W<warning>) to Clang-Tidy warning (clang-diagnostic-<warning>)
if (data.ruleId.startsWith("-W")) {
data.ruleId = "clang-diagnostic-" + data.ruleId.substring(2);
}
end = start;
} else {
if (c == '[') {
issue.ruleId = issue.info.substring(start + 1, end);
if(issue.ruleId.startsWith("-W")) {
// Let's support also clang, the compiler, output
//
// clang reports warnings as -W<warning>
// clang-tidy reports the exact same warning as clang-diagnostic-<warning>, and no actual clang-tidy warning
// starts with "-W"
issue.ruleId = issue.ruleId.replaceFirst("^-W", "clang-diagnostic-");
}
issue.info = issue.info.substring(0, start - 1);
} else {
issue.aliasRuleIds.clear();
}
break;
data.ruleId = getDefaultRuleId(data.level);
}

return data;
}
}

String getDefaultRuleId() {
Map<String, String> map = Map.of(
"note", "",
"warning", "clang-diagnostic-warning",
"error", "clang-diagnostic-error",
"fatal error", "clang-diagnostic-error"
);
return null;
}

return map.getOrDefault(issue.level, "clang-diagnostic-unknown");
private static String getDefaultRuleId(String level) {
return map.getOrDefault(level, "clang-diagnostic-unknown");
}

@Override
public String toString() {
return getClass().getSimpleName();
}

private static class Issue {
private static class LineData {

private String path;
private String line;
private String column;
private String level;
private String ruleId;
private LinkedList<String> aliasRuleIds = new LinkedList<>();
private ArrayList<String> aliasRuleIds = new ArrayList<>();
private String info;
}

Expand Down
80 changes: 54 additions & 26 deletions cxx-sensors/src/test/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ void shouldReportDefaultRuleId() {
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("""
asd
asdasdfghtzsdfghjuio
asda
""")
asd
asdasdfghtzsdfghjuio
asda
""")
.build()
);

Expand Down Expand Up @@ -107,10 +107,10 @@ void shouldReportSameIssueInSameLineWithDifferentColumn() {
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("""
asd
output[outputPos++] = table[((input[inputPos + 1] & 0x0f) << 2) | (input[inputPos + 2] >> 6)];
asda
""")
asd
output[outputPos++] = table[((input[inputPos + 1] & 0x0f) << 2) | (input[inputPos + 2] >> 6)];
asda
""")
.build()
);

Expand Down Expand Up @@ -142,11 +142,11 @@ void shouldRemoveDuplicateIssues() {
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("""
asd
_identityFunction,
_identityFunction) {
asda
""")
asd
_identityFunction,
_identityFunction) {
asda
""")
.build()
);

Expand All @@ -172,10 +172,10 @@ void shouldReportLineIfColumnIsInvalid() {
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("""
asd
X
asda
""")
asd
X
asda
""")
.build()
);

Expand Down Expand Up @@ -227,10 +227,10 @@ void shouldReportAliasRuleIds() {
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("""
asd
output[outputPos++] = table[((input[inputPos + 1] & 0x0f) << 2) | (input[inputPos + 2] >> 6)];
asda
""")
asd
output[outputPos++] = table[((input[inputPos + 1] & 0x0f) << 2) | (input[inputPos + 2] >> 6)];
asda
""")
.build()
);

Expand Down Expand Up @@ -259,7 +259,11 @@ void shouldReport(String reportFile) {
context.fileSystem().add(TestInputFileBuilder
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("asd\nasdasdgghs\nasda\n")
.initMetadata("""
asd
asdasdgghs
asda
""")
.build()
);

Expand All @@ -281,7 +285,11 @@ void shouldReportClangWarnings() {
context.fileSystem().add(TestInputFileBuilder
.create("ProjectKey", "to_array.hpp")
.setLanguage("cxx")
.initMetadata("asd\nasdbsdfghtz\nasda\n")
.initMetadata("""
asd
asdbsdfghtz
asda
""")
.build()
);

Expand All @@ -306,7 +314,11 @@ void shouldReportMixedClangWarnings() {
context.fileSystem().add(TestInputFileBuilder
.create("ProjectKey", "to_array.hpp")
.setLanguage("cxx")
.initMetadata("asd\nasdcsdfghtz\nasda\n")
.initMetadata("""
asd
asdcsdfghtz
asda
""")
.build()
);

Expand All @@ -331,7 +343,19 @@ void shouldReportFlow() {
context.fileSystem().add(TestInputFileBuilder
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("ab\nab\nab\nab\nab\nab\nabcdefg\nabdefgrqwe\nab\nab\nabcdefghijklm\n")
.initMetadata("""
ab
ab
ab
ab
ab
ab
abcdefg
abdefgrqwe
ab
ab
abcdefghijklm
""")
.build()
);

Expand All @@ -357,7 +381,11 @@ void invalidReportReportsNoIssues() {
context.fileSystem().add(TestInputFileBuilder
.create("ProjectKey", "sources/utils/code_chunks.cpp")
.setLanguage("cxx")
.initMetadata("asd\nasdas\nasda\n")
.initMetadata("""
asd
asdas
asda
""")
.build()
);

Expand Down

0 comments on commit 666c53b

Please sign in to comment.