From 51e0d1c6883cc07ce2168a189f18c4a6569955dd Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 10 Sep 2022 23:17:06 +0200
Subject: [PATCH 01/15] support add-argument action

---
 .../src/Development/IDE/GHC/ExactPrint.hs     |  46 +++-
 .../src/Development/IDE/Plugin/CodeAction.hs  | 120 +++++++---
 plugins/hls-refactor-plugin/test/Main.hs      | 212 +++++++++++++++++-
 3 files changed, 341 insertions(+), 37 deletions(-)

diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
index d56b513a79..9e0d54d9e6 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
@@ -29,7 +29,10 @@ module Development.IDE.GHC.ExactPrint
       addParensToCtxt,
       modifyAnns,
       removeComma,
+      modifySmallestDeclWithM,
+      modifyMgMatchesT,
       -- * Helper function
+      spanContainsRange,
       eqSrcSpan,
       epl,
       epAnn,
@@ -42,7 +45,7 @@ module Development.IDE.GHC.ExactPrint
       ExceptStringT (..),
       TransformT,
       Log(..),
-    )
+      )
 where
 
 import           Control.Applicative                     (Alternative)
@@ -98,10 +101,11 @@ import           GHC                                     (EpAnn (..),
                                                           SrcSpanAnnA,
                                                           TrailingAnn (AddCommaAnn),
                                                           emptyComments,
-                                                          spanAsAnchor)
+                                                          spanAsAnchor, spans)
 import           GHC.Parser.Annotation                   (AnnContext (..),
                                                           DeltaPos (SameLine),
                                                           EpaLocation (EpaDelta))
+import Data.Maybe (fromMaybe)
 #endif
 
 ------------------------------------------------------------------------------
@@ -114,10 +118,10 @@ instance Pretty Log where
 
 instance Show (Annotated ParsedSource) where
   show _ = "<Annotated ParsedSource>"
- 
+
 instance NFData (Annotated ParsedSource) where
   rnf = rwhnf
- 
+
 data GetAnnotatedParsedSource = GetAnnotatedParsedSource
   deriving (Eq, Show, Typeable, GHC.Generic)
 
@@ -430,6 +434,32 @@ graftDecls dst decs0 = Graft $ \dflags a -> do
             | otherwise = DL.singleton (L src e) <> go rest
     modifyDeclsT (pure . DL.toList . go) a
 
+modifySmallestDeclWithM ::
+    forall a.
+    (HasDecls a) =>
+    (SrcSpan -> Bool) ->
+    (LHsDecl GhcPs -> TransformT (Either String) (Maybe [LHsDecl GhcPs])) ->
+    a ->
+    TransformT (Either String) a
+modifySmallestDeclWithM validSpan f a = do
+    let modifyMatchingDecl [] = pure DL.empty
+        modifyMatchingDecl (e@(L src _) : rest)
+            | validSpan $ locA src = do
+                decs' <- fromMaybe [e] <$> f e
+                pure $ DL.fromList decs' <> DL.fromList rest
+            | otherwise = (DL.singleton e <>) <$> modifyMatchingDecl rest
+    modifyDeclsT (fmap DL.toList . modifyMatchingDecl) a
+
+modifyMgMatchesT ::
+  Monad m =>
+  MatchGroup GhcPs (LHsExpr GhcPs)
+  -> (LMatch GhcPs (LHsExpr GhcPs) -> TransformT m (LMatch GhcPs (LHsExpr GhcPs)))
+  -> TransformT m (MatchGroup GhcPs (LHsExpr GhcPs))
+modifyMgMatchesT (MG xMg (L locMatches matches) originMg) f = do
+    matches' <- forM matches f
+    let decl' = (MG xMg (L locMatches matches') originMg)
+    pure decl'
+
 graftSmallestDeclsWithM ::
     forall a.
     (HasDecls a) =>
@@ -623,6 +653,14 @@ eqSrcSpanA l r = leftmost_smallest l r == EQ
 #endif
 
 #if MIN_VERSION_ghc(9,2,0)
+
+spanContainsRange :: SrcSpan -> Range -> Bool
+spanContainsRange srcSpan Range {..} =
+  srcSpan `spans` positionToTuple _start && srcSpan `spans` positionToTuple _end
+  where
+    positionToTuple :: Position -> (Int, Int)
+    positionToTuple (Position l c) = (fromIntegral l + 1, fromIntegral c)
+
 addParensToCtxt :: Maybe EpaLocation -> AnnContext -> AnnContext
 addParensToCtxt close_dp = addOpen . addClose
   where
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index 01c3b555c1..1eb8f4f906 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -38,7 +38,7 @@ import           Data.Ord                                          (comparing)
 import qualified Data.Set                                          as S
 import qualified Data.Text                                         as T
 import qualified Data.Text.Utf16.Rope                              as Rope
-import           Data.Tuple.Extra                                  (fst3)
+import           Data.Tuple.Extra                                  (fst3, first)
 import           Development.IDE.Types.Logger                      hiding (group)
 import           Development.IDE.Core.Rules
 import           Development.IDE.Core.RuleTypes
@@ -62,7 +62,7 @@ import           Development.IDE.Types.Exports
 import           Development.IDE.Types.Location
 import           Development.IDE.Types.Options
 import qualified GHC.LanguageExtensions                            as Lang
-import           Ide.PluginUtils                                   (subRange)
+import           Ide.PluginUtils                                   (subRange, makeDiffTextEdit)
 import           Ide.Types
 import qualified Language.LSP.Server                               as LSP
 import           Language.LSP.Types                                (ApplyWorkspaceEditParams(..), CodeAction (..),
@@ -82,7 +82,7 @@ import           Language.LSP.Types                                (ApplyWorkspa
                                                                     WorkspaceEdit (WorkspaceEdit, _changeAnnotations, _changes, _documentChanges),
                                                                     type (|?) (InR),
                                                                     uriToFilePath)
-import           GHC.Exts                                           (fromList)
+import           GHC.Exts                                           (IsList (fromList))
 import           Language.LSP.VFS                                  (VirtualFile,
                                                                     _file_text)
 import           Text.Regex.TDFA                                   (mrAfter,
@@ -96,7 +96,9 @@ import           GHC                                               (AddEpAnn (Ad
                                                                     EpAnn (..),
                                                                     EpaLocation (..),
                                                                     LEpaComment,
-                                                                    LocatedA)
+                                                                    LocatedA, spans)
+import Language.Haskell.GHC.ExactPrint (runTransformFromT, noAnnSrcSpanDP1, runTransform, runTransformT)
+import GHC.Types.SrcLoc (generatedSrcSpan)
 
 #else
 import           Language.Haskell.GHC.ExactPrint.Types             (Annotation (annsDP),
@@ -167,6 +169,7 @@ bindingsPluginDescriptor recorder plId = mkExactprintPluginDescriptor recorder $
     , wrap suggestImplicitParameter
 #endif
     , wrap suggestNewDefinition
+    , wrap suggestAddArgument
     , wrap suggestDeleteUnusedBinding
     ]
     plId
@@ -242,7 +245,7 @@ extendImportHandler' ideState ExtendImport {..}
                   Nothing -> newThing
                   Just p  -> p <> "(" <> newThing <> ")"
             t <- liftMaybe $ snd <$> newImportToEdit n ps (fromMaybe "" contents)
-            return (nfp, WorkspaceEdit {_changes=Just (fromList [(doc,List [t])]), _documentChanges=Nothing, _changeAnnotations=Nothing})
+            return (nfp, WorkspaceEdit {_changes=Just (GHC.Exts.fromList [(doc,List [t])]), _documentChanges=Nothing, _changeAnnotations=Nothing})
   | otherwise =
     mzero
 
@@ -389,7 +392,7 @@ suggestHideShadow ps fileContents mTcM mHar Diagnostic {_message, _range}
   | otherwise = []
   where
     L _ HsModule {hsmodImports} = astA ps
- 
+
     suggests identifier modName s
       | Just tcM <- mTcM,
         Just har <- mHar,
@@ -845,34 +848,93 @@ suggestReplaceIdentifier contents Diagnostic{_range=_range,..}
         = [ ("Replace with ‘" <> name <> "’", [mkRenameEdit contents _range name]) | name <- renameSuggestions ]
     | otherwise = []
 
+matchVariableNotInScope :: T.Text -> Maybe (T.Text, Maybe T.Text)
+matchVariableNotInScope message
+  --     * Variable not in scope:
+  --         suggestAcion :: Maybe T.Text -> Range -> Range
+  --     * Variable not in scope:
+  --         suggestAcion
+  | Just (name, typ) <- matchVariableNotInScopeTyped message = Just (name, Just typ)
+  | Just name <- matchVariableNotInScopeUntyped message = Just (name, Nothing)
+  | otherwise = Nothing
+  where
+    matchVariableNotInScopeTyped message
+      | Just [name, typ] <- matchRegexUnifySpaces message "Variable not in scope: ([^ ]+) :: ([^*•]+)" =
+          Just (name, typ)
+      | otherwise = Nothing
+    matchVariableNotInScopeUntyped message
+      | Just [name] <- matchRegexUnifySpaces message "Variable not in scope: ([^ ]+)" =
+          Just name
+      | otherwise = Nothing
+
+matchFoundHole :: T.Text -> Maybe (T.Text, T.Text)
+matchFoundHole message
+  | Just [name, typ] <- matchRegexUnifySpaces message "Found hole: _([^ ]+) :: ([^*•]+) Or perhaps" =
+      Just (name, typ)
+  | otherwise = Nothing
+
+matchFoundHoleIncludeUnderscore :: T.Text -> Maybe (T.Text, T.Text)
+matchFoundHoleIncludeUnderscore message = first ("_" <>) <$> matchFoundHole message
+
 suggestNewDefinition :: IdeOptions -> ParsedModule -> Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])]
-suggestNewDefinition ideOptions parsedModule contents Diagnostic{_message, _range}
---     * Variable not in scope:
---         suggestAcion :: Maybe T.Text -> Range -> Range
-    | Just [name, typ] <- matchRegexUnifySpaces message "Variable not in scope: ([^ ]+) :: ([^*•]+)"
-    = newDefinitionAction ideOptions parsedModule _range name typ
-    | Just [name, typ] <- matchRegexUnifySpaces message "Found hole: _([^ ]+) :: ([^*•]+) Or perhaps"
-    , [(label, newDefinitionEdits)] <- newDefinitionAction ideOptions parsedModule _range name typ
-    = [(label, mkRenameEdit contents _range name : newDefinitionEdits)]
-    | otherwise = []
-    where
-      message = unifySpaces _message
+suggestNewDefinition ideOptions parsedModule contents Diagnostic {_message, _range}
+  | Just (name, typ) <- matchVariableNotInScope message =
+      newDefinitionAction ideOptions parsedModule _range name typ
+  | Just (name, typ) <- matchFoundHole message,
+    [(label, newDefinitionEdits)] <- newDefinitionAction ideOptions parsedModule _range name (Just typ) =
+      [(label, mkRenameEdit contents _range name : newDefinitionEdits)]
+  | otherwise = []
+  where
+    message = unifySpaces _message
 
-newDefinitionAction :: IdeOptions -> ParsedModule -> Range -> T.Text -> T.Text -> [(T.Text, [TextEdit])]
-newDefinitionAction IdeOptions{..} parsedModule Range{_start} name typ
-    | Range _ lastLineP : _ <-
+newDefinitionAction :: IdeOptions -> ParsedModule -> Range -> T.Text -> Maybe T.Text -> [(T.Text, [TextEdit])]
+newDefinitionAction IdeOptions {..} parsedModule Range {_start} name typ
+  | Range _ lastLineP : _ <-
       [ realSrcSpanToRange sp
-      | (L (locA -> l@(RealSrcSpan sp _)) _) <- hsmodDecls
-      , _start `isInsideSrcSpan` l]
-    , nextLineP <- Position{ _line = _line lastLineP + 1, _character = 0}
-    = [ ("Define " <> sig
-        , [TextEdit (Range nextLineP nextLineP) (T.unlines ["", sig, name <> " = _"])]
-        )]
-    | otherwise = []
+        | (L (locA -> l@(RealSrcSpan sp _)) _) <- hsmodDecls,
+          _start `isInsideSrcSpan` l
+      ],
+    nextLineP <- Position {_line = _line lastLineP + 1, _character = 0} =
+      [ ( "Define " <> sig,
+          [TextEdit (Range nextLineP nextLineP) (T.unlines ["", sig, name <> " = _"])]
+        )
+      ]
+  | otherwise = []
   where
     colon = if optNewColonConvention then " : " else " :: "
-    sig = name <> colon <> T.dropWhileEnd isSpace typ
-    ParsedModule{pm_parsed_source = L _ HsModule{hsmodDecls}} = parsedModule
+    sig = name <> colon <> T.dropWhileEnd isSpace (fromMaybe "_" typ)
+    ParsedModule {pm_parsed_source = L _ HsModule {hsmodDecls}} = parsedModule
+
+suggestAddArgument :: ParsedModule -> Diagnostic -> [(T.Text, [TextEdit])]
+suggestAddArgument parsedModule Diagnostic {_message, _range}
+  | Just (name, typ) <- matchVariableNotInScope message = addArgumentAction parsedModule _range name typ
+  | Just (name, typ) <- matchFoundHoleIncludeUnderscore message = addArgumentAction parsedModule _range name (Just typ)
+  | otherwise = []
+  where
+    message = unifySpaces _message
+
+-- TODO use typ to modify type signature
+addArgumentAction :: ParsedModule -> Range -> T.Text -> Maybe T.Text -> [(T.Text, [TextEdit])]
+addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
+  do
+    let addArgToMatch = \(L locMatch (Match xMatch ctxMatch pats rhs)) -> do
+          let unqualName = mkRdrUnqual $ mkVarOcc $ T.unpack name
+          let newPat = L (noAnnSrcSpanDP1 generatedSrcSpan) $ VarPat NoExtField (noLocA unqualName)
+          pure $ L locMatch (Match xMatch ctxMatch (pats <> [newPat]) rhs)
+        insertArg = \case
+          (L locDecl (ValD xVal (FunBind xFunBind idFunBind mg coreFunBind))) -> do
+            mg' <- modifyMgMatchesT mg addArgToMatch
+            let decl' = L locDecl (ValD xVal (FunBind xFunBind idFunBind mg' coreFunBind))
+            pure $ Just [decl']
+          _ -> pure Nothing
+    case runTransformT $ modifySmallestDeclWithM (`spanContainsRange` range) insertArg (makeDeltaAst parsedSource) of
+      Left err -> error $ "Error when inserting argument: " <> err
+      Right (newSource, _, _) ->
+        let diff = makeDiffTextEdit (T.pack $ exactPrint parsedSource) (T.pack $ exactPrint newSource)
+         in [("Add argument ‘" <> name <> "’ to function", fromLspList $ diff)]
+
+fromLspList :: List a -> [a]
+fromLspList (List a) = a
 
 suggestFillTypeWildcard :: Diagnostic -> [(T.Text, TextEdit)]
 suggestFillTypeWildcard Diagnostic{_range=_range,..}
diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs
index 2a81b9085e..50ad768203 100644
--- a/plugins/hls-refactor-plugin/test/Main.hs
+++ b/plugins/hls-refactor-plugin/test/Main.hs
@@ -311,6 +311,7 @@ codeActionTests = testGroup "code actions"
   , fillTypedHoleTests
   , addSigActionTests
   , insertNewDefinitionTests
+  , addFunctionArgumentTests
   , deleteUnusedDefinitionTests
   , addInstanceConstraintTests
   , addFunctionConstraintTests
@@ -2025,7 +2026,7 @@ insertNewDefinitionTests = testGroup "insert new definition actions"
       docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ txtB ++ txtB')
       _ <- waitForDiagnostics
       InR action@CodeAction { _title = actionTitle } : _
-                  <- sortOn (\(InR CodeAction{_title=x}) -> x) <$>
+                  <- filter (\(InR CodeAction{_title=x}) -> "Define" `T.isPrefixOf` x) <$>
                      getCodeActions docB (R 0 0 0 50)
       liftIO $ actionTitle @?= "Define select :: [Bool] -> Bool"
       executeCodeAction action
@@ -2049,7 +2050,7 @@ insertNewDefinitionTests = testGroup "insert new definition actions"
       docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ txtB ++ txtB')
       _ <- waitForDiagnostics
       InR action@CodeAction { _title = actionTitle } : _
-                  <- sortOn (\(InR CodeAction{_title=x}) -> x) <$>
+                  <- filter (\(InR CodeAction{_title=x}) -> "Define" `T.isPrefixOf` x) <$>
                      getCodeActions docB (R 0 0 0 50)
       liftIO $ actionTitle @?= "Define select :: [Bool] -> Bool"
       executeCodeAction action
@@ -2083,7 +2084,7 @@ insertNewDefinitionTests = testGroup "insert new definition actions"
     docB <- createDoc "ModuleB.hs" "haskell" (T.unlines start)
     _ <- waitForDiagnostics
     InR action@CodeAction { _title = actionTitle } : _
-                <- sortOn (\(InR CodeAction{_title=x}) -> x) <$>
+                <- filter (\(InR CodeAction{_title=x}) -> "Define" `T.isPrefixOf` x) <$>
                     getCodeActions docB (R 1 0 0 50)
     liftIO $ actionTitle @?= "Define select :: Int -> Bool"
     executeCodeAction action
@@ -2109,14 +2110,217 @@ insertNewDefinitionTests = testGroup "insert new definition actions"
     docB <- createDoc "ModuleB.hs" "haskell" (T.unlines start)
     _ <- waitForDiagnostics
     InR action@CodeAction { _title = actionTitle } : _
-                <- sortOn (\(InR CodeAction{_title=x}) -> x) <$>
+                <- filter (\(InR CodeAction{_title=x}) -> "Define" `T.isPrefixOf` x) <$>
                     getCodeActions docB (R 1 0 0 50)
     liftIO $ actionTitle @?= "Define select :: Int -> Bool"
     executeCodeAction action
     contentAfterAction <- documentContents docB
     liftIO $ contentAfterAction @?= T.unlines expected
+    , testSession "insert new function definition - untyped error" $ do
+      let txtB =
+            ["foo = select"
+            ]
+          txtB' =
+            [""
+            ,"someOtherCode = ()"
+            ]
+      docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ txtB ++ txtB')
+      _ <- waitForDiagnostics
+      InR action@CodeAction { _title = actionTitle } : _
+                  <- filter (\(InR CodeAction{_title=x}) -> "Define" `T.isPrefixOf` x) <$>
+                     getCodeActions docB (R 0 0 0 50)
+      liftIO $ actionTitle @?= "Define select :: _"
+      executeCodeAction action
+      contentAfterAction <- documentContents docB
+      liftIO $ contentAfterAction @?= T.unlines (txtB ++
+        [ ""
+        , "select :: _"
+        , "select = _"
+        ]
+        ++ txtB')
   ]
 
+addFunctionArgumentTests :: TestTree
+addFunctionArgumentTests =
+  testGroup
+    "add function argument"
+    [ testSession "simple" $ do
+        let foo =
+              [ "foo True = select [True]",
+                "",
+                "foo False = False"
+              ]
+            foo' =
+              [ "foo True select = select [True]",
+                "",
+                "foo False select = False"
+              ]
+            someOtherCode =
+              [ "",
+                "someOtherCode = ()"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo ++ someOtherCode)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 0 0 0 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines (foo' ++ someOtherCode),
+      testSession "comments" $ do
+        let foo =
+              [ "foo -- c1",
+                "  True -- c2",
+                "  = -- c3",
+                "    select [True]",
+                "",
+                "foo False = False"
+              ]
+            -- TODO improve behavior slightly?
+            foo' =
+              [ "foo -- c1",
+                "  True select -- c2",
+                "  = -- c3",
+                "    select [True]",
+                "",
+                "foo False select = False"
+              ]
+            someOtherCode =
+              [ "",
+                "someOtherCode = ()"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo ++ someOtherCode)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 3 0 3 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines (foo' ++ someOtherCode),
+      testSession "leading decls" $ do
+        let foo =
+              [ "module Foo where",
+                "",
+                "bar = 1",
+                "",
+                "foo True = select [True]",
+                "",
+                "foo False = False"
+              ]
+            foo' =
+              [ "module Foo where",
+                "",
+                "bar = 1",
+                "",
+                "foo True select = select [True]",
+                "",
+                "foo False select = False"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 4 0 4 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines foo',
+      testSession "hole" $ do
+        let foo =
+              [ "module Foo where",
+                "",
+                "bar = 1",
+                "",
+                "foo True = _select [True]",
+                "",
+                "foo False = False"
+              ]
+            foo' =
+              [ "module Foo where",
+                "",
+                "bar = 1",
+                "",
+                "foo True _select = _select [True]",
+                "",
+                "foo False _select = False"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 4 0 4 50)
+        liftIO $ actionTitle @?= "Add argument ‘_select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines foo',
+      testSession "untyped error" $ do
+        let foo =
+              [ "foo = select"
+              ]
+            foo' =
+              [ "foo select = select"
+              ]
+            someOtherCode =
+              [ "",
+                "someOtherCode = ()"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo ++ someOtherCode)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 0 0 0 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines (foo' ++ someOtherCode),
+      testSession "untyped error" $ do
+        let foo =
+              [ "foo = select"
+              ]
+            foo' =
+              [ "foo select = select"
+              ]
+            someOtherCode =
+              [ "",
+                "someOtherCode = ()"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo ++ someOtherCode)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 0 0 0 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines (foo' ++ someOtherCode),
+      testSession "where clause" $ do
+        let foo =
+              [ "foo True = False ",
+                "  where",
+                "    bar = select",
+                "",
+                "foo False = False"
+              ]
+            -- TODO improve this behaviour (should add select to bar, not foo)
+            foo' =
+              [ "foo True select = False ",
+                "  where",
+                "    bar = select",
+                "",
+                "foo False select = False"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 2 0 2 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines foo'
+    ]
 
 deleteUnusedDefinitionTests :: TestTree
 deleteUnusedDefinitionTests = testGroup "delete unused definition action"

From cae508a20d8053175e7f04e58fbada5bb0a0101f Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sun, 2 Oct 2022 01:50:51 -0700
Subject: [PATCH 02/15] respond to review comments

---
 ghcide/src/Development/IDE/GHC/Error.hs       |  4 ++
 .../src/Ide/Plugin/QualifyImportedNames.hs    |  6 +-
 .../src/Development/IDE/GHC/ExactPrint.hs     | 62 ++++++++++---------
 .../src/Development/IDE/Plugin/CodeAction.hs  | 11 ++--
 4 files changed, 43 insertions(+), 40 deletions(-)

diff --git a/ghcide/src/Development/IDE/GHC/Error.hs b/ghcide/src/Development/IDE/GHC/Error.hs
index b16d908c58..c9345038f5 100644
--- a/ghcide/src/Development/IDE/GHC/Error.hs
+++ b/ghcide/src/Development/IDE/GHC/Error.hs
@@ -24,6 +24,7 @@ module Development.IDE.GHC.Error
   , zeroSpan
   , realSpan
   , isInsideSrcSpan
+  , spanContainsRange
   , noSpan
 
   -- * utilities working with severities
@@ -119,6 +120,9 @@ p `isInsideSrcSpan` r = case srcSpanToRange r of
   Just (Range sp ep) -> sp <= p && p <= ep
   _                  -> False
 
+spanContainsRange :: SrcSpan -> Range -> Bool
+spanContainsRange srcSpan Range {..} = _start `isInsideSrcSpan` srcSpan && _end `isInsideSrcSpan` srcSpan
+
 -- | Convert a GHC severity to a DAML compiler Severity. Severities below
 -- "Warning" level are dropped (returning Nothing).
 toDSeverity :: GHC.Severity -> Maybe D.DiagnosticSeverity
diff --git a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
index 6d78cee625..ef714e0711 100644
--- a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
+++ b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
@@ -87,16 +87,12 @@ descriptor pluginId = (defaultPluginDescriptor pluginId) {
     ]
 }
 
-isRangeWithinSrcSpan :: Range -> SrcSpan -> Bool
-isRangeWithinSrcSpan (Range start end) srcSpan =
-  isInsideSrcSpan start srcSpan && isInsideSrcSpan end srcSpan
-
 findLImportDeclAt :: Range -> ParsedModule -> Maybe (LImportDecl GhcPs)
 findLImportDeclAt range parsedModule
   | ParsedModule {..} <- parsedModule
   , L _ hsModule <- pm_parsed_source
   , locatedImportDecls <- hsmodImports hsModule =
-      find (\ (L (locA -> srcSpan) _) -> isRangeWithinSrcSpan range srcSpan) locatedImportDecls
+      find (\ (L (locA -> srcSpan) _) -> srcSpan `spanContainsRange` range) locatedImportDecls
 
 makeCodeActions :: Uri -> [TextEdit] -> [a |? CodeAction]
 makeCodeActions uri textEdits = [InR CodeAction {..} | not (null textEdits)]
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
index 9e0d54d9e6..5116f3f702 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
@@ -20,6 +20,10 @@ module Development.IDE.GHC.ExactPrint
       transform,
       transformM,
       ExactPrint(..),
+#if MIN_VERSION_ghc(9,2,1)
+      modifySmallestDeclWithM,
+      modifyMgMatchesT,
+#endif
 #if !MIN_VERSION_ghc(9,2,0)
       Anns,
       Annotate,
@@ -29,10 +33,7 @@ module Development.IDE.GHC.ExactPrint
       addParensToCtxt,
       modifyAnns,
       removeComma,
-      modifySmallestDeclWithM,
-      modifyMgMatchesT,
       -- * Helper function
-      spanContainsRange,
       eqSrcSpan,
       epl,
       epAnn,
@@ -106,6 +107,7 @@ import           GHC.Parser.Annotation                   (AnnContext (..),
                                                           DeltaPos (SameLine),
                                                           EpaLocation (EpaDelta))
 import Data.Maybe (fromMaybe)
+import Development.IDE.GHC.Error (isInsideSrcSpan)
 #endif
 
 ------------------------------------------------------------------------------
@@ -434,31 +436,38 @@ graftDecls dst decs0 = Graft $ \dflags a -> do
             | otherwise = DL.singleton (L src e) <> go rest
     modifyDeclsT (pure . DL.toList . go) a
 
+#if MIN_VERSION_ghc(9,2,1)
+-- | Replace the smallest declaration whose SrcSpan satisfies the given condition with a new
+-- list of declarations.
+--
+-- For example, if you would like to move a where-clause-defined variable to the same
+-- level as its parent HsDecl, you could use this function.
 modifySmallestDeclWithM ::
-    forall a.
-    (HasDecls a) =>
-    (SrcSpan -> Bool) ->
-    (LHsDecl GhcPs -> TransformT (Either String) (Maybe [LHsDecl GhcPs])) ->
-    a ->
-    TransformT (Either String) a
+  forall a.
+  (HasDecls a) =>
+  (SrcSpan -> Bool) ->
+  (LHsDecl GhcPs -> TransformT (Either String) [LHsDecl GhcPs]) ->
+  a ->
+  TransformT (Either String) a
 modifySmallestDeclWithM validSpan f a = do
-    let modifyMatchingDecl [] = pure DL.empty
-        modifyMatchingDecl (e@(L src _) : rest)
-            | validSpan $ locA src = do
-                decs' <- fromMaybe [e] <$> f e
-                pure $ DL.fromList decs' <> DL.fromList rest
-            | otherwise = (DL.singleton e <>) <$> modifyMatchingDecl rest
-    modifyDeclsT (fmap DL.toList . modifyMatchingDecl) a
-
+  let modifyMatchingDecl [] = pure DL.empty
+      modifyMatchingDecl (e@(L src _) : rest)
+        | validSpan $ locA src = do
+            decs' <- f e
+            pure $ DL.fromList decs' <> DL.fromList rest
+        | otherwise = (DL.singleton e <>) <$> modifyMatchingDecl rest
+  modifyDeclsT (fmap DL.toList . modifyMatchingDecl) a
+
+-- | Modify the each LMatch in a MatchGroup
 modifyMgMatchesT ::
   Monad m =>
-  MatchGroup GhcPs (LHsExpr GhcPs)
-  -> (LMatch GhcPs (LHsExpr GhcPs) -> TransformT m (LMatch GhcPs (LHsExpr GhcPs)))
-  -> TransformT m (MatchGroup GhcPs (LHsExpr GhcPs))
+  MatchGroup GhcPs (LHsExpr GhcPs) ->
+  (LMatch GhcPs (LHsExpr GhcPs) -> TransformT m (LMatch GhcPs (LHsExpr GhcPs))) ->
+  TransformT m (MatchGroup GhcPs (LHsExpr GhcPs))
 modifyMgMatchesT (MG xMg (L locMatches matches) originMg) f = do
-    matches' <- forM matches f
-    let decl' = (MG xMg (L locMatches matches') originMg)
-    pure decl'
+  matches' <- mapM f matches
+  pure $ MG xMg (L locMatches matches') originMg
+#endif
 
 graftSmallestDeclsWithM ::
     forall a.
@@ -654,13 +663,6 @@ eqSrcSpanA l r = leftmost_smallest l r == EQ
 
 #if MIN_VERSION_ghc(9,2,0)
 
-spanContainsRange :: SrcSpan -> Range -> Bool
-spanContainsRange srcSpan Range {..} =
-  srcSpan `spans` positionToTuple _start && srcSpan `spans` positionToTuple _end
-  where
-    positionToTuple :: Position -> (Int, Int)
-    positionToTuple (Position l c) = (fromIntegral l + 1, fromIntegral c)
-
 addParensToCtxt :: Maybe EpaLocation -> AnnContext -> AnnContext
 addParensToCtxt close_dp = addOpen . addClose
   where
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index 1eb8f4f906..978c08b68e 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -99,6 +99,7 @@ import           GHC                                               (AddEpAnn (Ad
                                                                     LocatedA, spans)
 import Language.Haskell.GHC.ExactPrint (runTransformFromT, noAnnSrcSpanDP1, runTransform, runTransformT)
 import GHC.Types.SrcLoc (generatedSrcSpan)
+import Debug.Trace (trace)
 
 #else
 import           Language.Haskell.GHC.ExactPrint.Types             (Annotation (annsDP),
@@ -917,7 +918,7 @@ suggestAddArgument parsedModule Diagnostic {_message, _range}
 addArgumentAction :: ParsedModule -> Range -> T.Text -> Maybe T.Text -> [(T.Text, [TextEdit])]
 addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
   do
-    let addArgToMatch = \(L locMatch (Match xMatch ctxMatch pats rhs)) -> do
+    let addArgToMatch (L locMatch (Match xMatch ctxMatch pats rhs)) = do
           let unqualName = mkRdrUnqual $ mkVarOcc $ T.unpack name
           let newPat = L (noAnnSrcSpanDP1 generatedSrcSpan) $ VarPat NoExtField (noLocA unqualName)
           pure $ L locMatch (Match xMatch ctxMatch (pats <> [newPat]) rhs)
@@ -925,13 +926,13 @@ addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
           (L locDecl (ValD xVal (FunBind xFunBind idFunBind mg coreFunBind))) -> do
             mg' <- modifyMgMatchesT mg addArgToMatch
             let decl' = L locDecl (ValD xVal (FunBind xFunBind idFunBind mg' coreFunBind))
-            pure $ Just [decl']
-          _ -> pure Nothing
+            pure [decl']
+          decl -> pure [decl]
     case runTransformT $ modifySmallestDeclWithM (`spanContainsRange` range) insertArg (makeDeltaAst parsedSource) of
-      Left err -> error $ "Error when inserting argument: " <> err
+      Left err -> trace ("Error when inserting argument: " <> err) []
       Right (newSource, _, _) ->
         let diff = makeDiffTextEdit (T.pack $ exactPrint parsedSource) (T.pack $ exactPrint newSource)
-         in [("Add argument ‘" <> name <> "’ to function", fromLspList $ diff)]
+         in [("Add argument ‘" <> name <> "’ to function", fromLspList diff)]
 
 fromLspList :: List a -> [a]
 fromLspList (List a) = a

From be159c69a11506bd9988f33d47a81a0c57099da4 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Fri, 7 Oct 2022 19:57:52 -0700
Subject: [PATCH 03/15] review: add ability to report errors in CodeAction api

---
 .../src/Development/IDE/Plugin/CodeAction.hs  | 10 ++--
 .../Development/IDE/Plugin/CodeAction/Args.hs | 47 +++++++++++--------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index 978c08b68e..bc97be4348 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -906,16 +906,16 @@ newDefinitionAction IdeOptions {..} parsedModule Range {_start} name typ
     sig = name <> colon <> T.dropWhileEnd isSpace (fromMaybe "_" typ)
     ParsedModule {pm_parsed_source = L _ HsModule {hsmodDecls}} = parsedModule
 
-suggestAddArgument :: ParsedModule -> Diagnostic -> [(T.Text, [TextEdit])]
+suggestAddArgument :: ParsedModule -> Diagnostic -> Either ResponseError [(T.Text, [TextEdit])]
 suggestAddArgument parsedModule Diagnostic {_message, _range}
   | Just (name, typ) <- matchVariableNotInScope message = addArgumentAction parsedModule _range name typ
   | Just (name, typ) <- matchFoundHoleIncludeUnderscore message = addArgumentAction parsedModule _range name (Just typ)
-  | otherwise = []
+  | otherwise = pure []
   where
     message = unifySpaces _message
 
 -- TODO use typ to modify type signature
-addArgumentAction :: ParsedModule -> Range -> T.Text -> Maybe T.Text -> [(T.Text, [TextEdit])]
+addArgumentAction :: ParsedModule -> Range -> T.Text -> Maybe T.Text -> Either ResponseError [(T.Text, [TextEdit])]
 addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
   do
     let addArgToMatch (L locMatch (Match xMatch ctxMatch pats rhs)) = do
@@ -929,10 +929,10 @@ addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
             pure [decl']
           decl -> pure [decl]
     case runTransformT $ modifySmallestDeclWithM (`spanContainsRange` range) insertArg (makeDeltaAst parsedSource) of
-      Left err -> trace ("Error when inserting argument: " <> err) []
+      Left err -> Left $ responseError ("Error when inserting argument: " <> T.pack err)
       Right (newSource, _, _) ->
         let diff = makeDiffTextEdit (T.pack $ exactPrint parsedSource) (T.pack $ exactPrint newSource)
-         in [("Add argument ‘" <> name <> "’ to function", fromLspList diff)]
+         in pure [("Add argument ‘" <> name <> "’ to function", fromLspList diff)]
 
 fromLspList :: List a -> [a]
 fromLspList (List a) = a
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
index ef5c7b623a..06395ef261 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
@@ -15,7 +15,7 @@ where
 import           Control.Concurrent.STM.Stats                 (readTVarIO)
 import           Control.Monad.Reader
 import           Control.Monad.Trans.Maybe
-import           Data.Either                                  (fromRight)
+import           Data.Either                                  (fromRight, partitionEithers)
 import qualified Data.HashMap.Strict                          as Map
 import           Data.IORef.Extra
 import           Data.Maybe                                   (fromMaybe)
@@ -39,6 +39,8 @@ import           Ide.Plugin.Config                            (Config)
 import           Ide.Types
 import qualified Language.LSP.Server                          as LSP
 import           Language.LSP.Types
+import Control.Monad.Except (ExceptT(..))
+import Control.Monad.Trans.Except (runExceptT)
 
 type CodeActionTitle = T.Text
 
@@ -46,7 +48,7 @@ type CodeActionPreferred = Bool
 
 type GhcideCodeActionResult = [(CodeActionTitle, Maybe CodeActionKind, Maybe CodeActionPreferred, [TextEdit])]
 
-type GhcideCodeAction = ReaderT CodeActionArgs IO GhcideCodeActionResult
+type GhcideCodeAction = ExceptT ResponseError (ReaderT CodeActionArgs IO) GhcideCodeActionResult
 
 -------------------------------------------------------------------------------------------------
 
@@ -79,13 +81,15 @@ runGhcideCodeAction state (CodeActionParams _ _ (TextDocumentIdentifier uri) _ra
   caaHar <- onceIO $ runRule GetHieAst
   caaBindings <- onceIO $ runRule GetBindings
   caaGblSigs <- onceIO $ runRule GetGlobalBindingTypeSigs
-  liftIO $
-    concat
-      <$> sequence
-        [ runReaderT codeAction caa
+  results <- liftIO $
+
+      sequence
+        [ runReaderT (runExceptT codeAction) caa
           | caaDiagnostic <- diags,
             let caa = CodeActionArgs {..}
         ]
+  let (errs, successes) = partitionEithers results
+  pure $ concat successes
 
 mkCA :: T.Text -> Maybe CodeActionKind -> Maybe Bool -> [Diagnostic] -> WorkspaceEdit -> (Command |? CodeAction)
 mkCA title kind isPreferred diags edit =
@@ -194,39 +198,44 @@ instance ToCodeAction a => ToCodeAction [a] where
 instance ToCodeAction a => ToCodeAction (Maybe a) where
   toCodeAction = maybe (pure []) toCodeAction
 
+instance ToCodeAction a => ToCodeAction (Either ResponseError a) where
+  toCodeAction = either (\err -> ExceptT $ ReaderT $ \_ -> pure $ Left err) toCodeAction
+
 instance ToTextEdit a => ToCodeAction (CodeActionTitle, a) where
-  toCodeAction (title, te) = ReaderT $ \caa -> pure . (title,Just CodeActionQuickFix,Nothing,) <$> toTextEdit caa te
+  toCodeAction (title, te) = ExceptT $ ReaderT $ \caa -> Right . pure . (title,Just CodeActionQuickFix,Nothing,) <$> toTextEdit caa te
 
 instance ToTextEdit a => ToCodeAction (CodeActionTitle, CodeActionKind, a) where
-  toCodeAction (title, kind, te) = ReaderT $ \caa -> pure . (title,Just kind,Nothing,) <$> toTextEdit caa te
+  toCodeAction (title, kind, te) = ExceptT $ ReaderT $ \caa -> Right . pure . (title,Just kind,Nothing,) <$> toTextEdit caa te
 
 instance ToTextEdit a => ToCodeAction (CodeActionTitle, CodeActionPreferred, a) where
-  toCodeAction (title, isPreferred, te) = ReaderT $ \caa -> pure . (title,Just CodeActionQuickFix,Just isPreferred,) <$> toTextEdit caa te
+  toCodeAction (title, isPreferred, te) = ExceptT $ ReaderT $ \caa -> Right . pure . (title,Just CodeActionQuickFix,Just isPreferred,) <$> toTextEdit caa te
 
 instance ToTextEdit a => ToCodeAction (CodeActionTitle, CodeActionKind, CodeActionPreferred, a) where
-  toCodeAction (title, kind, isPreferred, te) = ReaderT $ \caa -> pure . (title,Just kind,Just isPreferred,) <$> toTextEdit caa te
+  toCodeAction (title, kind, isPreferred, te) = ExceptT $ ReaderT $ \caa -> Right . pure . (title,Just kind,Just isPreferred,) <$> toTextEdit caa te
 
 -------------------------------------------------------------------------------------------------
 
 toCodeAction1 :: (ToCodeAction r) => (CodeActionArgs -> IO (Maybe a)) -> (Maybe a -> r) -> GhcideCodeAction
-toCodeAction1 get f = ReaderT $ \caa -> get caa >>= flip runReaderT caa . toCodeAction . f
+toCodeAction1 get f = ExceptT . ReaderT $ \caa -> do
+                          caaMay <- get caa
+                          flip runReaderT caa . runExceptT . toCodeAction . f $ caaMay
 
 toCodeAction2 :: (ToCodeAction r) => (CodeActionArgs -> IO (Maybe a)) -> (a -> r) -> GhcideCodeAction
-toCodeAction2 get f = ReaderT $ \caa ->
+toCodeAction2 get f = ExceptT . ReaderT $ \caa ->
   get caa >>= \case
-    Just x -> flip runReaderT caa . toCodeAction . f $ x
-    _      -> pure []
+    Just x -> flip runReaderT caa . runExceptT . toCodeAction . f $ x
+    _      -> pure $ Right []
 
 toCodeAction3 :: (ToCodeAction r) => (CodeActionArgs -> IO a) -> (a -> r) -> GhcideCodeAction
-toCodeAction3 get f = ReaderT $ \caa -> get caa >>= flip runReaderT caa . toCodeAction . f
+toCodeAction3 get f = ExceptT . ReaderT $ \caa -> get caa >>= flip runReaderT caa . runExceptT . toCodeAction . f
 
 -- | this instance returns a delta AST, useful for exactprint transforms
 instance ToCodeAction r => ToCodeAction (ParsedSource -> r) where
 #if !MIN_VERSION_ghc(9,3,0)
-  toCodeAction f = ReaderT $ \caa@CodeActionArgs {caaAnnSource = x} ->
+  toCodeAction f = ExceptT . ReaderT $ \caa@CodeActionArgs {caaAnnSource = x} ->
     x >>= \case
-      Just s -> flip runReaderT caa . toCodeAction . f . astA $ s
-      _      -> pure []
+      Just s -> flip runReaderT caa . runExceptT . toCodeAction . f . astA $ s
+      _      -> pure $ Right []
 #else
   toCodeAction f = ReaderT $ \caa@CodeActionArgs {caaParsedModule = x} ->
     x >>= \case
@@ -241,7 +250,7 @@ instance ToCodeAction r => ToCodeAction (IdeOptions -> r) where
   toCodeAction = toCodeAction3 caaIdeOptions
 
 instance ToCodeAction r => ToCodeAction (Diagnostic -> r) where
-  toCodeAction f = ReaderT $ \caa@CodeActionArgs {caaDiagnostic = x} -> flip runReaderT caa . toCodeAction $ f x
+  toCodeAction f = ExceptT . ReaderT $ \caa@CodeActionArgs {caaDiagnostic = x} -> flip runReaderT caa . runExceptT . toCodeAction $ f x
 
 instance ToCodeAction r => ToCodeAction (Maybe ParsedModule -> r) where
   toCodeAction = toCodeAction1 caaParsedModule

From 3c6fba3c5eceef2d8dd07f556168cd74d68735a8 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Fri, 7 Oct 2022 22:38:24 -0700
Subject: [PATCH 04/15] review: use already-defined function

---
 ghcide/src/Development/IDE/GHC/Error.hs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ghcide/src/Development/IDE/GHC/Error.hs b/ghcide/src/Development/IDE/GHC/Error.hs
index c9345038f5..5d7d054cc5 100644
--- a/ghcide/src/Development/IDE/GHC/Error.hs
+++ b/ghcide/src/Development/IDE/GHC/Error.hs
@@ -44,6 +44,7 @@ import           Development.IDE.GHC.Orphans       ()
 import           Development.IDE.Types.Diagnostics as D
 import           Development.IDE.Types.Location
 import           GHC
+import Language.LSP.Types (isSubrangeOf)
 
 
 diagFromText :: T.Text -> D.DiagnosticSeverity -> SrcSpan -> T.Text -> FileDiagnostic
@@ -121,7 +122,7 @@ p `isInsideSrcSpan` r = case srcSpanToRange r of
   _                  -> False
 
 spanContainsRange :: SrcSpan -> Range -> Bool
-spanContainsRange srcSpan Range {..} = _start `isInsideSrcSpan` srcSpan && _end `isInsideSrcSpan` srcSpan
+spanContainsRange srcSpan range = maybe False (range `isSubrangeOf`) $ srcSpanToRange srcSpan
 
 -- | Convert a GHC severity to a DAML compiler Severity. Severities below
 -- "Warning" level are dropped (returning Nothing).

From 4be40b7ab6c86b3f1175211838701c35b980e126 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 8 Oct 2022 09:09:20 -0700
Subject: [PATCH 05/15] attempts at cpp

---
 .../src/Ide/Plugin/QualifyImportedNames.hs          |  3 ++-
 .../src/Development/IDE/GHC/ExactPrint.hs           |  7 ++-----
 .../src/Development/IDE/Plugin/CodeAction.hs        | 13 ++++++++-----
 .../src/Development/IDE/Plugin/CodeAction/Args.hs   |  3 +--
 plugins/hls-refactor-plugin/test/Main.hs            |  6 +++++-
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
index ef714e0711..08a8cc0e81 100644
--- a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
+++ b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
@@ -74,6 +74,7 @@ import           Language.LSP.Types                (CodeAction (CodeAction, _com
                                                     WorkspaceEdit (WorkspaceEdit, _changeAnnotations, _changes, _documentChanges),
                                                     type (|?) (InR),
                                                     uriToNormalizedFilePath)
+import Development.IDE (spanContainsRange)
 
 thenCmp :: Ordering -> Ordering -> Ordering
 {-# INLINE thenCmp #-}
@@ -128,7 +129,7 @@ data ImportedBy = ImportedBy {
 }
 
 isRangeWithinImportedBy :: Range -> ImportedBy -> Bool
-isRangeWithinImportedBy range (ImportedBy _ srcSpan) = isRangeWithinSrcSpan range srcSpan
+isRangeWithinImportedBy range (ImportedBy _ srcSpan) = spanContainsRange srcSpan range
 
 globalRdrEnvToNameToImportedByMap :: GlobalRdrEnv -> NameEnv [ImportedBy]
 globalRdrEnvToNameToImportedByMap =
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
index 5116f3f702..80ceed5267 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
@@ -46,7 +46,7 @@ module Development.IDE.GHC.ExactPrint
       ExceptStringT (..),
       TransformT,
       Log(..),
-      )
+    )
 where
 
 import           Control.Applicative                     (Alternative)
@@ -102,12 +102,10 @@ import           GHC                                     (EpAnn (..),
                                                           SrcSpanAnnA,
                                                           TrailingAnn (AddCommaAnn),
                                                           emptyComments,
-                                                          spanAsAnchor, spans)
+                                                          spanAsAnchor)
 import           GHC.Parser.Annotation                   (AnnContext (..),
                                                           DeltaPos (SameLine),
                                                           EpaLocation (EpaDelta))
-import Data.Maybe (fromMaybe)
-import Development.IDE.GHC.Error (isInsideSrcSpan)
 #endif
 
 ------------------------------------------------------------------------------
@@ -662,7 +660,6 @@ eqSrcSpanA l r = leftmost_smallest l r == EQ
 #endif
 
 #if MIN_VERSION_ghc(9,2,0)
-
 addParensToCtxt :: Maybe EpaLocation -> AnnContext -> AnnContext
 addParensToCtxt close_dp = addOpen . addClose
   where
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index bc97be4348..c8c8cbf939 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -82,7 +82,7 @@ import           Language.LSP.Types                                (ApplyWorkspa
                                                                     WorkspaceEdit (WorkspaceEdit, _changeAnnotations, _changes, _documentChanges),
                                                                     type (|?) (InR),
                                                                     uriToFilePath)
-import           GHC.Exts                                           (IsList (fromList))
+import           GHC.Exts                                           (fromList)
 import           Language.LSP.VFS                                  (VirtualFile,
                                                                     _file_text)
 import           Text.Regex.TDFA                                   (mrAfter,
@@ -96,10 +96,9 @@ import           GHC                                               (AddEpAnn (Ad
                                                                     EpAnn (..),
                                                                     EpaLocation (..),
                                                                     LEpaComment,
-                                                                    LocatedA, spans)
-import Language.Haskell.GHC.ExactPrint (runTransformFromT, noAnnSrcSpanDP1, runTransform, runTransformT)
+                                                                    LocatedA)
+import Language.Haskell.GHC.ExactPrint (noAnnSrcSpanDP1, runTransformT)
 import GHC.Types.SrcLoc (generatedSrcSpan)
-import Debug.Trace (trace)
 
 #else
 import           Language.Haskell.GHC.ExactPrint.Types             (Annotation (annsDP),
@@ -170,7 +169,9 @@ bindingsPluginDescriptor recorder plId = mkExactprintPluginDescriptor recorder $
     , wrap suggestImplicitParameter
 #endif
     , wrap suggestNewDefinition
+#if MIN_VERSION_ghc(9,2,1)
     , wrap suggestAddArgument
+#endif
     , wrap suggestDeleteUnusedBinding
     ]
     plId
@@ -388,7 +389,7 @@ suggestHideShadow ps fileContents mTcM mHar Diagnostic {_message, _range}
     Just matched <- allMatchRegexUnifySpaces _message "imported from ‘([^’]+)’ at ([^ ]*)",
     mods <- [(modName, s) | [_, modName, s] <- matched],
     result <- nubOrdBy (compare `on` fst) $ mods >>= uncurry (suggests identifier),
-    hideAll <- ("Hide " <> identifier <> " from all occurence imports", concat $ snd <$> result) =
+    hideAll <- ("Hide " <> identifier <> " from all occurence imports", concatMap snd result) =
     result <> [hideAll]
   | otherwise = []
   where
@@ -906,6 +907,7 @@ newDefinitionAction IdeOptions {..} parsedModule Range {_start} name typ
     sig = name <> colon <> T.dropWhileEnd isSpace (fromMaybe "_" typ)
     ParsedModule {pm_parsed_source = L _ HsModule {hsmodDecls}} = parsedModule
 
+#if MIN_VERSION_ghc(9,2,1)
 suggestAddArgument :: ParsedModule -> Diagnostic -> Either ResponseError [(T.Text, [TextEdit])]
 suggestAddArgument parsedModule Diagnostic {_message, _range}
   | Just (name, typ) <- matchVariableNotInScope message = addArgumentAction parsedModule _range name typ
@@ -933,6 +935,7 @@ addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
       Right (newSource, _, _) ->
         let diff = makeDiffTextEdit (T.pack $ exactPrint parsedSource) (T.pack $ exactPrint newSource)
          in pure [("Add argument ‘" <> name <> "’ to function", fromLspList diff)]
+#endif
 
 fromLspList :: List a -> [a]
 fromLspList (List a) = a
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
index 06395ef261..283b5f4cf5 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
@@ -39,8 +39,7 @@ import           Ide.Plugin.Config                            (Config)
 import           Ide.Types
 import qualified Language.LSP.Server                          as LSP
 import           Language.LSP.Types
-import Control.Monad.Except (ExceptT(..))
-import Control.Monad.Trans.Except (runExceptT)
+import Control.Monad.Except (ExceptT(..), runExceptT)
 
 type CodeActionTitle = T.Text
 
diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs
index 50ad768203..6cb6982351 100644
--- a/plugins/hls-refactor-plugin/test/Main.hs
+++ b/plugins/hls-refactor-plugin/test/Main.hs
@@ -311,7 +311,6 @@ codeActionTests = testGroup "code actions"
   , fillTypedHoleTests
   , addSigActionTests
   , insertNewDefinitionTests
-  , addFunctionArgumentTests
   , deleteUnusedDefinitionTests
   , addInstanceConstraintTests
   , addFunctionConstraintTests
@@ -320,6 +319,9 @@ codeActionTests = testGroup "code actions"
   , exportUnusedTests
   , addImplicitParamsConstraintTests
   , removeExportTests
+#if MIN_VERSION_ghc(9,2,1)
+  , addFunctionArgumentTests
+#endif
   ]
 
 insertImportTests :: TestTree
@@ -2140,6 +2142,7 @@ insertNewDefinitionTests = testGroup "insert new definition actions"
         ++ txtB')
   ]
 
+#if MIN_VERSION_ghc(9,2,1)
 addFunctionArgumentTests :: TestTree
 addFunctionArgumentTests =
   testGroup
@@ -2321,6 +2324,7 @@ addFunctionArgumentTests =
         contentAfterAction <- documentContents docB
         liftIO $ contentAfterAction @?= T.unlines foo'
     ]
+#endif
 
 deleteUnusedDefinitionTests :: TestTree
 deleteUnusedDefinitionTests = testGroup "delete unused definition action"

From 2627118ace78bc2aca28e842b1dab60f8b99b825 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 8 Oct 2022 10:44:13 -0700
Subject: [PATCH 06/15] fix format error

---
 .../src/Ide/Plugin/QualifyImportedNames.hs                      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
index 08a8cc0e81..f677936a23 100644
--- a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
+++ b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
@@ -21,6 +21,7 @@ import qualified Data.Map.Strict                   as Map
 import           Data.Maybe                        (mapMaybe)
 import           Data.Text                         (Text)
 import qualified Data.Text                         as Text
+import           Development.IDE                   (spanContainsRange)
 import           Development.IDE.Core.RuleTypes    (GetFileContents (GetFileContents),
                                                     GetHieAst (GetHieAst),
                                                     HieAstResult (HAR, refMap),
@@ -74,7 +75,6 @@ import           Language.LSP.Types                (CodeAction (CodeAction, _com
                                                     WorkspaceEdit (WorkspaceEdit, _changeAnnotations, _changes, _documentChanges),
                                                     type (|?) (InR),
                                                     uriToNormalizedFilePath)
-import Development.IDE (spanContainsRange)
 
 thenCmp :: Ordering -> Ordering -> Ordering
 {-# INLINE thenCmp #-}

From 9388c5444ac727ea1de8f16858cfabfe40d6bdad Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Mon, 10 Oct 2022 00:02:54 -0700
Subject: [PATCH 07/15] fix broken test

---
 plugins/hls-refactor-plugin/hls-refactor-plugin.cabal | 2 ++
 plugins/hls-refactor-plugin/test/Main.hs              | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/hls-refactor-plugin/hls-refactor-plugin.cabal b/plugins/hls-refactor-plugin/hls-refactor-plugin.cabal
index d61d1593ee..c6ac02128a 100644
--- a/plugins/hls-refactor-plugin/hls-refactor-plugin.cabal
+++ b/plugins/hls-refactor-plugin/hls-refactor-plugin.cabal
@@ -109,6 +109,8 @@ test-suite tests
     , extra
     , text-rope
     , containers
+    -- ghc is included to enable the MIN_VERSION_ghc macro
+    , ghc
     , ghcide
     , ghcide-test-utils
     , shake
diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs
index 6cb6982351..f714b29575 100644
--- a/plugins/hls-refactor-plugin/test/Main.hs
+++ b/plugins/hls-refactor-plugin/test/Main.hs
@@ -57,6 +57,7 @@ import           Text.Regex.TDFA                          ((=~))
 import           Development.IDE.Plugin.CodeAction        (matchRegExMultipleImports)
 import           Test.Hls
 
+import           Control.Applicative                      (liftA2)
 import qualified Development.IDE.Plugin.CodeAction        as Refactor
 import qualified Development.IDE.Plugin.HLS.GhcIde        as GhcIde
 
@@ -1488,7 +1489,7 @@ extendImportTests = testGroup "extend import actions"
             actionsOrCommands <- getCodeActions docB range
             let codeActions =
                   filter
-                    (T.isPrefixOf "Add" . codeActionTitle)
+                    (liftA2 (&&) (T.isPrefixOf "Add") (not . T.isPrefixOf "Add argument") . codeActionTitle)
                     [ca | InR ca <- actionsOrCommands]
                 actualTitles = codeActionTitle <$> codeActions
             -- Note that we are not testing the order of the actions, as the

From 3a0a8f918e4c629445e7c6281ec099fffe7b55d4 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 15 Oct 2022 19:30:40 -0700
Subject: [PATCH 08/15] doc: add self to codeowners; add doc to features.md

---
 CODEOWNERS       | 1 +
 docs/features.md | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/CODEOWNERS b/CODEOWNERS
index 27cc7e20d4..ebe2b307ab 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -24,6 +24,7 @@
 /plugins/hls-qualify-imported-names-plugin @eddiemundo
 /plugins/hls-refine-imports-plugin
 /plugins/hls-rename-plugin @OliverMadine
+/plugins/hls-refactor-plugin @santiweight
 /plugins/hls-retrie-plugin @pepeiborra
 /plugins/hls-code-range-plugin @kokobd
 /plugins/hls-splice-plugin @konn
diff --git a/docs/features.md b/docs/features.md
index 793b66a61e..9dec6f1b25 100644
--- a/docs/features.md
+++ b/docs/features.md
@@ -270,6 +270,14 @@ Known Limitations:
 
 ![Link to Docs](../plugins/hls-change-type-signature-plugin/README.md)
 
+### Add argument to function
+
+Provided by: `hls-refactor-plugin`
+
+Code action kind: `quickfix`
+
+Add an undefined variable as an argument to the top-level binding.
+
 ### Convert to GADT syntax
 
 Provided by: `hls-gadt-plugin`

From 57b4edbeec91ad31e07b58194cca88eb4b252256 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 15 Oct 2022 19:34:35 -0700
Subject: [PATCH 09/15] formatting

---
 .../src/Development/IDE/Plugin/CodeAction.hs             | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index 8b33dcdf8e..75aaa90339 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -39,7 +39,6 @@ import qualified Data.Set                                          as S
 import qualified Data.Text                                         as T
 import qualified Data.Text.Utf16.Rope                              as Rope
 import           Data.Tuple.Extra                                  (first)
-import           Development.IDE.Types.Logger                      hiding (group)
 import           Development.IDE.Core.Rules
 import           Development.IDE.Core.RuleTypes
 import           Development.IDE.Core.Service
@@ -60,10 +59,13 @@ import           Development.IDE.Plugin.Completions.Types
 import           Development.IDE.Plugin.TypeLenses                 (suggestSignature)
 import           Development.IDE.Types.Exports
 import           Development.IDE.Types.Location
+import           Development.IDE.Types.Logger                      hiding
+                                                                   (group)
 import           Development.IDE.Types.Options
 import           GHC.Exts                                          (fromList)
 import qualified GHC.LanguageExtensions                            as Lang
-import           Ide.PluginUtils                                   (subRange, makeDiffTextEdit)
+import           Ide.PluginUtils                                   (makeDiffTextEdit,
+                                                                    subRange)
 import           Ide.Types
 import qualified Language.LSP.Server                               as LSP
 import           Language.LSP.Types                                (ApplyWorkspaceEditParams (..),
@@ -99,8 +101,9 @@ import           GHC                                               (AddEpAnn (Ad
                                                                     EpaLocation (..),
                                                                     LEpaComment,
                                                                     LocatedA)
-import           Language.Haskell.GHC.ExactPrint                   (noAnnSrcSpanDP1, runTransformT)
 import           GHC.Types.SrcLoc                                  (generatedSrcSpan)
+import           Language.Haskell.GHC.ExactPrint                   (noAnnSrcSpanDP1,
+                                                                    runTransformT)
 #else
 import           Language.Haskell.GHC.ExactPrint.Types             (Annotation (annsDP),
                                                                     DeltaPos,

From 123a4f0a13a6f4b2f202ae3a94494c7cb3201267 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 15 Oct 2022 19:35:15 -0700
Subject: [PATCH 10/15] formatting

---
 ghcide/src/Development/IDE/GHC/Error.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ghcide/src/Development/IDE/GHC/Error.hs b/ghcide/src/Development/IDE/GHC/Error.hs
index 5d7d054cc5..e19cbca427 100644
--- a/ghcide/src/Development/IDE/GHC/Error.hs
+++ b/ghcide/src/Development/IDE/GHC/Error.hs
@@ -44,7 +44,7 @@ import           Development.IDE.GHC.Orphans       ()
 import           Development.IDE.Types.Diagnostics as D
 import           Development.IDE.Types.Location
 import           GHC
-import Language.LSP.Types (isSubrangeOf)
+import           Language.LSP.Types                (isSubrangeOf)
 
 
 diagFromText :: T.Text -> D.DiagnosticSeverity -> SrcSpan -> T.Text -> FileDiagnostic

From 9a232d2da1bfe9a50d22be61c8276283e97a43a7 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 15 Oct 2022 19:36:47 -0700
Subject: [PATCH 11/15] fix an import

---
 .../src/Development/IDE/Plugin/CodeAction.hs              | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index 75aaa90339..c53ba27c7a 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -91,6 +91,11 @@ import           Language.LSP.VFS                                  (VirtualFile,
 import qualified Text.Fuzzy.Parallel                               as TFP
 import           Text.Regex.TDFA                                   (mrAfter,
                                                                     (=~), (=~~))
+#if MIN_VERSION_ghc(9,2,1)
+import           GHC.Types.SrcLoc                                  (generatedSrcSpan)
+import           Language.Haskell.GHC.ExactPrint                   (noAnnSrcSpanDP1,
+                                                                    runTransformT)
+#endif
 #if MIN_VERSION_ghc(9,2,0)
 import           GHC                                               (AddEpAnn (AddEpAnn),
                                                                     Anchor (anchor_op),
@@ -101,9 +106,6 @@ import           GHC                                               (AddEpAnn (Ad
                                                                     EpaLocation (..),
                                                                     LEpaComment,
                                                                     LocatedA)
-import           GHC.Types.SrcLoc                                  (generatedSrcSpan)
-import           Language.Haskell.GHC.ExactPrint                   (noAnnSrcSpanDP1,
-                                                                    runTransformT)
 #else
 import           Language.Haskell.GHC.ExactPrint.Types             (Annotation (annsDP),
                                                                     DeltaPos,

From f2659a7b86555f8f42ef93b1bc65a64c7f3fab13 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 5 Nov 2022 11:16:15 -0700
Subject: [PATCH 12/15] review

---
 ghcide/src/Development/IDE/GHC/Error.hs       |  5 +++--
 .../src/Development/IDE/GHC/ExactPrint.hs     | 22 ++++++++++---------
 .../src/Development/IDE/Plugin/CodeAction.hs  | 21 ++++++++++++++++--
 3 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/ghcide/src/Development/IDE/GHC/Error.hs b/ghcide/src/Development/IDE/GHC/Error.hs
index e19cbca427..a8a7acce27 100644
--- a/ghcide/src/Development/IDE/GHC/Error.hs
+++ b/ghcide/src/Development/IDE/GHC/Error.hs
@@ -121,8 +121,9 @@ p `isInsideSrcSpan` r = case srcSpanToRange r of
   Just (Range sp ep) -> sp <= p && p <= ep
   _                  -> False
 
-spanContainsRange :: SrcSpan -> Range -> Bool
-spanContainsRange srcSpan range = maybe False (range `isSubrangeOf`) $ srcSpanToRange srcSpan
+-- Returns Nothing if the SrcSpan does not represent a valid range
+spanContainsRange :: SrcSpan -> Range -> Maybe Bool
+spanContainsRange srcSpan range = (range `isSubrangeOf`) <$> srcSpanToRange srcSpan
 
 -- | Convert a GHC severity to a DAML compiler Severity. Severities below
 -- "Warning" level are dropped (returning Nothing).
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
index aadd81bcd2..9716a4ff90 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
@@ -435,25 +435,27 @@ graftDecls dst decs0 = Graft $ \dflags a -> do
     modifyDeclsT (pure . DL.toList . go) a
 
 #if MIN_VERSION_ghc(9,2,1)
+
 -- | Replace the smallest declaration whose SrcSpan satisfies the given condition with a new
 -- list of declarations.
 --
 -- For example, if you would like to move a where-clause-defined variable to the same
 -- level as its parent HsDecl, you could use this function.
 modifySmallestDeclWithM ::
-  forall a.
-  (HasDecls a) =>
-  (SrcSpan -> Bool) ->
-  (LHsDecl GhcPs -> TransformT (Either String) [LHsDecl GhcPs]) ->
+  forall a m.
+  (HasDecls a, Monad m) =>
+  (SrcSpan -> m Bool) ->
+  (LHsDecl GhcPs -> TransformT m [LHsDecl GhcPs]) ->
   a ->
-  TransformT (Either String) a
+  TransformT m a
 modifySmallestDeclWithM validSpan f a = do
   let modifyMatchingDecl [] = pure DL.empty
-      modifyMatchingDecl (e@(L src _) : rest)
-        | validSpan $ locA src = do
-            decs' <- f e
-            pure $ DL.fromList decs' <> DL.fromList rest
-        | otherwise = (DL.singleton e <>) <$> modifyMatchingDecl rest
+      modifyMatchingDecl (e@(L src _) : rest) =
+        lift (validSpan $ locA src) >>= \case
+            True -> do
+              decs' <- f e
+              pure $ DL.fromList decs' <> DL.fromList rest
+            False -> (DL.singleton e <>) <$> modifyMatchingDecl rest
   modifyDeclsT (fmap DL.toList . modifyMatchingDecl) a
 
 -- | Modify the each LMatch in a MatchGroup
diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
index c53ba27c7a..442ffcb253 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
@@ -97,6 +97,7 @@ import           Language.Haskell.GHC.ExactPrint                   (noAnnSrcSpan
                                                                     runTransformT)
 #endif
 #if MIN_VERSION_ghc(9,2,0)
+import           Extra                                             (maybeToEither)
 import           GHC                                               (AddEpAnn (AddEpAnn),
                                                                     Anchor (anchor_op),
                                                                     AnchorOperation (..),
@@ -949,6 +950,20 @@ newDefinitionAction IdeOptions {..} parsedModule Range {_start} name typ
     ParsedModule {pm_parsed_source = L _ HsModule {hsmodDecls}} = parsedModule
 
 #if MIN_VERSION_ghc(9,2,1)
+-- When GHC tells us that a variable is not bound, it will tell us either:
+--  - there is an unbound variable with a given type
+--  - there is an unbound variable (GHC provides no type suggestion)
+--
+-- When we receive either of these errors, we produce a text edit that will add a new argument (as a new pattern in the
+-- last position of each LHS of the top-level bindings for this HsDecl).
+--
+-- TODO Include logic to also update the type signature of a binding
+--
+-- NOTE When adding a new argument to a declaration, the corresponding argument's type in declaration's signature might
+--      not be the last type in the signature, such as:
+--         foo :: a -> b -> c -> d
+--         foo a b = \c -> ...
+--      In this case a new argument would have to add its type between b and c in the signature.
 suggestAddArgument :: ParsedModule -> Diagnostic -> Either ResponseError [(T.Text, [TextEdit])]
 suggestAddArgument parsedModule Diagnostic {_message, _range}
   | Just (name, typ) <- matchVariableNotInScope message = addArgumentAction parsedModule _range name typ
@@ -971,11 +986,13 @@ addArgumentAction (ParsedModule _ parsedSource _ _) range name _typ =
             let decl' = L locDecl (ValD xVal (FunBind xFunBind idFunBind mg' coreFunBind))
             pure [decl']
           decl -> pure [decl]
-    case runTransformT $ modifySmallestDeclWithM (`spanContainsRange` range) insertArg (makeDeltaAst parsedSource) of
-      Left err -> Left $ responseError ("Error when inserting argument: " <> T.pack err)
+    case runTransformT $ modifySmallestDeclWithM spanContainsRangeOrErr insertArg (makeDeltaAst parsedSource) of
+      Left err -> Left err
       Right (newSource, _, _) ->
         let diff = makeDiffTextEdit (T.pack $ exactPrint parsedSource) (T.pack $ exactPrint newSource)
          in pure [("Add argument ‘" <> name <> "’ to function", fromLspList diff)]
+      where
+        spanContainsRangeOrErr = maybeToEither (responseError "SrcSpan was not valid range") . (`spanContainsRange` range)
 #endif
 
 fromLspList :: List a -> [a]

From 136c232e9604c43d6689ab73dce2933185e24f6a Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 5 Nov 2022 11:17:04 -0700
Subject: [PATCH 13/15] formatting

---
 .../src/Development/IDE/Plugin/CodeAction/Args.hs           | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
index 283b5f4cf5..82e0134fcb 100644
--- a/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
+++ b/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Args.hs
@@ -15,7 +15,8 @@ where
 import           Control.Concurrent.STM.Stats                 (readTVarIO)
 import           Control.Monad.Reader
 import           Control.Monad.Trans.Maybe
-import           Data.Either                                  (fromRight, partitionEithers)
+import           Data.Either                                  (fromRight,
+                                                               partitionEithers)
 import qualified Data.HashMap.Strict                          as Map
 import           Data.IORef.Extra
 import           Data.Maybe                                   (fromMaybe)
@@ -30,6 +31,8 @@ import           Development.IDE.GHC.ExactPrint
 import           Development.IDE.Plugin.CodeAction.ExactPrint (Rewrite,
                                                                rewriteToEdit)
 #endif
+import           Control.Monad.Except                         (ExceptT (..),
+                                                               runExceptT)
 import           Development.IDE.Plugin.TypeLenses            (GetGlobalBindingTypeSigs (GetGlobalBindingTypeSigs),
                                                                GlobalBindingTypeSigsResult)
 import           Development.IDE.Spans.LocalBindings          (Bindings)
@@ -39,7 +42,6 @@ import           Ide.Plugin.Config                            (Config)
 import           Ide.Types
 import qualified Language.LSP.Server                          as LSP
 import           Language.LSP.Types
-import Control.Monad.Except (ExceptT(..), runExceptT)
 
 type CodeActionTitle = T.Text
 

From c39b6f39b231d269bce5a0a2a9ffa232e67e02db Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sat, 5 Nov 2022 11:45:10 -0700
Subject: [PATCH 14/15] add testcase with comments

---
 plugins/hls-refactor-plugin/test/Main.hs | 27 ++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/plugins/hls-refactor-plugin/test/Main.hs b/plugins/hls-refactor-plugin/test/Main.hs
index 75a5cf87a1..124f28acf1 100644
--- a/plugins/hls-refactor-plugin/test/Main.hs
+++ b/plugins/hls-refactor-plugin/test/Main.hs
@@ -2345,6 +2345,33 @@ addFunctionArgumentTests =
         liftIO $ actionTitle @?= "Add argument ‘select’ to function"
         executeCodeAction action
         contentAfterAction <- documentContents docB
+        liftIO $ contentAfterAction @?= T.unlines foo',
+      testSession "where clause" $ do
+        let foo =
+              [ "foo -- c1",
+                "  -- | c2",
+                "  {- c3 -} True -- c4",
+                "  = select",
+                "",
+                "foo False = False"
+              ]
+            -- TODO could use improvement here...
+            foo' =
+              [ "foo -- c1",
+                "  -- | c2",
+                "  {- c3 -} True select -- c4",
+                "  = select",
+                "",
+                "foo False select = False"
+              ]
+        docB <- createDoc "ModuleB.hs" "haskell" (T.unlines $ foo)
+        _ <- waitForDiagnostics
+        InR action@CodeAction {_title = actionTitle} : _ <-
+          filter (\(InR CodeAction {_title = x}) -> "Add" `isPrefixOf` T.unpack x)
+            <$> getCodeActions docB (R 3 0 3 50)
+        liftIO $ actionTitle @?= "Add argument ‘select’ to function"
+        executeCodeAction action
+        contentAfterAction <- documentContents docB
         liftIO $ contentAfterAction @?= T.unlines foo'
     ]
 #endif

From 0fce8308785066e785cbaa87be789be670f6e2f9 Mon Sep 17 00:00:00 2001
From: Santiago Weight <santiago.weight@lmns.com>
Date: Sun, 6 Nov 2022 11:55:53 -0800
Subject: [PATCH 15/15] fix build error

---
 .../src/Ide/Plugin/QualifyImportedNames.hs                  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
index f677936a23..62d39bfd6f 100644
--- a/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
+++ b/plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
@@ -18,7 +18,7 @@ import qualified Data.HashMap.Strict               as HashMap
 import           Data.List                         (sortOn)
 import qualified Data.List                         as List
 import qualified Data.Map.Strict                   as Map
-import           Data.Maybe                        (mapMaybe)
+import           Data.Maybe                        (fromMaybe, mapMaybe)
 import           Data.Text                         (Text)
 import qualified Data.Text                         as Text
 import           Development.IDE                   (spanContainsRange)
@@ -93,7 +93,7 @@ findLImportDeclAt range parsedModule
   | ParsedModule {..} <- parsedModule
   , L _ hsModule <- pm_parsed_source
   , locatedImportDecls <- hsmodImports hsModule =
-      find (\ (L (locA -> srcSpan) _) -> srcSpan `spanContainsRange` range) locatedImportDecls
+      find (\ (L (locA -> srcSpan) _) -> fromMaybe False $ srcSpan `spanContainsRange` range) locatedImportDecls
 
 makeCodeActions :: Uri -> [TextEdit] -> [a |? CodeAction]
 makeCodeActions uri textEdits = [InR CodeAction {..} | not (null textEdits)]
@@ -129,7 +129,7 @@ data ImportedBy = ImportedBy {
 }
 
 isRangeWithinImportedBy :: Range -> ImportedBy -> Bool
-isRangeWithinImportedBy range (ImportedBy _ srcSpan) = spanContainsRange srcSpan range
+isRangeWithinImportedBy range (ImportedBy _ srcSpan) = fromMaybe False $ spanContainsRange srcSpan range
 
 globalRdrEnvToNameToImportedByMap :: GlobalRdrEnv -> NameEnv [ImportedBy]
 globalRdrEnvToNameToImportedByMap =