Skip to content

Commit 0212cd3

Browse files
authored
Fix handling clippy paths - issue #65 (#66)
* fix issue #65 * Add unit tests * Add license header * fix code smells
1 parent 45acb3b commit 0212cd3

File tree

6 files changed

+262
-13
lines changed

6 files changed

+262
-13
lines changed

community-rust-plugin/pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@
132132
<artifactId>maven-compiler-plugin</artifactId>
133133
<version>3.8.1</version>
134134
<configuration>
135-
<source>10</source>
136-
<target>10</target>
135+
<source>11</source>
136+
<target>11</target>
137137
</configuration>
138138
</plugin>
139139
<plugin>

community-rust-plugin/src/main/java/org/elegoff/plugins/communityrust/clippy/ClippyJsonReportReader.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.io.IOException;
2929
import java.io.InputStream;
3030
import java.io.InputStreamReader;
31-
import java.nio.file.Paths;
3231
import java.util.function.Consumer;
3332
import java.util.stream.Stream;
3433
import javax.annotation.Nullable;
@@ -46,16 +45,15 @@ public class ClippyJsonReportReader {
4645
private static final String VISIT_MSG = "for further information visit";
4746
private static final String MESSAGE = "message";
4847
private final JSONParser jsonParser = new JSONParser();
49-
private final String projectDir;
48+
5049
private final Consumer<ClippyIssue> consumer;
5150

52-
private ClippyJsonReportReader(String projectDir, Consumer<ClippyIssue> consumer) {
53-
this.projectDir = projectDir;
51+
private ClippyJsonReportReader(Consumer<ClippyIssue> consumer) {
5452
this.consumer = consumer;
5553
}
5654

57-
static void read(InputStream in, String projectDir, Consumer<ClippyIssue> consumer) throws IOException, ParseException {
58-
new ClippyJsonReportReader(projectDir, consumer).read(in);
55+
static void read(InputStream in, Consumer<ClippyIssue> consumer) throws IOException, ParseException {
56+
new ClippyJsonReportReader(consumer).read(in);
5957
}
6058

6159
private static String suggestedMessage(JSONObject obj) {
@@ -113,6 +111,7 @@ private void read(InputStream in) throws IOException, ParseException {
113111
}
114112
}
115113

114+
116115
private void onResult(JSONObject result) {
117116

118117
var clippyIssue = new ClippyIssue();
@@ -132,7 +131,7 @@ private void onResult(JSONObject result) {
132131
return; // Exit silently when JSON is not compliant
133132

134133
JSONObject span = (JSONObject) spans.get(0);
135-
clippyIssue.filePath = Paths.get(this.projectDir, (String) span.get("file_name")).toString();
134+
clippyIssue.filePath = (String) span.get("file_name");
136135
clippyIssue.message = (String) message.get(MESSAGE);
137136
JSONArray children = (JSONArray) message.get("children");
138137

community-rust-plugin/src/main/java/org/elegoff/plugins/communityrust/clippy/ClippySensor.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Set;
2929
import java.util.stream.Collectors;
3030
import org.elegoff.plugins.communityrust.language.RustLanguage;
31+
import org.sonar.api.batch.fs.InputFile;
3132
import org.sonar.api.batch.fs.TextRange;
3233
import org.sonar.api.batch.rule.Severity;
3334
import org.sonar.api.batch.sensor.Sensor;
@@ -40,6 +41,8 @@
4041
import org.sonarsource.analyzer.commons.ExternalReportProvider;
4142
import org.sonarsource.analyzer.commons.internal.json.simple.parser.ParseException;
4243

44+
import javax.annotation.CheckForNull;
45+
4346
import static org.apache.commons.lang.StringUtils.isEmpty;
4447

4548
public class ClippySensor implements Sensor {
@@ -51,13 +54,22 @@ public class ClippySensor implements Sensor {
5154
private static final Long DEFAULT_CONSTANT_DEBT_MINUTES = 5L;
5255
private static final int MAX_LOGGED_FILE_NAMES = 20;
5356

54-
private static void saveIssue(SensorContext context, ClippyJsonReportReader.ClippyIssue clippyIssue, Set<String> unresolvedInputFiles) {
57+
private FileAdjustor fileAdjustor;
58+
59+
@CheckForNull
60+
private InputFile inputFile(SensorContext context, String filePath) {
61+
String relativePath = fileAdjustor.relativePath(filePath);
62+
return context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(relativePath));
63+
}
64+
65+
private void saveIssue(SensorContext context, ClippyJsonReportReader.ClippyIssue clippyIssue, Set<String> unresolvedInputFiles) {
5566
if (isEmpty(clippyIssue.ruleKey) || isEmpty(clippyIssue.filePath) || isEmpty(clippyIssue.message)) {
5667
LOG.debug("Missing information for ruleKey:'{}', filePath:'{}', message:'{}'", clippyIssue.ruleKey, clippyIssue.filePath, clippyIssue.message);
5768
return;
5869
}
5970

60-
var inputFile = context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(clippyIssue.filePath));
71+
InputFile inputFile = inputFile(context, clippyIssue.filePath);
72+
6173
if (inputFile == null) {
6274
unresolvedInputFiles.add(clippyIssue.filePath);
6375
return;
@@ -97,6 +109,7 @@ private static Severity toSonarQubeSeverity(String severity) {
97109
@Override
98110
public void execute(SensorContext context) {
99111
Set<String> unresolvedInputFiles = new HashSet<>();
112+
fileAdjustor = FileAdjustor.create(context);
100113
List<File> reportFiles = ExternalReportProvider.getReportFiles(context, reportPathKey());
101114
reportFiles.forEach(report -> importReport(report, context, unresolvedInputFiles));
102115
logUnresolvedInputFiles(unresolvedInputFiles);
@@ -107,8 +120,7 @@ private void importReport(File rawReport, SensorContext context, Set<String> unr
107120

108121
try {
109122
InputStream in = ClippyJsonReportReader.toJSON(rawReport);
110-
String projectDir = rawReport.getParent();
111-
ClippyJsonReportReader.read(in, projectDir, clippyIssue -> saveIssue(context, clippyIssue, unresolvedInputFiles));
123+
ClippyJsonReportReader.read(in, clippyIssue -> saveIssue(context, clippyIssue, unresolvedInputFiles));
112124
} catch (IOException | ParseException e) {
113125
LOG.error("No issues information will be saved as the report file '{}' can't be read. " +
114126
e.getClass().getSimpleName() + ": " + e.getMessage(), rawReport, e);
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* Community Rust Plugin
3+
* Copyright (C) 2021-2023 Eric Le Goff
4+
* mailto:community-rust AT pm DOT me
5+
* http://github.com/elegoff/sonar-rust
6+
*
7+
* This program is free software; you can redistribute it and/or
8+
* modify it under the terms of the GNU Lesser General Public
9+
* License as published by the Free Software Foundation; either
10+
* version 3 of the License, or (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public License
18+
* along with this program; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
20+
*/
21+
package org.elegoff.plugins.communityrust.clippy;
22+
23+
24+
import java.io.File;
25+
import java.nio.file.Path;
26+
import org.sonar.api.batch.fs.FileSystem;
27+
import org.sonar.api.batch.sensor.SensorContext;
28+
29+
/**
30+
* Adjust file paths from clippy reports into a path relative to the project.
31+
* The context in which the report was created and the analyzer context may possibly be different.
32+
*/
33+
public class FileAdjustor {
34+
35+
private final FileSystem fileSystem;
36+
37+
private int relativePathOffset = 0;
38+
39+
private FileAdjustor(FileSystem fileSystem) {
40+
this.fileSystem = fileSystem;
41+
}
42+
43+
public static FileAdjustor create(SensorContext context) {
44+
return new FileAdjustor(context.fileSystem());
45+
}
46+
47+
public String relativePath(String fileName) {
48+
49+
if (isKnown(fileName)) {
50+
return fileName;
51+
}
52+
53+
String normalizedPath = chooseSeparator(fileName);
54+
if (normalizedPath == null) {
55+
return fileName;
56+
}
57+
58+
Path path = Path.of(normalizedPath);
59+
int pathNameCount = path.getNameCount();
60+
61+
if (relativePathOffset > 0) {
62+
if (path.getNameCount() > relativePathOffset) {
63+
path = path.subpath(relativePathOffset, pathNameCount);
64+
if (isKnown(path)) {
65+
return path.toString();
66+
}
67+
}
68+
return fileName;
69+
}
70+
71+
72+
for (int i = 1; i < pathNameCount; i++) {
73+
Path subpath = path.subpath(i, pathNameCount);
74+
if (isKnown(subpath)) {
75+
relativePathOffset = i;
76+
return subpath.toString();
77+
}
78+
}
79+
80+
return fileName;
81+
}
82+
83+
84+
private static String chooseSeparator(String path) {
85+
return isWindows() ? windowsSeparators(path) : unixSeparators(path);
86+
}
87+
88+
private static boolean isWindows() {
89+
return File.separatorChar == '\\';
90+
}
91+
92+
private static String unixSeparators(String path) {
93+
return path.indexOf(92) != -1 ? path.replace('\\', '/') : path;
94+
}
95+
96+
private static String windowsSeparators(String path) {
97+
return path.indexOf(47) != -1 ? path.replace('/', '\\') : path;
98+
}
99+
100+
101+
private boolean isKnown(Path path) {
102+
return isKnown(path.toString());
103+
}
104+
105+
private boolean isKnown(String path) {
106+
return fileSystem.hasFiles(fileSystem.predicates().hasPath(path));
107+
}
108+
}
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/**
2+
* Community Rust Plugin
3+
* Copyright (C) 2021-2023 Eric Le Goff
4+
* mailto:community-rust AT pm DOT me
5+
* http://github.com/elegoff/sonar-rust
6+
*
7+
* This program is free software; you can redistribute it and/or
8+
* modify it under the terms of the GNU Lesser General Public
9+
* License as published by the Free Software Foundation; either
10+
* version 3 of the License, or (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public License
18+
* along with this program; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
20+
*/
21+
package org.elegoff.plugins.communityrust.clippy;
22+
23+
24+
import java.nio.file.Path;
25+
import java.nio.file.Paths;
26+
import java.util.Arrays;
27+
import org.junit.Before;
28+
import org.junit.Test;
29+
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
30+
import org.sonar.api.batch.sensor.internal.SensorContextTester;
31+
32+
import static org.assertj.core.api.Assertions.assertThat;
33+
34+
public class FileAdjustorTest {
35+
36+
private static final Path PROJECT_DIR = Paths.get("src", "test", "resources", "FileAdjustor");
37+
private static final String MODULE_KEY = "FileAdjustor";
38+
39+
private FileAdjustor fileAdjustor;
40+
41+
private SensorContextTester context;
42+
43+
@Before
44+
public void setup() {
45+
context = SensorContextTester.create(PROJECT_DIR);
46+
addInputFiles("main.rs", "subfolder/main.rs");
47+
fileAdjustor = FileAdjustor.create(context);
48+
}
49+
50+
@Test
51+
public void should_return_relative_path_when_all_files_are_known() {
52+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "main.rs"))).isEqualTo("main.rs");
53+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "subfolder", "main.rs"))).isEqualTo(path("subfolder", "main.rs"));
54+
}
55+
56+
@Test
57+
public void should_return_relative_path_when_first_file_is_in_subfolder() {
58+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "subfolder", "main.rs"))).isEqualTo(path("subfolder", "main.rs"));
59+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "main.rs"))).isEqualTo("main.rs");
60+
}
61+
62+
63+
@Test
64+
public void should_not_return_relative_when_files_are_unknown() {
65+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "unknown.rs"))).isEqualTo(path("foo", "bar", "FileAdjustor", "unknown.rs"));
66+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "subfolder", "unknown.rs"))).isEqualTo(path("foo", "bar", "FileAdjustor", "subfolder", "unknown.rs"));
67+
}
68+
69+
@Test
70+
public void should_return_relative_when_first_file_is_unknown() {
71+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "unknown.rs"))).isEqualTo(path("foo", "bar", "FileAdjustor", "unknown.rs"));
72+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "subfolder", "main.rs"))).isEqualTo(path("subfolder", "main.rs"));
73+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "main.rs"))).isEqualTo("main.rs");
74+
}
75+
76+
@Test
77+
public void should_return_relative_path_when_already_relative() {
78+
assertThat(fileAdjustor.relativePath("main.rs")).isEqualTo("main.rs");
79+
assertThat(fileAdjustor.relativePath(path("subfolder", "main.rs"))).isEqualTo(path("subfolder", "main.rs"));
80+
}
81+
82+
@Test
83+
public void test() {
84+
assertThat(fileAdjustor.relativePath(path("unknown", "main.rs"))).isEqualTo(path("main.rs"));
85+
assertThat(fileAdjustor.relativePath("main.rs")).isEqualTo("main.rs");
86+
}
87+
88+
@Test
89+
public void should_return_relative_path_for_second_file_when_unknown() {
90+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileHandler", "main.rs"))).isEqualTo("main.rs");
91+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileHandler", "subfolder", "unknown.rs"))).isEqualTo(path("foo", "bar", "FileHandler", "subfolder", "unknown.rs"));
92+
}
93+
94+
@Test
95+
public void should_return_relative_path_for_unix_path() {
96+
assertThat(fileAdjustor.relativePath("/foo/bar/FileAdjustor/main.rs")).isEqualTo("main.rs");
97+
assertThat(fileAdjustor.relativePath("/foo/bar/FileAdjustor/subfolder/main.rs")).isEqualTo(path("subfolder", "main.rs"));
98+
}
99+
100+
@Test
101+
public void should_return_relative_path_for_fqn_windows_path() {
102+
assertThat(fileAdjustor.relativePath("C:\\foo\\bar\\FileAdjustor\\main.rs")).isEqualTo("main.rs");
103+
assertThat(fileAdjustor.relativePath("C:\\foo\\bar\\FileAdjustor\\subfolder\\main.rs")).isEqualTo(path("subfolder", "main.rs"));
104+
}
105+
106+
@Test
107+
public void should_return_relative_path_for_relative_windows_path() {
108+
assertThat(fileAdjustor.relativePath("main.rs")).isEqualTo("main.rs");
109+
assertThat(fileAdjustor.relativePath("subfolder\\main.rs")).isEqualTo("subfolder\\main.rs");
110+
}
111+
112+
@Test
113+
public void should_return_handle_shorter_path() {
114+
assertThat(fileAdjustor.relativePath(path("foo", "bar", "FileAdjustor", "main.rs"))).isEqualTo("main.rs");
115+
assertThat(fileAdjustor.relativePath(path("bar", "main.rs"))).isEqualTo(path("bar","main.rs"));
116+
}
117+
118+
119+
private void addInputFiles(String... paths) {
120+
Arrays.stream(paths).forEach(path -> context.fileSystem().add(TestInputFileBuilder.create(MODULE_KEY, path).build()));
121+
}
122+
123+
private static String path(String first, String... more) {
124+
return Path.of(first, more).toString();
125+
}
126+
127+
128+
}

pom.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@
6363
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
6464
<sonar.version>9.7.1.62043</sonar.version>
6565
<sonar.plugin.api.version>9.11.0.290</sonar.plugin.api.version>
66+
<sonar.branch.name>fix-handling-clippy-paths</sonar.branch.name>
67+
<sonar.branch.target>master</sonar.branch.target>
6668

6769
<!-- Advertise minimal required JRE version -->
6870
<jre.min.version>11</jre.min.version>

0 commit comments

Comments
 (0)