Skip to content

Commit 4f75be3

Browse files
authored
Merge pull request #2177 from guwirth/clangtidy-refactoring
Clang-Tidy refactoring
2 parents ca42944 + 5f51a5b commit 4f75be3

File tree

10 files changed

+286
-124
lines changed

10 files changed

+286
-124
lines changed
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* C++ Community Plugin (cxx plugin)
3+
* Copyright (C) 2010-2021 SonarOpenCommunity
4+
* http://github.com/SonarOpenCommunity/sonar-cxx
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.cxx.sensors.clangtidy;
21+
22+
import java.io.File;
23+
import java.io.IOException;
24+
import java.util.LinkedList;
25+
import java.util.Map;
26+
import java.util.regex.Pattern;
27+
import org.sonar.api.utils.log.Logger;
28+
import org.sonar.api.utils.log.Loggers;
29+
import org.sonar.cxx.sensors.utils.TextScanner;
30+
import org.sonar.cxx.utils.CxxReportIssue;
31+
32+
public class ClangTidyParser {
33+
34+
private static final Logger LOG = Loggers.get(ClangTidyParser.class);
35+
36+
private static final String REGEX = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): (.+)";
37+
private static final Pattern PATTERN = Pattern.compile(REGEX);
38+
39+
private final CxxClangTidySensor sensor;
40+
private Issue issue = null;
41+
42+
public ClangTidyParser(CxxClangTidySensor sensor) {
43+
this.sensor = sensor;
44+
}
45+
46+
public void parse(File report, String defaultEncoding) throws IOException {
47+
try ( var scanner = new TextScanner(report, defaultEncoding)) {
48+
LOG.debug("Encoding='{}'", scanner.encoding());
49+
50+
CxxReportIssue currentIssue = null;
51+
while (scanner.hasNextLine()) {
52+
if (!parseLine(scanner.nextLine())) {
53+
continue;
54+
}
55+
if ("note".equals(issue.level)) {
56+
if (currentIssue != null) {
57+
currentIssue.addFlowElement(issue.path, issue.line, issue.column, issue.info);
58+
}
59+
} else {
60+
if (currentIssue != null) {
61+
sensor.saveUniqueViolation(currentIssue);
62+
}
63+
currentIssue = new CxxReportIssue(issue.ruleId, issue.path, issue.line, issue.column, issue.info);
64+
for (var aliasRuleId : issue.aliasRuleIds) {
65+
currentIssue.addAliasRuleId(aliasRuleId);
66+
}
67+
}
68+
}
69+
if (currentIssue != null) {
70+
sensor.saveUniqueViolation(currentIssue);
71+
}
72+
}
73+
}
74+
75+
private boolean parseLine(String data) {
76+
var matcher = PATTERN.matcher(data);
77+
issue = null;
78+
if (matcher.matches()) {
79+
issue = new Issue();
80+
// group: 1 2 3 4 5
81+
// <path>:<line>:<column>: <level>: <info> [ruleIds]
82+
// sample:
83+
// c:\a\file.cc:5:20: warning: txt txt [clang-diagnostic-writable-strings]
84+
var m = matcher.toMatchResult();
85+
issue.path = m.group(1); // relative paths
86+
issue.line = m.group(2); // 1...n
87+
issue.column = m.group(3); // 1...n
88+
issue.level = m.group(4); // error, warning, note, ...
89+
issue.info = m.group(5); // info [ruleIds]
90+
91+
// Clang-Tidy column numbers are from 1...n and SQ is using 0...n
92+
try {
93+
issue.column = Integer.toString(Integer.parseInt(issue.column) - 1);
94+
} catch (java.lang.NumberFormatException e) {
95+
issue.column = "";
96+
}
97+
98+
splitRuleIds(); // info [ruleId, aliasId, ...]
99+
}
100+
101+
return issue != null;
102+
}
103+
104+
private void splitRuleIds() {
105+
issue.ruleId = getDefaultRuleId();
106+
107+
if (!issue.info.endsWith("]")) { // [...]
108+
return;
109+
}
110+
111+
var end = issue.info.length() - 1;
112+
for (var start = issue.info.length() - 2; start >= 0; start--) {
113+
var c = issue.info.charAt(start);
114+
if (Character.isLetterOrDigit(c) || c == '-' || c == '.' || c == '_') {
115+
continue;
116+
} else if (c == ',') {
117+
var aliasId = issue.info.substring(start + 1, end);
118+
if (!"-warnings-as-errors".equals(aliasId)) {
119+
issue.aliasRuleIds.addFirst(aliasId);
120+
}
121+
end = start;
122+
} else {
123+
if (c == '[') {
124+
issue.ruleId = issue.info.substring(start + 1, end);
125+
issue.info = issue.info.substring(0, start - 1);
126+
} else {
127+
issue.aliasRuleIds.clear();
128+
}
129+
break;
130+
}
131+
}
132+
}
133+
134+
String getDefaultRuleId() {
135+
Map<String, String> map = Map.of(
136+
"note", "",
137+
"warning", "clang-diagnostic-warning",
138+
"error", "clang-diagnostic-error",
139+
"fatal error", "clang-diagnostic-error"
140+
);
141+
142+
return map.getOrDefault(issue.level, "clang-diagnostic-unknown");
143+
}
144+
145+
@Override
146+
public String toString() {
147+
return getClass().getSimpleName();
148+
}
149+
150+
private static class Issue {
151+
152+
private String path;
153+
private String line;
154+
private String column;
155+
private String level;
156+
private String ruleId;
157+
private LinkedList<String> aliasRuleIds = new LinkedList<>();
158+
private String info;
159+
}
160+
161+
}

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

Lines changed: 4 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,12 @@
2323
import java.nio.charset.StandardCharsets;
2424
import java.util.Arrays;
2525
import java.util.Collections;
26-
import java.util.LinkedList;
2726
import java.util.List;
28-
import java.util.Map;
29-
import java.util.regex.Pattern;
3027
import org.sonar.api.batch.sensor.SensorDescriptor;
3128
import org.sonar.api.config.PropertyDefinition;
3229
import org.sonar.api.resources.Qualifiers;
33-
import org.sonar.api.utils.log.Logger;
34-
import org.sonar.api.utils.log.Loggers;
3530
import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor;
3631
import org.sonar.cxx.sensors.utils.InvalidReportException;
37-
import org.sonar.cxx.sensors.utils.TextScanner;
38-
import org.sonar.cxx.utils.CxxReportIssue;
3932

4033
/**
4134
* Sensor for clang-tidy
@@ -46,8 +39,6 @@ public class CxxClangTidySensor extends CxxIssuesReportSensor {
4639
public static final String REPORT_ENCODING_DEF = "sonar.cxx.clangtidy.encoding";
4740
public static final String DEFAULT_ENCODING_DEF = StandardCharsets.UTF_8.name();
4841

49-
private static final Logger LOG = Loggers.get(CxxClangTidySensor.class);
50-
5142
public static List<PropertyDefinition> properties() {
5243
return Collections.unmodifiableList(Arrays.asList(
5344
PropertyDefinition.builder(REPORT_PATH_KEY)
@@ -84,35 +75,10 @@ public void describe(SensorDescriptor descriptor) {
8475

8576
@Override
8677
protected void processReport(File report) {
87-
String reportEncoding = context.config().get(REPORT_ENCODING_DEF).orElse(DEFAULT_ENCODING_DEF);
88-
89-
try ( var scanner = new TextScanner(report, reportEncoding)) {
90-
LOG.debug("Encoding='{}'", scanner.encoding());
91-
92-
// sample:
93-
// c:\a\file.cc:5:20: warning: ... conversion from string literal to 'char *' [clang-diagnostic-writable-strings]
94-
CxxReportIssue currentIssue = null;
95-
while (scanner.hasNextLine()) {
96-
var issue = Issue.create(scanner.nextLine());
97-
if (issue != null) {
98-
if ("note".equals(issue.level)) {
99-
if (currentIssue != null) {
100-
currentIssue.addFlowElement(issue.path, issue.line, issue.column, issue.info);
101-
}
102-
} else {
103-
if (currentIssue != null) {
104-
saveUniqueViolation(currentIssue);
105-
}
106-
currentIssue = new CxxReportIssue(issue.ruleId, issue.path, issue.line, issue.column, issue.info);
107-
for (var aliasRuleId : issue.aliasRuleIds) {
108-
currentIssue.addAliasRuleId(aliasRuleId);
109-
}
110-
}
111-
}
112-
}
113-
if (currentIssue != null) {
114-
saveUniqueViolation(currentIssue);
115-
}
78+
var parser = new ClangTidyParser(this);
79+
try {
80+
String defaultEncoding = context.config().get(REPORT_ENCODING_DEF).orElse(DEFAULT_ENCODING_DEF);
81+
parser.parse(report, defaultEncoding);
11682
} catch (final java.io.IOException
11783
| java.lang.IllegalArgumentException
11884
| java.lang.IllegalStateException
@@ -131,73 +97,4 @@ protected String getRuleRepositoryKey() {
13197
return CxxClangTidyRuleRepository.KEY;
13298
}
13399

134-
private static class Issue {
135-
136-
private static final String REGEX = "(.+|[a-zA-Z]:\\\\.+):([0-9]+):([0-9]+): ([^:]+): (.+)";
137-
private static final Pattern PATTERN = Pattern.compile(REGEX);
138-
139-
private String path;
140-
private String line;
141-
private String column;
142-
private String level;
143-
private String ruleId;
144-
private LinkedList<String> aliasRuleIds = new LinkedList<>();
145-
private String info;
146-
147-
static Issue create(String data) {
148-
var matcher = PATTERN.matcher(data);
149-
if (matcher.matches()) {
150-
var issue = new Issue();
151-
// group: 1 2 3 4 5
152-
// <path>:<line>:<column>: <level>: <info>
153-
var m = matcher.toMatchResult();
154-
issue.path = m.group(1); // relative paths
155-
issue.line = m.group(2);
156-
issue.column = m.group(3);
157-
issue.level = m.group(4); // error, warning, note, ...
158-
issue.info = m.group(5); // info [ruleIds]
159-
160-
issue.splitRuleId();
161-
return issue;
162-
}
163-
return null;
164-
}
165-
166-
void splitRuleId() {
167-
ruleId = getDefaultRuleId();
168-
169-
if (info.endsWith("]")) { // [ruleIds]
170-
var end = info.length() - 1;
171-
for (var start = info.length() - 2; start >= 0; start--) {
172-
var c = info.charAt(start);
173-
if (!(Character.isLetterOrDigit(c) || c == '-' || c == '.' || c == '_')) {
174-
if (c == ',') {
175-
var aliasId = info.substring(start + 1, end);
176-
if (!"warnings-as-errors".equals(aliasId)) {
177-
aliasRuleIds.addFirst(aliasId);
178-
}
179-
end = start;
180-
continue;
181-
} else if (c == '[') {
182-
ruleId = info.substring(start + 1, end);
183-
info = info.substring(0, start - 1);
184-
}
185-
break;
186-
}
187-
}
188-
}
189-
}
190-
191-
String getDefaultRuleId() {
192-
Map<String, String> map = Map.of(
193-
"note", "",
194-
"warning", "clang-diagnostic-warning",
195-
"error", "clang-diagnostic-error",
196-
"fatal error", "clang-diagnostic-error"
197-
);
198-
199-
return map.getOrDefault(level, "clang-diagnostic-unknown");
200-
}
201-
}
202-
203100
}

cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxIssuesReportSensor.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,13 @@ private TextRange getRange(CxxReportLocation location, InputFile inputFile) {
131131
if (column < 0) {
132132
return inputFile.selectLine(line);
133133
} else {
134-
// since we do not have more information, we select only one character
135-
return inputFile.newRange(line, column, line, column + 1);
134+
try {
135+
// since we do not have more information, we select only one character
136+
return inputFile.newRange(line, column, line, column + 1);
137+
} catch (IllegalArgumentException e) {
138+
// second try without column number: sometimes locations is behind last valid column
139+
return inputFile.selectLine(line);
140+
}
136141
}
137142
}
138143

cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public InputFile getInputFileIfInProject(String path) {
8181
inputFile = getInputFileTryRealPath(path);
8282

8383
if (inputFile == null) {
84-
LOG.warn("Cannot find the file '{}' in project '{}' with baseDir '{}', skipping.",
84+
LOG.warn("Cannot find the file '{}' in project '{}' with baseDir '{}', skipping",
8585
path, context.project().key(), context.fileSystem().baseDir());
8686
notFoundFiles.add(path);
8787
}

cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxUtils.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,23 +63,21 @@ public static String getStackTrace(final Throwable throwable) {
6363
/**
6464
* validateRecovery
6565
*
66+
* @param msg
6667
* @param ex
6768
* @param config
6869
*/
6970
public static void validateRecovery(String msg, Exception ex, Configuration config) {
70-
var message = msg;
71-
var cause = ex.getCause();
72-
if (cause != null) {
73-
message += ", cause='" + cause.toString().replaceAll("[\\r\\n]+", " ") + "'";
74-
}
71+
var message = (msg + ", cause='" + ExceptionUtils.getRootCauseMessage(ex) + "'")
72+
.replaceAll("\\R+", " ");
7573
Optional<Boolean> recovery = config.getBoolean(CxxReportSensor.ERROR_RECOVERY_KEY);
7674
if (recovery.isPresent() && recovery.get()) {
7775
LOG.warn(message + ", skipping");
7876
return;
7977
}
8078
LOG.info("Error recovery is disabled");
8179
LOG.error(message + ", stop analysis");
82-
throw new IllegalStateException(ex.getMessage(), ex.getCause());
80+
throw new IllegalStateException(msg, ex);
8381
}
8482

8583
/**

0 commit comments

Comments
 (0)