Compiler could warn about type variable shadowing, and hint about ScopedTypeVariables

[stusmith]

GHC already warns about variable shadowing:

$ cat test.hs
module Test where

timesTwoPlusOne x
    = timesTwo x + 1
  where timesTwo x = x * 2

$ ghc -fwarn-name-shadowing test.hs
...
Warning:
    This binding for `x' shadows the existing binding
      bound at <location>

However the similar warning doesn't happen for type variables.

$ cat T9244.hs
module T9244 where

import Control.Exception

tryMaybe :: IO a -> IO (Maybe a)
tryMaybe action = do
    result <- (try action) :: IO (Either SomeException a)
    return $ case result of
        Left _  -> Nothing
        Right v -> Just v

$ ghc -fwarn-name-shadowing T9244.hs
...
    Couldn't match type `a' with `a1'
      `a' is a rigid type variable bound by
          the type signature for tryMaybe :: IO a -> IO (Maybe a)
          at types.hs:<line>:13
      `a1' is a rigid type variable bound by
           an expression type signature: IO (Either SomeException a1)
           at types.hs:<line>:15
    Expected type: IO a1
      Actual type: IO a
...

Here, I thought that the 'a' in the function's type declaration was the same 'a' in the expression type declaration. However in Haskell 98, they are completely different variables.

Suggestion: if a type variable is renamed by the compiler due to a clash with another type variable, issue a warning that the second shadows the first, and give a hint about using -XScopedTypeVariables and forall.

Alternative suggestion: if an error is displayed, where the error contains a renamed type variable, issue a hint that the second shadows the first, and give a hint about using -XScopedTypeVariables and forall.

[simonpj]

I think that's a fine idea. The easiest place to try this might be the renamer. At the moment the type signature for tryMaybe

tryMaybe :: IO a -> IO (Maybe a)

does not bring a into scope in the body. You need -XScopedTypeVariables, and you need an explicit forall:

tryMaybe :: forall a. IO a -> IO (Maybe a)

But it might be reasonable for the renamer to suggest that this is perhaps what you intended, when it comes across that a mentioned in

(try action) :: IO (Either SomeException a)

Simon

[thomie]

Also reported as #1316, #3691, #11438 and maybe others.

@goldfire writes in ticket:11438#comment:1 (I'm moving the discussion here):

An easy way of suggesting ScopedTypeVariables just came to mind: pretend the extension is always on. When looking up a type variable, if the extension is off but the variable would be in scope otherwise, suggest the extension, while returning a lookup failure (because the variable really isn't in scope!). Getting caught on ScopedTypeVariables is a fairly common occurrence in my experience, so I think it's worth putting in a bit of effort to do better here. (Even better would be to look for type variables in a signature that's missing a forall to suggest adding the forall, but that can be a separate task.)

[thomie]

@goldfire in #10581:

The ScopedTypeVariables extension is one of the few that doesn't recommend itself when it should be used.

I recognize that this may be hard to do, but here is a strawman proposal: Always behave as if ScopedTypeVariables is on at binding sites for type variables (in an annotation with forall). Then, at occurrences of type variables, check if the extension is on. If a type variable is in scope but the extension is off, remember that the user probably wants the extension, but then rename the type variable occurrence away from the in-scope one. If a type error ensues, we've remembered that the lack of ScopedTypeVariables may be to blame, and we recommend turning it on.

[thomie]

#11539 requests to warn about missing forall when -XScopedTypeVariables is on.

[thomie]

Another request for this feature: #12716.

[bgamari]

It doesn't look like this will happen for 8.2.

[kanetw]

Working on this, but it seems pretty complicated.

[simonpj]

Before implementing much, it'd be good to write a specification of what you are trying to implement. I think it may be something like this:

• When compiling without -XScopedTypevariables...
• ...and type variable a would have been in scope if you were compiling with -XScopedTypeVariables...
• ...and when you are about to add implicit quantification over a
• ...then emit a warning

Example

f :: forall a. [a] -> [a]
f = ....(let g :: a -> a
             g = ...
         in blah)....

With -XScopedTypeVariables the tyvar a would have been in scope in the body of f. Because it isn't in scope, we are going to implicitly quantify over a in the type signature for g, treating its signature as g :: forall a. a->a. That implicit quantification is what we want to warn about.

Question: what if we had instead written

f :: [a] -> [a]
f = ....(let g :: a -> a
             g = ...
         in blah)....

Now, even with -XScopedTypeVariables the tyvar a would not be in scope. Do we want to warn then too? I guess so.

Writing all this down, with a series of examples, would be helpful.

I can see that it's a bit tricky to implement. We kind of need an extra set of tyvars that would be in scope if -XScopedTypeVariables was on, but aren't in scope because it isn't. Is it worth the fuss? Perhaps. Anyway, it'd be worth explaining your proposed implementation path before investing effort in implementing it.

[bgamari]

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

[bgamari]

These won't be fixed for 8.6, bumping to 8.8.

[osa1]

Bumping milestones of low-priority tickets.