Create Rule: MapGetWithNotNullAssertionOperator

[smyachenkov]

This PR adds new rule MapGetWithNotNullAssertionOperator to potential-bugs ruleset.

Rule MapGetWithNotNullAssertionOperator reports not-null asserted calls of map.get()!! or map[]!! methods and recommends to use map.getValue(), map.getOrDefault() or map.getOrElse().

This rule is based on an inspection MapGetWithNotNullAssertionOperatorInspection from IntelliJ IDEA.

Noncompliant Code

val map = emptyMap<String, String>()
map["key"]!!

val map = emptyMap<String, String>()
map.get("key")!!

Compliant Code

val map = emptyMap<String, String>()
map["key"]

val map = emptyMap<String, String>()
map.getValue("key")

val map = emptyMap<String, String>()
map.getOrDefault("key", "")

val map = emptyMap<String, String>()
map.getOrElse("key", { "" })

[codecov-io]

Codecov Report

Merging #2171 into master will increase coverage by 0.01%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##             master    #2171      +/-   ##
============================================
+ Coverage     80.58%   80.59%   +0.01%     
- Complexity     2024     2028       +4     
============================================
  Files           337      338       +1     
  Lines          5836     5850      +14     
  Branches       1065     1066       +1     
============================================
+ Hits           4703     4715      +12     
  Misses          564      564              
- Partials        569      571       +2

Impacted Files	Coverage Δ	Complexity Δ
...sch/detekt/rules/providers/PotentialBugProvider.kt	100% <100%> (ø)	3 <0> (ø)	⬇️
...t/rules/bugs/MapGetWithNotNullAssertionOperator.kt	84.61% <84.61%> (ø)	4 <4> (?)

---

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 ffbd6e1...86b8dae. Read the comment docs.

[schalkms]

Thanks for contributing this rule!
This is an awesome first contribution! 🎉
I attached my feedback and one question to this code review.

[schalkms]

What about this check from the original rule?
I think it's missing

if (expression.getReplacementData() == null) return

[3flex]

Looking at the IDEA implementation I think that's only used to make sure it's possible for IDEA to recommend a quick fix, I don't think it's used to check for the issue itself. The tests in this PR look comprehensive.

[smyachenkov]

That is correct, getReplacementData function in original inspection is used for generation of the replacement code. I left it out because I think it's enough to check that expression.operationToken is !! and that is expression on the left is resolved to Map.get.

[3flex]

This rule is great! Well done. Once you've addressed feedback from @schalkms and myself I'd be happy to merge this.