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 rule deprecation mechanism and deprecate DuplicateCaseInWhenExpression, MissingWhenCase, RedundantElseInWhen #5309

Merged
merged 9 commits into from Oct 4, 2022

Conversation

VitalyVPinchuk
Copy link
Contributor

@VitalyVPinchuk VitalyVPinchuk commented Sep 17, 2022

Close (#4973)
Exactly the same as deprecation.properties save for filename deprecation.rules.
Uses deprecation.properties to write deprecated rules.

@github-actions
Copy link

github-actions bot commented Sep 17, 2022

Messages
📖

Thank you very much for making our website better ❤️!

Generated by 🚫 dangerJS against 77c61c3

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 for that! Looks good!
I also prefer using the existing single file for this use case, as Nico mentioned.

@VitalyVPinchuk
Copy link
Contributor Author

Updated, and now it uses single file.

VitalyVPinchuk and others added 2 commits September 18, 2022 17:52
Co-authored-by: Nicola Corti <corti.nico@gmail.com>
…enerator/collection/Rule.kt

Co-authored-by: Nicola Corti <corti.nico@gmail.com>
@cortinico cortinico added the notable changes Marker for notable changes in the changelog label Sep 19, 2022
@cortinico
Copy link
Member

You'll have to suppress the deprecation locally inside PotentialBugProvider as otherwise the CI is red.

w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (23, 13): 'DuplicateCaseInWhenExpression' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (24, 13): 'ElseCaseInsteadOfExhaustiveWhen' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (37, 13): 'MissingWhenCase' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (39, 13): 'RedundantElseInWhen' is deprecated. Rule deprecated as compiler performs this check by default

@cortinico cortinico added this to the 1.22.0 milestone Sep 19, 2022
@VitalyVPinchuk
Copy link
Contributor Author

You'll have to suppress the deprecation locally inside PotentialBugProvider as otherwise the CI is red.

w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (23, 13): 'DuplicateCaseInWhenExpression' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (24, 13): 'ElseCaseInsteadOfExhaustiveWhen' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (37, 13): 'MissingWhenCase' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt: (39, 13): 'RedundantElseInWhen' is deprecated. Rule deprecated as compiler performs this check by default

I did it like this, but it doesn't help

    @Suppress("DEPRECATION")
    override fun instance(config: Config): RuleSet = RuleSet(

@cortinico
Copy link
Member

I think you'll have to inline it:

        listOf(
            AvoidReferentialEquality(config),
            Deprecation(config),
            DontDowncastCollectionTypes(config),
            DoubleMutabilityForCollection(config),
            @Suppress("DEPRECATION") DuplicateCaseInWhenExpression(config),
            ElseCaseInsteadOfExhaustiveWhen(config),
            EqualsAlwaysReturnsTrueOrFalse(config),

@cortinico
Copy link
Member

You have more warnings:

w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/DuplicateCaseInWhenExpressionSpec.kt: (9, 27): 'DuplicateCaseInWhenExpression' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/ElseCaseInsteadOfExhaustiveWhenSpec.kt: (13, 27): 'ElseCaseInsteadOfExhaustiveWhen' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt: (17, 31): 'MissingWhenCase' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt: (312, 31): 'MissingWhenCase' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhenSpec.kt: (13, 27): 'RedundantElseInWhen' is deprecated. Rule deprecated as compiler performs this check by default

@VitalyVPinchuk
Copy link
Contributor Author

I think you'll have to inline it:

        listOf(
            AvoidReferentialEquality(config),
            Deprecation(config),
            DontDowncastCollectionTypes(config),
            DoubleMutabilityForCollection(config),
            @Suppress("DEPRECATION") DuplicateCaseInWhenExpression(config),
            ElseCaseInsteadOfExhaustiveWhen(config),
            EqualsAlwaysReturnsTrueOrFalse(config),

Yes, that worked. Thanks!
This way suppressed rules are ignored by RuleSetProviderCollector.kt, so I slightly modified visitCallExpression.

?.map { if (it is KtAnnotatedExpression) it.lastChild as KtCallExpression else it }

@VitalyVPinchuk
Copy link
Contributor Author

You have more warnings:

w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/DuplicateCaseInWhenExpressionSpec.kt: (9, 27): 'DuplicateCaseInWhenExpression' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/ElseCaseInsteadOfExhaustiveWhenSpec.kt: (13, 27): 'ElseCaseInsteadOfExhaustiveWhen' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt: (17, 31): 'MissingWhenCase' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt: (312, 31): 'MissingWhenCase' is deprecated. Rule deprecated as compiler performs this check by default
w: /home/runner/work/detekt/detekt/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhenSpec.kt: (13, 27): 'RedundantElseInWhen' is deprecated. Rule deprecated as compiler performs this check by default

Right.
I didn't see them while running ./gradlew clean build. Maybe there's a simple command to purge Gradle's cache without downloading dependencies?

@cortinico
Copy link
Member

I didn't see them while running ./gradlew clean build. Maybe there's a simple command to purge Gradle's cache without downloading dependencies?

You should be able to replicate locally with:

./gradlew build -x detekt -PwarningsAsErrors=true

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.

I like your solution :)!

But I really think that we should not deprecate ElseCaseInsteadOfExhaustiveWhen.

@cortinico
Copy link
Member

Add rule deprecation mechanism

Can we also update the title of this PR mentioning which rule we deprecated?

@VitalyVPinchuk VitalyVPinchuk changed the title Add rule deprecation mechanism Add rule deprecation mechanism and deprecate DuplicateCaseInWhenExpression, MissingWhenCase, RedundantElseInWhen Sep 21, 2022
- Revert docs for 1.21.0
- Exclude ElseCaseInsteadOfExhaustiveWhen rule
- Mark deprecated with Strikethrough font.
@codecov
Copy link

codecov bot commented Sep 21, 2022

Codecov Report

Merging #5309 (77c61c3) into main (15c19aa) will increase coverage by 85.84%.
The diff coverage is 88.88%.

@@             Coverage Diff             @@
##             main    #5309       +/-   ##
===========================================
+ Coverage        0   85.84%   +85.84%     
- Complexity      0     3727     +3727     
===========================================
  Files           0      513      +513     
  Lines           0    12035    +12035     
  Branches        0     2232     +2232     
===========================================
+ Hits            0    10331    +10331     
- Misses          0      611      +611     
- Partials        0     1093     +1093     
Impacted Files Coverage Δ
...t/generator/collection/RuleSetProviderCollector.kt 78.31% <0.00%> (ø)
...detekt/rules/bugs/DuplicateCaseInWhenExpression.kt 100.00% <ø> (ø)
...ab/arturbosch/detekt/rules/bugs/MissingWhenCase.kt 92.85% <ø> (ø)
...rturbosch/detekt/rules/bugs/RedundantElseInWhen.kt 92.30% <ø> (ø)
...arturbosch/detekt/generator/printer/RulePrinter.kt 96.96% <75.00%> (ø)
...lab/arturbosch/detekt/generator/collection/Rule.kt 100.00% <100.00%> (ø)
...urbosch/detekt/generator/collection/RuleVisitor.kt 89.89% <100.00%> (ø)
...osch/detekt/generator/printer/DeprecatedPrinter.kt 100.00% <100.00%> (ø)
...ator/printer/defaultconfig/RuleSetConfigPrinter.kt 88.46% <100.00%> (ø)
...turbosch/detekt/rules/bugs/PotentialBugProvider.kt 100.00% <100.00%> (ø)
... and 513 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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.

LGTM Thank you so much for the contribution.

@BraisGabin BraisGabin merged commit 8784c78 into detekt:main Oct 4, 2022
@VitalyVPinchuk VitalyVPinchuk deleted the rules_deprecation branch October 5, 2022 09:50
antonis added a commit to wordpress-mobile/WordPress-Android that referenced this pull request Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation notable changes Marker for notable changes in the changelog rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants