hlint icon indicating copy to clipboard operation
hlint copied to clipboard

Bad suggestion in the presence of MonadComprehensions

Open hseg opened this issue 4 years ago • 5 comments

[x | True] = return x in the presence of MonadComprehensions, but hlint incorrectly suggests replacing it with [x] instead. Using a trivial guard here for homogeneity in a long list of clauses like

fromMaybe [] . sequence $
   [ [f x y | True]
   , [g x r | r <- k]
   , [h x | True]
   , [j r s | r <- l, s <- m]
   ]

cf #775

hseg avatar Jun 27 '21 19:06 hseg

Looking at that code it seems like GHC knows whether it is a monad comp or not, so just a case of checking for that case. We should also check that monad comprehensions are not enabled by default. Patch welcome!

ndmitchell avatar Jul 02 '21 13:07 ndmitchell

Monad comprehensions are enabled by default.

f :: Maybe Int
f = [ x + y | x <- Just 1, y <- Just 2 ]

main = print f

A quick play with this program suggests parsing succeeds regardless of whether the extension is in effect or not. The extension is required to be enabled for the program to type however.

I expect therefore disabling the extension by default won't have an effect, good or bad.

shayne-fletcher avatar Jul 02 '21 16:07 shayne-fletcher

I think the GHC syntax tree is different if monad comprehensions are enabled, so we should probably disable monad comps, then if they are enabled and reflected in the syntax tree it is probably because a user enabled them explicitly, and then we should not apply the [x | True] modification.

ndmitchell avatar Jul 03 '21 18:07 ndmitchell

The parser list rule (for comprehensions)

        | texp '|' flattenedpquals

inspects the parser's MonadComprehensionBit in order to calculate correctly the "statement context" (list or monad) via

checkMonadComp = do
    monadComprehensions <- getBit MonadComprehensionsBit
    return $ if monadComprehensions
                then MonadComp
                else ListComp

So yes, you are right, the parse tree is different when the rule is enabled from when it's not.

I will offer a PR that stops MonadComprehensions from being enabled by default.

shayne-fletcher avatar Jul 03 '21 18:07 shayne-fletcher

Was going through my old open tickets, this is still broken on Hlint 3.8:

{-# LANGUAGE MonadComprehensions #-}

f x = [x | True]

g = fromMaybe [] . sequence $
   [ [f x y | True]
   , [g x r | r <- k]
   , [h x | True]
   , [j r s | r <- l, s <- m]
   ]

emits (regardless of MonadComprehensions):

|| Test.hs:1:7-16: Suggestion: Redundant True guards
|| Found:
||   [x | True]
|| Perhaps:
||   [x]
|| 
|| Test.hs:4:6-19: Suggestion: Redundant True guards
|| Found:
||   [f x y | True]
|| Perhaps:
||   [f x y]
|| 
|| Test.hs:6:6-17: Suggestion: Redundant True guards
|| Found:
||   [h x | True]
|| Perhaps:
||   [h x]
|| 
|| 3 hints

hseg avatar Aug 20 '24 13:08 hseg