From 4addc841db5bd4c49dc6a717a79d44343337379e Mon Sep 17 00:00:00 2001 From: guwirth Date: Fri, 8 Nov 2024 17:36:11 +0100 Subject: [PATCH] fix technical debt - ClangTidyParser --- .../sensors/clangtidy/ClangTidyParser.java | 146 ++++++++---------- .../clangtidy/CxxClangTidySensorTest.java | 80 ++++++---- 2 files changed, 121 insertions(+), 105 deletions(-) diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/ClangTidyParser.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/ClangTidyParser.java index 89d3a25cc..b46537bd6 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/ClangTidyParser.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/ClangTidyParser.java @@ -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; @@ -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 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; @@ -48,107 +54,89 @@ 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 // ::: : [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) to Clang-Tidy warning (clang-diagnostic-) + 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 - // clang-tidy reports the exact same warning as clang-diagnostic-, 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 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 @@ -156,14 +144,14 @@ 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 aliasRuleIds = new LinkedList<>(); + private ArrayList aliasRuleIds = new ArrayList<>(); private String info; } 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 d69760ca5..a2689e114 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 @@ -76,10 +76,10 @@ void shouldReportDefaultRuleId() { .create("ProjectKey", "sources/utils/code_chunks.cpp") .setLanguage("cxx") .initMetadata(""" - asd - asdasdfghtzsdfghjuio - asda - """) + asd + asdasdfghtzsdfghjuio + asda + """) .build() ); @@ -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() ); @@ -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() ); @@ -172,10 +172,10 @@ void shouldReportLineIfColumnIsInvalid() { .create("ProjectKey", "sources/utils/code_chunks.cpp") .setLanguage("cxx") .initMetadata(""" - asd - X - asda - """) + asd + X + asda + """) .build() ); @@ -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() ); @@ -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() ); @@ -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() ); @@ -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() ); @@ -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() ); @@ -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() );