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

return false as soon as a final equals method is found #2341

Closed
wants to merge 8 commits into from

Conversation

troosan
Copy link
Contributor

@troosan troosan commented Dec 5, 2018

This PR fixes issue reported here: https://community.sonarsource.com/t/rule-s2160-false-positive-when-class-has-one-parent-with-non-final-equals-method/4885

  • Use the following formatting style: SonarSource/sonar-developer-toolset
  • Unit tests are passing and you provided a unit test for your fix
  • ITs should pass : To run ITs locally, checkout the README of the project.
  • If there is a Jira ticket available, please make your commits and pull request start with the ticket number (SONARJAVA-XXXX)

@troosan
Copy link
Contributor Author

troosan commented Dec 6, 2018

tests seem to have failed. I had the same locally, after relaunching they passed

@troosan
Copy link
Contributor Author

troosan commented Dec 6, 2018

@m-g-sonar
Copy link

Hey @troosan ,

Thanks for the contribution. I did not had the time to have a look yet, but can you rebase your branch on top of our master? The issue which was making the travis build fail has been solved.

Once this is rebased, I'll have a look at your PR and review it.

Cheers,
Michael

@troosan
Copy link
Contributor Author

troosan commented Dec 19, 2018

@m-g-sonar Thanks, I rebased locally and tried to build but I'm getting the following error:

[ERROR] Failed to execute goal on project java-frontend: Could not resolve dependencies for project org.sonarsource.java:java-frontend:jar:5.10.0-SNAPSHOT: Could not find artifact org.sonarsource.analyzer-commons:sonar-xml-parsing:jar:1.10.0.396 in central (https://repo.maven.apache.org/maven2) -> [Help 1]

Are you sure that library has been published?

@troosan
Copy link
Contributor Author

troosan commented Dec 19, 2018

@m-g-sonar never mind it seems the dependency exists for Jenkins.

@m-g-sonar
Copy link

@troosan Yeah, sorry for that. We are currently reworking the XML handling of the SonarJava plugin... And I did not realized we are relying on a dependency which is not yet on maven central. So you won't be able to build locally. That's pretty bad.

Currently, the new artifact is only visible from our own infrastructure. This should get back to normal in a few days. In the meantime, travis-ci will pass, as we configured it to rely on our own base.

If you want to check your build locally, a workaround could be to:

  1. build the master branch of the following project locally: https://github.com/SonarSource/sonar-analyzer-commons
  2. change the associated version of the dependency in the parent pom of SonarJava, in order to rely on a snapshot version

@troosan
Copy link
Contributor Author

troosan commented Dec 19, 2018

@troosan Yeah, sorry for that. We are currently reworking the XML handling of the SonarJava plugin... And I did not realized we are relying on a dependency which is not yet on maven central. So you won't be able to build locally. That's pretty bad.

Currently, the new artifact is only visible from our own infrastructure. This should get back to normal in a few days. In the meantime, travis-ci will pass, as we configured it to rely on our own base.

If you want to check your build locally, a workaround could be to:

  1. build the master branch of the following project locally: https://github.com/SonarSource/sonar-analyzer-commons
  2. change the associated version of the dependency in the parent pom of SonarJava, in order to rely on a snapshot version

that's indeed what I did :-)

@m-g-sonar
Copy link

@troosan thanks again for your contribution! I merged the pull request (#2375) which included your fix, rebased. I slightly reworked the rule at the same time. Your commit 9a33d05 is now part of our master. I'm consequently closing this PR.

Thanks again for helping us making SonarJava every time a bit better.

Cheers,
Michael

@m-g-sonar m-g-sonar closed this Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants