Skip to content
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

ghcide: Implements a CodeAction to disambiguate ambiguous symbols #1264

Merged
merged 52 commits into from
Jan 31, 2021
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
b35aa37
wip
konn Jan 26, 2021
cbcb0f0
Draft completed
konn Jan 26, 2021
a068257
Removes Unuseds
konn Jan 26, 2021
e506ddf
Redundant extension
konn Jan 26, 2021
a995c5e
linting
konn Jan 26, 2021
169db8f
Makes HLint happy
konn Jan 26, 2021
4e92ee9
tweak for transfer annotation logic (not working)
konn Jan 26, 2021
72477dd
Initial implementation done
konn Jan 27, 2021
65724fa
Import list reorder
konn Jan 27, 2021
4ed8abc
Remove redundant fmt
konn Jan 27, 2021
80aafa7
Merge branch 'master' into hide-import
konn Jan 28, 2021
197a8a3
Merge branch 'master' into hide-import
konn Jan 28, 2021
d99f84c
Merge branch 'master' into hide-import
konn Jan 29, 2021
30f2f16
lint
konn Jan 29, 2021
9e656c5
Missing import
konn Jan 29, 2021
3e68f24
Merge branch 'master' into hide-import
konn Jan 29, 2021
d19b4fb
Excludes false-positive qualified imports
konn Jan 29, 2021
eb3cd11
A first test (not enough though)
konn Jan 29, 2021
0239aa0
fmt.sh
konn Jan 29, 2021
a6ddab2
Some more test cases
konn Jan 29, 2021
4816c84
More test cases
konn Jan 29, 2021
3b6c048
Ah! CRLF have bitten me!
konn Jan 29, 2021
ed73a63
Merge branch 'master' into hide-import
konn Jan 30, 2021
dff4d03
Tentative workaround for #1274
konn Jan 30, 2021
0696e5a
Wait much to ensure rewrite suggestion to be collected
konn Jan 30, 2021
c2907dd
Tests for type suggestion
konn Jan 30, 2021
41d6c75
Slightly more wait
konn Jan 30, 2021
223808b
A little smarter waiting strartegy for actions
konn Jan 30, 2021
abbebf5
Merge branch 'master' into hide-import
konn Jan 30, 2021
0721ef9
Import list slim up
konn Jan 30, 2021
7487446
Adjusted to the master
konn Jan 30, 2021
b5b7f03
Update ghcide/src/Development/IDE/Plugin/CodeAction.hs
konn Jan 30, 2021
d4075dc
Rewrote using `expectDiagnostics`
konn Jan 30, 2021
130afec
Case for Prelude.++
konn Jan 30, 2021
a3486ac
Corrects test name
konn Jan 30, 2021
c252aca
Renames `rawIEWrapName` to `unqualIEWrapName`, and moved to the appro…
konn Jan 30, 2021
f6594fd
Rewrote qualifying strategy with `Rewrite`
konn Jan 30, 2021
e694694
Use exactprint also for hideImplicitPreludeSymbol
konn Jan 30, 2021
0550b6f
Unify exact actions and `suggestImportDisambiguation`
konn Jan 30, 2021
bf99f31
Moves a comment to the right place
konn Jan 30, 2021
1a736ad
Merge branch 'master' into hide-import
konn Jan 30, 2021
d96d612
Won't panic on errornous input, let HLS keep going
konn Jan 30, 2021
10cc815
No, each errornous rewrite must not be dropped seprately, but discard…
konn Jan 30, 2021
82345d1
Update ghcide/src/Development/IDE/Spans/Common.hs
konn Jan 31, 2021
191df1b
ieNames
konn Jan 31, 2021
3b2b4e8
Makes Splice plugin compiles
konn Jan 31, 2021
ced501f
Merge branch 'master' into hide-import
konn Jan 31, 2021
d75b51e
Stop using nfp
konn Jan 31, 2021
13b9f2c
Since there is global `setEntryDPT dp00`, we don't add offset here
konn Jan 31, 2021
54f5a49
Merge branch 'master' into hide-import
konn Jan 31, 2021
4aae21a
Removes redundant (why warned here?)
konn Jan 31, 2021
a45667b
Made `hideImplicitPreludeSymbol` total
konn Jan 31, 2021
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
18 changes: 18 additions & 0 deletions ghcide/src/Development/IDE/GHC/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ module Development.IDE.GHC.Error
, realSrcLocToPosition
, realSrcSpanToLocation
, srcSpanToFilename
, rangeToSrcSpan
, rangeToRealSrcSpan
, positionToRealSrcLoc
, zeroSpan
, realSpan
, isInsideSrcSpan
Expand All @@ -39,6 +42,7 @@ import Panic
import ErrUtils
import SrcLoc
import qualified Outputable as Out
import Data.String (fromString)



Expand Down Expand Up @@ -102,6 +106,20 @@ srcSpanToLocation src = do
-- important that the URI's we produce have been properly normalized, otherwise they point at weird places in VS Code
pure $ Location (fromNormalizedUri $ filePathToUri' $ toNormalizedFilePath' fs) rng

rangeToSrcSpan :: NormalizedFilePath -> Range -> SrcSpan
rangeToSrcSpan = fmap RealSrcSpan . rangeToRealSrcSpan

rangeToRealSrcSpan
:: NormalizedFilePath -> Range -> RealSrcSpan
rangeToRealSrcSpan nfp =
mkRealSrcSpan
<$> positionToRealSrcLoc nfp . _start
<*> positionToRealSrcLoc nfp . _end

positionToRealSrcLoc :: NormalizedFilePath -> Position -> RealSrcLoc
positionToRealSrcLoc nfp (Position l c)=
mkRealSrcLoc (fromString $ fromNormalizedFilePath nfp) (l + 1) (c + 1)

isInsideSrcSpan :: Position -> SrcSpan -> Bool
p `isInsideSrcSpan` r = case srcSpanToRange r of
Just (Range sp ep) -> sp <= p && p <= ep
Expand Down
197 changes: 181 additions & 16 deletions ghcide/src/Development/IDE/Plugin/CodeAction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,22 @@ import qualified Data.Text as T
import Text.Regex.TDFA (mrAfter, (=~), (=~~))
import Outputable (Outputable, ppr, showSDoc, showSDocUnsafe)
import Data.Function
import Control.Arrow ((>>>))
import Control.Arrow ((>>>), second)
import Data.Functor
import Control.Applicative ((<|>))
import Safe (atMay)
import Bag (isEmptyBag)
import qualified Data.HashSet as Set
import Control.Concurrent.Extra (readVar)
import Development.IDE.GHC.Util (printRdrName)
import Development.IDE.GHC.Util (printRdrName, prettyPrint)
import Ide.PluginUtils (subRange)
import Ide.Types
import qualified Data.DList as DL
import Development.IDE.Spans.Common
import OccName
import qualified GHC.LanguageExtensions as Lang
import Control.Lens (alaf)
import Data.Monoid (Ap(..))

descriptor :: PluginId -> PluginDescriptor IdeState
descriptor plId =
Expand Down Expand Up @@ -88,7 +94,7 @@ codeAction lsp state _ (TextDocumentIdentifier uri) _range CodeActionContext{_di
df = ms_hspp_opts . pm_mod_summary <$> parsedModule
actions =
[ mkCA title [x] edit
| x <- xs, (title, tedit) <- suggestAction exportsMap ideOptions parsedModule text df annotatedPS x
| x <- xs, (title, tedit) <- suggestAction exportsMap ideOptions mbFile parsedModule text df annotatedPS x
, let edit = WorkspaceEdit (Just $ Map.singleton uri $ List tedit) Nothing
]
actions' = caRemoveRedundantImports parsedModule text diag xs uri
Expand All @@ -103,26 +109,37 @@ mkCA title diags edit =
rewrite ::
Maybe DynFlags ->
Maybe (Annotated ParsedSource) ->
(DynFlags -> ParsedSource -> [(T.Text, Rewrite)]) ->
(DynFlags -> ParsedSource -> [(T.Text, [Rewrite])]) ->
[(T.Text, [TextEdit])]
rewrite (Just df) (Just ps) f
| Right edit <- (traverse . traverse) (rewriteToEdit df (annsA ps)) (f df $ astA ps) = edit
| Right edit <- (traverse . traverse)
(alaf Ap foldMap (rewriteToEdit df (annsA ps)))
(f df $ astA ps) = edit
rewrite _ _ _ = []

suggestAction
:: ExportsMap
-> IdeOptions
-> Maybe NormalizedFilePath
-> Maybe ParsedModule
-> Maybe T.Text
-> Maybe DynFlags
-> Maybe (Annotated ParsedSource)
-> Diagnostic
-> [(T.Text, [TextEdit])]
suggestAction packageExports ideOptions parsedModule text df annSource diag = concat
suggestAction packageExports ideOptions mbFile parsedModule text df annSource diag =
concat
-- Order these suggestions by priority
[ suggestSignature True diag
, rewrite df annSource $ \_ ps -> suggestExtendImport packageExports ps diag
, suggestFillTypeWildcard diag
]
++ concat
[ rewrite df annSource $ \df ps ->
suggestImportDisambiguation nfp df ps diag
| nfp <- maybeToList mbFile
] ++
concat [
suggestFillTypeWildcard diag
, suggestFixConstructorImport text diag
, suggestModuleTypo diag
, suggestReplaceIdentifier text diag
Expand Down Expand Up @@ -301,8 +318,8 @@ suggestDeleteUnusedBinding
let findSig (L (RealSrcSpan l) (SigD _ sig)) = findRelatedSigSpan indexedContent name l sig
findSig _ = []
in
[extendForSpaces indexedContent $ toRange l]
++ concatMap findSig hsmodDecls
extendForSpaces indexedContent (toRange l) :
concatMap findSig hsmodDecls
_ -> concatMap (findRelatedSpanForMatch indexedContent name) matches
findRelatedSpans _ _ _ = []

Expand Down Expand Up @@ -377,7 +394,7 @@ suggestDeleteUnusedBinding
then
let findSig (L (RealSrcSpan l) sig) = findRelatedSigSpan indexedContent name l sig
findSig _ = []
in [extendForSpaces indexedContent $ toRange l] ++ concatMap findSig lsigs
in extendForSpaces indexedContent (toRange l) : concatMap findSig lsigs
else concatMap (findRelatedSpanForMatch indexedContent name) matches
findRelatedSpanForHsBind _ _ _ _ = []

Expand Down Expand Up @@ -655,7 +672,7 @@ getIndentedGroupsBy pred inp = case dropWhile (not.pred) inp of
indentation :: T.Text -> Int
indentation = T.length . T.takeWhile isSpace

suggestExtendImport :: ExportsMap -> ParsedSource -> Diagnostic -> [(T.Text, Rewrite)]
suggestExtendImport :: ExportsMap -> ParsedSource -> Diagnostic -> [(T.Text, [Rewrite])]
suggestExtendImport exportsMap (L _ HsModule {hsmodImports}) Diagnostic{_range=_range,..}
| Just [binding, mod, srcspan] <-
matchRegexUnifySpaces _message
Expand All @@ -676,7 +693,7 @@ suggestExtendImport exportsMap (L _ HsModule {hsmodImports}) Diagnostic{_range=_
Just decl <- findImportDeclByRange decls range,
Just ident <- lookupExportMap binding mod
= [ ( "Add " <> renderImportStyle importStyle <> " to the import list of " <> mod
, uncurry extendImport (unImportStyle importStyle) decl
, [uncurry extendImport (unImportStyle importStyle) decl]
)
| importStyle <- NE.toList $ importStyles ident
]
Expand All @@ -694,6 +711,143 @@ suggestExtendImport exportsMap (L _ HsModule {hsmodImports}) Diagnostic{_range=_
, parent = Nothing
, isDatacon = False}

data HidingMode = HideOthers [ModuleTarget]
| ToQualified ModuleName
deriving (Show)

data ModuleTarget
= ExistingImp (NonEmpty (LImportDecl GhcPs))
| ImplicitPrelude [LImportDecl GhcPs]
deriving (Show)

targetImports :: ModuleTarget -> [LImportDecl GhcPs]
targetImports (ExistingImp ne) = NE.toList ne
targetImports (ImplicitPrelude xs) = xs

oneAndOthers :: [a] -> [(a, [a])]
oneAndOthers = go
where
go [] = []
go (x : xs) = (x, xs) : map (second (x :)) (go xs)

isPreludeImplicit :: DynFlags -> Bool
isPreludeImplicit = xopt Lang.ImplicitPrelude

-- | Suggests disambiguation for ambiguous symbols.
suggestImportDisambiguation ::
NormalizedFilePath ->
DynFlags ->
ParsedSource ->
Diagnostic ->
[(T.Text, [Rewrite])]
suggestImportDisambiguation nfp df ps@(L _ HsModule {hsmodImports}) diag@Diagnostic {..}
| Just [ambiguous] <-
matchRegexUnifySpaces
_message
"Ambiguous occurrence ‘([^’]+)’"
, Just modules <-
map last
<$> allMatchRegexUnifySpaces _message "imported from ‘([^’]+)’" =
suggestions ambiguous modules
| otherwise = []
where
locDic =
fmap (NE.fromList . DL.toList) $
Map.fromListWith (<>) $
map
( \i@(L _ idecl) ->
( T.pack $ moduleNameString $ unLoc $ ideclName idecl
, DL.singleton i
)
)
hsmodImports
toModuleTarget "Prelude"
| isPreludeImplicit df
= Just $ ImplicitPrelude $
maybe [] NE.toList (Map.lookup "Prelude" locDic)
toModuleTarget mName = ExistingImp <$> Map.lookup mName locDic

suggestions symbol mods
| Just targets <- mapM toModuleTarget mods =
sortOn fst
[ ( renderUniquify mode modNameText symbol
, disambiguateSymbol nfp ps diag symbol mode
)
| (modTarget, restImports) <- oneAndOthers targets
, let modName = targetModuleName modTarget
modNameText = T.pack $ moduleNameString modName
, mode <-
HideOthers restImports :
[ ToQualified qual
| ExistingImp imps <- [modTarget]
, L _ qual <- nubOrd $ mapMaybe (ideclAs . unLoc)
$ NE.toList imps
]
++ [ToQualified modName
| any (occursUnqualified symbol . unLoc)
(targetImports modTarget)
|| case modTarget of
ImplicitPrelude{} -> True
_ -> False
]
]
| otherwise = []
renderUniquify HideOthers {} modName symbol =
"Use " <> modName <> " for " <> symbol <> ", hiding other imports"
renderUniquify (ToQualified qual) _ symbol =
"Replace with qualified: "
<> T.pack (moduleNameString qual)
<> "."
<> symbol

occursUnqualified :: T.Text -> ImportDecl GhcPs -> Bool
occursUnqualified symbol ImportDecl{..}
| isNothing ideclAs = Just False /=
-- I don't find this particularly comprehensible,
-- but HLint suggested me to do so...
(ideclHiding <&> \(isHiding, L _ ents) ->
let occurs = any ((symbol `symbolOccursIn`) . unLoc) ents
in isHiding && not occurs || not isHiding && occurs
)
occursUnqualified _ _ = False

symbolOccursIn :: T.Text -> IE GhcPs -> Bool
symbolOccursIn symb = any ((== symb). showNameWithoutUniques) . ieNames

targetModuleName :: ModuleTarget -> ModuleName
targetModuleName ImplicitPrelude{} = mkModuleName "Prelude"
targetModuleName (ExistingImp (L _ ImportDecl{..} :| _)) =
unLoc ideclName
targetModuleName (ExistingImp _) =
error "Cannot happen!"

disambiguateSymbol ::
NormalizedFilePath ->
ParsedSource ->
Diagnostic ->
T.Text ->
HidingMode ->
[Rewrite]
disambiguateSymbol nfp pm Diagnostic {..} (T.unpack -> symbol) = \case
(HideOthers hiddens0) ->
[ hideSymbol symbol idecl
| ExistingImp idecls <- hiddens0
, idecl <- NE.toList idecls
]
++ mconcat
[ if null imps
then [hideImplicitPreludeSymbol symbol pm]
else hideSymbol symbol <$> imps
| ImplicitPrelude imps <- hiddens0
]
(ToQualified qualMod) ->
let occSym = mkVarOcc symbol
rdr = Qual qualMod occSym
in [Rewrite (rangeToSrcSpan nfp _range) $ \df -> do
konn marked this conversation as resolved.
Show resolved Hide resolved
liftParseAST @(HsExpr GhcPs) df $ prettyPrint $ HsVar @GhcPs noExtField
$ L (UnhelpfulSpan "") rdr
]

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

Expand All @@ -711,13 +865,13 @@ suggestFixConstructorImport _ Diagnostic{_range=_range,..}
in [("Fix import of " <> fixedImport, [TextEdit _range fixedImport])]
| otherwise = []
-- | Suggests a constraint for a declaration for which a constraint is missing.
suggestConstraint :: DynFlags -> ParsedSource -> Diagnostic -> [(T.Text, Rewrite)]
suggestConstraint :: DynFlags -> ParsedSource -> Diagnostic -> [(T.Text, [Rewrite])]
suggestConstraint df parsedModule diag@Diagnostic {..}
| Just missingConstraint <- findMissingConstraint _message
= let codeAction = if _message =~ ("the type signature for:" :: String)
then suggestFunctionConstraint df parsedModule
else suggestInstanceConstraint df parsedModule
in codeAction diag missingConstraint
in map (second (:[])) $ codeAction diag missingConstraint
| otherwise = []
where
findMissingConstraint :: T.Text -> Maybe T.Text
Expand Down Expand Up @@ -773,14 +927,14 @@ suggestInstanceConstraint df (L _ HsModule {hsmodDecls}) Diagnostic {..} missing
suggestImplicitParameter ::
ParsedSource ->
Diagnostic ->
[(T.Text, Rewrite)]
[(T.Text, [Rewrite])]
suggestImplicitParameter (L _ HsModule {hsmodDecls}) Diagnostic {_message, _range}
| Just [implicitT] <- matchRegexUnifySpaces _message "Unbound implicit parameter \\(([^:]+::.+)\\) arising",
Just (L _ (ValD _ FunBind {fun_id = L _ funId})) <- findDeclContainingLoc (_start _range) hsmodDecls,
Just (TypeSig _ _ HsWC {hswc_body = HsIB {hsib_body}}) <- findSigOfDecl (== funId) hsmodDecls
=
[( "Add " <> implicitT <> " to the context of " <> T.pack (printRdrName funId)
, appendConstraint (T.unpack implicitT) hsib_body)]
, [appendConstraint (T.unpack implicitT) hsib_body])]
| otherwise = []

findTypeSignatureName :: T.Text -> Maybe T.Text
Expand Down Expand Up @@ -1098,11 +1252,22 @@ rangesForBinding' _ _ = []
matchRegexUnifySpaces :: T.Text -> T.Text -> Maybe [T.Text]
matchRegexUnifySpaces message = matchRegex (unifySpaces message)

-- | 'allMatchRegex' combined with 'unifySpaces'
allMatchRegexUnifySpaces :: T.Text -> T.Text -> Maybe [[T.Text]]
allMatchRegexUnifySpaces message =
allMatchRegex (unifySpaces message)

-- | Returns Just (the submatches) for the first capture, or Nothing.
matchRegex :: T.Text -> T.Text -> Maybe [T.Text]
matchRegex message regex = case message =~~ regex of
Just (_ :: T.Text, _ :: T.Text, _ :: T.Text, bindings) -> Just bindings
Nothing -> Nothing

-- | Returns Just (all matches) for the first capture, or Nothing.
allMatchRegex :: T.Text -> T.Text -> Maybe [[T.Text]]
allMatchRegex message regex = message =~~ regex


unifySpaces :: T.Text -> T.Text
unifySpaces = T.unwords . T.words

Expand Down
Loading