Split up -Wunused-imports into three warningswip/unused-imports

This splits up `-Wunused-imports` into three warnings:

`-Wunused-explicit-imports`: Warn when an explicitly imported identifier
is not used.
  import Foo (bar)

`-Wredundant-imports`: Warn when nothing from an import is used.
  -- Nothing from Foo is used
  import Foo

`-Wunused-source-imports`: Warn when a source import is unecessary
  -- Can just `import Foo`.
  import {-# SOURCE #-} Foo

These three flags are implied by `-Wunused-imports`

The motiviation for this is that some library authors don't want to
always enable `-Wredundant-imports` as it makes downstream library
changes introduce warnings in libraries.

Fixes #21879

Also fixes #22182 by supporting -Werror=unused-imports and -Werror=unused-binds

5 files changed, 63 insertions, 21 deletions

diff --git a/compiler/GHC/Driver/Errors/Ppr.hs b/compiler/GHC/Driver/Errors/Ppr.hs
index 8f0ffa4a4d..e5aee036c3 100644
--- a/compiler/GHC/Driver/Errors/Ppr.hs
+++ b/compiler/GHC/Driver/Errors/Ppr.hs
@@ -222,7 +222,7 @@ instance Diagnostic DriverMessage where
     DriverUnusedPackages{}
       -> WarningWithFlag Opt_WarnUnusedPackages
     DriverUnnecessarySourceImports{}
-      -> WarningWithFlag Opt_WarnUnusedImports
+      -> WarningWithFlag Opt_WarnUnusedSourceImports
     DriverDuplicatedModuleDeclaration{}
       -> ErrorWithoutFlag
     DriverModuleNotFound{}
diff --git a/compiler/GHC/Driver/Flags.hs b/compiler/GHC/Driver/Flags.hs
index fd23d2e81e..279dacc681 100644
--- a/compiler/GHC/Driver/Flags.hs
+++ b/compiler/GHC/Driver/Flags.hs
@@ -18,6 +18,7 @@ module GHC.Driver.Flags
    , minusWeverythingOpts
    , minusWcompatOpts
    , unusedBindsFlags
+   , unusedImportsFlags
    )
 where
 
@@ -538,7 +539,9 @@ data WarningFlag =
    | Opt_WarnUnusedTopBinds
    | Opt_WarnUnusedLocalBinds
    | Opt_WarnUnusedPatternBinds
-   | Opt_WarnUnusedImports
+   | Opt_WarnUnusedSourceImports
+   | Opt_WarnUnusedExplicitImports
+   | Opt_WarnRedundantImports
    | Opt_WarnUnusedMatches
    | Opt_WarnUnusedTypePatterns
    | Opt_WarnUnusedForalls
@@ -685,7 +688,9 @@ warnFlagNames wflag = case wflag of
   Opt_WarnUntickedPromotedConstructors            -> "unticked-promoted-constructors" :| []
   Opt_WarnUnusedDoBind                            -> "unused-do-bind" :| []
   Opt_WarnUnusedForalls                           -> "unused-foralls" :| []
-  Opt_WarnUnusedImports                           -> "unused-imports" :| []
+  Opt_WarnUnusedExplicitImports                   -> "unused-explicit-imports" :| []
+  Opt_WarnUnusedSourceImports                     -> "unused-source-imports" :| []
+  Opt_WarnRedundantImports                        -> "redundant-imports" :| []
   Opt_WarnUnusedLocalBinds                        -> "unused-local-binds" :| []
   Opt_WarnUnusedMatches                           -> "unused-matches" :| []
   Opt_WarnUnusedPatternBinds                      -> "unused-pattern-binds" :| []
@@ -738,6 +743,7 @@ warningGroups :: [(String, [WarningFlag])]
 warningGroups =
     [ ("compat",       minusWcompatOpts)
     , ("unused-binds", unusedBindsFlags)
+    , ("unused-imports", unusedImportsFlags)
     , ("default",      standardWarnings)
     , ("extra",        minusWOpts)
     , ("all",          minusWallOpts)
@@ -825,7 +831,9 @@ minusWOpts
         Opt_WarnUnusedPatternBinds,
         Opt_WarnUnusedMatches,
         Opt_WarnUnusedForalls,
-        Opt_WarnUnusedImports,
+        Opt_WarnUnusedExplicitImports,
+        Opt_WarnUnusedSourceImports,
+        Opt_WarnRedundantImports,
         Opt_WarnIncompletePatterns,
         Opt_WarnDodgyExports,
         Opt_WarnDodgyImports,
@@ -875,3 +883,10 @@ unusedBindsFlags = [ Opt_WarnUnusedTopBinds
                    , Opt_WarnUnusedLocalBinds
                    , Opt_WarnUnusedPatternBinds
                    ]
+
+-- | Things you get with -Wunused-binds
+unusedImportsFlags :: [WarningFlag]
+unusedImportsFlags = [ Opt_WarnUnusedExplicitImports
+                     , Opt_WarnUnusedSourceImports
+                     , Opt_WarnRedundantImports
+                     ]
diff --git a/compiler/GHC/Driver/Make.hs b/compiler/GHC/Driver/Make.hs
index d1f9ba0104..9a259d9e96 100644
--- a/compiler/GHC/Driver/Make.hs
+++ b/compiler/GHC/Driver/Make.hs
@@ -1483,7 +1483,7 @@ modNodeMapUnionWith f (ModNodeMap m) (ModNodeMap n) = ModNodeMap (M.unionWith f
 warnUnnecessarySourceImports :: GhcMonad m => [SCC ModSummary] -> m ()
 warnUnnecessarySourceImports sccs = do
   diag_opts <- initDiagOpts <$> getDynFlags
-  when (diag_wopt Opt_WarnUnusedImports diag_opts) $ do
+  when (diag_wopt Opt_WarnUnusedSourceImports diag_opts) $ do
     let check ms =
            let mods_in_this_cycle = map ms_mod_name ms in
            [ warn i | m <- ms, i <- ms_home_srcimps m,
diff --git a/compiler/GHC/Driver/Session.hs b/compiler/GHC/Driver/Session.hs
index 9d03c434f6..864e93e35b 100644
--- a/compiler/GHC/Driver/Session.hs
+++ b/compiler/GHC/Driver/Session.hs
@@ -2916,6 +2916,10 @@ dynamic_flags_deps = [
   , make_ord_flag defHiddenFlag "fwarn-unused-binds" (NoArg enableUnusedBinds)
   , make_ord_flag defHiddenFlag "fno-warn-unused-binds" (NoArg
                                                             disableUnusedBinds)
+  , make_ord_flag defFlag "Wunused-imports" (NoArg enableUnusedImports)
+  , make_ord_flag defFlag "Wno-unused-imports" (NoArg disableUnusedImports)
+  , make_ord_flag defHiddenFlag "fwarn-unused-imports" (NoArg enableUnusedImports)
+  , make_ord_flag defHiddenFlag "fno-warn-unused-imports" (NoArg disableUnusedImports)
 
         ------ Safe Haskell flags -------------------------------------------
   , make_ord_flag defFlag "fpackage-trust"   (NoArg setPackageTrust)
@@ -2952,12 +2956,7 @@ dynamic_flags_deps = [
  ++ [ (NotDeprecated, unrecognisedWarning "W"),
       (Deprecated,    unrecognisedWarning "fwarn-"),
       (Deprecated,    unrecognisedWarning "fno-warn-") ]
- ++ [ make_ord_flag defFlag "Werror=compat"
-        (NoArg (mapM_ setWErrorFlag minusWcompatOpts))
-    , make_ord_flag defFlag "Wno-error=compat"
-        (NoArg (mapM_ unSetFatalWarningFlag minusWcompatOpts))
-    , make_ord_flag defFlag "Wwarn=compat"
-        (NoArg (mapM_ unSetFatalWarningFlag minusWcompatOpts)) ]
+ ++ concatMap werrorFlagGroup ["compat", "unused-binds", "unused-imports"]
  ++ map (mkFlag turnOn  "f"         setExtensionFlag  ) fLangFlagsDeps
  ++ map (mkFlag turnOff "fno-"      unSetExtensionFlag) fLangFlagsDeps
  ++ map (mkFlag turnOn  "X"         setExtensionFlag  ) xFlagsDeps
@@ -2965,6 +2964,16 @@ dynamic_flags_deps = [
  ++ map (mkFlag turnOn  "X"         setLanguage       ) languageFlagsDeps
  ++ map (mkFlag turnOn  "X"         setSafeHaskell    ) safeHaskellFlagsDeps
 
+werrorFlagGroup :: String -> [(Deprecation, Flag (CmdLineP DynFlags))]
+werrorFlagGroup name =
+    let Just opts = lookup name warningGroups
+    in [ make_ord_flag defFlag ("Werror=" ++ name)
+        (NoArg (mapM_ setWErrorFlag opts))
+       , make_ord_flag defFlag ("Wno-error=" ++ name)
+        (NoArg (mapM_ unSetFatalWarningFlag opts))
+       , make_ord_flag defFlag ("Wwarn=" ++ name)
+         (NoArg (mapM_ unSetFatalWarningFlag opts)) ]
+
 -- | This is where we handle unrecognised warning flags. We only issue a warning
 -- if -Wunrecognised-warning-flags is set. See #11429 for context.
 unrecognisedWarning :: String -> Flag (CmdLineP DynFlags)
@@ -3291,7 +3300,9 @@ wWarningFlagsDeps = mconcat [
   warnSpec    Opt_WarnUntickedPromotedConstructors,
   warnSpec    Opt_WarnUnusedDoBind,
   warnSpec    Opt_WarnUnusedForalls,
-  warnSpec    Opt_WarnUnusedImports,
+  warnSpec    Opt_WarnUnusedSourceImports,
+  warnSpec    Opt_WarnUnusedExplicitImports,
+  warnSpec    Opt_WarnRedundantImports,
   warnSpec    Opt_WarnUnusedLocalBinds,
   warnSpec    Opt_WarnUnusedMatches,
   warnSpec    Opt_WarnUnusedPatternBinds,
@@ -4034,6 +4045,11 @@ enableUnusedBinds = mapM_ setWarningFlag unusedBindsFlags
 disableUnusedBinds :: DynP ()
 disableUnusedBinds = mapM_ unSetWarningFlag unusedBindsFlags
 
+enableUnusedImports, disableUnusedImports :: DynP ()
+enableUnusedImports = mapM_ setWarningFlag unusedImportsFlags
+disableUnusedImports = mapM_ unSetWarningFlag unusedImportsFlags
+
+
 -- | Things you get with `-dlint`.
 enableDLint :: DynP ()
 enableDLint = do
diff --git a/compiler/GHC/Rename/Names.hs b/compiler/GHC/Rename/Names.hs
index dce4daa562..ace32e7314 100644
--- a/compiler/GHC/Rename/Names.hs
+++ b/compiler/GHC/Rename/Names.hs
@@ -1725,8 +1725,8 @@ warnUnusedImportDecls gbl_env hsc_src
                        (vcat [ text "Uses:" <+> ppr uses
                              , text "Import usage" <+> ppr usage])
 
-       ; whenWOptM Opt_WarnUnusedImports $
-         mapM_ (warnUnusedImport Opt_WarnUnusedImports fld_env) usage
+      ; unused_flags <- mkUnusedImportFlags
+      ; mapM_ (warnUnusedImport unused_flags fld_env) usage
 
        ; whenGOptM Opt_D_dump_minimal_imports $
          printMinimalImports hsc_src usage }
@@ -1828,9 +1828,15 @@ mkImportMap gres
           best_imp_spec = bestImport (bagToList imp_specs)
           add _ gres = gre : gres
 
-warnUnusedImport :: WarningFlag -> NameEnv (FieldLabelString, Parent)
+data UnusedImportFlags = UnusedImportFlags { warnRedundantImports :: Bool, warnUnusedExplicitImports :: Bool }
+
+mkUnusedImportFlags :: RnM UnusedImportFlags
+mkUnusedImportFlags = UnusedImportFlags <$> woptM Opt_WarnRedundantImports <*> woptM Opt_WarnUnusedExplicitImports
+
+warnUnusedImport :: UnusedImportFlags -> NameEnv (FieldLabelString, Parent)
                  -> ImportDeclUsage -> RnM ()
-warnUnusedImport flag fld_env (L loc decl, used, unused)
+warnUnusedImport (UnusedImportFlags False False) _ _ = return ()
+warnUnusedImport flags fld_env (L loc decl, used, unused)
 
   -- Do not warn for 'import M()'
   | Just (Exactly, L _ []) <- ideclImportList decl
@@ -1844,8 +1850,9 @@ warnUnusedImport flag fld_env (L loc decl, used, unused)
 
   -- Nothing used; drop entire declaration
   | null used
+  , warnRedundantImports flags
   = let dia = mkTcRnUnknownMessage $
-          mkPlainDiagnostic (WarningWithFlag flag) noHints msg1
+          mkPlainDiagnostic (WarningWithFlag Opt_WarnRedundantImports) noHints msg1
     in addDiagnosticAt (locA loc) dia
 
   -- Everything imported is used; nop
@@ -1854,17 +1861,21 @@ warnUnusedImport flag fld_env (L loc decl, used, unused)
 
   -- Only one import is unused, with `SrcSpan` covering only the unused item instead of
   -- the whole import statement
-  | Just (_, L _ imports) <- ideclImportList decl
+  | warnUnusedExplicitImports flags
+  , Just (_, L _ imports) <- ideclImportList decl
   , length unused == 1
   , Just (L loc _) <- find (\(L _ ie) -> ((ieName ie) :: Name) `elem` unused) imports
-  = let dia = mkTcRnUnknownMessage $ mkPlainDiagnostic (WarningWithFlag flag) noHints msg2
+  = let dia = mkTcRnUnknownMessage $ mkPlainDiagnostic (WarningWithFlag Opt_WarnUnusedExplicitImports) noHints msg2
     in addDiagnosticAt (locA loc) dia
 
   -- Some imports are unused
-  | otherwise
-  = let dia = mkTcRnUnknownMessage $ mkPlainDiagnostic (WarningWithFlag flag) noHints msg2
+  | warnUnusedExplicitImports flags
+  = let dia = mkTcRnUnknownMessage $ mkPlainDiagnostic (WarningWithFlag Opt_WarnUnusedExplicitImports) noHints msg2
     in addDiagnosticAt (locA loc) dia
 
+  | otherwise
+  = return ()
+
   where
     msg1 = vcat [ pp_herald <+> quotes pp_mod <+> is_redundant
                 , nest 2 (text "except perhaps to import instances from"