gleam icon indicating copy to clipboard operation
gleam copied to clipboard

Unused string concatenation does not generate warning

Open LilyRose2798 opened this issue 1 year ago • 2 comments

It is very easy to make the mistake of leaving out a <> in a long statement of many string concatenations, i.e.: "foo" <> a <> "bar" b <> "baz The issue is the because this is valid code and will be parsed as two separate expressions, and only the last expression will be returned from the block, you'll end up with the previous concatenations before the missing <> being dead/unused code. You'll get no warning either to indicate that you've done something unintentional either. Ideally this unused code should generate a warning to make it easier to see that you've made a mistake. This would require checking for expression that only contain <> and, are not the last statement in the block, and are not assigned to anything. This issue also applies to basically any pure function or operator as well, such as 1 + 2 + 3 4 + 5 + 6 which will discard the 1 + 2 + 3 as it is parsed as a separate unused expression. The only way to solve this generally is to know whether all the function calls and operators within an expression are pure or not, and give a warning on any unused expressions that only contain pure functions and operators. Given that there doesn't look to be a way currently to mark functions as pure (though a list of pure/unpure functions in the stdlib could be made for the compiler to use), I think the best solution currently is to just stick to operators, since as far as I'm aware there are no unpure operators, and just put the warning on any unused expressions that only contain operators (so no function calls).

LilyRose2798 avatar Mar 20 '24 08:03 LilyRose2798

Generalising on this, how about reporting a warning for all statements that evaluate to a type that is not Nil, are not let/let assert, and are not the last statement in a block? (Think Rust's #[must_use], or Zig's "unused value" errors)

If the intention was to actually discard the resulting value, one could assign it to the blank identifier, or pipe it to a discard function:

import gleam/io

pub fn discard(_: a) -> Nil {
  Nil
}

pub fn impure_function() -> String {
  io.println("I am a side effect :>")
  "some maybe-useful return value"
}

pub fn main() -> Nil {
  let _ = impure_function()
  impure_function() |> discard
  Nil
}

Fri3dNstuff avatar Mar 20 '24 11:03 Fri3dNstuff

Good idea @LilyRose2798, thank you. Let's do this for all operators other than pipe.

Generalising on this, how about reporting a warning for all statements that evaluate to a type that is not Nil, are not let/let assert, and are not the last statement in a block?

No plans to do that as it would be very annoying to have to write let _ = on every line.

lpil avatar Mar 20 '24 19:03 lpil

I'm planning to work on this one!

giacomocavalieri avatar Mar 22 '24 08:03 giacomocavalieri