More import related hints

Now for unqualified imports. Improves upon #11071.

Unfortunately, it seems that since 7.10, ghc will not print all
out-of-scope errors.

Test Plan: test suite updated

Reviewers: austin, thomie, bgamari

Reviewed By: bgamari

Differential Revision: https://phabricator.haskell.org/D1478

GHC Trac Issues: #11071

compiler/main/HscTypes.hs

@@ -1032,11 +1032,12 @@ emptyModDetails
 type ImportedMods = ModuleEnv [ImportedModsVal]
 data ImportedModsVal
  = ImportedModsVal {
-        imv_name :: ModuleName,         -- ^ The name the module is imported with
-        imv_span :: SrcSpan,            -- ^ the source span of the whole import
-        imv_is_safe :: IsSafeImport,    -- ^ whether this is a safe import
-        imv_is_hiding :: Bool,          -- ^ whether this is an "hiding" import
-        imv_all_exports :: GlobalRdrEnv -- ^ all the things the module could provide
+        imv_name :: ModuleName,          -- ^ The name the module is imported with
+        imv_span :: SrcSpan,             -- ^ the source span of the whole import
+        imv_is_safe :: IsSafeImport,     -- ^ whether this is a safe import
+        imv_is_hiding :: Bool,           -- ^ whether this is an "hiding" import
+        imv_all_exports :: GlobalRdrEnv, -- ^ all the things the module could provide
+        imv_qualified :: Bool            -- ^ whether this is a qualified import
         }
 
 -- | A ModGuts is carried through the compiler, accumulating stuff as it goes

compiler/rename/RnEnv.hs

@@ -1637,10 +1637,9 @@ unboundNameX where_look rdr_name extra
           else do { local_env  <- getLocalRdrEnv
                   ; global_env <- getGlobalRdrEnv
                   ; impInfo <- getImports
-                  ; let suggestions1 = unknownNameSuggestions_ where_look
-                                         dflags global_env local_env rdr_name
-                  ; let suggestions2 = importSuggestions dflags impInfo rdr_name
-                  ; addErr (err $$ suggestions1 $$ suggestions2) }
+                  ; let suggestions = unknownNameSuggestions_ where_look
+                                        dflags global_env local_env impInfo rdr_name
+                  ; addErr (err $$ suggestions) }
         ; return (mkUnboundName rdr_name) }
 
 unknownNameErr :: SDoc -> RdrName -> SDoc
@@ -1656,113 +1655,25 @@ type HowInScope = Either SrcSpan ImpDeclSpec
      -- Left loc    =>  locally bound at loc
      -- Right ispec =>  imported as specified by ispec
 
--- | Generate helpful suggestions if a qualified name Mod.foo is not in scope.
-importSuggestions :: DynFlags -> ImportAvails -> RdrName -> SDoc
-importSuggestions _dflags imports rdr_name
-  | not (isQual rdr_name) = Outputable.empty
-  | null interesting_imports
-  = hsep
-      [ ptext (sLit "No module named")
-      , quotes (ppr mod_name)
-      , ptext (sLit "is imported.")
-      ]
-  | null helpful_imports
-  , [(mod,_)] <- interesting_imports
-  = hsep
-      [ ptext (sLit "Module")
-      , quotes (ppr mod)
-      , ptext (sLit "does not export")
-      , quotes (ppr occ_name) <> dot
-      ]
-  | null helpful_imports
-  , mods <- map fst interesting_imports
-  = hsep
-      [ ptext (sLit "Neither")
-      , quotedListWithNor (map ppr mods)
-      , ptext (sLit "exports")
-      , quotes (ppr occ_name) <> dot
-      ]
-  | [(mod,imv)] <- helpful_imports_non_hiding
-  = fsep
-      [ ptext (sLit "Perhaps you want to add")
-      , quotes (ppr occ_name)
-      , ptext (sLit "to the import list")
-      , ptext (sLit "in the import of")
-      , quotes (ppr mod)
-      , parens (ppr (imv_span imv)) <> dot
-      ]
-  | not (null helpful_imports_non_hiding)
-  = fsep
-      [ ptext (sLit "Perhaps you want to add")
-      , quotes (ppr occ_name)
-      , ptext (sLit "to one of these import lists:")
-      ]
-    $$
-    nest 2 (vcat
-        [ quotes (ppr mod) <+> parens (ppr (imv_span imv))
-        | (mod,imv) <- helpful_imports_non_hiding
-        ])
-  | [(mod,imv)] <- helpful_imports_hiding
-  = fsep
-      [ ptext (sLit "Perhaps you want to remove")
-      , quotes (ppr occ_name)
-      , ptext (sLit "from the explicit hiding list")
-      , ptext (sLit "in the import of")
-      , quotes (ppr mod)
-      , parens (ppr (imv_span imv)) <> dot
-      ]
-  | not (null helpful_imports_hiding)
-  = fsep
-      [ ptext (sLit "Perhaps you want to remove")
-      , quotes (ppr occ_name)
-      , ptext (sLit "from the hiding clauses")
-      , ptext (sLit "in one of these imports:")
-      ]
-    $$
-    nest 2 (vcat
-        [ quotes (ppr mod) <+> parens (ppr (imv_span imv))
-        | (mod,imv) <- helpful_imports_hiding
-        ])
-  | otherwise
-  = Outputable.empty
- where
-  Just (mod_name, occ_name) = isQual_maybe rdr_name
-
-  -- What import statements provide "Mod" at all
-  interesting_imports = [ (mod, imp)
-    | (mod, mod_imports) <- moduleEnvToList (imp_mods imports)
-    , Just imp <- return $ pick mod_imports
-    ]
-
-  -- We want to keep only one for each original module; preferably one with an
-  -- explicit import list (for no particularly good reason)
-  pick :: [ImportedModsVal] -> Maybe ImportedModsVal
-  pick = listToMaybe . sortBy (compare `on` prefer) . filter select
-    where select imv = imv_name imv == mod_name
-          prefer imv = (imv_is_hiding imv, imv_span imv)
-
-  -- Which of these would export a 'foo'
-  -- (all of these are restricted imports, because if they were not, we
-  -- wouldn't have an out-of-scope error in the first place)
-  helpful_imports = filter helpful interesting_imports
-    where helpful (_,imv)
-            = not . null $ lookupGlobalRdrEnv (imv_all_exports imv) occ_name
-
-  -- Which of these do that because of an explicit hiding list resp. an
-  -- explicit import list
-  (helpful_imports_hiding, helpful_imports_non_hiding)
-    = partition (imv_is_hiding . snd) helpful_imports
 
 -- | Called from the typechecker (TcErrors) when we find an unbound variable
 unknownNameSuggestions :: DynFlags
-                       -> GlobalRdrEnv -> LocalRdrEnv
+                       -> GlobalRdrEnv -> LocalRdrEnv -> ImportAvails
                        -> RdrName -> SDoc
 unknownNameSuggestions = unknownNameSuggestions_ WL_Any
 
 unknownNameSuggestions_ :: WhereLooking -> DynFlags
+                       -> GlobalRdrEnv -> LocalRdrEnv -> ImportAvails
+                       -> RdrName -> SDoc
+unknownNameSuggestions_ where_look dflags global_env local_env imports tried_rdr_name =
+    similarNameSuggestions where_look dflags global_env local_env tried_rdr_name $$
+    importSuggestions dflags imports tried_rdr_name
+
+
+similarNameSuggestions :: WhereLooking -> DynFlags
                         -> GlobalRdrEnv -> LocalRdrEnv
                         -> RdrName -> SDoc
-unknownNameSuggestions_ where_look dflags global_env
+similarNameSuggestions where_look dflags global_env
                         local_env tried_rdr_name
   = case suggest of
       []  -> Outputable.empty
@@ -1876,6 +1787,113 @@ unknownNameSuggestions_ where_look dflags global_env
       = [ (mkRdrQual (is_as ispec) (nameOccName n), Right ispec)
         | i <- is, let ispec = is_decl i, is_qual ispec ]
 
+-- | Generate helpful suggestions if a qualified name Mod.foo is not in scope.
+importSuggestions :: DynFlags -> ImportAvails -> RdrName -> SDoc
+importSuggestions _dflags imports rdr_name
+  | not (isQual rdr_name || isUnqual rdr_name) = Outputable.empty
+  | null interesting_imports
+  , Just name <- mod_name
+  = hsep
+      [ ptext (sLit "No module named")
+      , quotes (ppr name)
+      , ptext (sLit "is imported.")
+      ]
+  | is_qualified
+  , null helpful_imports
+  , [(mod,_)] <- interesting_imports
+  = hsep
+      [ ptext (sLit "Module")
+      , quotes (ppr mod)
+      , ptext (sLit "does not export")
+      , quotes (ppr occ_name) <> dot
+      ]
+  | is_qualified
+  , null helpful_imports
+  , mods <- map fst interesting_imports
+  = hsep
+      [ ptext (sLit "Neither")
+      , quotedListWithNor (map ppr mods)
+      , ptext (sLit "exports")
+      , quotes (ppr occ_name) <> dot
+      ]
+  | [(mod,imv)] <- helpful_imports_non_hiding
+  = fsep
+      [ ptext (sLit "Perhaps you want to add")
+      , quotes (ppr occ_name)
+      , ptext (sLit "to the import list")
+      , ptext (sLit "in the import of")
+      , quotes (ppr mod)
+      , parens (ppr (imv_span imv)) <> dot
+      ]
+  | not (null helpful_imports_non_hiding)
+  = fsep
+      [ ptext (sLit "Perhaps you want to add")
+      , quotes (ppr occ_name)
+      , ptext (sLit "to one of these import lists:")
+      ]
+    $$
+    nest 2 (vcat
+        [ quotes (ppr mod) <+> parens (ppr (imv_span imv))
+        | (mod,imv) <- helpful_imports_non_hiding
+        ])
+  | [(mod,imv)] <- helpful_imports_hiding
+  = fsep
+      [ ptext (sLit "Perhaps you want to remove")
+      , quotes (ppr occ_name)
+      , ptext (sLit "from the explicit hiding list")
+      , ptext (sLit "in the import of")
+      , quotes (ppr mod)
+      , parens (ppr (imv_span imv)) <> dot
+      ]
+  | not (null helpful_imports_hiding)
+  = fsep
+      [ ptext (sLit "Perhaps you want to remove")
+      , quotes (ppr occ_name)
+      , ptext (sLit "from the hiding clauses")
+      , ptext (sLit "in one of these imports:")
+      ]
+    $$
+    nest 2 (vcat
+        [ quotes (ppr mod) <+> parens (ppr (imv_span imv))
+        | (mod,imv) <- helpful_imports_hiding
+        ])
+  | otherwise
+  = Outputable.empty
+ where
+  is_qualified = isQual rdr_name
+  (mod_name, occ_name) = case rdr_name of
+    Unqual occ_name        -> (Nothing, occ_name)
+    Qual mod_name occ_name -> (Just mod_name, occ_name)
+    _                      -> error "importSuggestions: dead code"
+
+
+  -- What import statements provide "Mod" at all
+  -- or, if this is an unqualified name, are not qualified imports
+  interesting_imports = [ (mod, imp)
+    | (mod, mod_imports) <- moduleEnvToList (imp_mods imports)
+    , Just imp <- return $ pick mod_imports
+    ]
+
+  -- We want to keep only one for each original module; preferably one with an
+  -- explicit import list (for no particularly good reason)
+  pick :: [ImportedModsVal] -> Maybe ImportedModsVal
+  pick = listToMaybe . sortBy (compare `on` prefer) . filter select
+    where select imv = case mod_name of Just name -> imv_name imv == name
+                                        Nothing   -> not (imv_qualified imv)
+          prefer imv = (imv_is_hiding imv, imv_span imv)
+
+  -- Which of these would export a 'foo'
+  -- (all of these are restricted imports, because if they were not, we
+  -- wouldn't have an out-of-scope error in the first place)
+  helpful_imports = filter helpful interesting_imports
+    where helpful (_,imv)
+            = not . null $ lookupGlobalRdrEnv (imv_all_exports imv) occ_name
+
+  -- Which of these do that because of an explicit hiding list resp. an
+  -- explicit import list
+  (helpful_imports_hiding, helpful_imports_non_hiding)
+    = partition (imv_is_hiding . snd) helpful_imports
+
 {-
 ************************************************************************
 *                                                                      *

compiler/rename/RnNames.hs

@@ -284,6 +284,7 @@ rnImportDecl this_mod
             , imv_is_safe     = mod_safe'
             , imv_is_hiding   = is_hiding
             , imv_all_exports = potential_gres
+            , imv_qualified   = qual_only
             }
     let imports
           = (calculateAvails dflags iface mod_safe' want_boot)

compiler/typecheck/TcErrors.hs

@@ -724,9 +724,10 @@ mkHoleError ctxt ct@(CHoleCan { cc_occ = occ, cc_hole = hole_sort })
                        -- Suggest possible in-scope variables in the message
   = do { dflags  <- getDynFlags
        ; rdr_env <- getGlobalRdrEnv
+       ; impInfo <- getImports
        ; mkLongErrAt (RealSrcSpan (tcl_loc lcl_env)) out_of_scope_msg
                      (unknownNameSuggestions dflags rdr_env
-                               (tcl_rdr lcl_env) (mkRdrUnqual occ)) }
+                               (tcl_rdr lcl_env) impInfo (mkRdrUnqual occ)) }
 
   | otherwise  -- Explicit holes, like "_" or "_f"
   = do { (ctxt, binds_doc, ct) <- relevantBindings False ctxt ct

testsuite/tests/annotations/should_fail/annfail11.stderr

 
-annfail11.hs:3:1:
+annfail11.hs:3:1: error:
     Not in scope: ‘length’
+    Perhaps you want to add ‘length’ to the import list
+    in the import of ‘Prelude’ (annfail11.hs:1:8-16).
     In the annotation:
       {-# ANN length "Cannot annotate other modules yet" #-}
 
-annfail11.hs:4:1:
+annfail11.hs:4:1: error:
     Not in scope: type constructor or class ‘Integer’
+    Perhaps you want to add ‘Integer’ to the import list
+    in the import of ‘Prelude’ (annfail11.hs:1:8-16).
     In the annotation:
       {-# ANN type Integer "Cannot annotate other modules yet" #-}

testsuite/tests/module/mod114.stderr

 
-mod114.hs:3:16: Not in scope: type constructor or class ‘Stuff’
+mod114.hs:3:16: error:
+    Not in scope: type constructor or class ‘Stuff’
+    Perhaps you want to remove ‘Stuff’ from the explicit hiding list
+    in the import of ‘Mod114_Help’ (mod114.hs:4:1-36).

testsuite/tests/module/mod124.stderr

 
-mod124.hs:6:6: Not in scope: type constructor or class ‘T’
+mod124.hs:6:6: error:
+    Not in scope: type constructor or class ‘T’
+    Perhaps you want to remove ‘T’ from the explicit hiding list
+    in the import of ‘Mod124_A’ (mod124.hs:4:1-26).

testsuite/tests/module/mod125.stderr

-
-mod125.hs:7:5: error: Data constructor not in scope: T
+
+mod125.hs:7:5: error:
+    Data constructor not in scope: T
+    Perhaps you want to remove ‘T’ from the explicit hiding list
+    in the import of ‘Mod125_A’ (mod125.hs:4:1-26).

testsuite/tests/module/mod126.stderr

-
-mod126.hs:7:5: error: Data constructor not in scope: T
+
+mod126.hs:7:5: error:
+    Data constructor not in scope: T
+    Perhaps you want to remove ‘T’ from the explicit hiding list
+    in the import of ‘Mod126_A’ (mod126.hs:4:1-26).

testsuite/tests/module/mod127.stderr

 
-mod127.hs:6:6: Not in scope: type constructor or class ‘T’
+mod127.hs:6:6: error:
+    Not in scope: type constructor or class ‘T’
+    Perhaps you want to remove ‘T’ from the explicit hiding list
+    in the import of ‘Mod127_A’ (mod127.hs:4:1-26).

testsuite/tests/module/mod130.stderr

-
-mod130.hs:7:5: error:
-    Variable not in scope: (<) :: Integer -> Int -> Int
+
+mod130.hs:7:5: error:
+    Variable not in scope: (<) :: Integer -> Int -> Int
+    Perhaps you want to remove ‘<’ from the explicit hiding list
+    in the import of ‘Prelude’ (mod130.hs:4:1-33).

testsuite/tests/module/mod29.stderr

 
-mod29.hs:6:12: Not in scope: type constructor or class ‘Char’
+mod29.hs:6:12: error:
+    Not in scope: type constructor or class ‘Char’
+    Perhaps you want to add ‘Char’ to the import list in the import of
+    ‘Prelude’ (mod29.hs:5:1-19).

testsuite/tests/module/mod36.stderr

-
-mod36.hs:5:5: error: Variable not in scope: const
+
+mod36.hs:5:5: error:
+    Variable not in scope: const
+    Perhaps you want to remove ‘const’ from the explicit hiding list
+    in the import of ‘Prelude’ (mod36.hs:3:1-32).

testsuite/tests/module/mod87.stderr

-
-mod87.hs:4:5: error:
-    Data constructor not in scope: Left :: Char -> t
+
+mod87.hs:4:5: error:
+    Data constructor not in scope: Left :: Char -> t
+    Perhaps you want to add ‘Left’ to the import list in the import of
+    ‘Prelude’ (mod87.hs:3:1-22).

testsuite/tests/module/mod97.stderr

-
-mod97.hs:4:9: error:
-    Variable not in scope: (==) :: Char -> Char -> t
+
+mod97.hs:4:9: error:
+    Variable not in scope: (==) :: Char -> Char -> t
+    Perhaps you want to add ‘==’ to the import list in the import of
+    ‘Prelude’ (mod97.hs:3:1-18).

testsuite/tests/rename/should_fail/T11071a.hs

+module T11071 where
+
+import Data.List (lines)
+import Data.IntMap ()
+import Data.Ord hiding (Down)
+import Prelude hiding (True)
+
+ignore :: a -> IO ()
+ignore = const (return ())
+
+main = do
+    ignore intersperse       -- missing in import list (one import)
+    ignore foldl'            -- missing in import list (two imports)
+    ignore Down              -- explicitly hidden
+    ignore True              -- explicitly hidden from prelude (not really special)
+    ignore foobar            -- genuinely out of scope

testsuite/tests/rename/should_fail/T11071a.stderr

+
+T11071a.hs:12:12: error:
+    Variable not in scope: intersperse
+    Perhaps you want to add ‘intersperse’ to the import list
+    in the import of ‘Data.List’ (T11071a.hs:3:1-24).
+
+T11071a.hs:13:12: error:
+    Variable not in scope: foldl'
+    Perhaps you meant one of these:
+      ‘foldl’ (imported from Prelude), ‘foldl1’ (imported from Prelude),
+      ‘foldr’ (imported from Prelude)
+    Perhaps you want to add ‘foldl'’ to one of these import lists:
+      ‘Data.IntMap’ (T11071a.hs:4:1-21)
+      ‘Data.List’ (T11071a.hs:3:1-24)
+
+T11071a.hs:14:12: error:
+    Data constructor not in scope: Down
+    Perhaps you want to remove ‘Down’ from the explicit hiding list
+    in the import of ‘Data.Ord’ (T11071a.hs:5:1-29).
+
+T11071a.hs:15:12: error:
+    Data constructor not in scope: True
+    Perhaps you want to remove ‘True’ from the explicit hiding list
+    in the import of ‘Prelude’ (T11071a.hs:6:1-28).
+
+T11071a.hs:16:12: error: Variable not in scope: foobar

testsuite/tests/rename/should_fail/all.T

@@ -139,3 +139,4 @@ test('T10668', normal, compile_fail, [''])
 test('T5001b', normal, compile_fail, [''])
 test('T10781', normal, compile_fail, [''])
 test('T11071', normal, compile_fail, [''])
+test('T11071a', normal, compile_fail, [''])