Skip to content

Commit

Permalink
chore: Update to sonar-java 6.9.0 (#156)
Browse files Browse the repository at this point in the history
* Upgrade to sonar-java 6.9

* Fix issues with paths

* Ignore the code factory test for now

* Refactor RuleVerifier

* Move comment in pom into proper scope

* Add comment that we may want to set the Java version

* Format code with spotless

* Fix weird comment formatting

* Apply optimization by only running symbolic execution if necessary

* Bump spoon version to 8.3.0
  • Loading branch information
slarse authored Oct 29, 2020
1 parent c76d525 commit 1961033
Show file tree
Hide file tree
Showing 12 changed files with 183 additions and 34 deletions.
22 changes: 11 additions & 11 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,29 @@
<dependency>
<groupId>fr.inria.gforge.spoon</groupId>
<artifactId>spoon-core</artifactId>
<version>8.2.0-beta-12</version>
<version>8.3.0</version>
<exclusions>
<exclusion>
<!-- must be excluded as it is signed, which causes problems with unsigned version from sonar -->
<groupId>org.eclipse.jdt</groupId>
<artifactId>org.eclipse.jdt.core</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.sonarsource.java</groupId>
<artifactId>java-checks-testkit</artifactId>
<version>5.14.0-SNAPSHOT</version>
<version>6.9.0.23563</version>
</dependency>
<dependency>
<groupId>org.sonarsource.java</groupId>
<artifactId>java-frontend</artifactId>
<version>5.14.0-SNAPSHOT</version>
<version>6.9.0.23563</version>
</dependency>
<dependency>
<groupId>org.sonarsource.java</groupId>
<artifactId>java-checks</artifactId>
<version>5.14.0-SNAPSHOT</version>
<version>6.9.0.23563</version>
</dependency>
<dependency>
<groupId>com.martiansoftware</groupId>
Expand Down Expand Up @@ -92,13 +99,6 @@
<maven.compiler.source>1.8</maven.compiler.source>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
<repositories>
<repository>
<id>fermadeiral-maven-repository-snapshots</id>
<name>fermadeiral's Maven Repository</name>
<url>https://fermadeiral.github.io/maven-repository/snapshots/</url>
</repository>
</repositories>
<build>
<plugins>
<plugin>
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/sorald/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import java.io.File;
import java.util.Collections;
import java.util.List;
import org.sonar.java.checks.*;
import org.sonar.java.se.checks.*;
import org.sonar.plugins.java.api.JavaFileScanner;
import sorald.sonar.Checks;

Expand Down
7 changes: 4 additions & 3 deletions src/main/java/sorald/DefaultRepair.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ public void repair() {
}

Launcher launcher = createLauncher(inputDirPath, outputDirPath);
File inputBaseDir = FileUtils.getClosestDirectory(new File(inputDirPath));

Processor processor = createProcessor(ruleKey, inputDirPath);
Processor processor = createProcessor(ruleKey, inputDirPath, inputBaseDir);
this.addedProcessors.add((SoraldAbstractProcessor) processor);
Factory factory = launcher.getFactory();
ProcessingManager processingManager = new QueueProcessingManager(factory);
Expand Down Expand Up @@ -108,11 +109,11 @@ private Launcher createLauncher(String inputDirPath, String outputDirPath) {
return initLauncher(launcher, outputDirPath);
}

private Processor createProcessor(Integer ruleKey, String inputDirPath) {
private Processor createProcessor(Integer ruleKey, String inputDirPath, File inputBaseDir) {
SoraldAbstractProcessor processor = createBaseProcessor(ruleKey);
if (processor != null) {
return processor
.initResource(inputDirPath)
.initResource(inputDirPath, inputBaseDir)
.setMaxFixes(this.config.getMaxFixesPerRule());
}
return null;
Expand Down
39 changes: 39 additions & 0 deletions src/main/java/sorald/FileUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package sorald;

import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;

public class FileUtils {

/**
* Compare the two given paths as absolute, normalized paths.
*
* @param lhs A path.
* @param rhs A path.
* @return Whether or not the paths are equal as absolute, normalized paths.
*/
public static boolean pathAbsNormEqual(String lhs, String rhs) {
return pathAbsNormEqual(Paths.get(lhs), Paths.get(rhs));
}

/**
* Compare the two given paths as absolute, normalized paths.
*
* @param lhs A path.
* @param rhs A path.
* @return Whether or not the paths are equal as absolute, normalized paths.
*/
public static boolean pathAbsNormEqual(Path lhs, Path rhs) {
return lhs.toAbsolutePath().normalize().equals(rhs.toAbsolutePath().normalize());
}

/**
* @param file A file.
* @return The given file if it is a directory, or its parent directory if it is not a
* directory.
*/
public static File getClosestDirectory(File file) {
return file.isDirectory() ? file : file.getParentFile();
}
}
9 changes: 6 additions & 3 deletions src/main/java/sorald/SegmentRepair.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ public void repair() {
try {
Launcher launcher = createLauncher(segment, outputDirPath);

SoraldAbstractProcessor processor = createProcessor(ruleKey, segment, nbFixes);
File inputBaseDir =
FileUtils.getClosestDirectory(new File(config.getOriginalFilesPath()));
SoraldAbstractProcessor processor =
createProcessor(ruleKey, segment, nbFixes, inputBaseDir);
if (!this.processorNbsRepaired.containsKey(processor.getClass().getSimpleName())) {
this.processorNbsRepaired.put(processor.getClass().getSimpleName(), nbFixes);
}
Expand Down Expand Up @@ -89,11 +92,11 @@ private Launcher createLauncher(List<Node> segment, String outputDirPath) {
}

private SoraldAbstractProcessor createProcessor(
Integer ruleKey, List<Node> segment, int cachedNbFixes) throws Exception {
Integer ruleKey, List<Node> segment, int cachedNbFixes, File baseDir) {
SoraldAbstractProcessor processor = createBaseProcessor(ruleKey);
if (processor != null) {
return processor
.initResource(segment)
.initResource(segment, baseDir)
.setMaxFixes(this.config.getMaxFixesPerRule())
.setNbFixes(cachedNbFixes);
}
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/sorald/SoraldConfig.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package sorald;

import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -58,7 +59,8 @@ public RepairStrategy getRepairStrategy() {
}

public void setOriginalFilesPath(String originalFilesPath) {
this.originalFilesPath = originalFilesPath;
this.originalFilesPath =
Paths.get(originalFilesPath).normalize().toAbsolutePath().toString();
}

public String getOriginalFilesPath() {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/sorald/miner/MineSonarWarnings.java
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ private static Map<String, Integer> extractWarnings(String projectPath) {
}
for (JavaFileScanner javaFileScanner : SONAR_CHECK_INSTANCES) {
Set<RuleViolation> ruleViolations =
RuleVerifier.analyze(filesToScan, javaFileScanner);
RuleVerifier.analyze(filesToScan, file, javaFileScanner);
warnings.putIfAbsent(
javaFileScanner.getClass().getSimpleName(), ruleViolations.size());
}
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/sorald/processor/SoraldAbstractProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.stream.Stream;
import org.sonar.plugins.java.api.JavaFileScanner;
import sorald.Constants;
import sorald.FileUtils;
import sorald.UniqueTypesCollector;
import sorald.segment.Node;
import sorald.sonar.RuleVerifier;
Expand All @@ -29,7 +30,7 @@ public SoraldAbstractProcessor() {}

public abstract JavaFileScanner getSonarCheck();

public SoraldAbstractProcessor initResource(String originalFilesPath) {
public SoraldAbstractProcessor initResource(String originalFilesPath, File baseDir) {
JavaFileScanner sonarCheck = getSonarCheck();
try {
List<String> filesToScan = new ArrayList<>();
Expand All @@ -46,14 +47,14 @@ public SoraldAbstractProcessor initResource(String originalFilesPath) {
e.printStackTrace();
}
}
ruleViolations = RuleVerifier.analyze(filesToScan, sonarCheck);
ruleViolations = RuleVerifier.analyze(filesToScan, baseDir, sonarCheck);
} catch (Exception e) {
e.printStackTrace();
}
return this;
}

public SoraldAbstractProcessor initResource(List<Node> segment) throws Exception {
public SoraldAbstractProcessor initResource(List<Node> segment, File baseDir) {
JavaFileScanner sonarCheck = getSonarCheck();
List<String> filesToScan = new ArrayList<>();
for (Node node : segment) {
Expand All @@ -71,7 +72,7 @@ public SoraldAbstractProcessor initResource(List<Node> segment) throws Exception
}
}

ruleViolations = RuleVerifier.analyze(filesToScan, sonarCheck);
ruleViolations = RuleVerifier.analyze(filesToScan, baseDir, sonarCheck);
return this;
}

Expand Down Expand Up @@ -111,7 +112,8 @@ public boolean isToBeProcessedAccordingToSonar(CtElement element) {
}

for (RuleViolation ruleViolation : ruleViolations) {
if (ruleViolation.getLineNumber() == line && ruleViolation.getFileName().equals(file)) {
if (ruleViolation.getLineNumber() == line
&& FileUtils.pathAbsNormEqual(ruleViolation.getFileName(), file)) {
return true;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/sorald/sonar/Checks.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.sonar.java.checks.naming.MethodNamedHashcodeOrEqualCheck;
import org.sonar.java.checks.serialization.CustomSerializationMethodCheck;
import org.sonar.java.checks.serialization.ExternalizableClassConstructorCheck;
import org.sonar.java.checks.serialization.NonSerializableWriteCheck;
import org.sonar.java.checks.serialization.SerializableFieldInSerializableClassCheck;
import org.sonar.java.checks.serialization.SerializableObjectInSessionCheck;
import org.sonar.java.checks.serialization.SerializableSuperConstructorCheck;
Expand Down
115 changes: 108 additions & 7 deletions src/main/java/sorald/sonar/RuleVerifier.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
package sorald.sonar;

import java.util.Arrays;
import static java.nio.charset.StandardCharsets.UTF_8;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.sonar.java.checks.verifier.MultipleFilesJavaCheckVerifier;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.fs.internal.DefaultFileSystem;
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
import org.sonar.api.batch.sensor.internal.SensorContextTester;
import org.sonar.java.AnalyzerMessage;
import org.sonar.java.SonarComponents;
import org.sonar.java.ast.JavaAstScanner;
import org.sonar.java.checks.verifier.JavaCheckVerifier;
import org.sonar.java.model.VisitorsBridge;
import org.sonar.java.se.SymbolicExecutionMode;
import org.sonar.plugins.java.api.JavaFileScanner;

/** Adapter class for interfacing with sonar-java's verification and analysis facilities. */
Expand All @@ -18,7 +34,7 @@ public class RuleVerifier {
*/
@SuppressWarnings("UnstableApiUsage")
public static void verifyHasIssue(String filename, JavaFileScanner check) {
verifyHasIssue(Arrays.asList(filename), check);
JavaCheckVerifier.newVerifier().onFile(filename).withCheck(check).verifyIssues();
}

/**
Expand All @@ -29,19 +45,32 @@ public static void verifyHasIssue(String filename, JavaFileScanner check) {
*/
@SuppressWarnings("UnstableApiUsage")
public static void verifyHasIssue(List<String> filesToScan, JavaFileScanner check) {
MultipleFilesJavaCheckVerifier.verify(filesToScan, check, true);
filesToScan.forEach(filename -> verifyHasIssue(filename, check));
}

/**
* Analyze the files with respect to check.
*
* @param filesToScan A list of paths to files.
* @param baseDir The base directory of the current project.
* @param check A Sonar check.
* @return All messages produced by the analyzer, for all files.
*/
@SuppressWarnings("UnstableApiUsage")
public static Set<RuleViolation> analyze(List<String> filesToScan, JavaFileScanner check) {
return MultipleFilesJavaCheckVerifier.verify(filesToScan, check, false).stream()
public static Set<RuleViolation> analyze(
List<String> filesToScan, File baseDir, JavaFileScanner check) {
List<InputFile> inputFiles =
filesToScan.stream()
.map(filename -> toInputFile(baseDir, filename))
.collect(Collectors.toList());

SoraldSonarComponents sonarComponents = createSonarComponents(baseDir);
JavaAstScanner scanner =
createAstScanner(sonarComponents, Collections.singletonList(check));

scanner.scan(inputFiles);

return sonarComponents.getMessages().stream()
.map(RuleViolation::new)
.collect(Collectors.toSet());
}
Expand All @@ -54,6 +83,78 @@ public static Set<RuleViolation> analyze(List<String> filesToScan, JavaFileScann
*/
@SuppressWarnings("UnstableApiUsage")
public static void verifyNoIssue(String filename, JavaFileScanner check) {
MultipleFilesJavaCheckVerifier.verifyNoIssue(Arrays.asList(filename), check);
JavaCheckVerifier.newVerifier().onFile(filename).withCheck(check).verifyNoIssues();
}

private static InputFile toInputFile(File baseDir, String filename) {
// must append a separator to the basedir string as Sonar appends the filenames directly to
// it
final String baseDirStr = baseDir.toString() + File.separator;
try {
return new TestInputFileBuilder(baseDirStr, filename)
.setContents(new String(Files.readAllBytes(Paths.get(filename)), UTF_8))
.setCharset(UTF_8)
.setLanguage("java")
.build();
} catch (IOException e) {
throw new RuntimeException("failed to read file " + filename);
}
}

@SuppressWarnings("UnstableApiUsage")
private static JavaAstScanner createAstScanner(
SonarComponents sonarComponents, List<JavaFileScanner> checks) {
JavaAstScanner scanner = new JavaAstScanner(sonarComponents);
VisitorsBridge visitorsBridge =
new VisitorsBridge(
checks,
// TODO set the classpath to something reasonable
Collections.emptyList(),
sonarComponents,
SymbolicExecutionMode.getMode(checks.toArray(new JavaFileScanner[0])));
// TODO we may want to set the version of the visitors bridge, as without setting it the
// implementation defaults to the highest version supported by the parser (currently
// Java 14)
// visitorsBridge.setJavaVersion(new JavaVersionImpl(8));
scanner.setVisitorBridge(visitorsBridge);
return scanner;
}

private static SoraldSonarComponents createSonarComponents(File baseDir) {
// FIXME The SensorContextTester is an internal and unstable component in sonar,
// we should implement our own SensorContext
SensorContextTester context = SensorContextTester.create(baseDir);
SoraldSonarComponents sonarComponents = new SoraldSonarComponents(context.fileSystem());
sonarComponents.setSensorContext(context);
return sonarComponents;
}

/**
* A simple subclass of SonarComponents that stores all analyzer messages. These are by default
* stored in a storage container, but it seems easier for our use case to just intercept them.
*
* <p>This IS a bit of a hack, so it wouldn't be unreasonable to try to do this the "proper
* way".
*/
private static class SoraldSonarComponents extends SonarComponents {
private final List<AnalyzerMessage> messages;

public SoraldSonarComponents(DefaultFileSystem fs) {
// FIXME I'm very unsure about supplying null for all of these constructor values.
// They should not be null, but at this time I don't know that to put there, and
// with our current usage this hack appears to work.
super(null, fs, null, null, null, null);
messages = new ArrayList<>();
}

@Override
public void reportIssue(AnalyzerMessage analyzerMessage) {
super.reportIssue(analyzerMessage);
messages.add(analyzerMessage);
}

public List<AnalyzerMessage> getMessages() {
return Collections.unmodifiableList(messages);
}
}
}
1 change: 1 addition & 0 deletions src/test/java/sorald/processor/ProcessorTestHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ static Stream<ProcessorTestCase<?>> getTestCaseStream() {
.map(File::listFiles)
.flatMap(Arrays::stream)
.filter(file -> file.getName().endsWith(".java"))
.filter(file -> !file.getName().startsWith("IGNORE"))
.map(ProcessorTestHelper::toProcessorTestCase);
}

Expand Down
Loading

0 comments on commit 1961033

Please sign in to comment.