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

Rework issues from sonarsource.io #41

Closed
hohwille opened this issue Oct 21, 2019 · 7 comments
Closed

Rework issues from sonarsource.io #41

hohwille opened this issue Oct 21, 2019 · 7 comments

Comments

@hohwille
Copy link
Member

With #40 we are now analyzing our own plugin code. The issues found by SonarQube should be analysed and reworked where possible/makes sense (e.g. remove outcommented code, get rid of System.out).
https://sonarcloud.io/project/issues?id=devonfw_sonar-devon4j-plugin&resolved=false&types=CODE_SMELL

@lmarniazman
Copy link
Member

What about the 'bugs' shown in the analysis?

lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 2, 2019
lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 9, 2019
lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 9, 2019
lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 13, 2019
lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 13, 2019
@lmarniazman
Copy link
Member

About half of the code smells have been fixed with PR #50, so the quality gate is green now. However, there are still seven code smells and three bugs open, so I will look into fixing them in a seperate PR.

lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 19, 2019
lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Dec 19, 2019
@hohwille
Copy link
Member Author

hohwille commented Jan 9, 2020

3 bugs left:

  • tree may be null - valid point. Just add null check and return immediately in that case.
  • tree may be null 2 - same problem
  • getter and property mismatch - actually intended. However if that is the last remaining issue, why not renaming the fields accordingly (errors -> errorsMutable and errorsView -> errors) to make SonarQube happy.

@lmarniazman
Copy link
Member

As far as I can see, there already is a null check for the two NPE. I added an if(this.tree == null) {return;} after a call of getTreeInstance(context);

lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Jan 16, 2020
lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Jan 16, 2020
@hohwille
Copy link
Member Author

IMHO all done and issue can be closed. Correct or am I missing something?

@lmarniazman
Copy link
Member

lmarniazman commented Jan 31, 2020

Yes, issues were fixed with PR #64.

lmarniazman added a commit to lmarniazman/sonar-devon4j-plugin that referenced this issue Feb 7, 2020
hohwille pushed a commit that referenced this issue Apr 24, 2020
@hohwille
Copy link
Member Author

All fixed now, hence closing.

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

No branches or pull requests

2 participants