Fix hanging redundant import on Unicode function (#2870)

* Revert partial changes from #2483

Instead of using a low-level `is_ident` that only works on ascii
characters. Instead match directly on the '_' character. The original
functionality has been restored (using `isAlpha` instead).

This commit removes the now unneccessary `is_ident` compat layer

* Add regression test for unicode functions

Co-authored-by: Pepe Iborra <pepeiborra@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

ghcide/src/Development/IDE/GHC/Compat/Util.hs

@@ -69,8 +69,6 @@ module Development.IDE.GHC.Compat.Util (
     stringToStringBuffer,
     nextChar,
     atEnd,
-    -- * Char
-    is_ident
     ) where
 
 #if MIN_VERSION_ghc(9,0,0)
@@ -83,7 +81,6 @@ import           GHC.Data.FastString
 import           GHC.Data.Maybe
 import           GHC.Data.Pair
 import           GHC.Data.StringBuffer
-import           GHC.Parser.CharClass    (is_ident)
 import           GHC.Types.Unique
 import           GHC.Types.Unique.DFM
 import           GHC.Utils.Fingerprint
@@ -93,7 +90,6 @@ import           GHC.Utils.Panic         hiding (try)
 #else
 import           Bag
 import           BooleanFormula
-import           Ctype                   (is_ident)
 import           EnumSet
 import qualified Exception
 import           FastString

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

@@ -41,8 +41,8 @@ import qualified Data.Rope.UTF16                                   as Rope
 import qualified Data.Set                                          as S
 import qualified Data.Text                                         as T
 import           Data.Tuple.Extra                                  (fst3)
-import           Development.IDE.Core.RuleTypes
 import           Development.IDE.Core.Rules
+import           Development.IDE.Core.RuleTypes
 import           Development.IDE.Core.Service
 import           Development.IDE.GHC.Compat
 import           Development.IDE.GHC.Compat.Util
@@ -1631,8 +1631,11 @@ rangesForBindingImport _ _ = []
 wrapOperatorInParens :: String -> String
 wrapOperatorInParens x =
   case uncons x of
-    Just (h, _t) -> if is_ident h then x else "(" <> x <> ")"
-    Nothing      -> mempty
+    -- see #2483 and #2859
+    -- common lens functions use the _ prefix, and should not be wrapped in parens
+    Just ('_', _t) -> x
+    Just (h, _t)   -> if isAlpha h then x else "(" <> x <> ")"
+    Nothing        -> mempty
 
 smallerRangesForBindingExport :: [LIE GhcPs] -> String -> [Range]
 smallerRangesForBindingExport lies b =
@@ -1788,8 +1791,8 @@ renderImportStyle (ImportAllConstructors p) = p <> "(..)"
 
 -- | Used for extending import lists
 unImportStyle :: ImportStyle -> (Maybe String, String)
-unImportStyle (ImportTopLevel x)    = (Nothing, T.unpack x)
-unImportStyle (ImportViaParent x y) = (Just $ T.unpack y, T.unpack x)
+unImportStyle (ImportTopLevel x)        = (Nothing, T.unpack x)
+unImportStyle (ImportViaParent x y)     = (Just $ T.unpack y, T.unpack x)
 unImportStyle (ImportAllConstructors x) = (Just $ T.unpack x, wildCardSymbol)
 
 

ghcide/test/exe/Main.hs

@@ -1343,6 +1343,34 @@ removeImportTests = testGroup "remove import actions"
             , "main = print stuffB"
             ]
       liftIO $ expectedContentAfterAction @=? contentAfterAction
+  , testSession "redundant binding - unicode regression " $ do
+      let contentA = T.unlines
+            [ "module ModuleA where"
+            , "data A = A"
+            , "ε :: Double"
+            , "ε = 0.5"
+            ]
+      _docA <- createDoc "ModuleA.hs" "haskell" contentA
+      let contentB = T.unlines
+            [ "{-# OPTIONS_GHC -Wunused-imports #-}"
+            , "module ModuleB where"
+            , "import ModuleA (A(..), ε)"
+            , "a = A"
+            ]
+      docB <- createDoc "ModuleB.hs" "haskell" contentB
+      _ <- waitForDiagnostics
+      [InR action@CodeAction { _title = actionTitle }, _]
+          <- getCodeActions docB (Range (Position 2 0) (Position 2 5))
+      liftIO $ "Remove ε from import" @=? actionTitle
+      executeCodeAction action
+      contentAfterAction <- documentContents docB
+      let expectedContentAfterAction = T.unlines
+            [ "{-# OPTIONS_GHC -Wunused-imports #-}"
+            , "module ModuleB where"
+            , "import ModuleA (A(..))"
+            , "a = A"
+            ]
+      liftIO $ expectedContentAfterAction @=? contentAfterAction
   , testSession "redundant operator" $ do
       let contentA = T.unlines
             [ "module ModuleA where"