Appropriately deal with files that don't end with a line terminator

hls-plugin-api/src/Ide/PluginUtils.hs

@@ -1,5 +1,7 @@
 {-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE TypeFamilies      #-}
+{-# LANGUAGE BlockArguments #-}
+{-# LANGUAGE LambdaCase #-}
 
 module Ide.PluginUtils
   ( -- * LSP Range manipulation functions
@@ -13,6 +15,7 @@ module Ide.PluginUtils
     makeDiffTextEditAdditive,
     diffText,
     diffText',
+    diffTextEdit,
     pluginDescToIdePlugins,
     idePluginsToPluginDesc,
     getClientConfig,
@@ -41,23 +44,56 @@ where
 
 import           Control.Arrow               ((&&&))
 import           Control.Lens                (_head, _last, re, (%~), (^.))
-import           Data.Algorithm.Diff
-import           Data.Algorithm.DiffOutput
+import Data.Algorithm.Diff ( getGroupedDiff )
+import Data.Algorithm.DiffOutput
+    ( LineRange(..), DiffOperation(..), diffToLineRanges )
 import           Data.Char                   (isPrint, showLitChar)
 import           Data.Functor                (void)
 import qualified Data.Map                    as M
 import qualified Data.Text                   as T
 import           Data.Void                   (Void)
-import           Ide.Plugin.Config
-import           Ide.Plugin.Properties
-import           Ide.Types
+import Ide.Plugin.Config ( Config, PluginConfig(plcConfig) )
+import Ide.Plugin.Properties
+    ( HasProperty,
+      KeyNameProxy,
+      Properties,
+      ToHsType,
+      useProperty,
+      (&) )
+import Ide.Types
+    ( PluginDescriptor(pluginCommands, pluginId),
+      IdePlugins(IdePlugins),
+      PluginId,
+      PluginCommand(commandId),
+      getProcessID,
+      configForPlugin,
+      mkLspCommand,
+      mkLspCmdId,
+      getPid,
+      installSigUsr1Handler,
+      PluginMethod(handlesRequest) )
 import qualified Language.LSP.Protocol.Lens  as L
-import           Language.LSP.Protocol.Types
-import           Language.LSP.Server
+import Language.LSP.Protocol.Types
+    ( Range(Range),
+      Position(Position),
+      ClientCapabilities(ClientCapabilities),
+      VersionedTextDocumentIdentifier,
+      WorkspaceEdit(WorkspaceEdit),
+      TextEdit(..),
+      TextDocumentEdit(TextDocumentEdit),
+      type (|?)(InL),
+      WorkspaceClientCapabilities(_workspaceEdit),
+      WorkspaceEditClientCapabilities(WorkspaceEditClientCapabilities),
+      positionInRange,
+      _versionedTextDocumentIdentifier,
+      isSubrangeOf )
+import Language.LSP.Server ( MonadLsp, getConfig )
 import           System.FilePath             ((</>))
 import qualified Text.Megaparsec             as P
 import qualified Text.Megaparsec.Char        as P
 import qualified Text.Megaparsec.Char.Lexer  as P
+import Control.Lens.Setter (over)
+import Data.Tuple.Extra (both)
 
 -- ---------------------------------------------------------------------
 
@@ -119,18 +155,25 @@ makeDiffTextEditAdditive :: T.Text -> T.Text -> [TextEdit]
 makeDiffTextEditAdditive f1 f2 = diffTextEdit f1 f2 SkipDeletions
 
 diffTextEdit :: T.Text -> T.Text -> WithDeletions -> [TextEdit]
-diffTextEdit fText f2Text withDeletions = r
+diffTextEdit fText f2Text withDeletions
+  = newlineFix $ map diffOperationToTextEdit diffOps
   where
-    r = map diffOperationToTextEdit diffOps
-    d = getGroupedDiff (lines $ T.unpack fText) (lines $ T.unpack f2Text)
-
+    (linesL, linesR) = both (lines . T.unpack) (fText, f2Text)
+
+    newlineFix = over _last \case edit | T.last fText /= '\n'
+                                        , let lineCount = fromIntegral (length linesL)
+                                              insertionLine = edit ^. L.range . L.start . L.line
+                                        , insertionLine >= lineCount
+                                         -> edit & L.newText %~ rotateRight
+                                              where rotateRight xs = T.last xs `T.cons` T.init xs
+                                  edit -> edit
     diffOps =
       filter
         (\x -> (withDeletions == IncludeDeletions) || not (isDeletion x))
-        (diffToLineRanges d)
-
-    isDeletion (Deletion _ _) = True
-    isDeletion _              = False
+        (diffToLineRanges $ getGroupedDiff linesL linesR)
+      where
+        isDeletion (Deletion _ _) = True
+        isDeletion _              = False
 
     diffOperationToTextEdit :: DiffOperation LineRange -> TextEdit
     diffOperationToTextEdit (Change fm to) = TextEdit range nt

hls-plugin-api/test/Ide/PluginUtilsTest.hs

@@ -20,9 +20,9 @@ import           Ide.Plugin.Properties       (KeyNamePath (..),
                                               usePropertyByPath,
                                               usePropertyByPathEither)
 import qualified Ide.Plugin.RangeMap         as RangeMap
-import           Ide.PluginUtils             (extractTextInRange, unescape)
+import           Ide.PluginUtils             (extractTextInRange, unescape, diffTextEdit, WithDeletions (IncludeDeletions))
 import           Language.LSP.Protocol.Types (Position (..), Range (Range),
-                                              UInt, isSubrangeOf)
+                                              UInt, isSubrangeOf, TextEdit(_newText))
 import           Test.Tasty
 import           Test.Tasty.Golden           (goldenVsStringDiff)
 import           Test.Tasty.HUnit
@@ -31,6 +31,7 @@ import           Test.Tasty.QuickCheck
 tests :: TestTree
 tests = testGroup "PluginUtils"
     [ unescapeTest
+    , diffTextEditTest
     , extractTextInRangeTest
     , localOption (QuickCheckMaxSize 10000) $
         testProperty "RangeMap-List filtering identical" $
@@ -56,6 +57,56 @@ unescapeTest = testGroup "unescape"
         unescape "\"\\n\\t\"" @?= "\"\\n\\t\""
     ]
 
+diffTextEditTest :: TestTree
+diffTextEditTest = testGroup "diffTextEdit"
+    [ testGroup "inserting line at EOF"
+      [ testCase "both newline-terminated (linux-style vs linux-style)"
+          $ diffTextEditComplete "foo\n"
+                                 "foo\nbar\n"
+                        @?= [textEdit "bar\n"
+                                      (mkRange 1 0 1 0)]
+      , testCase "neither newline-terminated (win-style vs win-style)"
+          $ diffTextEditComplete "foo"
+                                 "foo\nbar"
+                      @?= [textEdit "\nbar"
+                                    (mkRange 0 3 0 3)]
+      , testCase "only left newline-terminated"
+          $ diffTextEditComplete "foo\n"
+                                 "foo\nbar"
+                        @?= [textEdit "bar"
+                                      (mkRange 1 0 1 0)]
+      , testCase "only right newline-terminated"
+          $ diffTextEditComplete "foo"
+                                 "foo\nbar\n"
+                      @?= [textEdit "\nbar\n"
+                                    (mkRange 0 3 0 3)]
+      ]
+    , testGroup "deleting line at EOF"
+      [ testCase "both newline-terminated (linux-style vs linux-style)"
+          $ diffTextEditComplete "foo\nbar\n"
+                                 "foo\n"
+                        @?= [textEdit ""
+                                      (mkRange 1 0 1 4)]
+      , testCase "neither newline-terminated (win-style vs win-style)"
+          $ diffTextEditComplete "foo\nbar"
+                                 "foo"
+                      @?= [textEdit ""
+                                    (mkRange 0 3 1 3)]
+      , testCase "only left newline-terminated"
+          $ diffTextEditComplete "foo\nbar"
+                                 "foo\n"
+                        @?= [textEdit ""
+                                      (mkRange 1 0 1 3)]
+      , testCase "only right newline-terminated"
+          $ diffTextEditComplete "foo\nbar\n"
+                                 "foo"
+                      @?= [textEdit ""
+                                    (mkRange 0 3 1 4)]
+      ]
+    ]
+    where diffTextEditComplete from to = diffTextEdit from to IncludeDeletions
+          textEdit = flip TextEdit
+
 extractTextInRangeTest :: TestTree
 extractTextInRangeTest = testGroup "extractTextInRange"
     [ testCase "inline range" $

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

@@ -46,6 +46,8 @@ codeActionTests = testGroup
       ]
   , goldenWithClass "Creates a placeholder for '=='" "T1" "eq" $
       getActionByTitle "Add placeholders for '=='"
+  , goldenWithClass "Like previous one, but this file has no line terminator" "T1W" "eq" $
+      getActionByTitle "Add placeholders for '=='"
   , goldenWithClass "Creates a placeholder for '/='" "T1" "ne" $
       getActionByTitle "Add placeholders for '/='"
   , goldenWithClass "Creates a placeholder for both '==' and '/='" "T1" "all" $

plugins/hls-class-plugin/test/testdata/T1W.eq.expected.hs

@@ -0,0 +1,6 @@
+module T1 where
+
+data X = X
+
+instance Eq X where
+  (==) = _

plugins/hls-class-plugin/test/testdata/T1W.hs

@@ -0,0 +1,5 @@
+module T1 where
+
+data X = X
+
+instance Eq X where