Add -Wmissing-deriving-strategies
authorchessai <chessai1996@gmail.com>
Wed, 26 Dec 2018 17:12:37 +0000 (12:12 -0500)
committerBen Gamari <ben@smart-cactus.org>
Sun, 6 Jan 2019 12:27:09 +0000 (07:27 -0500)
Warn users when -XDerivingStrategies is enabled but not used, at each
potential use site.

add -Wmissing-deriving-strategies

Reviewers: bgamari, RyanGlScott

Subscribers: andrewthad, rwbarton, carter

GHC Trac Issues: #15798

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

13 files changed:
compiler/main/DynFlags.hs
compiler/rename/RnSource.hs
compiler/typecheck/TcDerivUtils.hs
docs/users_guide/8.8.1-notes.rst
docs/users_guide/extending_ghc.rst
docs/users_guide/using-warnings.rst
testsuite/tests/rename/should_compile/T15798a.hs [new file with mode: 0644]
testsuite/tests/rename/should_compile/T15798a.stderr [new file with mode: 0644]
testsuite/tests/rename/should_compile/T15798b.hs [new file with mode: 0644]
testsuite/tests/rename/should_compile/T15798b.stderr [new file with mode: 0644]
testsuite/tests/rename/should_compile/T15798c.hs [new file with mode: 0644]
testsuite/tests/rename/should_compile/T15798c.stderr [new file with mode: 0644]
testsuite/tests/rename/should_compile/all.T

index 02ad366..7296809 100644 (file)
@@ -834,6 +834,7 @@ data WarningFlag =
    | Opt_WarnStarBinder                   -- Since 8.6
    | Opt_WarnImplicitKindVars             -- Since 8.6
    | Opt_WarnSpaceAfterBang
+   | Opt_WarnMissingDerivingStrategies    -- Since 8.8
    deriving (Eq, Show, Enum)
 
 data Language = Haskell98 | Haskell2010
@@ -4022,6 +4023,7 @@ wWarningFlagsDeps = [
   flagSpec "wrong-do-bind"               Opt_WarnWrongDoBind,
   flagSpec "missing-pattern-synonym-signatures"
                                     Opt_WarnMissingPatternSynonymSignatures,
+  flagSpec "missing-deriving-strategies" Opt_WarnMissingDerivingStrategies,
   flagSpec "simplifiable-class-constraints" Opt_WarnSimplifiableClassConstraints,
   flagSpec "missing-home-modules"        Opt_WarnMissingHomeModules,
   flagSpec "unrecognised-warning-flags"  Opt_WarnUnrecognisedWarningFlags,
index 5ec4e05..ca35e94 100644 (file)
@@ -1004,6 +1004,7 @@ rnSrcDerivDecl (DerivDecl _ ty mds overlap)
            <- rnLDerivStrategy DerivDeclCtx mds $ \strat_tvs ppr_via_ty ->
               rnAndReportFloatingViaTvs strat_tvs loc ppr_via_ty "instance" $
               rnHsSigWcType BindUnlessForall DerivDeclCtx ty
+       ; warnNoDerivStrat mds' loc
        ; return (DerivDecl noExt ty' mds' overlap, fvs) }
   where
     loc = getLoc $ hsib_body $ hswc_body ty
@@ -1705,6 +1706,29 @@ rnDataDefn doc (HsDataDefn { dd_ND = new_or_data, dd_cType = cType
            ; return (cL loc ds', fvs) }
 rnDataDefn _ (XHsDataDefn _) = panic "rnDataDefn"
 
+warnNoDerivStrat :: Maybe (LDerivStrategy GhcRn)
+                 -> SrcSpan
+                 -> RnM ()
+warnNoDerivStrat mds loc
+  = do { dyn_flags <- getDynFlags
+       ; when (wopt Opt_WarnMissingDerivingStrategies dyn_flags) $
+           case mds of
+             Nothing -> addWarnAt
+               (Reason Opt_WarnMissingDerivingStrategies)
+               loc
+               (if xopt LangExt.DerivingStrategies dyn_flags
+                 then no_strat_warning
+                 else no_strat_warning $+$ deriv_strat_nenabled
+               )
+             _ -> pure ()
+       }
+  where
+    no_strat_warning :: SDoc
+    no_strat_warning = text "No deriving strategy specified. Did you want stock"
+                       <> text ", newtype, or anyclass?"
+    deriv_strat_nenabled :: SDoc
+    deriv_strat_nenabled = text "Use DerivingStrategies to specify a strategy."
+
 rnLHsDerivingClause :: HsDocContext -> LHsDerivingClause GhcPs
                     -> RnM (LHsDerivingClause GhcRn, FreeVars)
 rnLHsDerivingClause doc
@@ -1715,6 +1739,7 @@ rnLHsDerivingClause doc
   = do { (dcs', dct', fvs)
            <- rnLDerivStrategy doc dcs $ \strat_tvs ppr_via_ty ->
               mapFvRn (rn_deriv_ty strat_tvs ppr_via_ty) dct
+       ; warnNoDerivStrat dcs' loc
        ; pure ( cL loc (HsDerivingClause { deriv_clause_ext = noExt
                                          , deriv_clause_strategy = dcs'
                                          , deriv_clause_tys = cL loc' dct' })
index 86205de..5f48e5f 100644 (file)
@@ -62,7 +62,7 @@ import ListSetOps (assocMaybe)
 -- is a simple reader around 'TcRn'.
 type DerivM = ReaderT DerivEnv TcRn
 
--- | Is GHC processing a stanalone deriving declaration?
+-- | Is GHC processing a standalone deriving declaration?
 isStandaloneDeriv :: DerivM Bool
 isStandaloneDeriv = asks (go . denv_ctxt)
   where
index 69d5397..cd4c00d 100644 (file)
@@ -84,6 +84,10 @@ Compiler
 
 - The deprecated ghc-flag ``-Wamp`` has been removed.
 
+- Add new :ghc-flag:`-Wmissing-deriving-strategies` flag that warns users when they are not
+  taking advantage of :extension:`DerivingStrategies`. The warning is supplied at each
+  ``deriving`` site.
+
 Runtime system
 ~~~~~~~~~~~~~~
 
index a913684..02847c9 100644 (file)
@@ -851,6 +851,7 @@ In general, the ``pluginRecompile`` field has the following type::
 
 The ``PluginRecompile`` data type is an enumeration determining how the plugin
 should affect recompilation. ::
+
     data PluginRecompile = ForceRecompile | NoForceRecompile | MaybeRecompile Fingerprint
 
 A plugin which declares itself impure using ``ForceRecompile`` will always
index 6a6166b..03ca184 100644 (file)
@@ -904,6 +904,27 @@ of ``-W(no-)*``.
     This option isn't enabled by default because it can be very noisy,
     and it often doesn't indicate a bug in the program.
 
+.. ghc-flag:: -Wmissing-deriving-strategies
+    :shortdesc: warn when a deriving clause is missing a deriving strategy
+    :type: dynamic
+    :reverse: -Wno-missing-deriving-strategies
+    :category:
+
+    :since: 8.8.1
+
+    The datatype below derives the ``Eq`` typeclass, but doesn't specify a
+    strategy. When :ghc-flag:`-Wmissing-deriving-strategies` is enabled,
+    the compiler will emit a warning about this. ::
+
+        data Foo a = Foo a
+          deriving (Eq)
+
+    The compiler will warn here that the deriving clause doesn't specify a
+    strategy. If the warning is enabled, but :extension:`DerivingStrategies` is
+    not enabled, the compiler will suggest turning on the
+    :extension:`DerivingStrategies` extension. This option is not on by default,
+    having to be turned on manually or with :ghc-flag:`-Weverything`.
+
 .. ghc-flag:: -Wmissing-fields
     :shortdesc: warn when fields of a record are uninitialised
     :type: dynamic
diff --git a/testsuite/tests/rename/should_compile/T15798a.hs b/testsuite/tests/rename/should_compile/T15798a.hs
new file mode 100644 (file)
index 0000000..d34e55b
--- /dev/null
@@ -0,0 +1,12 @@
+{-# LANGUAGE DerivingStrategies #-}
+
+{-# OPTIONS_GHC -Wmissing-deriving-strategies #-}
+
+module T15798a () where
+
+data Foo a = Foo a
+  deriving stock (Eq)
+
+data Bar a = Bar a
+  deriving (Eq, Show)
+  deriving stock (Ord)
diff --git a/testsuite/tests/rename/should_compile/T15798a.stderr b/testsuite/tests/rename/should_compile/T15798a.stderr
new file mode 100644 (file)
index 0000000..6832205
--- /dev/null
@@ -0,0 +1,3 @@
+
+T15798a.hs:11:3: warning: [-Wmissing-deriving-strategies]
+    No deriving strategy specified. Did you want stock, newtype, or anyclass?
diff --git a/testsuite/tests/rename/should_compile/T15798b.hs b/testsuite/tests/rename/should_compile/T15798b.hs
new file mode 100644 (file)
index 0000000..8504d6f
--- /dev/null
@@ -0,0 +1,9 @@
+{-# LANGUAGE StandaloneDeriving #-}
+
+{-# OPTIONS_GHC -Wmissing-deriving-strategies #-}
+
+module T15798b () where
+
+data Foo a = Foo a
+
+deriving instance Eq a => Eq (Foo a)
diff --git a/testsuite/tests/rename/should_compile/T15798b.stderr b/testsuite/tests/rename/should_compile/T15798b.stderr
new file mode 100644 (file)
index 0000000..de1b09e
--- /dev/null
@@ -0,0 +1,4 @@
+
+T15798b.hs:9:19: warning: [-Wmissing-deriving-strategies]
+    No deriving strategy specified. Did you want stock, newtype, or anyclass?
+    Use DerivingStrategies to specify a strategy.
diff --git a/testsuite/tests/rename/should_compile/T15798c.hs b/testsuite/tests/rename/should_compile/T15798c.hs
new file mode 100644 (file)
index 0000000..f3048ed
--- /dev/null
@@ -0,0 +1,6 @@
+{-# OPTIONS_GHC -Wmissing-deriving-strategies #-}
+
+module T15798c () where
+
+data Foo a = Foo a
+  deriving (Eq)
diff --git a/testsuite/tests/rename/should_compile/T15798c.stderr b/testsuite/tests/rename/should_compile/T15798c.stderr
new file mode 100644 (file)
index 0000000..868266a
--- /dev/null
@@ -0,0 +1,4 @@
+
+T15798c.hs:6:3: warning: [-Wmissing-deriving-strategies]
+    No deriving strategy specified. Did you want stock, newtype, or anyclass?
+    Use DerivingStrategies to specify a strategy.
index b6c06c1..0bcd25c 100644 (file)
@@ -163,3 +163,7 @@ test('T14747', [], multimod_compile, ['T14747', '-v0'])
 test('T15149', [], multimod_compile, ['T15149', '-v0'])
 test('T13064', normal, compile, [''])
 test('T15994', [], run_command, ['$MAKE -s --no-print-directory T15994'])
+test('T15798a', normal, compile, [''])
+test('T15798b', normal, compile, [''])
+test('T15798c', normal, compile, [''])
+