Add warn-missing-export-lists
authorEvan Rutledge Borden <eborden@frontrowed.com>
Tue, 21 Nov 2017 23:06:01 +0000 (18:06 -0500)
committerBen Gamari <ben@smart-cactus.org>
Wed, 22 Nov 2017 02:11:16 +0000 (21:11 -0500)
Many industrial users have aligned around the idea that implicit exports
are an anti-pattern. They lead to namespace pollution and byzantine
naming schemes. They also prevent GHC's dead code analysis and create
more obstacles to optimization. This warning allows teams/projects to
warn on or enforce via -Werror explicit export lists.

This warning also serves as a complement to warn-missing-import-lists.

This was originally discussed here:
https://github.com/ghc-proposals/ghc-proposals/pull/93

Test Plan: Three new minimal tests have been added to the type checker.

Reviewers: bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie

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

compiler/main/DynFlags.hs
compiler/typecheck/TcRnExports.hs
docs/users_guide/8.4.1-notes.rst
docs/users_guide/using-warnings.rst
testsuite/tests/typecheck/should_compile/MissingExportList01.hs [new file with mode: 0644]
testsuite/tests/typecheck/should_compile/MissingExportList02.hs [new file with mode: 0644]
testsuite/tests/typecheck/should_compile/all.T
testsuite/tests/typecheck/should_fail/MissingExportList03.hs [new file with mode: 0644]
testsuite/tests/typecheck/should_fail/MissingExportList03.stderr [new file with mode: 0644]
testsuite/tests/typecheck/should_fail/all.T

index cbf3ab7..ffc8614 100644 (file)
@@ -683,6 +683,7 @@ data WarningFlag =
    | Opt_WarnUnbangedStrictPatterns       -- Since 8.2
    | Opt_WarnMissingHomeModules           -- Since 8.2
    | Opt_WarnPartialFields                -- Since 8.4
+   | Opt_WarnMissingExportList
    deriving (Eq, Show, Enum)
 
 data Language = Haskell98 | Haskell2010
@@ -3639,6 +3640,7 @@ wWarningFlagsDeps = [
   flagSpec "identities"                  Opt_WarnIdentities,
   flagSpec "missing-fields"              Opt_WarnMissingFields,
   flagSpec "missing-import-lists"        Opt_WarnMissingImportList,
+  flagSpec "missing-export-lists"        Opt_WarnMissingExportList,
   depFlagSpec "missing-local-sigs"       Opt_WarnMissingLocalSignatures
     "it is replaced by -Wmissing-local-signatures",
   flagSpec "missing-local-signatures"    Opt_WarnMissingLocalSignatures,
index a79e30d..e2b6a61 100644 (file)
@@ -181,10 +181,15 @@ exports_from_avail Nothing rdr_env _imports _this_mod
    -- The same as (module M) where M is the current module name,
    -- so that's how we handle it, except we also export the data family
    -- when a data instance is exported.
-  = let avails =
-          map fix_faminst . gresToAvailInfo
-            . filter isLocalGRE . globalRdrEnvElts $ rdr_env
-    in return (Nothing, avails)
+  = do {
+    ; warnMissingExportList <- woptM Opt_WarnMissingExportList
+    ; warnIfFlag Opt_WarnMissingExportList
+        warnMissingExportList
+        (missingModuleExportWarn $ moduleName _this_mod)
+    ; let avails =
+            map fix_faminst . gresToAvailInfo
+              . filter isLocalGRE . globalRdrEnvElts $ rdr_env
+    ; return (Nothing, avails) }
   where
     -- #11164: when we define a data instance
     -- but not data family, re-export the family
@@ -659,6 +664,11 @@ nullModuleExport :: ModuleName -> SDoc
 nullModuleExport mod
   = text "The export item `module" <+> ppr mod <> ptext (sLit "' exports nothing")
 
+missingModuleExportWarn :: ModuleName -> SDoc
+missingModuleExportWarn mod
+  = text "The export item `module" <+> ppr mod <>
+    ptext (sLit "' is missing an export list")
+
 
 dodgyExportWarn :: Name -> SDoc
 dodgyExportWarn item
index 103559c..1acab73 100644 (file)
@@ -108,6 +108,9 @@ Language
 Compiler
 ~~~~~~~~
 
+- Add warning flag :ghc-flag:`-Wmissing-export-lists` which causes the type
+  checker to warn when a module does not include an explicit export list.
+
 - The ``configure`` script now no longer accepts ``--with-TOOL`` flags (e.g.
   ``--with-nm``, ``--with-ld``, etc.). Instead, these are taken from environment
   variables, as is typical in ``autoconf`` scripts. For instance,
index c6a86bb..cf41c28 100644 (file)
@@ -74,6 +74,7 @@ The following flags are simple ways to select standard "packages" of warnings:
         * :ghc-flag:`-Wimplicit-prelude`
         * :ghc-flag:`-Wmissing-local-signatures`
         * :ghc-flag:`-Wmissing-exported-signatures`
+        * :ghc-flag:`-Wmissing-export-lists`
         * :ghc-flag:`-Wmissing-import-lists`
         * :ghc-flag:`-Wmissing-home-modules`
         * :ghc-flag:`-Widentities`
@@ -858,6 +859,31 @@ of ``-W(no-)*``.
     fields are initialised with bottoms), it is often an indication of a
     programmer error.
 
+.. ghc-flag:: -Wmissing-export-lists
+    :shortdesc: warn when a module declaration does not explicitly list all
+        exports
+    :type: dynamic
+    :reverse: -fnowarn-missing-export-lists
+    :category:
+
+    :since: 8.4.1
+
+    .. index::
+       single: missing export lists, warning
+       single: export lists, missing
+
+    This flag warns if you declare a module without declaring an explicit
+    export list. For example ::
+
+        module M where
+
+          p x = x
+
+    The :ghc-flag:`-Wmissing-export-lists` flag will warn that ``M`` does not
+    declare an export list. Declaring an explicit export list for ``M`` enables
+    GHC dead code analysis, prevents accidental export of names and can ease
+    optimizations like inlining.
+
 .. ghc-flag:: -Wmissing-import-lists
     :shortdesc: warn when an import declaration does not explicitly list all the
         names brought into scope
diff --git a/testsuite/tests/typecheck/should_compile/MissingExportList01.hs b/testsuite/tests/typecheck/should_compile/MissingExportList01.hs
new file mode 100644 (file)
index 0000000..62b9509
--- /dev/null
@@ -0,0 +1,5 @@
+{-# OPTIONS_GHC -Werror -fwarn-missing-export-lists #-}
+module ShouldCompile (foo) where
+
+foo :: String
+foo = "foo"
diff --git a/testsuite/tests/typecheck/should_compile/MissingExportList02.hs b/testsuite/tests/typecheck/should_compile/MissingExportList02.hs
new file mode 100644 (file)
index 0000000..15c31b9
--- /dev/null
@@ -0,0 +1,5 @@
+{-# OPTIONS_GHC -Werror -fwarn-missing-export-lists #-}
+module ShouldCompile () where
+
+foo :: String
+foo = "foo"
index 03e7091..417e43e 100644 (file)
@@ -582,3 +582,5 @@ test('T14363', normal, compile, [''])
 test('T14363a', normal, compile, [''])
 test('T7169', normal, compile, [''])
 test('T14434', [], run_command, ['$MAKE -s --no-print-directory T14434'])
+test('MissingExportList01', normal, compile, [''])
+test('MissingExportList02', normal, compile, [''])
diff --git a/testsuite/tests/typecheck/should_fail/MissingExportList03.hs b/testsuite/tests/typecheck/should_fail/MissingExportList03.hs
new file mode 100644 (file)
index 0000000..4a5ecf0
--- /dev/null
@@ -0,0 +1,5 @@
+{-# OPTIONS_GHC -Werror -fwarn-missing-export-lists #-}
+module ShouldFail where
+
+foo :: String
+foo = "foo"
diff --git a/testsuite/tests/typecheck/should_fail/MissingExportList03.stderr b/testsuite/tests/typecheck/should_fail/MissingExportList03.stderr
new file mode 100644 (file)
index 0000000..f4258de
--- /dev/null
@@ -0,0 +1,2 @@
+MissingExportList03.hs:1:1: [-Wmissing-export-lists, -Werror=missing-export-lists]
+    The export item `module ShouldFail' is missing an export list
index ca0264b..553e10a 100644 (file)
@@ -461,3 +461,4 @@ test('T14232', normal, compile_fail, [''])
 test('T14325', normal, compile_fail, [''])
 test('T14350', normal, compile_fail, [''])
 test('T14390', normal, compile_fail, [''])
+test('MissingExportList03', normal, compile_fail, [''])