Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve violations from sonarcloud.io #217

Open
romani opened this issue Apr 30, 2019 · 8 comments
Open

Resolve violations from sonarcloud.io #217

romani opened this issue Apr 30, 2019 · 8 comments
Assignees
Labels

Comments

@romani
Copy link
Member

romani commented Apr 30, 2019

https://sonarcloud.io/dashboard?id=checkstyle_sonar-checkstyle

lets try to resolve all violations in our code, if reasonable.

@romani romani changed the title Resolve violations from sonarcloud Resolve violations from sonarcloud.io Apr 30, 2019
@muhlba91
Copy link
Contributor

muhlba91 commented May 8, 2019

I started looking through them and found those issues to be discussed:

For the other issues, it's quite straight forward to resolve them.

@romani
Copy link
Member Author

romani commented May 11, 2019

For the other issues, it's quite straight forward to resolve them.

please do. the less violations the better. No need to fix the all in one PR, even 1 violation in separate PR is ok. Continuous improvements ...

CheckstyleRulesDefinition::extractRulesData -> uses deprecated (and without provided alternative)

I see clear meassage in javadoc that plugin need to host this functionality in their own code.
https://github.com/SonarSource/sslr-squid-bridge/blob/master/src/main/java/org/sonar/squidbridge/rules/ExternalDescriptionLoader.java#L33
Lets just copy such files to us.

RuleFinder is not deprecated; only marked deprecated wrongly in our used API version - we investigated this issue before already

I do not see deprecation in latest code
https://github.com/SonarSource/sonarqube/blob/master/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleFinder.java
should be upgrade to new version ?

file() is deprecated without replacement

I see suggestion on what to use instead -
https://github.com/SonarSource/sonarqube/blob/master/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/InputFile.java#L91

we have a lot of usage of RulePriority but Sonar still uses that one

https://github.com/SonarSource/sonarqube/blob/master/sonar-plugin-api/src/main/java/org/sonar/api/rules/RulePriority.java#L30
Please open issue on them to let when describe what is better way to define this.

I actually don't know if it's wise to change this one

It is false positive for sure, but ... according to
https://jira.sonarsource.com/browse/RSPEC-1075 we probably can remove leading "/" and pass validation.
Or according to https://github.com/SonarSource/sonar-dotnet/pull/556/files#diff-4682fc1f8b2440f08cbc43fa1a536fa0R56 , name variable as xxxxLocation and pass :).

@muhlba91
Copy link
Contributor

I created issues for:

Working on the others.

@muhlba91 muhlba91 self-assigned this May 14, 2019
muhlba91 pushed a commit to muhlba91/sonar-checkstyle that referenced this issue May 14, 2019
muhlba91 pushed a commit to muhlba91/sonar-checkstyle that referenced this issue May 14, 2019
muhlba91 pushed a commit to muhlba91/sonar-checkstyle that referenced this issue May 14, 2019
@rnveach
Copy link
Member

rnveach commented May 14, 2019

@romani Is it possible to put sonar as part of CI so we can see violations are fixed and won't come back?

@romani
Copy link
Member Author

romani commented May 14, 2019

Yes, they do have PR processing now, but I didn't have time to set up it.
There is still problem to set up custom gate, suppression, rules. it will require extra scripting to make it prevents new issue to appear.

@romani
Copy link
Member Author

romani commented Jan 22, 2020

No spacial scripting is required now, now sonarqube is completely customizable.
We just need to resolve or mark as won't fix all issues

@romani
Copy link
Member Author

romani commented Jan 22, 2020

this i the only major violations that is left - https://sonarcloud.io/project/issues?id=checkstyle_sonar-checkstyle&open=AWpuVgQeyRjJygCrGbZ-&resolved=false&severities=MAJOR

Al other related to deprecation resolving it is part of #58.

So as this major is resolved we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants