ameba icon indicating copy to clipboard operation
ameba copied to clipboard

Identify unintended assignment in condition

Open straight-shoota opened this issue 2 years ago • 1 comments

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:

  1. The assignment target is a fresh local variable.
  2. The assignment target is used in the block or subsequent parts of the condition.
  3. The assigned value is not a constant or literal.
  4. 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).
  5. 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.

straight-shoota avatar Nov 05 '22 21:11 straight-shoota

  1. 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.

straight-shoota avatar Nov 15 '22 11:11 straight-shoota