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

#7360: Add 'assign' to operatorSet of UnusedImport #7361

Merged
merged 4 commits into from
Jul 3, 2024

Conversation

PhilippNowak96
Copy link
Contributor

This PR adds assign to the operatorSet of the UnusedImport rule so that kotlin-dsl assignments are not reporting unused imports.

Resolves #7360

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.50%. Comparing base (da020ea) to head (17516a6).

Current head 17516a6 differs from pull request most recent head 609a198

Please upload reports for the commit 609a198 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7361   +/-   ##
=========================================
  Coverage     84.50%   84.50%           
  Complexity     4168     4168           
=========================================
  Files           573      573           
  Lines         11897    11900    +3     
  Branches       2461     2462    +1     
=========================================
+ Hits          10053    10056    +3     
  Misses          591      591           
  Partials       1253     1253           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BraisGabin
Copy link
Member

🤔 sorry, but I can't find assign as an operator on the kotlin documentation: https://kotlinlang.org/docs/operator-overloading.html where is it documented? Or is this gradle specific?

@PhilippNowak96
Copy link
Contributor Author

@BraisGabin this seems to be some gradle specific stuff. I read this article about it https://blog.gradle.org/simpler-kotlin-dsl-property-assignment and then verified it by the test

@3flex
Copy link
Member

3flex commented Jun 13, 2024

Yes it's Gradle specific and implemented as a compiler plugin applied by Gradle when it compiles scripts.

Kotlin offers the extension point to customise behaviour of the assign operator (and possibly others though I'm not sure).

Given this is Gradle specific, and in line with our general philosophy of not building library or tool-specific config as defaults, I think it would be better to introduce a config option for additionalOperators where things like this can be added for those that need it.

@PhilippNowak96
Copy link
Contributor Author

Thanks @3flex for the clarification ☺️ I like the idea introducing a separate set for non language default operators. Changes have been pushed.

PhilippNowak96 and others added 2 commits June 21, 2024 06:43
@schalkms schalkms merged commit d4db424 into detekt:main Jul 3, 2024
19 checks passed
@3flex
Copy link
Member

3flex commented Jul 4, 2024

Sorry I didn't check this more closely - in my earlier comment I meant that the assign operator should be configurable and something to set in the YAML config. It's been implemented here as a hardcoded value in the rule but it's highly specific to Gradle so should be something that can be configured by users who are using that for Gradle analysis and not part of the standard rule config which should be framework-, tool- and library-agnostic.

I'll open a new issue.

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.

UnusedImport reported for import org.gradle.kotlin.dsl.assign in kotlin-dsl
6 participants