Many projects such as (https://github.com/eclipse/eclipse.platform.ui) contains many sonar warnings but we are unable to repair them due to various reasons (below are some of them - issue #124 , #125)

• The project is too big (out of heap memory)
• File parsing error causing null pointer exception

Therefore we need to somehow in a nice way split these projects up in a way such that we can still repair the non-error part as big as possible in such cases. Thus, segment repair is proposed by me , where the ideal goal is to repair as good as the default repair (number of fixes are the same) and allow easy rollback and recovery if the repair attempt failed and savage the situation somehow instead of complete fail.

@monperrus @fermadeiral

I tried using the release 1.0 version of the jar, when i test against rule 2095 i am getting below error.

C:\softwares\sonar-qube-repair>java -jar sonarqube-repair-1.0-jar-with-dependencies.jar 2095
Applying sonarquberepair.processor.sonarbased.UnclosedResourcesProcessor...
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.

Sending GET request to SonarQube API : https://sonarcloud.io/api/issues/search?resolved=false&ps=500&rules=squid:S2095&componentKeys=
Response Code : 400
java.lang.Exception: ERROR : Wrong Reponse Code from Sonarqube API. Check Internet Connection
at sonarquberepair.processor.sonarbased.SonarWebAPIBasedProcessor.parse(SonarWebAPIBasedProcessor.java:66)
at sonarquberepair.processor.sonarbased.SonarWebAPIBasedProcessor.(SonarWebAPIBasedProcessor.java:23)
at sonarquberepair.processor.sonarbased.UnclosedResourcesProcessor.(UnclosedResourcesProcessor.java:19)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(Unknown Source)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source)
at java.lang.reflect.Constructor.newInstance(Unknown Source)
at sonarquberepair.Main.repair(Main.java:32)
at sonarquberepair.Main.main(Main.java:62)
Exception in thread "main" java.lang.NullPointerException

Could you please check ?

The jar-with-dependencies does not work anymore.

Bug reproduction steps:

git clone [email protected]:SpoonLabs/sonarqube-repair.git
cd sonarqube-repair
./init.sh
mvn package -DskipTests
java -jar target/sonarqube-repair-1.1-SNAPSHOT-jar-with-dependencies.jar --originalFilesPath src/test/resources/ --rulesKey 2116

Error output:

WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance.
Exception in thread "main" java.lang.ExceptionInInitializerError
	at sonarquberepair.DefaultRepair.repair(DefaultRepair.java:33)
	at sonarquberepair.Main.start(Main.java:84)
	at sonarquberepair.Main.main(Main.java:90)
Caused by: java.lang.UnsupportedOperationException: No class provided, and an appropriate one cannot be found.
	at org.apache.logging.log4j.LogManager.callerClass(LogManager.java:576)
	at org.apache.logging.log4j.LogManager.getLogger(LogManager.java:601)
	at org.apache.logging.log4j.LogManager.getLogger(LogManager.java:588)
	at spoon.Launcher.<clinit>(Launcher.java:589)
	... 3 more

On first sight it could very possibly be a dependency collision between LogManager of sonarjava and spoon-core by checking line at spoon.Launcher.<clinit>(Launcher.java:589) . Since spoon-core use log4j , its LogManager can use the method LogManager.getLogger() without any arguments, while sonarjava use slf4j , its LogManager need at least 1 argument causing the error above.

WDYT ? @fermadeiral , @monperrus :)

@fermadeiral you can access to the groupId on https://issues.sonatype.org/browse/OSSRH-53806

The following two tests are failing in my computer (but not in Travis):

[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   FirstFitSegmentationAlgorithmTest.segmentationTest:45 expected:<DummyT[wo].java> but was:<DummyT[hree].java>
[ERROR]   SoraldTreeBuilderAlgorithmTest.treeBuildTest:36 expected:<DummyT[wo].java> but was:<DummyT[hree].java>
[INFO] 
[ERROR] Tests run: 31, Failures: 2, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

@henry-lp, because of that, I should not deploy. Could you please try to run mvn clean test in your computer to see if you also have test failures?

We have two pretty printing strategies, sniper and normal. We currently let the user choose which strategy she wants to use. However, the sniper strategy is very desired, and failure might happen using it, where no repair at all is produced. We should automatically do a second repair trial with the normal strategy in case of failure with the sniper strategy.

The two tests in SegmentStrategyTest.java have pretty unclear goals. I changed the directory structure in the test resources, which caused a code coverage diff in FirstFitSegmentDiffAlgorithm (see coverage diff here). The tests still pass, though, so I'm not certain what they're actually supposed to test, and there's no documentation to clarify it.

I'll try to figure it out, but if anyone has any suggestions please feel free to add them!

Throughout sorald, there is heavy use of raw String values to represent paths. This is really hard to deal with consistently, especially when relative paths start getting mixed with absolute paths. Simple operations such as checking if a path exists, is a file or directory, or resolving a new path relative to some other path, are needlessly tedious and error-prone.

We should replace all raw String values with either Path and/or File values. New code that I write will use these data types where it makes sense, and then the legacy code can be adapted bit-by-bit.

I want to fix multiple issues example:
java -jar target/sonarqube-repair-1.1-SNAPSHOT-jar-with-dependencies.jar rule1 rule2 rule3 ....

Does the API allow me to do that ?

at some point, we'll need to prioritize the warnings to repair. on this topic, see

"Developer-Driven Code Smell Prioritization" https://fabiano-pecorelli.github.io/publications/conferences/C7.pdf

The project has both had a name change and a major uplift to the UI since the last release. With a new release, we could also update the README with usage instructions for the pre-built jar. It's a bit clunky to have to build the project from source.

@fermadeiral

per bug type

There are errors in CI https://travis-ci.com/kth-tcs/sonarqube-repair/builds/15260387 starting with

org.json.JSONException: JSONObject["line"] not found.
	at org.json.JSONObject.get(JSONObject.java:471)
	at Bug.init(Bug.java:35)
	at Bug.<init>(Bug.java:22)
	at Bug.createSetOfBugs(Bug.java:71)

We need to improve the tests such that those errors break the build.

i.e. get the "offline" mode working, without the Web API (that was the initial idea actually)

(per our discussion on today's meeting)

we post paper link

The default repair strategy is highly inefficient when repairing multiple rules. In short, it parses the input file, scans it for one rule, repairs any violations, and then prints to an intermediate file. For the next rule, it parses the intermediate file, and so on. I don't know the original reasoning for this approach. The whole thing can be found here: https://github.com/SpoonLabs/sorald/blob/c76d525603345ad82149951ce3f2c7339ce6236d/src/main/java/sorald/DefaultRepair.java#L31-L88

This incurs a heavy performance penalty when processing multiple rules, as parsing is heavy with Spoon, and actually writing content to a file is an inherently slow operation. In essence, when processing multiple rules, most of the running time is likely spent parsing and printing.

In theory, this printing to an intermediate file should not be necessary, as we should have parse(print(ast)) == ast. Of course, practice is not theory. In particular the sniper printer is sensitive to transformations, and may cause issues if we try to apply too many at the same time. From a theoretical perspective, the end goal should however be to apply all transformations on the AST, without printing and reparsing thrown into the mix.

In practice, this may not be a problem. It is reasonable to assume that, when repairing rules with Sorald, developers will want to fix only one or perhaps a few closely related rules at a time. We however need some data to support this assumption.

To summarize, repairing multiple rules with Sorald is currently fairly slow going, and writing to disk and reparsing is very likely the bottleneck. Although it should be noted that I haven't actually done any profiling, there's a slim chance that I'm wrong. But I don't think so. :)

The sonar checks of different types are mixed in Constants.java, i.e., the checks related to bugs are mixed with the ones related to code smells (DeadStoreCheck and SerializableFieldInSerializableClassCheck). Thinking about the miner, it's better for us to organize the checks in a separate way so that data collection will be organized and easier to be analyzed by us.

TODOs:

• add link to rule doc
• add link to implementation classes
• add missing sections according to https://github.com/monperrus/misc/blob/master/README-template.md

sonarepaired and sona test should go in src/test/resources

To simplify further maintenance of the test suite, it'd be preferable to fully adopt JUnit5. This will in the short term make #153 much easier to solve, and in the long term lead to less headache.

It's really not difficult, the current test suite uses very little JUnit4-specific functionality.

This must be done after #144 is merged, as that touches so many parts of the test suite.

The way we use sonarjava only works when the noncompliant line is marked with // Noncompliant, which is why our tests pass, but on real projects no issue will be found by sonarjava and consequently sonarqube-repair will not apply any change. I will search for a workaround, but I started to think that it's not a good idea to use sonarjava. Any help is welcome.

We depend on sonar-java 5.14, we should depend on a recent version, see https://search.maven.org/search?q=org.sonarsource.java

However, as noted by @fermadeiral , there is a dependency conflict to analyze and fix, see #88 (comment)

Given rules for repair as 2116,2116 . It should only repair once :) .

The Constants.SORALD_WORKSPACE path is used for every test, but it's not cleaned up. This means that if a source file is used for two tests, it's output path may very well be the same between the two tests. Therefore, the second test may pass without doing anything if the first test produced the correct output.

I see two reasonable solutions.

1. Use a temporary directory for the workspace
2. Clean up the workspace after or before each test

I prefer solution 1, but because of the current setup it may be less work to implement solution 2.

put everything in master

in order to have single branch.

delete the other branches from the repo

and document them

Currently, the warnings miner loops through the checks and applies them one-by-one, which essentially means re-parsing the file and traversing the AST for each check. Running time can be drastically reduced by applying all checks in one pass, which will be easy to do once #156 is merged.

If multiple rules are specified along with the segment strategy, only the first rule is processed, while the rest are ignored. The issue is right here: https://github.com/SpoonLabs/sorald/blob/c76d525603345ad82149951ce3f2c7339ce6236d/src/main/java/sorald/SegmentRepair.java#L29-L30

There's no loop to iterate through the rules, as is the case for the default strategy.

I've been poking around the test suite for the processors (i.e. src/test/java/sorald/processor), and I've found that there is a whole lot of code duplication. Most of the test classes are borderline identical, and generally follow this pattern:

public class SomeCheckProcessorTest {
    @Test
    public void test() throws Exception {
        String fileName = "SomeFile.java";
        String pathToBuggyFile = Constants.PATH_TO_RESOURCES_FOLDER + fileName;
        String pathToRepairedFile = Constants.SORALD_WORKSPACE + "/" + Constants.SPOONED +"/" + fileName;

        JavaCheckVerifier.verify(pathToBuggyFile, new SomeCheck());
        Main.main(new String[]{
                Constants.ARG_SYMBOL + Constants.ARG_ORIGINAL_FILES_PATH,pathToBuggyFile,
                Constants.ARG_SYMBOL + Constants.ARG_RULE_KEYS,"<KEY>",
                Constants.ARG_SYMBOL + Constants.ARG_WORKSPACE,Constants.SORALD_WORKSPACE});
        TestHelper.removeComplianceComments(pathToRepairedFile);
        JavaCheckVerifier.verifyNoIssue(pathToRepairedFile, new ArrayHashCodeAndToStringCheck());
    }
}

There is some minor variation in the arguments to Main.main and variable names, but most classes are otherwise semantically identical. If Sorald is to scale with more rules and more tests, I think this must be revamped.

As a solution, I propose that we write a single parameterized test, and keep a directory with "broken" Java files (such as the current src/test/resources directory). Then, each file can be automatically turned into a separate test case. If we for a moment ignore the problem with the variation in command line arguments, a test case file could be named as SomethingInformative_<SONAR_CUBE_KEY>.java, and be used to generate a test case for the rule with key <SONAR_CUBE_KEY> (e.g. EqualsOnAtomicClass_2204.java to test rule 2204).

I have yet to figure out the best solution for the variation in arguments to Main.main, but I'm sure a reasonable solution can be found through some trial and error.

As an example, I've done something similar to this in Spork, an AST-based merge tool. There, I have one directory per test case containing the three input files for the merge, as well as the expected output file as an oracle. Then, source providers that look like this generate input for a handful of test cases that look like this. This gives me hundreds of test cases with very few lines of code, and adding a new test case only requires the addition of new input, there's no additional logic.

I could get to work on this right away. Any thoughts?

Binary files are not pushed to the repo. We have to:

• remove target/classes from the repo
• add target in .gitignore

Hi @HarisAdzemovic, @nharrand, and @monperrus.

What do you think about creating a release with the current version of Sonarqube-repair? I thought about it because I think it's nice to register the end point in the project's history concerning the results reported in Haris' dissertation, which would be also the starting point of a new evolution round (of the tool and the data collection).

Cheers,
Fernanda

See #145

Does anyone know a good way of finding the key to a rule in the SonarQube docs? Right now, the only way I know how is to click a rule and look at the URL. Example:

https://rules.sonarsource.com/java/type/Bug/RSPEC-4973

There I can see that the key is 4973, but it's not obvious anywhere else that I can tell. Perhaps the URL is enough of a "good" way, but it seems a little bit like a hack and I just wanted to know if there's an obvious method I'm overlooking.

For better readability and easier review for PRs, the user should be able to specify max number of fixes per commit. WDYT ? :)

@monperrus @fermadeiral

I can start working on this after this PR is merged.

This was run in Repairnator after importing in the pom.xml .

mons.ProgressReport - 0/0 source files have been analyzed 
java.lang.NullPointerException
	at org.sonar.java.checks.verifier.MultipleFilesJavaCheckVerifier.scanFiles(MultipleFilesJavaCheckVerifier.java:116)
	at org.sonar.java.checks.verifier.MultipleFilesJavaCheckVerifier.verify(MultipleFilesJavaCheckVerifier.java:91)
	at org.sonar.java.checks.verifier.MultipleFilesJavaCheckVerifier.verify(MultipleFilesJavaCheckVerifier.java:58)
	at sonarquberepair.processor.SQRAbstractProcessor.<init>(SQRAbstractProcessor.java:39)
	at sonarquberepair.processor.ArrayHashCodeAndToStringProcessor.<init>(ArrayHashCodeAndToStringProcessor.java:19)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at sonarquberepair.DefaultRepair.repair(DefaultRepair.java:51)
	at sonarquberepair.Main.start(Main.java:84)
	at sonarquberepair.Main.main(Main.java:90)
	at fr.inria.spirals.repairnator.process.step.repair.SonarQubeRepair.businessExecute(SonarQubeRepair.java:48)
	at fr.inria.spirals.repairnator.process.step.AbstractStep.execute(AbstractStep.java:336)
	at fr.inria.spirals.repairnator.process.step.repair.AbstractRepairStep.execute(AbstractRepairStep.java:60)
	at fr.inria.spirals.repairnator.process.step.AbstractStep.executeNextStep(AbstractStep.java:188)
	at fr.inria.spirals.repairnator.process.step.AbstractStep.execute(AbstractStep.java:353)
	at fr.inria.spirals.repairnator.process.step.AbstractStep.executeNextStep(AbstractStep.java:188)
	at fr.inria.spirals.repairnator.process.step.AbstractStep.execute(AbstractStep.java:353)
	at fr.inria.spirals.repairnator.process.inspectors.components.RunInspector4DefaultGit.run(RunInspector4DefaultGit.java:109)
	at fr.inria.spirals.repairnator.process.inspectors.ProjectInspector.run(ProjectInspector.java:301)
	at fr.inria.spirals.repairnator.pipeline.GithubMainProcess.run(GithubMainProcess.java:126)
	at fr.inria.spirals.repairnator.pipeline.BranchLauncher.main(BranchLauncher.java:97)
	at fr.inria.spirals.repairnator.pipeline.BranchLauncher.launch(BranchLauncher.java:103)
	at fr.inria.spirals.repairnator.pipeline.Launcher.main(Launcher.java:67)
done

Apparently the local changes are not there, which explain why we still get null (bug in the front end).
:)

and rename ProcessorTest into SonarRepairTest

with the merging of INRIA/spoon#2265 .

Spoon failed which in turn failed Sorald.

Reproduction procedure after packaging the Jar file of Sorald:

git clone https://github.com/eclipse/rdf4j Repo4Analyze

java -jar target/sorald-1.1-SNAPSHOT-jar-with-dependencies.jar --originalFilesPath Repo4Analyze/ --ruleKeys 2164 --maxFixesPerRule 1

@fermadeiral @monperrus

Here is to keep a collection of sniper mode bug before submitting to spoon.

Another issue related to #141 is that I've found the test cases for processors to be fairly weak. They only assert that the repaired file contains no issues according to sonar-java. That's not a strong assertion, as I found when I very incorrectly re-implemented the CompareStringsBoxedTypesWithEqualsProcessor.java. In fact, one can go as far as replace the processor implementation with this:

	@Override
	public void process(CtElement element) {
		super.process(element);
		element.delete();
	}

it passes the test suite just fine. I found you can do something clearly incorrect like that with several other of the processors as well. I'm a little bit hesitant to get to work with refactorings as my first attempt at one was massively incorrect, yet the test suite said nothing about it.

I'm note sure about the best way to proceed here, but one solution would be to manually add an expected output file to each test case. This should typically not be too hard, given that sorald is template-based. I've found that many of the SonarCube rules contain a non-compliant example, and a directly related compliant example, which would give the oracle for free.

A straight equality check with Spoon between the expected output and the actual output may be enough, but it could be too strict (e.g. formatting differences could come into play). I worked extensively with equality checks during my thesis, and so have quite a few backup plans if the equality check with Spoon is too strict. For example, gumtree-spoon could be used, and a zero-diff would indicate equality.

Thoughts on this?

Per discussion , this rule does not add an import line for java.util.Arrays after the repair on output, which cause compilation error on the repaired code.

Case: henry-lp/SonarQubeRepairTests@84b2746

Hi everyone,

We started to have a lot of big problems since we started to use sonarjava (e.g., #94 and #96). Well, sonarjava is not meant to be used as we want, and right now I see no way to make things work with sonarjava. My suggestion: to remove sonarjava from the project.

However, we cannot switch back to how it was before (without sonarjava) easily, because there are processors that previously relied on sonar web API, and it was not nice at all to have incompatible processors. My second suggestion: 1) to remove the code-smell-related processors (Unused assignments should be removed and Fields in a "Serializable" class should either be transient or serializable), and 2) to work on the processor that is bug-related and previously used the sonar web API (Resources should be closed) to work without the sonar web API, I could do it myself.

We need a stable and focused ground. Processors only based on Spoon might be enough to us, and leaving code in the project that will not be used (which was the case of the processors using sonar web API) only complicates things too.

WDYT?

When a repair breaks an existing test, one can use AssertFixer to fix the test automatically.

with a short description of how the repair is implemented

We currently don't have any style enforcement, and it shows in some places. For example CompareToReturnValueProcessor (along with most other processors) uses tabs for indentation, while the entire sorald.segment subpackage uses spaces.

I don't think we need anything rigorous, but at the very least we should decide upon and enforce a style of indentation.

Error when using rule 2184: Math operands should be cast before assignment when applying on the following projects

• https://github.com/eclipse/kura
• https://github.com/eclipse/hono

Reproduction:

git clone https://github.com/eclipse/hono Repo4Analyze

java -jar target/sorald-1.1-SNAPSHOT-jar-with-dependencies.jar --originalFilesPath Repo4Analyze/ --ruleKeys 2184

@fermadeiral WDYT ? :)