Skip to content

Suggest hiding imports when local definition exists #2320

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
Show file tree
Hide file tree
Changes from all 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
26 changes: 15 additions & 11 deletions ghcide/src/Development/IDE/Plugin/CodeAction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,9 @@ suggestImportDisambiguation df (Just txt) ps@(L _ HsModule {hsmodImports}) fileC
"Ambiguous occurrence ‘([^’]+)’"
, Just modules <-
map last
<$> allMatchRegexUnifySpaces _message "imported from ‘([^’]+)’" =
suggestions ambiguous modules
<$> allMatchRegexUnifySpaces _message "imported from ‘([^’]+)’"
, local <- matchRegexUnifySpaces _message "defined at .+:[0-9]+:[0-9]+" =
suggestions ambiguous modules (isJust local)
| otherwise = []
where
locDic =
Expand All @@ -906,16 +907,16 @@ suggestImportDisambiguation df (Just txt) ps@(L _ HsModule {hsmodImports}) fileC
-- > removeAllDuplicates [1, 1, 2, 3, 2] = [3]
removeAllDuplicates = map head . filter ((==1) <$> length) . group . sort
hasDuplicate xs = length xs /= length (S.fromList xs)
suggestions symbol mods
suggestions symbol mods local
| hasDuplicate mods = case mapM toModuleTarget (removeAllDuplicates mods) of
Just targets -> suggestionsImpl symbol (map (, []) targets)
Just targets -> suggestionsImpl symbol (map (, []) targets) local
Nothing -> []
| otherwise = case mapM toModuleTarget mods of
Just targets -> suggestionsImpl symbol (oneAndOthers targets)
Just targets -> suggestionsImpl symbol (oneAndOthers targets) local
Nothing -> []
suggestionsImpl symbol targetsWithRestImports =
suggestionsImpl symbol targetsWithRestImports local =
sortOn fst
[ ( renderUniquify mode modNameText symbol
[ ( renderUniquify mode modNameText symbol False
, disambiguateSymbol ps fileContents diag symbol mode
)
| (modTarget, restImports) <- targetsWithRestImports
Expand All @@ -942,10 +943,14 @@ suggestImportDisambiguation df (Just txt) ps@(L _ HsModule {hsmodImports}) fileC
_ -> False
]
++ [HideOthers restImports | not (null restImports)]
] ++ [ ( renderUniquify mode T.empty symbol True
, disambiguateSymbol ps fileContents diag symbol mode
) | local, not (null targetsWithRestImports)
, let mode = HideOthers (uncurry (:) (head targetsWithRestImports))
]
renderUniquify HideOthers {} modName symbol =
"Use " <> modName <> " for " <> symbol <> ", hiding other imports"
renderUniquify (ToQualified _ qual) _ symbol =
renderUniquify HideOthers {} modName symbol local =
"Use " <> (if local then "local definition" else modName) <> " for " <> symbol <> ", hiding other imports"
renderUniquify (ToQualified _ qual) _ symbol _ =
"Replace with qualified: "
<> T.pack (moduleNameString qual)
<> "."
Expand Down Expand Up @@ -1005,7 +1010,6 @@ disambiguateSymbol pm fileContents Diagnostic {..} (T.unpack -> symbol) = \case
liftParseAST @RdrName df $
prettyPrint $ L (mkGeneralSrcSpan "") rdr
]

findImportDeclByRange :: [LImportDecl GhcPs] -> Range -> Maybe (LImportDecl GhcPs)
findImportDeclByRange xs range = find (\(L l _)-> srcSpanToRange l == Just range) xs

Expand Down
14 changes: 14 additions & 0 deletions ghcide/test/data/hiding/HideFunctionWithoutLocal.expected.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module HideFunctionWithoutLocal where

import AVec (fromList)
import BVec (fromList)
import CVec hiding ((++), cons)
import DVec hiding ((++), cons, snoc)
import EVec as E hiding ((++))
import Prelude hiding ((++))

theOp = (++)

data Vec a

(++) = undefined
13 changes: 13 additions & 0 deletions ghcide/test/data/hiding/HideFunctionWithoutLocal.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module HideFunctionWithoutLocal where

import AVec (fromList)
import BVec (fromList, (++))
import CVec hiding (cons)
import DVec hiding (cons, snoc)
import EVec as E

theOp = (++)

data Vec a

(++) = undefined
9 changes: 9 additions & 0 deletions ghcide/test/data/hiding/HidePreludeLocalInfix.expected.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module HidePreludeLocalInfix where
import Prelude hiding ((++))

infixed xs ys = xs ++ ys

data Vec a

(++) :: Vec a -> Vec a -> Vec a
(++) = undefined
8 changes: 8 additions & 0 deletions ghcide/test/data/hiding/HidePreludeLocalInfix.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module HidePreludeLocalInfix where

infixed xs ys = xs ++ ys

data Vec a

(++) :: Vec a -> Vec a -> Vec a
(++) = undefined
10 changes: 10 additions & 0 deletions ghcide/test/exe/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1798,10 +1798,20 @@ suggestImportDisambiguationTests = testGroup "suggest import disambiguation acti
compareHideFunctionTo [(8,9),(10,8)]
"Use EVec for ++, hiding other imports"
"HideFunction.expected.append.E.hs"
, testCase "Hide functions without local" $
compareTwo
"HideFunctionWithoutLocal.hs" [(8,8)]
"Use local definition for ++, hiding other imports"
"HideFunctionWithoutLocal.expected.hs"
, testCase "Prelude" $
compareHideFunctionTo [(8,9),(10,8)]
"Use Prelude for ++, hiding other imports"
"HideFunction.expected.append.Prelude.hs"
, testCase "Prelude and local definition, infix" $
compareTwo
"HidePreludeLocalInfix.hs" [(2,19)]
"Use local definition for ++, hiding other imports"
"HidePreludeLocalInfix.expected.hs"
, testCase "AVec, indented" $
compareTwo "HidePreludeIndented.hs" [(3,8)]
"Use AVec for ++, hiding other imports"
Expand Down