Purescript: Proposal: Enable shadowing warnings in `case` patterns

Created on 17 May 2020  路  4Comments  路  Source: purescript/purescript

The compiler can help beginners avoid the following mistake by enabling warnings for shadowed variables in patterns of case expressions.

The naive user may write the following:

showVal :: Maybe Int -> Int -> String
showVal special v =
  let
    showV = show v
  in
    case special of
      Just v -> showV <> " is special"
      _ -> showV

main :: Effect Unit
main = render =<< withConsole do
  traverse_ log $ map (showVal $ Just 2) $ 1..3

https://try.purescript.org/?gist=cd41b818f5853806f836489993e523fb

and expect it to print:

2 is special
3

when it actually prints:

1 is special
2 is special
3 is special

The root cause is a misunderstanding of how pattern matching works with case. In this example, incorrectly assuming a match against the value of v, rather than the actual behavior of matching all Justs and overwriting v.

Shadowing warnings would help highlight this misunderstanding.

If the shadowing warnings are added, then would it be possible to support pattern matching against the value of v? This could be a nice feature, as it can result in cleaner syntax than if using guards.

There's another proposal to completely remove shadowing warnings in #3375, which is incompatible with this one.

enhancement

Most helpful comment

I wonder if anyone really would have a mental model that your expected thing would happen, that seems very strange to me :smile: there's a reason Lisp-2s are less common.

If the newly bound v was used it would be warned about, as we only detect shadowed names on usage rather than declaration I guess.

It should be marked as unused though, I'd prefer just to have that, rather than marking it shadowed too. #3189 should address that when merged.

All 4 comments

It would also be great to see an "unused variable v" warning within the case expression. Perhaps this is a job for the linter.

I wonder if anyone really would have a mental model that your expected thing would happen, that seems very strange to me :smile: there's a reason Lisp-2s are less common.

If the newly bound v was used it would be warned about, as we only detect shadowed names on usage rather than declaration I guess.

It should be marked as unused though, I'd prefer just to have that, rather than marking it shadowed too. #3189 should address that when merged.

3189 should address that when merged.

Is that the correct issue link?

Uh, no :smile: I meant #3819

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hdgarrood picture hdgarrood  路  3Comments

MonoidMusician picture MonoidMusician  路  3Comments

garrett-hopper picture garrett-hopper  路  3Comments

rightfold picture rightfold  路  3Comments

jnbooth picture jnbooth  路  3Comments