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

Add option to add a reason to ForbiddenMethodCall #4910

Merged
merged 5 commits into from Jul 23, 2022
Merged

Conversation

BraisGabin
Copy link
Member

Same as #4909 but now adding it to ForbiddenMethodCall

Important: This introduces a breaking change in ForbiddenMethodCall. It no longer allows comma-separated imports as a String. It should use yml lists. I'm targetting this to 1.22.0 because on 1.21.0 we added make that all those usages will be a warning. Tell me is we should be more conservative.

The reason that this is a draft is just to avoid accidental merges. But the code should be ready to merge.

Related with #3501

@codecov
Copy link

codecov bot commented Jun 4, 2022

Codecov Report

Merging #4910 (ae4f246) into main (3baa8f9) will decrease coverage by 0.00%.
The diff coverage is 90.47%.

@@             Coverage Diff              @@
##               main    #4910      +/-   ##
============================================
- Coverage     84.92%   84.91%   -0.01%     
- Complexity     3605     3614       +9     
============================================
  Files           502      502              
  Lines         11873    11887      +14     
  Branches       2227     2237      +10     
============================================
+ Hits          10083    10094      +11     
+ Misses          691      690       -1     
- Partials       1099     1103       +4     
Impacted Files Coverage Δ
...ain/kotlin/io/github/detekt/metrics/LinesOfCode.kt 94.44% <0.00%> (-1.02%) ⬇️
...rturbosch/detekt/rules/style/NewLineAtEndOfFile.kt 95.23% <50.00%> (-4.77%) ⬇️
...kotlin/io/gitlab/arturbosch/detekt/api/Location.kt 86.84% <100.00%> (-0.66%) ⬇️
...ls/src/main/kotlin/io/github/detekt/psi/KtFiles.kt 76.92% <100.00%> (+1.92%) ⬆️
...rturbosch/detekt/rules/bugs/UnusedUnaryOperator.kt 72.00% <100.00%> (+5.33%) ⬆️
...b/arturbosch/detekt/rules/style/ForbiddenImport.kt 96.00% <100.00%> (ø)
...turbosch/detekt/rules/style/ForbiddenMethodCall.kt 91.66% <100.00%> (+0.55%) ⬆️
...o/gitlab/arturbosch/detekt/rules/KtModifierList.kt 88.23% <0.00%> (-4.08%) ⬇️
.../rules/documentation/UndocumentedPublicFunction.kt 90.47% <0.00%> (-3.28%) ⬇️
...in/io/gitlab/arturbosch/detekt/rules/Traversing.kt 40.00% <0.00%> (-2.86%) ⬇️
... and 3 more

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 594a2da...ae4f246. 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.

👍

@marschwar
Copy link
Contributor

How Do we go about breaking changes in minor releases? Do we need an upgrade gradle task for users to call?

@BraisGabin BraisGabin enabled auto-merge (squash) July 23, 2022 12:45
@BraisGabin BraisGabin merged commit 855065b into main Jul 23, 2022
@BraisGabin BraisGabin deleted the fix-3501-2 branch July 23, 2022 12:56
VitalyVPinchuk pushed a commit to VitalyVPinchuk/detekt that referenced this pull request Jul 25, 2022
* Improve tests

* Improve issue message

* Removes that test support with comma-separated strings

* Add support for reasons in ForbiddenMethodCall

* Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt

Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com>

Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com>
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

3 participants