Detect deprecations

[3flex]

Closes #267

This rule uses the diagnostic warning built into the Kotlin compiler. Earlier discussion suggested that detekt could reimplement compiler warnings, so users have control over which compiler warnings will fail the build.

Implementing rules in this way implements checks for compiler warnings with minimal effort.

One issue is that suppressions for detekt compiler warnings are separated by underscore (like "UNCHECKED_CAST") while detekt uses CamelCase.

For rules that overlay compiler checks that have names with underscores we could match the rule ID so a single suppression (@Suppress("UNCHECKED_CAST")) would apply to both detekt and the compiler, but then the style of the rule ID differs from the rest of the rules which have CamelCase IDs.

We should pick one option and stick to it because there are many built in warnings that would be good to implement in detekt.

[3flex]

@Deprecated("deprecation message") causes generateDocumentation task to throw an error.

\@Deprecated("deprecation message") doesn't render correctly, though it does remove the syntax highlighting in IDE so @Deprecated isn't highlighted, so probably that syntax should be accepted by generateDocumentation task.

[arturbosch]

You need to use a html escape character for @.
I used it somewhere ... some rule with annotations I don't remember anymore :'D

[codecov-io]

Codecov Report

Merging #1913 into master will increase coverage by 0.01%.
The diff coverage is 92.85%.

@@             Coverage Diff              @@
##             master    #1913      +/-   ##
============================================
+ Coverage     80.45%   80.47%   +0.01%     
- Complexity     1968     2000      +32     
============================================
  Files           326      329       +3     
  Lines          5510     5602      +92     
  Branches       1022     1042      +20     
============================================
+ Hits           4433     4508      +75     
- Misses          543      545       +2     
- Partials        534      549      +15

Impacted Files	Coverage Δ	Complexity Δ
...urbosch/detekt/generator/collection/RuleVisitor.kt	87.23% <100%> (ø)	38 <0> (ø)	⬇️
...sch/detekt/rules/providers/PotentialBugProvider.kt	100% <100%> (ø)	3 <0> (ø)	⬇️
...gitlab/arturbosch/detekt/rules/bugs/Deprecation.kt	90.9% <90.9%> (ø)	5 <5> (?)
...osch/detekt/rules/bugs/UnsafeCallOnNullableType.kt	72.72% <0%> (-14.78%)	4% <0%> (+1%)
...ekt/rules/style/optional/PreferToOverPairSyntax.kt	85.71% <0%> (-14.29%)	7% <0%> (+2%)
.../gitlab/arturbosch/detekt/rules/bugs/UnsafeCast.kt	75% <0%> (-13.89%)	4% <0%> (-1%)
...itlab/arturbosch/detekt/cli/console/DebtSumming.kt	90% <0%> (-10%)	7% <0%> (+2%)
...turbosch/detekt/rules/KtClassOrObjectExtensions.kt	66.66% <0%> (-8.34%)	0% <0%> (ø)
...gitlab/arturbosch/detekt/rules/style/MayBeConst.kt	75% <0%> (-2.28%)	40% <0%> (-1%)
...etekt/rules/style/FunctionOnlyReturningConstant.kt	92% <0%> (-0.31%)	27% <0%> (-1%)
... and 15 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 f714f6e...1901317. Read the comment docs.

[arturbosch]

Looks good and super cool that we can just wrap compiler warnings!
We do have the aliases feature exactly for your concern.
Take a look at https://github.com/arturbosch/detekt/blob/master/detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/FunctionNaming.kt#L29

This way you can suppress detekt issues with normal IDEA/kotlinc suppressions.

[3flex]

Aliases work. In this case though the detekt and Kotlin suppression is effectively the same (Deprecation/DEPRECATION), so @Suppress("Deprecation") suppresses both (Kotlin is case insensitive). For Kotlin rules with underscores in them the detekt and Kotlin suppression will be different: might be UncheckedCast in detekt and UNCHECKED_CAST in Kotlin for example. User needs two suppressions. This might be surprising to users since behaviour would be inconsistent.

We should either match the suppression ID that Kotlin uses when wrapping rules, or make sure it's different, and do it consistently.

[arturbosch]

Maybe its too early in the morning for me to think but isn't this also handled by the aliases feature?
The rule UncheckedCast has:

class UnsafeCast(config: Config = Config.empty) : Rule(config) {

    override val defaultRuleIdAliases: Set<String> = setOf("UNCHECKED_CAST")

    override val issue: Issue = Issue("UnsafeCast",

So the user only needs to suppress the kotlin compiler warning and detekt will also be suppressed?
The difficulty here will be to try out different suppression order and realize that suppressing kotlinc first will also suppress detekt ;)

[3flex]

Understood - so if the rule is the same or very similar to the kotlinc rule, add a default alias like above. Thanks, that answers my question.

I'll find the correct KDoc string as mentioned above then merge.

[arturbosch]

Tested it. IntelliJ autogenerates @Suppress("DEPRECATION") but also @Suppress("deprecation") and @Suppress("Deprecation") works.
Detekt needs the exact name, so we would add DEPRECATION here.

[3flex]

Using @Deprecated in the noncompliant sample is maybe not possible at the moment... because generateDocumentation task parses KDoc top to bottom firstly looking for a comment then looking for KDoc tags, it parses up to to the line before @Deprecated and treats that as the entire comment, and parses @Deprecated as a tag then ignores the rest.

I tried HTML-escaping with @ which works here in markdown: @, but that's not rendered when generating docs... support may have to be added.

Maybe this one can go in without code samples? It's self-explanatory and I don't want to fix this in the doc generation code because it's an edge case currently.

Unless you can find the other example? I searched all KDoc in detekt-rules but cannot find any line that starts with @ that isn't a KDoc tag.

[arturbosch]

Wow I thought our generator will crash when no examples are provided.
Hm I was sure I already escaped @ somewhere ...

Edit: Yes here https://github.com/arturbosch/detekt/pull/1823/files#diff-e19d966035356c2d7cdf24a1957fa12aR1246