Skip to content

SONARJAVA-5049 Add CheckListGenerator#4818

Merged
irina-batinic-sonarsource merged 26 commits intomasterfrom
irina/SONARJAVA-5049
Jun 24, 2024
Merged

SONARJAVA-5049 Add CheckListGenerator#4818
irina-batinic-sonarsource merged 26 commits intomasterfrom
irina/SONARJAVA-5049

Conversation

@irina-batinic-sonarsource
Copy link
Copy Markdown
Contributor

@irina-batinic-sonarsource irina-batinic-sonarsource commented Jun 18, 2024

No description provided.

johann-beleites-sonarsource
johann-beleites-sonarsource reviewed Jun 18, 2024
View reviewed changes
Comment thread sonar-java-plugin/pom.xml Outdated
@irina-batinic-sonarsource irina-batinic-sonarsource marked this pull request as draft June 18, 2024 15:27
ValentinAebi-sonar
ValentinAebi-sonar reviewed Jun 19, 2024
View reviewed changes
Comment thread sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckListGenerator.java Outdated
Comment thread sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckListGenerator.java Outdated
@irina-batinic-sonarsource irina-batinic-sonarsource force-pushed the irina/SONARJAVA-5049 branch 3 times, most recently from c26054f to e82a951 Compare June 20, 2024 15:54
@irina-batinic-sonarsource irina-batinic-sonarsource marked this pull request as ready for review June 21, 2024 12:45
ValentinAebi-sonar
ValentinAebi-sonar reviewed Jun 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@ValentinAebi-sonar ValentinAebi-sonar left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really necessary to change the name of the class to GeneratedCheckList? Would it not be better to merely generate a class named CheckList?

Apart from this and the comment below, seems good to me

Comment thread check-list/src/main/java/org/sonar/java/CheckListGenerator.java Outdated
leonardo-pilastri-sonarsource
leonardo-pilastri-sonarsource reviewed Jun 21, 2024
View reviewed changes
Comment thread check-list/src/main/java/org/sonar/java/CheckListGenerator.java Outdated
Comment thread check-list/src/main/java/org/sonar/java/CheckListGenerator.java
johann-beleites-sonarsource
johann-beleites-sonarsource reviewed Jun 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@johann-beleites-sonarsource johann-beleites-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool stuff, just a couple of minor comments. My main concern is if we now need a multi-step process to build

Comment thread .cirrus.yml Outdated
Comment on lines +183 to +184
- mvn clean install -Dmaven.test.skip=true
- mvn compile --projects java-checks-test-sources --also-make-dependents
Copy link
Copy Markdown
Contributor

@johann-beleites-sonarsource johann-beleites-sonarsource Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to do this in two steps now? Is the documentation we need to update?

Copy link
Copy Markdown
Contributor Author

@irina-batinic-sonarsource irina-batinic-sonarsource Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command that was used before, mvn clean compile --projects java-checks-test-sources --also-make-dependents, if executed before compiling the whole project, fails because of compilation errors (GeneratedCheckList class doesn't exist).
Now, I updated the script a bit to fix it without building the whole project (see the latest version of this file).
Also, yes, the documentation needed a minor update, and it has been added.

Comment thread check-list/src/main/java/org/sonar/java/CheckListGenerator.java Outdated
Comment thread check-list/src/main/java/org/sonar/java/CheckListGenerator.java Outdated
leonardo-pilastri-sonarsource
leonardo-pilastri-sonarsource requested changes Jun 24, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@leonardo-pilastri-sonarsource leonardo-pilastri-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me, let's just remove sonarpedia.json and S1135.json from the PR and then we can merge

Comment thread sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1135.json
Copy link
Copy Markdown
Contributor

@leonardo-pilastri-sonarsource leonardo-pilastri-sonarsource Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this and sonarpedia.json update do not belong to this PR

Copy link
Copy Markdown
Contributor Author

@irina-batinic-sonarsource irina-batinic-sonarsource Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we completely remove it, some checks will fail.
S1135 is enabled on tests, but it had a Main scope defined.
I will create a separate PR to fix S1135 and remove it from this one.

irina-batinic-sonarsource and others added 16 commits June 24, 2024 12:12
@irina-batinic-sonarsource
SONARJAVA-5049 Add CheckListGenerator
90fa36e
@irina-batinic-sonarsource
Revert maxSize
3275b5d
@irina-batinic-sonarsource
Separate writing to a file to a method
2406170
@irina-batinic-sonarsource
Separate check files search and filtering to a method
23b60c2
@irina-batinic-sonarsource
Remove commons io library
feebe50
@leonardo-pilastri-sonarsource @irina-batinic-sonarsource
Fix windows file separator
9e5cb7c
@irina-batinic-sonarsource
Update some field names
f731fe5
@irina-batinic-sonarsource
Move CheckList generation to java-frontend.
387dc3a 
Replace usages of CheckList with GeneratedCheckList.
@irina-batinic-sonarsource
Try moving checkList generator to separate module.
22b60eb 
We need both Checks and AWS Checks compiled to generate list and to use it in Plugin module.
@irina-batinic-sonarsource
Build before sanity tests and autoscan
88720d6
@irina-batinic-sonarsource
Try fixing autocan and sanity
8a24b2b
@irina-batinic-sonarsource
Update gson version to be compatible with records.
0332699
@irina-batinic-sonarsource
Add GeneratedCheckListTest
86ac8a9
@irina-batinic-sonarsource
Add tests for CheckListGenerator
4c7fc94
@irina-batinic-sonarsource
Add tests for CheckListGenerator
093a35c
@irina-batinic-sonarsource
Updating gson version breaks build - 1 forbidden licenses used.
c349903 
Returning back to 2.8.9
@sonarqube-next
Copy link
Copy Markdown

sonarqube-next Bot commented Jun 24, 2024

Quality Gate passed Quality Gate passed

Issues
0 New issues
1 Fixed issue
4 Accepted issues

Measures
0 Security Hotspots
96.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

@irina-batinic-sonarsource irina-batinic-sonarsource enabled auto-merge (squash) June 24, 2024 10:32
@irina-batinic-sonarsource irina-batinic-sonarsource merged commit c9933de into master Jun 24, 2024
@irina-batinic-sonarsource irina-batinic-sonarsource deleted the irina/SONARJAVA-5049 branch June 24, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johann-beleites-sonarsource johann-beleites-sonarsource johann-beleites-sonarsource left review comments

@leonardo-pilastri-sonarsource leonardo-pilastri-sonarsource leonardo-pilastri-sonarsource approved these changes

+1 more reviewer

@ValentinAebi-sonar ValentinAebi-sonar ValentinAebi-sonar left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@irina-batinic-sonarsource @johann-beleites-sonarsource @leonardo-pilastri-sonarsource @ValentinAebi-sonar