Skip to content

Use config parameter for UseIfInsteadOfWhen rule #1987

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

Merged
merged 3 commits into from
Oct 6, 2019
Merged

Use config parameter for UseIfInsteadOfWhen rule #1987

merged 3 commits into from
Oct 6, 2019

Conversation

schalkms
Copy link
Member

@schalkms schalkms commented Oct 5, 2019

Deactivating the UseIfInsteadOfWhen rule had no effect, since the config
parameter was missing for the rule.
Fixes #1986

Deactivating the UseIfInsteadOfWhen rule had no effect, since the config
parameter was missing for the rule.
Fixes #1986
Copy link
Member Author

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

This should be a hotfix and go into v1.1.1
I'll also add a test to prevent that from happening in the future.

This test checks if a valid config has been passed to all detekt rules.
This prevents bugs like in #1986 form happening.
Copy link
Member

@arturbosch arturbosch left a comment

Choose a reason for hiding this comment

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

Awesome!

Reason:
Class.newInstance() bypasses exception checking;
prefer getDeclaredConstructor().newInstance()
http://errorprone.info/bugpattern/ClassNewInstance
@schalkms
Copy link
Member Author

schalkms commented Oct 5, 2019

should now also run on > Java8

@3flex 3flex merged commit 1c93b1e into detekt:master Oct 6, 2019
@arturbosch arturbosch added this to the 1.1.1 milestone Oct 6, 2019
@sschuberth
Copy link
Contributor

BTW, the 1.1.0 tag in Git seems to be wrong now as it includes this fix, however the issue only occurred after the 1.1.0 release.

@arturbosch
Copy link
Member

arturbosch commented Oct 8, 2019

Yeah, I forgot the github release tag.
Can I just checkout the right commit and tag it locally with git and it will be uploaded on the right place in GitHub :) ?
Will release 1.1.1 soon, a snapshot is up if you want to try out the fix: https://oss.jfrog.org/artifactory/oss-snapshot-local/io/gitlab/arturbosch/detekt/detekt-cli/1.1.0-SNAPSHOT/

@sschuberth
Copy link
Contributor

Well, yes, you can delete the wrong remote tag and push a new one under the same name, but of course that will not fix the local tags for anyone who fetched in between...

@schalkms schalkms deleted the fix-#1986-missing-config branch November 8, 2019 18:12
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Use config parameter for UseIfInsteadOfWhen rule

Deactivating the UseIfInsteadOfWhen rule had no effect, since the config
parameter was missing for the rule.
Fixes detekt#1986

* Add RuleProvider config test

This test checks if a valid config has been passed to all detekt rules.
This prevents bugs like in detekt#1986 form happening.

* Don't use deprecated class.newInstance() for Java9

Reason:
Class.newInstance() bypasses exception checking;
prefer getDeclaredConstructor().newInstance()
http://errorprone.info/bugpattern/ClassNewInstance
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Use config parameter for UseIfInsteadOfWhen rule

Deactivating the UseIfInsteadOfWhen rule had no effect, since the config
parameter was missing for the rule.
Fixes detekt#1986

* Add RuleProvider config test

This test checks if a valid config has been passed to all detekt rules.
This prevents bugs like in detekt#1986 form happening.

* Don't use deprecated class.newInstance() for Java9

Reason:
Class.newInstance() bypasses exception checking;
prefer getDeclaredConstructor().newInstance()
http://errorprone.info/bugpattern/ClassNewInstance
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.

Can't deactivate UseIfInsteadOfWhen rule
4 participants