Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identify unintended assignment in condition #294

Open
straight-shoota opened this issue Nov 5, 2022 · 1 comment
Open

Identify unintended assignment in condition #294

straight-shoota opened this issue Nov 5, 2022 · 1 comment
Labels

Comments

@straight-shoota
Copy link
Contributor

straight-shoota commented Nov 5, 2022

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
Copy link
Contributor Author

  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.

@Sija Sija modified the milestones: 1.3.1, v1.4.0, 1.4.0 Nov 15, 2022
@Sija Sija removed their assignment Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants