ameba
ameba copied to clipboard
Identify unintended assignment in condition
This is an idea for a rule that targets the same error type as #163, an unintended assignment in a condition (instead of equality comparison).
I think the only valid use case for an assignment in a condition is for eliminating a Nil
type from an expression. For that purpose, it's assigned to a local variable and the type restriction excluding Nil
applies inside the block.
So we could use a couple of indicators which identify this use case:
- The assignment target is a fresh local variable.
- The assignment target is used in the block or subsequent parts of the condition.
- The assigned value is not a constant or literal.
- The assigned value should not be a local variable (you don't need to assign it again, the type restriction would apply on the original already).
- The conditional expression is not a trailing one (because there's no body with a type restriction; I suppose it could be valid if another part of the condition uses the restricted variable, but that's pretty certainly not a good style for a trailing conditional).
There might be more indicators.
The original example from #163 would already violate 1), 2), 3) and 5) 😎
var = 1
puts "foo" if var = 1
I think this approach would be far superior to using Yoda conditionals for this use case.
- The assigned value is not a constant or literal.
Technically, there are some valid use cases for constants whose value is determined at runtime (and are nilable).
An example for that would be Process::INITIAL_PWD
.
if pwd = Process::INITIAL_PWD
# do something only if INITIAL_PWD exists
end
Example use case: https://github.com/crystal-lang/crystal/blob/ed211835f35e354a9047a36aa634a32c01844864/src/process/executable_path.cr#L178
I think this is pretty rare, though. Most constants have static values an non-nilable types. Not sure if there are any considerable examples besides INITIAL_PWD
, pretty sure there are none in stdlib.