Add AvoidReferentialEquality rule

[marschwar]

This closes #3363

[codecov]

Codecov Report

Merging #3924 (637afa7) into main (40abc3f) will increase coverage by 0.04%.
The diff coverage is 93.93%.

@@             Coverage Diff              @@
##               main    #3924      +/-   ##
============================================
+ Coverage     83.44%   83.49%   +0.04%     
- Complexity     3149     3162      +13     
============================================
  Files           456      458       +2     
  Lines          9014     9047      +33     
  Branches       1754     1757       +3     
============================================
+ Hits           7522     7554      +32     
  Misses          565      565              
- Partials        927      928       +1     

Impacted Files	Coverage Δ
...osch/detekt/rules/bugs/AvoidReferentialEquality.kt	91.30% <91.30%> (ø)
...itlab/arturbosch/detekt/api/internal/SimpleGlob.kt	100.00% <100.00%> (ø)
...turbosch/detekt/rules/bugs/PotentialBugProvider.kt	100.00% <100.00%> (ø)
...lin/io/gitlab/arturbosch/detekt/rules/TypeUtils.kt	100.00% <0.00%> (+100.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 40abc3f...637afa7. Read the comment docs.

[BraisGabin]

Could you add some tests that show that == is never flagged? Other where the type is String === Any and Any === String. And even one more with List === List (without any new pattern)

[marschwar]

What do you mean by String === Any ?

This does not compile.

val b = "s" === 42

[cortinico]

Great work 🚀

[marschwar]

After rebase, the branch fails on the CI. [edit]

* What went wrong:
Execution failed for task ':detekt-gradle-plugin:test'.
> Multiple build operations failed.
      Metaspace
      Metaspace
      Metaspace
   > Metaspace
   > Metaspace
   > Metaspace

Is there anythig I can do about it?

[cortinico]

Is there anythig I can do about it?

Can you try to ./gradlew --stop and re-run?
Also I don't think we merged anything that could have caused this.

[marschwar]

Sorry, I should have been more specific. It only happens in the CI.

[cortinico]

Are we ready to merge this?

[Marcono1234]

It looks like this does not cover all meta characters. For example [, ], $, ^, (, ) (and probably more) would still be interpreted as regex meta characters.
This is likely not an issue for valid class names (since they should not contain any of these characters?), but users might misuse these characters which could become a maintenance burden.

A saner (but more complex) implementation might be to use Regex.escape for all the literal pieces of the glob pattern.
For example (not tested):

fun buildRegex(globPattern: String): Regex {
    val patternStringBuilder = StringBuilder()
    var nextStart = 0
    
    globPattern.forEachIndexed { index, char ->
        val replacement = when(char) {
            '*' -> ".*"
            '?' -> "."
            else -> null
        }
        
        if (replacement != null) {
            // Append escaped previous text, if any
            if (nextStart < index) {
            	val escaped = Regex.escape(globPattern.substring(nextStart, index))
            	patternStringBuilder.append(escaped)
            }
            // Append meta character replacement
            patternStringBuilder.append(replacement)
            nextStart = index + 1
        }
    }
    
    // Append escaped remainder, if any
    if (nextStart < globPattern.length) {
        val escaped = Regex.escape(globPattern.substring(nextStart))
        patternStringBuilder.append(escaped)
    }
    
    return Regex(patternStringBuilder.toString())
}

---

Note however, that I am not a member of this project, so this is at most a suggestion.

[cortinico]

but users might misuse these characters which could become a maintenance burden.

Is this an edge case or are there a real examples to support this? I'm trying to understand from which prospective you're mentioning users misusing those characters

[Marcono1234]

It is probably an edge case.

It looks like the intention here was to create a simple glob pattern. However, a user might by accident use a regex pattern such as java.lang.(String|Integer) (which the current implementation permits) and then expect that this works in the future as well. This could become a maintenance burden because the intention here was not to support something like this.

But this is probably negligible, so feel free to ignore.