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.

Sign up for GitHub

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

Jump to bottom

don't crash when an unused operator import ends in . #2123

Merged
merged 9 commits into from Aug 24, 2021
Merged

don't crash when an unused operator import ends in . #2123

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
81ae9a8
replace head with safer uncons
tscholak Aug 22, 2021
f498cde
if empty, stay empty
tscholak Aug 22, 2021
3f50753
add test for removal of operators ending with '.'
tscholak Aug 22, 2021
3308b94
make remove-import-actions tests pass
tscholak Aug 22, 2021
4dc8f25
add more tests and debug output for modifyBinding
tscholak Aug 22, 2021
1891a60
implement reviewers suggestions
tscholak Aug 22, 2021
479f1fc
restore ghcide/bench/example/HLS
tscholak Aug 22, 2021
ded6838
Merge branch 'master' into fix-2025-2118-head-empty-list
tscholak Aug 22, 2021
c240fd3
Merge branch 'master' into fix-2025-2118-head-empty-list
pepeiborra Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 8 additions & 7 deletions ghcide/src/Development/IDE/Plugin/CodeAction.hs
View file
Open in desktop
Expand Up @@ -1483,20 +1483,21 @@ rangesForBindingImport :: ImportDecl GhcPs -> String -> [Range]
rangesForBindingImport ImportDecl{ideclHiding = Just (False, L _ lies)} b =
concatMap (mapMaybe srcSpanToRange . rangesForBinding' b') lies
where
b' = modifyBinding b
b' = wrapOperatorInParens b
rangesForBindingImport _ _ = []

modifyBinding :: String -> String
modifyBinding = wrapOperatorInParens . unqualify
where
wrapOperatorInParens x = if isAlpha (head x) then x else "(" <> x <> ")"
unqualify x = snd $ breakOnEnd "." x
wrapOperatorInParens :: String -> String
wrapOperatorInParens x =
case uncons x of
Just (h, _t) -> if isAlpha h then x else "(" <> x <> ")"
Nothing -> mempty

smallerRangesForBindingExport :: [LIE GhcPs] -> String -> [Range]
smallerRangesForBindingExport lies b =
concatMap (mapMaybe srcSpanToRange . ranges') lies
where
b' = modifyBinding b
unqualify = snd . breakOnEnd "."
b' = wrapOperatorInParens . unqualify $ b
ranges' (L _ (IEThingWith _ thing _ inners labels))
| showSDocUnsafe (ppr thing) == b' = []
| otherwise =
Expand Down
39 changes: 39 additions & 0 deletions ghcide/test/exe/Main.hs
View file
Open in desktop
Expand Up @@ -1150,6 +1150,33 @@ removeImportTests = testGroup "remove import actions"
, "type T = K.Type"
]
liftIO $ expectedContentAfterAction @=? contentAfterAction
, testSession "remove unused operators whose name ends with '.'" $ do
let contentA = T.unlines
[ "module ModuleA where"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional suggestion: I just realized there's (.|.) operator in Data.Bits in base, so we could potentially shrink the reproducer to just one module if you fancy minimalism 😉

{-# OPTIONS_GHC -Wunused-imports #-}
module ModuleA where
import Data.Bits ((.|.), xor)
x=xor

, "(@.) = 0 -- Must have an operator whose name ends with '.'"
, "a = 1 -- .. but also something else"
]
_docA <- createDoc "ModuleA.hs" "haskell" contentA
let contentB = T.unlines
[ "{-# OPTIONS_GHC -Wunused-imports #-}"
, "module ModuleB where"
, "import ModuleA (a, (@.))"
, "x = a -- Must use something from module A, but not (@.)"
]
docB <- createDoc "ModuleB.hs" "haskell" contentB
_ <- waitForDiagnostics
[InR action@CodeAction { _title = actionTitle }, _]
<- getCodeActions docB (Range (Position 2 0) (Position 2 5))
liftIO $ "Remove @. from import" @=? actionTitle
executeCodeAction action
contentAfterAction <- documentContents docB
let expectedContentAfterAction = T.unlines
[ "{-# OPTIONS_GHC -Wunused-imports #-}"
, "module ModuleB where"
, "import ModuleA (a)"
, "x = a -- Must use something from module A, but not (@.)"
]
liftIO $ expectedContentAfterAction @=? contentAfterAction
]

extendImportTests :: TestTree
Expand Down Expand Up @@ -3311,6 +3338,18 @@ removeExportTests = testGroup "remove export actions"
, "import qualified Data.List as M"
, "a :: ()"
, "a = ()"])
, testSession "qualified re-export ending in '.'" $ template
(T.unlines
[ "module A ((M.@.),a) where"
, "import qualified Data.List as M"
, "a :: ()"
, "a = ()"])
"Remove ‘M.@.’ from export"
(Just $ T.unlines
[ "module A (a) where"
, "import qualified Data.List as M"
, "a :: ()"
, "a = ()"])
, testSession "export module" $ template
(T.unlines
[ "module A (module B) where"
Expand Down