Skip to content

Fix remove constraint #1578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 2, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 62 additions & 39 deletions ghcide/src/Development/IDE/Plugin/CodeAction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ module Development.IDE.Plugin.CodeAction
, matchRegExMultipleImports
) where

import Bag (isEmptyBag)
import Bag (bagToList,
isEmptyBag)
import Control.Applicative ((<|>))
import Control.Arrow (second,
(>>>))
Expand Down Expand Up @@ -72,7 +73,6 @@ import Outputable (Outputable,
showSDocUnsafe)
import RdrName (GlobalRdrElt (..),
lookupGlobalRdrEnv)
import Safe (atMay)
import SrcLoc (realSrcSpanEnd,
realSrcSpanStart)
import TcRnTypes (ImportAvails (..),
Expand Down Expand Up @@ -162,6 +162,56 @@ findSigOfDecl pred decls =
any (pred . unLoc) idsSig
]

findSigOfDeclRanged :: Range -> [LHsDecl p] -> Either String (Sig p)
findSigOfDeclRanged range decls = do
dec <- findDeclContainingLocE (_start range) decls
case dec of
L _ (SigD _ sig@TypeSig {}) -> Right sig
L _ (ValD _ (bind :: HsBind p)) -> findSigOfBind range bind
_ -> Left "findSigOfDeclRanged"

findSigOfBind :: Range -> HsBind p -> Either String (Sig p)
findSigOfBind range bind =
case bind of
FunBind {} -> findSigOfLMatch (unLoc $ mg_alts (fun_matches bind))
_ -> Left "findSigOfBind"
where
findSigOfLMatch :: [LMatch p (LHsExpr p)] -> Either String (Sig p)
findSigOfLMatch ls = do
match <- findDeclContainingLocE (_start range) ls
findSigOfGRHSs (m_grhss (unLoc match))

findSigOfGRHSs :: GRHSs p (LHsExpr p) -> Either String (Sig p)
findSigOfGRHSs grhs = do
if _start range `isInsideSrcSpan` (getLoc $ grhssLocalBinds grhs)
then findSigOfBinds range (unLoc (grhssLocalBinds grhs)) -- where clause
else do
grhs <- findDeclContainingLocE (_start range) (grhssGRHSs grhs)
case unLoc grhs of
GRHS _ _ bd -> findSigOfExpr (unLoc bd)
_ -> Left "findSigOfGRHSs"

findSigOfExpr :: HsExpr p -> Either String (Sig p)
findSigOfExpr = go
where
go (HsLet _ binds _) = findSigOfBinds range (unLoc binds)
go _ = Left "findSigOfExpr"

findSigOfBinds :: Range -> HsLocalBinds p -> Either String (Sig p)
findSigOfBinds range = go
where
go (HsValBinds _ (ValBinds _ binds lsigs)) =
case unLoc <$> findDeclContainingLocE (_start range) lsigs of
Right sig' -> Right sig'
Left _ -> do
lHsBindLR <- findDeclContainingLocE (_start range) (bagToList binds)
findSigOfBind range (unLoc lHsBindLR)
go _ = Left "findSigOfBinds"

findDeclContainingLocE :: Position -> [Located a] -> Either String (Located a)
findDeclContainingLocE loc ls =
maybe (Left "findDeclContainingLoc") Right $ findDeclContainingLoc loc ls

findInstanceHead :: (Outputable (HsType p)) => DynFlags -> String -> [LHsDecl p] -> Maybe (LHsType p)
findInstanceHead df instanceHead decls =
listToMaybe
Expand All @@ -173,6 +223,7 @@ findInstanceHead df instanceHead decls =
findDeclContainingLoc :: Position -> [Located a] -> Maybe (Located a)
findDeclContainingLoc loc = find (\(L l _) -> loc `isInsideSrcSpan` l)


-- Single:
-- This binding for ‘mod’ shadows the existing binding
-- imported from ‘Prelude’ at haskell-language-server/ghcide/src/Development/IDE/Plugin/CodeAction.hs:10:8-40
Expand Down Expand Up @@ -1020,10 +1071,6 @@ suggestImplicitParameter (L _ HsModule {hsmodDecls}) Diagnostic {_message, _rang
findTypeSignatureName :: T.Text -> Maybe T.Text
findTypeSignatureName t = matchRegexUnifySpaces t "([^ ]+) :: " <&> head

findTypeSignatureLine :: T.Text -> T.Text -> Int
findTypeSignatureLine contents typeSignatureName =
T.splitOn (typeSignatureName <> " :: ") contents & head & T.lines & length

-- | Suggests a constraint for a type signature with any number of existing constraints.
suggestFunctionConstraint :: DynFlags -> ParsedSource -> Diagnostic -> T.Text -> [(T.Text, Rewrite)]

Expand Down Expand Up @@ -1062,31 +1109,26 @@ suggestFunctionConstraint df (L _ HsModule {hsmodDecls}) Diagnostic {..} missing
<> "` to the context of the type signature for `" <> typeSignatureName <> "`"

-- | Suggests the removal of a redundant constraint for a type signature.
removeRedundantConstraints :: Maybe T.Text -> Diagnostic -> [(T.Text, TextEdit)]
removeRedundantConstraints mContents Diagnostic{..}
removeRedundantConstraints :: DynFlags -> ParsedSource -> Diagnostic -> [(T.Text, Rewrite)]
removeRedundantConstraints df (L _ HsModule {hsmodDecls}) Diagnostic{..}
-- • Redundant constraint: Eq a
-- • In the type signature for:
-- foo :: forall a. Eq a => a -> a
-- • Redundant constraints: (Monoid a, Show a)
-- • In the type signature for:
-- foo :: forall a. (Num a, Monoid a, Eq a, Show a) => a -> Bool
| Just contents <- mContents
-- Account for both "Redundant constraint" and "Redundant constraints".
, True <- "Redundant constraint" `T.isInfixOf` _message
| "Redundant constraint" `T.isInfixOf` _message
, Just typeSignatureName <- findTypeSignatureName _message
, Right (TypeSig _ _ HsWC{hswc_body = HsIB {hsib_body = sig}})
<- findSigOfDeclRanged _range hsmodDecls
, Just redundantConstraintList <- findRedundantConstraints _message
, Just constraints <- findConstraints contents typeSignatureName
= let constraintList = parseConstraints constraints
newConstraints = buildNewConstraints constraintList redundantConstraintList
typeSignatureLine = findTypeSignatureLine contents typeSignatureName
typeSignatureFirstChar = T.length $ typeSignatureName <> " :: "
startOfConstraint = Position typeSignatureLine typeSignatureFirstChar
endOfConstraint = Position typeSignatureLine $
typeSignatureFirstChar + T.length (constraints <> " => ")
range = Range startOfConstraint endOfConstraint
in [(actionTitle redundantConstraintList typeSignatureName, TextEdit range newConstraints)]
, rewrite <- removeConstraint (pred df redundantConstraintList) sig
= [(actionTitle redundantConstraintList typeSignatureName, rewrite)]
| otherwise = []
where
pred df list a = showSDoc df (ppr a) `elem` (T.unpack <$> list)

parseConstraints :: T.Text -> [T.Text]
parseConstraints t = t
& (T.strip >>> stripConstraintsParens >>> T.splitOn ",")
Expand All @@ -1106,32 +1148,13 @@ removeRedundantConstraints mContents Diagnostic{..}
& (`matchRegexUnifySpaces` "Redundant constraints?: (.+)")
<&> (head >>> parseConstraints)

-- If the type signature is not formatted as expected (arbitrary number of spaces,
-- line feeds...), just fail.
findConstraints :: T.Text -> T.Text -> Maybe T.Text
findConstraints contents typeSignatureName = do
constraints <- contents
& T.splitOn (typeSignatureName <> " :: ")
& (`atMay` 1)
>>= (T.splitOn " => " >>> (`atMay` 0))
guard $ not $ "\n" `T.isInfixOf` constraints || T.strip constraints /= constraints
return constraints

formatConstraints :: [T.Text] -> T.Text
formatConstraints [] = ""
formatConstraints [constraint] = constraint
formatConstraints constraintList = constraintList
& T.intercalate ", "
& \cs -> "(" <> cs <> ")"

formatConstraintsWithArrow :: [T.Text] -> T.Text
formatConstraintsWithArrow [] = ""
formatConstraintsWithArrow cs = cs & formatConstraints & (<> " => ")

buildNewConstraints :: [T.Text] -> [T.Text] -> T.Text
buildNewConstraints constraintList redundantConstraintList =
formatConstraintsWithArrow $ constraintList \\ redundantConstraintList

actionTitle :: [T.Text] -> T.Text -> T.Text
actionTitle constraintList typeSignatureName =
"Remove redundant constraint" <> (if length constraintList == 1 then "" else "s") <> " `"
Expand Down
14 changes: 14 additions & 0 deletions ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module Development.IDE.Plugin.CodeAction.ExactPrint (

-- * Utilities
appendConstraint,
removeConstraint,
extendImport,
hideSymbol,
liftParseAST,
Expand Down Expand Up @@ -118,6 +119,19 @@ fixParens openDP closeDP ctxt@(L _ elems) = do
dropHsParTy (L _ (HsParTy _ ty)) = ty
dropHsParTy other = other

removeConstraint ::
-- | Predicate: Which contect to drop.
(LHsType GhcPs -> Bool) ->
LHsType GhcPs ->
Rewrite
removeConstraint pred = go
where
go (L l it@HsQualTy{hst_ctxt = L l' ctxt}) = Rewrite l $ \_ -> do
return $ L l $ it{hst_ctxt = L l' $ filter (not . pred) ctxt}
go (L _ (HsParTy _ ty)) = go ty
go (L _ HsForAllTy{hst_body}) = go hst_body
go (L l other) = Rewrite l $ \_ -> return $ L l other

-- | Append a constraint at the end of a type context.
-- If no context is present, a new one will be created.
appendConstraint ::
Expand Down
94 changes: 85 additions & 9 deletions ghcide/test/exe/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2574,6 +2574,7 @@ addImplicitParamsConstraintTests =
"fCaller :: " <> mkContext contextCaller <> "()",
"fCaller = fBase"
]

removeRedundantConstraintsTests :: TestTree
removeRedundantConstraintsTests = let
header =
Expand All @@ -2582,6 +2583,13 @@ removeRedundantConstraintsTests = let
, ""
]

headerExt :: [T.Text] -> [T.Text]
headerExt exts =
redunt : extTxt ++ ["module Testing where"]
where
redunt = "{-# OPTIONS_GHC -Wredundant-constraints #-}"
extTxt = map (\ext -> "{-# LANGUAGE " <> ext <> " #-}") exts

redundantConstraintsCode :: Maybe T.Text -> T.Text
redundantConstraintsCode mConstraint =
let constraint = maybe "" (\c -> "" <> c <> " => ") mConstraint
Expand All @@ -2598,11 +2606,62 @@ removeRedundantConstraintsTests = let
, "foo x = x == 1"
]

typeSignatureSpaces :: T.Text
typeSignatureSpaces = T.unlines $ header <>
[ "foo :: (Num a, Eq a, Monoid a) => a -> Bool"
, "foo x = x == 1"
]
typeSignatureSpaces :: Maybe T.Text -> T.Text
typeSignatureSpaces mConstraint =
let constraint = maybe "(Num a, Eq a)" (\c -> "(Num a, Eq a, " <> c <> ")") mConstraint
in T.unlines $ header <>
[ "foo :: " <> constraint <> " => a -> Bool"
, "foo x = x == 1"
]

redundantConstraintsForall :: Maybe T.Text -> T.Text
redundantConstraintsForall mConstraint =
let constraint = maybe "" (\c -> "" <> c <> " => ") mConstraint
in T.unlines $ headerExt ["RankNTypes"] <>
[ "foo :: forall a. " <> constraint <> "a -> a"
, "foo = id"
]

typeSignatureNested :: Maybe T.Text -> T.Text
typeSignatureNested mConstraint =
let constraint = maybe "" (\c -> "" <> c <> " => ") mConstraint
in T.unlines $ header <>
[ "f :: Int -> ()"
, "f = g"
, " where"
, " g :: " <> constraint <> "a -> ()"
, " g _ = ()"
]

typeSignatureNested' :: Maybe T.Text -> T.Text
typeSignatureNested' mConstraint =
let constraint = maybe "" (\c -> "" <> c <> " => ") mConstraint
in T.unlines $ header <>
[ "f :: Int -> ()"
, "f ="
, " let"
, " g :: Int -> ()"
, " g = h"
, " where"
, " h :: " <> constraint <> "a -> ()"
, " h _ = ()"
, " in g"
]

typeSignatureNested'' :: Maybe T.Text -> T.Text
typeSignatureNested'' mConstraint =
let constraint = maybe "" (\c -> "" <> c <> " => ") mConstraint
in T.unlines $ header <>
[ "f :: Int -> ()"
, "f = g"
, " where"
, " g :: Int -> ()"
, " g = "
, " let"
, " h :: " <> constraint <> "a -> ()"
, " h _ = ()"
, " in h"
]

typeSignatureMultipleLines :: T.Text
typeSignatureMultipleLines = T.unlines $ header <>
Expand All @@ -2615,7 +2674,7 @@ removeRedundantConstraintsTests = let
check actionTitle originalCode expectedCode = testSession (T.unpack actionTitle) $ do
doc <- createDoc "Testing.hs" "haskell" originalCode
_ <- waitForDiagnostics
actionsOrCommands <- getCodeActions doc (Range (Position 4 0) (Position 4 maxBound))
actionsOrCommands <- getAllCodeActions doc
chosenAction <- liftIO $ pickActionWithTitle actionTitle actionsOrCommands
executeCodeAction chosenAction
modifiedCode <- documentContents doc
Expand All @@ -2641,9 +2700,26 @@ removeRedundantConstraintsTests = let
"Remove redundant constraints `(Monoid a, Show a)` from the context of the type signature for `foo`"
(redundantMixedConstraintsCode $ Just "Monoid a, Show a")
(redundantMixedConstraintsCode Nothing)
, checkPeculiarFormatting
"should do nothing when constraints contain an arbitrary number of spaces"
typeSignatureSpaces
, check
"Remove redundant constraint `Eq a` from the context of the type signature for `g`"
(typeSignatureNested $ Just "Eq a")
(typeSignatureNested Nothing)
, check
"Remove redundant constraint `Eq a` from the context of the type signature for `h`"
(typeSignatureNested' $ Just "Eq a")
(typeSignatureNested' Nothing)
, check
"Remove redundant constraint `Eq a` from the context of the type signature for `h`"
(typeSignatureNested'' $ Just "Eq a")
(typeSignatureNested'' Nothing)
, check
"Remove redundant constraint `Eq a` from the context of the type signature for `foo`"
(redundantConstraintsForall $ Just "Eq a")
(redundantConstraintsForall Nothing)
, check
"Remove redundant constraints `(Monoid a, Show a)` from the context of the type signature for `foo`"
(typeSignatureSpaces $ Just "Monoid a, Show a")
(typeSignatureSpaces Nothing)
, checkPeculiarFormatting
"should do nothing when constraints contain line feeds"
typeSignatureMultipleLines
Expand Down