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

NonBooleanPropertyPrefixedWithIs: Allow boolean function reference #4684

Merged

Conversation

kvn-stgl
Copy link
Contributor

@kvn-stgl kvn-stgl commented Apr 5, 2022

This PR will allow boolean function reference properties for the NonBooleanPropertyPrefixedWithIs rule. (And fix issue #4675).
It also fixes a little issue that unknown types are not null, the string is just empty. This should also fix issue #4674.

@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #4684 (5637b61) into main (e9d26d0) will decrease coverage by 0.00%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##               main    #4684      +/-   ##
============================================
- Coverage     84.58%   84.58%   -0.01%     
- Complexity     3434     3441       +7     
============================================
  Files           492      492              
  Lines         11329    11335       +6     
  Branches       2058     2061       +3     
============================================
+ Hits           9583     9588       +5     
  Misses          701      701              
- Partials       1045     1046       +1     
Impacted Files Coverage Δ
...t/rules/naming/NonBooleanPropertyPrefixedWithIs.kt 82.05% <50.00%> (+0.23%) ⬆️
.../arturbosch/detekt/rules/style/CanBeNonNullable.kt 72.61% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9d26d0...5637b61. Read the comment docs.

Copy link
Member

@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.

Thanks @kvn-stgl for the nice contribution!
May I ask you to correct the used code snippets in the tests, so that those two code snippets are actually valid Kotlin code. This repo runs compile checks on CI for snippets in tests. It would then make the CI green.

Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

Good one!

Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

Good one!

val isDebuggable get() = BuildConfig.DEBUG
}
"""
val findings = subject.compileAndLintWithContext(env, code)
Copy link
Member

Choose a reason for hiding this comment

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

This one should be just lintWithContext. You doesn't want to compile it because it is meant to no compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. Thanks for the hint!

@BraisGabin BraisGabin merged commit 8b6c5ae into detekt:main Apr 6, 2022
@kvn-stgl kvn-stgl deleted the feat/allow-boolean-function-reference branch April 6, 2022 08:01
@cortinico cortinico added this to the 1.20.0 milestone Apr 6, 2022
@cortinico cortinico added the rules label Apr 6, 2022
chao2zhang pushed a commit that referenced this pull request Apr 8, 2022
…4684)

* NonBooleanPropertyPrefixedWithIs: Allow boolean function reference properties

* fix detekt

* replace compileAndLintWithContext with lintWithContext
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 this pull request may close these issues.

None yet

4 participants