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

fix(java): improve property override detection #692

Merged
merged 7 commits into from
Aug 10, 2019

Conversation

RomainMuller
Copy link
Contributor

The previous code only checked whether a methodd name was a get/setter
by checking if it's name started by get or set and was at least 4
characters long. This is insufficient, as a method called settings
would be misidentified.

Changed to expect getters to have zero parameters, and setters to have
exactly one parameter and to match a corresponding getter.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

The previous code only checked whether a methodd name was a get/setter
by checking if it's name started by `get` or `set` and was at least 4
characters long. This is insufficient, as a method called `settings`
would be misidentified.

Changed to expect getters to have zero parameters, and setters to have
exactly one parameter and to match a corresponding getter.
@RomainMuller RomainMuller requested review from bmaizels and a team as code owners August 9, 2019 19:34
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Are the package-lock.json changes intentional?

@RomainMuller
Copy link
Contributor Author

I don't know what caused the package-lock.json changes but they keep coming back if I try to undo them. I am actually suspecting the no-diff enforcement of the Travis build is somehow not working here...

@RomainMuller RomainMuller merged commit d90b304 into master Aug 10, 2019
@RomainMuller RomainMuller deleted the rmuller/misidentified-getter branch August 10, 2019 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants