Skip to content

Commit 8999342

Browse files
author
kokobd
committed
#2414 fix new import position
1 parent 44be741 commit 8999342

File tree

2 files changed

+73
-39
lines changed

2 files changed

+73
-39
lines changed

ghcide/src/Development/IDE/Plugin/CodeAction.hs

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ module Development.IDE.Plugin.CodeAction
1919

2020
import Control.Applicative ((<|>))
2121
import Control.Arrow (second,
22-
(>>>),
23-
(&&&))
22+
(&&&),
23+
(>>>))
2424
import Control.Concurrent.STM.Stats (atomically)
2525
import Control.Monad (guard, join,
2626
msum)
@@ -41,6 +41,7 @@ import qualified Data.Rope.UTF16 as Rope
4141
import qualified Data.Set as S
4242
import qualified Data.Text as T
4343
import Data.Tuple.Extra (fst3)
44+
import Debug.Trace
4445
import Development.IDE.Core.Rules
4546
import Development.IDE.Core.RuleTypes
4647
import Development.IDE.Core.Service
@@ -58,6 +59,12 @@ import Development.IDE.Plugin.TypeLenses (suggestSigna
5859
import Development.IDE.Types.Exports
5960
import Development.IDE.Types.Location
6061
import Development.IDE.Types.Options
62+
import GHC (AddEpAnn (AddEpAnn),
63+
Anchor (anchor),
64+
AnnsModule (am_main),
65+
DeltaPos (..),
66+
EpAnn (..),
67+
EpaLocation (..))
6168
import qualified GHC.LanguageExtensions as Lang
6269
import Ide.PluginUtils (subRange)
6370
import Ide.Types
@@ -1386,34 +1393,66 @@ newImportToEdit (unNewImport -> imp) ps fileContents
13861393
-- the import will be inserted at line zero if there are no pragmas,
13871394
-- * otherwise inserted one line after the last file-header pragma
13881395
newImportInsertRange :: ParsedSource -> T.Text -> Maybe (Range, Int)
1389-
newImportInsertRange (L _ HsModule {..}) fileContents
1396+
newImportInsertRange ps@(L _ HsModule {..}) fileContents
13901397
| Just ((l, c), col) <- case hsmodImports of
1391-
[] -> findPositionNoImports (fmap reLoc hsmodName) (fmap reLoc hsmodExports) fileContents
1392-
_ -> findPositionFromImportsOrModuleDecl (map reLoc hsmodImports) last True
1398+
[] -> (\line -> ((line, 0), 0)) <$> findPositionNoImports ps fileContents
1399+
_ -> findPositionFromImportsOrModuleDecl (map reLoc hsmodImports) last
13931400
, let insertPos = Position (fromIntegral l) (fromIntegral c)
13941401
= Just (Range insertPos insertPos, col)
13951402
| otherwise = Nothing
13961403

13971404
-- | Insert the import under the Module declaration exports if they exist, otherwise just under the module declaration.
13981405
-- If no module declaration exists, then no exports will exist either, in that case
13991406
-- insert the import after any file-header pragmas or at position zero if there are no pragmas
1400-
findPositionNoImports :: Maybe (Located ModuleName) -> Maybe (Located [LIE name]) -> T.Text -> Maybe ((Int, Int), Int)
1401-
findPositionNoImports Nothing _ fileContents = findNextPragmaPosition fileContents
1402-
findPositionNoImports _ (Just hsmodExports) _ = findPositionFromImportsOrModuleDecl hsmodExports id False
1403-
findPositionNoImports (Just hsmodName) _ _ = findPositionFromImportsOrModuleDecl hsmodName id False
1407+
findPositionNoImports :: ParsedSource -> T.Text -> Maybe Int
1408+
findPositionNoImports (L _ HsModule {..}) fileContents =
1409+
case hsmodName of
1410+
Nothing -> Just $ findNextPragmaPosition fileContents
1411+
Just hsmodName' -> case hsmodAnn of
1412+
EpAnn _ annsModule _ ->
1413+
let prevSrcSpan = maybe (getLoc hsmodName') getLoc hsmodExports
1414+
in do
1415+
whereLocation <- fmap NE.head . NE.nonEmpty . mapMaybe filterWhere . am_main $ annsModule
1416+
epaLocationToLine prevSrcSpan whereLocation
1417+
EpAnnNotUsed -> Nothing
1418+
where
1419+
filterWhere (AddEpAnn AnnWhere loc) = Just loc
1420+
filterWhere _ = Nothing
1421+
1422+
epaLocationToLine :: SrcSpan -> EpaLocation -> Maybe Int
1423+
epaLocationToLine _ (EpaSpan sp) =
1424+
let loc = realSrcSpanEnd sp
1425+
in Just $ srcLocLine loc
1426+
epaLocationToLine (UnhelpfulSpan _) _ = Nothing
1427+
epaLocationToLine (RealSrcSpan prevSrcSpan _) (EpaDelta deltaPos _) =
1428+
case deltaPos of
1429+
SameLine _ -> Just prevEndLine
1430+
DifferentLine line _ -> Just $ prevEndLine + line
1431+
where
1432+
prevEndLine = srcLocLine (realSrcSpanEnd prevSrcSpan)
1433+
1434+
showAddEpAnns :: [AddEpAnn] -> String
1435+
showAddEpAnns = unlines . fmap showAddEpAnn
1436+
1437+
showAddEpAnn :: AddEpAnn -> String
1438+
showAddEpAnn (AddEpAnn keywordId loc) = show keywordId ++ "," ++ showEpaLocation loc
1439+
1440+
showEpaLocation :: EpaLocation -> String
1441+
showEpaLocation (EpaDelta pos _) = show pos
1442+
showEpaLocation _ = error "should not be EpaSpan"
14041443

1405-
findPositionFromImportsOrModuleDecl :: HasSrcSpan a => t -> (t -> a) -> Bool -> Maybe ((Int, Int), Int)
1406-
findPositionFromImportsOrModuleDecl hsField f hasImports = case getLoc (f hsField) of
1444+
findPositionFromImportsOrModuleDecl :: HasSrcSpan a => t -> (t -> a) -> Maybe ((Int, Int), Int)
1445+
findPositionFromImportsOrModuleDecl hsField f = case getLoc (f hsField) of
14071446
RealSrcSpan s _ ->
14081447
let col = calcCol s
14091448
in Just ((srcLocLine (realSrcSpanEnd s), col), col)
14101449
_ -> Nothing
1411-
where calcCol s = if hasImports then srcLocCol (realSrcSpanStart s) - 1 else 0
1450+
where calcCol s = srcLocCol (realSrcSpanStart s) - 1
14121451

14131452
-- | Find the position one after the last file-header pragma
14141453
-- Defaults to zero if there are no pragmas in file
1415-
findNextPragmaPosition :: T.Text -> Maybe ((Int, Int), Int)
1416-
findNextPragmaPosition contents = Just ((lineNumber, 0), 0)
1454+
findNextPragmaPosition :: T.Text -> Int
1455+
findNextPragmaPosition contents = lineNumber
14171456
where
14181457
lineNumber = afterLangPragma . afterOptsGhc $ afterShebang
14191458
afterLangPragma = afterPragma "LANGUAGE" contents'

ghcide/test/exe/Main.hs

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,17 @@ import System.Environment.Blank (getEnv, setEnv,
8585
unsetEnv)
8686
import System.Exit (ExitCode (ExitSuccess))
8787
import System.FilePath
88-
import System.IO.Extra hiding (withTempDir)
89-
import qualified System.IO.Extra
9088
import System.Info.Extra (isMac, isWindows)
89+
import qualified System.IO.Extra
90+
import System.IO.Extra hiding (withTempDir)
9191
import System.Mem (performGC)
9292
import System.Process.Extra (CreateProcess (cwd),
9393
createPipe, proc,
9494
readCreateProcessWithExitCode)
9595
import Test.QuickCheck
9696
-- import Test.QuickCheck.Instances ()
9797
import Control.Concurrent.Async
98-
import Control.Lens (to, (^.), (.~))
98+
import Control.Lens (to, (.~), (^.))
9999
import Control.Monad.Extra (whenJust)
100100
import Data.Function ((&))
101101
import Data.IORef
@@ -123,8 +123,9 @@ import qualified HieDbRetry
123123
import Ide.PluginUtils (pluginDescToIdePlugins)
124124
import Ide.Types
125125
import qualified Language.LSP.Types as LSP
126+
import Language.LSP.Types.Lens (didChangeWatchedFiles,
127+
workspace)
126128
import qualified Language.LSP.Types.Lens as L
127-
import Language.LSP.Types.Lens (workspace, didChangeWatchedFiles)
128129
import qualified Progress
129130
import System.Time.Extra
130131
import Test.Tasty
@@ -901,22 +902,16 @@ watchedFilesTests = testGroup "watched files"
901902

902903
insertImportTests :: TestTree
903904
insertImportTests = testGroup "insert import"
904-
[ expectFailBecause
905-
("'findPositionFromImportsOrModuleDecl' function adds import directly under line with module declaration, "
906-
++ "not accounting for case when 'where' keyword is placed on lower line")
907-
(checkImport
908-
"module where keyword lower in file no exports"
909-
"WhereKeywordLowerInFileNoExports.hs"
910-
"WhereKeywordLowerInFileNoExports.expected.hs"
911-
"import Data.Int")
912-
, expectFailBecause
913-
("'findPositionFromImportsOrModuleDecl' function adds import directly under line with module exports list, "
914-
++ "not accounting for case when 'where' keyword is placed on lower line")
915-
(checkImport
916-
"module where keyword lower in file with exports"
917-
"WhereDeclLowerInFile.hs"
918-
"WhereDeclLowerInFile.expected.hs"
919-
"import Data.Int")
905+
[ checkImport
906+
"module where keyword lower in file no exports"
907+
"WhereKeywordLowerInFileNoExports.hs"
908+
"WhereKeywordLowerInFileNoExports.expected.hs"
909+
"import Data.Int"
910+
, checkImport
911+
"module where keyword lower in file with exports"
912+
"WhereDeclLowerInFile.hs"
913+
"WhereDeclLowerInFile.expected.hs"
914+
"import Data.Int"
920915
, expectFailBecause
921916
"'findNextPragmaPosition' function doesn't account for case when shebang is not placed at top of file"
922917
(checkImport
@@ -5467,7 +5462,7 @@ completionDocTests =
54675462
-- We ignore doc uris since it points to the local path which determined by specific machines
54685463
case mn of
54695464
Nothing -> txt
5470-
Just n -> T.take n txt
5465+
Just n -> T.take n txt
54715466
| CompletionItem {_documentation = Just (CompletionDocMarkup (MarkupContent MkMarkdown txt)), ..} <- compls
54725467
, _label == label
54735468
]
@@ -5767,13 +5762,13 @@ knownBrokenFor = knownIssueFor Broken
57675762
knownIssueFor :: IssueSolution -> BrokenTarget -> String -> TestTree -> TestTree
57685763
knownIssueFor solution = go . \case
57695764
BrokenSpecific bos vers -> isTargetOS bos && isTargetGhc vers
5770-
BrokenForOS bos -> isTargetOS bos
5771-
BrokenForGHC vers -> isTargetGhc vers
5765+
BrokenForOS bos -> isTargetOS bos
5766+
BrokenForGHC vers -> isTargetGhc vers
57725767
where
57735768
isTargetOS = \case
57745769
Windows -> isWindows
5775-
MacOS -> isMac
5776-
Linux -> not isWindows && not isMac
5770+
MacOS -> isMac
5771+
Linux -> not isWindows && not isMac
57775772

57785773
isTargetGhc = elem ghcVersion
57795774

0 commit comments

Comments
 (0)