New rule: DontDowncastCollectionTypes

[cortinico]

I'm proposing a new rule: DontDowncastCollectionTypes

Down-casting immutable types from kotlin.collections should be discouraged.
The result of the downcast is platform specific and can lead to unexpected crashes.
Prefer to use instead the toMutable<Type>() functions.

Non-compliant

val list : List<Int> = getAList()
if (list is MutableList) {
    list.add(42)
}

(list as MutableList).add(42)

Compliant

val list : List<Int> = getAList()
list.toMutableList().add(42)

Happy to discuss if we want to adapt it to check casts against Java concrete collection types (e.g. ArrayList, etc.).

[codecov]

Codecov Report

Merging #3413 (a20691d) into master (2730336) will decrease coverage by 0.00%.
The diff coverage is 81.81%.

@@             Coverage Diff              @@
##             master    #3413      +/-   ##
============================================
- Coverage     80.38%   80.38%   -0.01%     
- Complexity     2748     2756       +8     
============================================
  Files           449      450       +1     
  Lines          8300     8330      +30     
  Branches       1587     1594       +7     
============================================
+ Hits           6672     6696      +24     
  Misses          774      774              
- Partials        854      860       +6     

Impacted Files	Coverage Δ	Complexity Δ
...h/detekt/rules/bugs/DontDowncastCollectionTypes.kt	81.25% <81.25%> (ø)	8.00 <8.00> (?)
...turbosch/detekt/rules/bugs/PotentialBugProvider.kt	100.00% <100.00%> (ø)	3.00 <0.00> (ø)
...ain/kotlin/io/github/detekt/metrics/LinesOfCode.kt	92.30% <0.00%> (-0.80%)	0.00% <0.00%> (ø%)
...gitlab/arturbosch/detekt/cli/runners/AstPrinter.kt	100.00% <0.00%> (ø)	4.00% <0.00%> (ø%)
...turbosch/detekt/rules/style/UnusedPrivateMember.kt	90.76% <0.00%> (+0.14%)	5.00% <0.00%> (ø%)

---

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 2730336...a20691d. Read the comment docs.

[cortinico]

This rule is inspired from Effective Kotlin: Item 1 - Limit Mutability (credits to @MarcinMoskala)

[BraisGabin]

Great rule! I think that it would be good to think of ArrayList at least because it exists in kotlin (with a type alias).

And I'm not sure about the name. But I can't think of anyone better.

[schalkms]

Great rule!

[cortinico]

Great rule! I think that it would be good to think of ArrayList at least because it exists in kotlin (with a type alias).

I've added those 5 type aliases to the mutable types to consider: https://github.com/JetBrains/kotlin/blob/46b7a774b558001c136be225cf4367fa09ba1aee/libraries/stdlib/jvm/src/kotlin/collections/TypeAliases.kt#L13-L17