Redundant usage of Boolean.not()

[osipxd]

Obsolete. Superseded by:

• New rule: RedundantExplicitOperatorCall #7034
• New rule: DoubleNegativeExpression #7035

Expected Behavior of the rule

Suggest to replace Boolean.not() by the operator ! if it is possible.

// Compliant
if (!isValid) { ... } 
val notValid = isValid?.not() // OK to use .not() for nullable types 

// Noncompliant
if (isValid.not()) { ... } // Call .not() could be replaced by the operator !

Context

• It might confuse if both of options are used for boolean iversion. It is harder to understand if a condition is negative because you have to check both the start and the end of the condition.
• Automatic refactorings like "Invert 'If' condition" doesn't work correctly with .not(). Applying of such refactoring produces double-negative expression !isValid.not()

[atulgpt]

Hi @osipxd I think you can use the ForbiddenMethod rule for this thing. Are you not able to do that?

[osipxd]

I think you can use the ForbiddenMethod rule for this thing.

Unfortunately not. When I add pattern kotlin.Boolean.not to ForbiddenMethod, detekt reports also !, because .not() is operator fun.

[osipxd]

I think, this rule might be generalized to "Redundant explicit usage of an operator" with configurable list of operators to be reported.

RedundantExplicitOperatorCall:
    enabled: true
    operators:
      - not
      - plusAssign

[osipxd]

By the way, @atulgpt. I've noticed that you prefer to use .not() (at least in this repository), can you elaborate on why should it be used instead of !?

I know the point that ! is easy to miss in expression, but I'm not sure that usage of explicit operator completely solves this problem. In addition to the reasons I've described in "Context" section, there is another one reason: until ! is not prohibited from usage, we cannot ensure there will no construction like !isValid.not(). However, this problem could be solved with something like "Double negative expression" rule.

Some statistics

Usages of ! in detekt project ~70 files
Usages of .not() in detekt project ~18 files (2 of them is usage of .not() on a nullable type)

[atulgpt]

By the way, @atulgpt. I've noticed that you prefer to use .not() (at least in this repository), can you elaborate on why should it be used instead of !?

I know the point that ! is easy to miss in expression, but I'm not sure that usage of explicit operator completely solves this problem. In addition to the reasons I've described in "Context" section, there is another one reason: until ! is not prohibited from usage, we cannot ensure there will no construction like !isValid.not(). However, this problem could be solved with something like "Double negative expression" rule.

Some statistics

Usages of ! in detekt project ~70 files
Usages of .not() in detekt project ~18 files (2 of them is usage of .not() on a nullable type)

Hi @osipxd, I think it's my preference as I somehow find .not very easy to use as it goes very well with my flow of thinking(as you have to apply it at the very end so I don't have to traverse the cursor back to add ! ) also it makes the expression easy to use in a chain of operators

But I am not against forbidding the use of .not in certain condition(it would have been awesome if we can accomplish this using ForbiddenMethod). It's just that I might not be using that rule in my projects

I completely agree with double Inverted rule as that pattern should be disallowed

[BraisGabin]

"Double negative expression" sounds like a great rule for detekt.

And about this rule I don't think we should have a rule that forbids the usage of .not. In general the people see detekt rules as something they should follow as good patterns. Even if they are not enable by default. So I think that your proposal of RedundantExplicitOperatorCall would be a better fit. Also we already have a rule to promote [] over get so maybe we can merge it (I can't recall the name now).

[osipxd]

Closing this one in favor of the created issues. Let's continue discussion there.