Skip to content

Version of let_unit_value that is an early pass #2011

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

Closed
Manishearth opened this issue Sep 2, 2017 · 0 comments · Fixed by #2016
Closed

Version of let_unit_value that is an early pass #2011

Manishearth opened this issue Sep 2, 2017 · 0 comments · Fixed by #2016
Labels
A-lint Area: New lints T-AST Type: Requires working with the AST

Comments

@Manishearth
Copy link
Member

For rust-lang/rust#44173

We should have a "use unit value" lint that checks for:

  • () being assigned to a variable
  • () being passed to a function

where the origin of the () is basically a semicolonless block

An example of an early pass is https://github.com/rust-lang-nursery/rust-clippy/blob/master/clippy_lints/src/precedence.rs

What we want is an early pass (runs on the AST before typechecking) with a check_expr and check_stmt that look for:

and then run a is_unit_expr check on their contents (the initializer for assignment/let, and each of the arguments for calls)

is_unit_expr will:

  • if it's a Block, return true if the last statement is not an Expr statement
  • if it's an if expr that has an else block, return true if both the if and else blocks have a non-expr final statement
  • We can do something similar for match exprs if we wish.
  • return false otherwise

We can also have it recurse, where if the last statement is a StmtKind::Expr, then recurse down and check if that expr evaluates to unit.

Then, if any of the is_unit_expr calls evaluates to true, throw the lint.

@zmanian is working on this

@Manishearth Manishearth added A-lint Area: New lints T-AST Type: Requires working with the AST labels Sep 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints T-AST Type: Requires working with the AST
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant