Rule `optimize-operands-order` reports potentially dangerous change when `len` of slice is used before slice accesing.

[moukoublen]

Describe the bug In this case:

if len(s) > 0 || s[0] == "" { // should not match, not safe
    // ...
}

Rule optimize-operands-order reports

for better performance 'len(s) == 0 || s[0] == ""' might be rewritten as 's[0] == "" || len(s) == 0'

Which could lead to panic if implemented.

To Reproduce Steps to reproduce the behavior:

1. I updated revive go install github.com/mgechev/revive@latest
2. I run it with the following flags & configuration file:

# no extra flags
revive -config revive.toml .

ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.3
errorCode = 0
warningCode = 0

[rule.optimize-operands-order]

Expected behavior I think in this case, where the len check is essential to happen before slice accessing, the rule should not match.

Additional context My first thought about fixing this was to not consider the built-in len function as a caller. I tried to change the isCaller function in the optimize-operands-order.go file so that it does not consider the built-in len function as a caller.

I opened a pr with this approach.

[chavacava]

Hi @moukoublen, thanks for reporting the issue and proposing a fix.

Personally, I use this rule to lint my code while I'm coding thus I can decide if the suggested modification is safe or not. That is because this rule can, as you pointed out, suggest unsafe/un-optimal code modifications.

[ccoVeille]

@moukoublen thanks for identifying and fixed that

Personally, I use this rule to lint my code while I'm coding thus I can decide if the suggested modification is safe or not.

@chavacava I agree with you, but some people are applying suggested changes blindly. It's great that @moukoublen introduced a safeguard