New lint: Have a semicolon on the last block statement if it returns nothing

[xFrednet]

What it does

Validates that the last block statement ends with a semicolon if it returns nothing.

Categories (optional)

• Kind: clippy::style

Advantages

• It leads to consistent block formatting.

• Extending the block with new code doesn't require a change in previous last line

  Example:

  // Valid code
  {
      println!("Hello")
  }
  
  // Extending it would cause a change to the old `println!("Hello")` line
  {
      println!("Hello"); // <-- here
      println!("World")
  } 

• It makes block linting easier for us

  • This is of course not the main reason but this causes some edge cases in a lint I'm currently working on.

Drawbacks

None.

Example

{
    println!("Hello world")
}

Could be written as:

{
    println!("Hello world");
}

[xFrednet]

I believe that this could be a good first issue. I'm not sure in which file it would be implemented by you could roughly do the following to implement this lint:

1. Filter out any rustc_hir::Block from the ast. (Maybe with check_block)
2. Check if the expr field is Some. (This means it has a statement without a semicolon at the end)
3. Get the type of expr from a LateContext instance
4. Check if the type is () with .is_unit() on the type.

@rustbot label +good first issue

[flip1995]

@rustbot label +good-first-issue

[flip1995]

Regarding the lint level: This should probably be pedantic, since it doesn't really matter. Great explanation how to implement this @xFrednet!

[camsteffen]

I can't get an example for this lint to compile. Am I missing something? (playground)

[xFrednet]

You're right @camsteffen the expression has to return the unit type () I've missed that. Thank you! I've updated the description. (playground)

[camsteffen]

Cool. I guess the difference is that let x = ... is not an expression - it's a statement.

[camsteffen]

Two questions.

1. What if the expression does not return (), but the value is unused?

   fn test() {
       let mut set = HashSet::new();
       {
           set.insert("hi")
       }
       println!("hi");
   }

2. Is this a job for rustfmt?

[flip1995]

1. Your example will produce a mismatched type error. You can only leave out the semicolon if the last expression in the block returns ()
2. No idea if rustfmt does type analysis.

[camsteffen]

Ah nevermind. I thought this might not require type checking.

[1c3t3a]

I've started to implement this on my own fork but ran into an issue while testing. I first tested the lint on the println!() macro where it worked alright. Looking at assert_eq!(), my lint fired even with a ; at the end of the line. As I assumed the assert_eq!() macro implementation doesn't end with a semicolon, but the println!() macro does. I know this might be a beginner question, but is there a way to check for a semicolon after the macro invocation? I've already experimented with in_macro but it didn't quite help.

[camsteffen]

Is the lint firing on the block that is part of the macro expansion?

macro_rules! assert_eq {
    ($left:expr, $right:expr) => ({ // <- don't lint on the block that starts here

You could do dbg!(block.span) and look at the line/column numbers to find out.

[1c3t3a]

Is the lint firing on the block that is part of the macro expansion?

macro_rules! assert_eq {
    ($left:expr, $right:expr) => ({ // <- don't lint on the block that starts here

You could do dbg!(block.span) and look at the line/column numbers to find out.

Actually yes, it's exactly that line.

[camsteffen]

If you need more help, head over to zulip.