Allow renaming when cursor is at end of alias

Author: izuzu

plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs

@@ -110,7 +110,7 @@ prepareRenameProvider state _pluginId (PrepareRenameParams (TextDocumentIdentifi
                     HAR{hieAst} <- handleGetHieAst state nfp
                     let spansWithNamesUnderCursor =
                             [ srcSpan
-                            | (names, srcSpan) <- getNamesSpansAtPoint' hieAst pos
+                            | (names, srcSpan) <- getNamesSpansAtPoint' hieAst lspPos
                             , not (null names)]
                     -- When this handler says that rename is invalid, VSCode shows "The element can't be renamed"
                     -- and doesn't even allow you to create full rename request.
@@ -304,10 +304,10 @@ getCodePointPosition state nfp pos = do
             "The cursor position is inside a Unicode surrogate pair."
         Just codePointPosition -> pure codePointPosition
 
--- TODO: 'getNamesAtPos' passes the LSP 'Position' directly to 'pointCommand',
+-- FIXME: 'getNamesAtPos' passes the LSP 'Position' directly to 'pointCommand',
 -- which treats '_character' as a code-point column. This is incorrect for
 -- files with supplementary-plane Unicode characters before the cursor.
--- Fixing it requires changes in ghcide, not here.
+-- Fixing it requires changes to 'pointCommand' in ghcide, not here.
 getNamesAtPos :: MonadIO m => IdeState -> NormalizedFilePath -> Position -> ExceptT PluginError m [Name]
 getNamesAtPos state nfp pos = do
     HAR{hieAst} <- handleGetHieAst state nfp
@@ -352,11 +352,13 @@ collectWith :: (Hashable a, Eq b) => (a -> b) -> HashSet a -> [(b, HashSet a)]
 collectWith f = map (\(a :| as) -> (f a, HS.fromList (a:as))) . groupWith f . HS.toList
 
 -- | A variant 'getNamesAtPoint' that does not expect a 'PositionMapping'
+-- FIXME: The use of 'pointCommand' is problematic. See 'getNamesAtPos' above.
 getNamesAtPoint' :: HieASTs a -> Position -> [Name]
 getNamesAtPoint' hf pos =
   concat $ pointCommand hf pos (rights . M.keys . getNodeIds)
 
 -- | A variant of `getNamesAtPoint'` that also returns source spans.
+-- FIXME: The use of 'pointCommand' is problematic. See 'getNamesAtPos' above.
 getNamesSpansAtPoint' :: HieASTs a -> Position -> [([Name], RealSrcSpan)]
 getNamesSpansAtPoint' hf pos =
   pointCommand hf pos $

plugins/hls-rename-plugin/src/Ide/Plugin/Rename/ImportAlias.hs

@@ -174,7 +174,7 @@ findAliasDeclAtPos pos imports = listToMaybe $ do
     Just locatedAlias <- [ideclAs importDecl]
     RealSrcSpan aliasDeclSpan _ <- [getLoc locatedAlias]
     let aliasDeclRange = realSrcSpanToCodePointRange aliasDeclSpan
-    guard (rangeContainsPosition aliasDeclRange pos)
+    guard (rangeContainsPositionInclusive aliasDeclRange pos)
     let aliasModuleName = unLoc (ideclName importDecl)
         aliasName = unLoc locatedAlias
         aliasIsShared = length (filter (== aliasName) allAliases) > 1
@@ -194,12 +194,12 @@ findAliasUseAtPos pos imports hsDecls =
             Qual qualifier _ <- [unLoc locatedRdrName]
             RealSrcSpan qualifiedNameSpan _ <- [getLoc locatedRdrName]
             let qualifiedNameRange = realSrcSpanToCodePointRange qualifiedNameSpan
-            guard (rangeContainsPosition qualifiedNameRange pos)
+            guard (rangeContainsPositionInclusive qualifiedNameRange pos)
             let qualifierLength = fromIntegral (moduleNameLength qualifier)
                 qualifierStart = qualifiedNameRange ^. VFS.start
                 qualifierRange = qualifiedNameRange
                     & VFS.end .~ (qualifierStart & VFS.character +~ qualifierLength)
-            guard (rangeContainsPosition qualifierRange pos)
+            guard (rangeContainsPositionInclusive qualifierRange pos)
             [(qualifierRange, qualifier)]
     in case qualifiersAtPos of
         [] -> Nothing
@@ -295,16 +295,18 @@ ambiguousAliasErrorMessage _ = ""
 ---------------------------------------------------------------------------------------------------
 -- Utility functions
 
--- | Check whether a 'CodePointRange' contains a 'CodePointPosition'
--- (inclusive start, exclusive end).
-rangeContainsPosition :: VFS.CodePointRange -> VFS.CodePointPosition -> Bool
-rangeContainsPosition
+-- | Check whether a 'CodePointRange' contains a 'CodePointPosition' (inclusive
+-- start, inclusive end).
+-- NOTE: The use of inclusive end allows the user to place the cursor at the end
+-- of an import alias and rename it.
+rangeContainsPositionInclusive :: VFS.CodePointRange -> VFS.CodePointPosition -> Bool
+rangeContainsPositionInclusive
     (VFS.CodePointRange
         (VFS.CodePointPosition startLine startColumn)
         (VFS.CodePointPosition endLine endColumn))
     (VFS.CodePointPosition posLine posColumn)
     =  (posLine > startLine || (posLine == startLine && posColumn >= startColumn))
-    && (posLine < endLine   || (posLine == endLine   && posColumn <  endColumn))
+    && (posLine < endLine   || (posLine == endLine   && posColumn <= endColumn))
 
 -- | Build a 'TextEdit' from a 'VFS.CodePointRange' and replacement text.
 -- Returns @Nothing@ if the range is out of bounds in the VFS.

plugins/hls-rename-plugin/test/Main.hs

@@ -37,6 +37,28 @@ prepareRenameTests = testGroup "PrepareRename"
         result <- prepareRename doc (Position 0 9)
         liftIO $ result @?= InR Null
 
+    , testCase "Import alias in declaration" $ runRenameSession "" $ do
+        doc <- openDoc "PrepareRename.hs" "haskell"
+        void waitForBuildQueue
+        let expected = InL (PrepareRenameResult (InL (Range (Position 2 24) (Position 2 25))))
+        resultAtStart <- prepareRename doc (Position 2 24)
+        liftIO $ resultAtStart @?= expected
+        resultAtEnd <- prepareRename doc (Position 2 25)
+        liftIO $ resultAtEnd @?= expected
+        resultOutside <- prepareRename doc (Position 2 26)
+        liftIO $ resultOutside /= expected @? "Cursor is outside alias"
+
+    , testCase "Import alias at use site" $ runRenameSession "" $ do
+        doc <- openDoc "PrepareRename.hs" "haskell"
+        void waitForBuildQueue
+        let expected = InL (PrepareRenameResult (InL (Range (Position 10 14) (Position 10 15))))
+        resultAtStart <- prepareRename doc (Position 10 14)
+        liftIO $ resultAtStart @?= expected
+        resultAtEnd <- prepareRename doc (Position 10 15)
+        liftIO $ resultAtEnd @?= expected
+        resultOutside <- prepareRename doc (Position 10 16)
+        liftIO $ resultOutside /= expected @? "Cursor is outside qualifier"
+
     , testCase "Function name" $ runRenameSession "" $ do
         doc <- openDoc "PrepareRename.hs" "haskell"
         void waitForBuildQueue
@@ -96,7 +118,7 @@ renameTests = testGroup "Identifier"
     , goldenWithRename "Import alias declaration" "ImportAlias" $ \doc ->
         rename doc (Position 1 14) "G"
     , goldenWithRename "Import alias at use site" "ImportAlias" $ \doc ->
-        rename doc (Position 5 6) "G"
+        rename doc (Position 5 10) "G"
     , goldenWithRename "Import alias declaration (shared by unrelated imports)" "ImportAliasShared" $ \doc ->
         rename doc (Position 1 31) "Maybe"
     , goldenWithRename "Import alias at use site (shared by unrelated imports)" "ImportAliasShared" $ \doc ->