refactor: Encapsulate SonarJava in sorald.sonar subpackage #582
+480
−344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Squash of issue/580-use-full-string-key
slarse
force-pushed
the
issue/579-encapsulate-sonar
branch
from
6f75efc to
cf20770
Compare
|
LGTM too. |
|
Let's merge then :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#579
This PR completely encapsulates all Sonar components into the
sorald.sonarpackage, with the corresponding generalizations being placed in thesorald.rulepackage.With this design, I managed to replace the current use of SonarJava with SonarLint core, without changing anythign outside of the
sorald.sonarpackage. So, the isolation is pretty good. There are still some leaky abstractions, but further work to support other static analyzers than Sonar should be relatively small. More importantly, we can now customize how we use Sonar in particular without having to rewire the entire application.I'm still not sure how to proceed with SonarJava, but after the problems I found in using SonarLint (see #583), I'm leaning towards sticking to SonarJava but using some higher-level APIs. Regardless, this PR facilitates any approach we might take with Sonar in the future.
This PR is pretty big but it's rather hard to split as it's a fundamental redesign. Give reviewing it a go and let me know if something is unclear.
@fermadeiral @khaes-kth whoever has the time to review this go ahead :)