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

VarCouldBeVal: Add configuration flag ignoreLateinitVar #4745

Merged
merged 1 commit into from
Apr 24, 2022

Conversation

rli
Copy link
Contributor

@rli rli commented Apr 21, 2022

I'm not entirely convinced that it makes sense to disable this across the entire codebase, but maybe someone else has a use-case.

Fixes #4731

@BraisGabin
Copy link
Member

I'm not entirely convinced that it makes sense to disable this across the entire codebase, but maybe someone else has a use-case.

I agree but how could we fix this otherwise? I mean, do you see any better way to fix this for your use case? In general I don't want to add a feature just because "maybe someone else has a use-case".

In your case, does it seem right to add an @Suppress to those files? Or to exclude the path of those files? Or to exclude them by package?

Sorry maybe I over-simplify your use case and I propouse a solution that doesn't fit your use case.

@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

Merging #4745 (7f6d958) into main (7960453) will decrease coverage by 0.00%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##               main    #4745      +/-   ##
============================================
- Coverage     84.69%   84.69%   -0.01%     
- Complexity     3418     3419       +1     
============================================
  Files           490      490              
  Lines         11235    11240       +5     
  Branches       2068     2068              
============================================
+ Hits           9516     9520       +4     
  Misses          675      675              
- Partials       1044     1045       +1     
Impacted Files Coverage Δ
...lab/arturbosch/detekt/rules/style/VarCouldBeVal.kt 80.39% <85.71%> (-0.03%) ⬇️

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 7960453...7f6d958. Read the comment docs.

@vladimirfx
Copy link

I have a myriad of use cases. 90% of all our lateinit usages are flagged. Our config now is polluted with ignoreAnnotated :

  VarCouldBeVal:
    active: true
    ignoreAnnotated:
      - 'javax.persistence.Version'
      - 'javax.ws.rs.core.Context'
      - 'javax.inject.Inject'
      - 'org.springframework.beans.factory.annotation.Autowired'

But all these excludes are NOT enough because there are cases where no annotations at field level (see @QuarkusTestResource which is set at class level).

Very waiting for merge.

@cortinico cortinico added this to the 1.21.0 milestone Apr 24, 2022
@cortinico cortinico merged commit 1e506d3 into detekt:main Apr 24, 2022
@cortinico cortinico removed the core label Apr 24, 2022
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.

VarCouldBeVal reported on private lateinit var
4 participants