Skip to content

Limit number of valid hole fits to 10 #4288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 9, 2024
Merged

Limit number of valid hole fits to 10 #4288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
990428f
ghcide: Pass -fmax-valid-hole-fits=10 to GHC
akshaymankar Jun 8, 2024
0d9bb43
hls-refactor-plugin: Ignore "Some hole fits suppressed" message when …
akshaymankar Jun 8, 2024
6a8e424
hls-refactor-plugin: More predictable hole fit for test
akshaymankar Jun 8, 2024
45b77f3
Merge branch 'master' into limit-hole-fits
michaelpj Jun 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions ghcide/src/Development/IDE/GHC/Compat.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ import Compat.HieTypes hiding
(nodeAnnotations)
import qualified Compat.HieTypes as GHC (nodeAnnotations)
import Compat.HieUtils
import Control.Applicative ((<|>))
import qualified Data.ByteString as BS
import Data.Coerce (coerce)
import Data.List (foldl')
Expand Down Expand Up @@ -434,7 +435,7 @@ setHieDir _f d = d { hieDir = Just _f}
dontWriteHieFiles :: DynFlags -> DynFlags
dontWriteHieFiles d = gopt_unset d Opt_WriteHie

setUpTypedHoles ::DynFlags -> DynFlags
setUpTypedHoles :: DynFlags -> DynFlags
setUpTypedHoles df
= flip gopt_unset Opt_AbstractRefHoleFits -- too spammy
$ flip gopt_unset Opt_ShowDocsOfHoleFits -- not used
Expand All @@ -447,9 +448,13 @@ setUpTypedHoles df
$ flip gopt_unset Opt_SortValidHoleFits
$ flip gopt_unset Opt_UnclutterValidHoleFits
$ df
{ refLevelHoleFits = Just 1 -- becomes slow at higher levels
, maxRefHoleFits = Just 10 -- quantity does not impact speed
, maxValidHoleFits = Nothing -- quantity does not impact speed
{ refLevelHoleFits = refLevelHoleFits df <|> Just 1 -- becomes slow at higher levels

-- Sometimes GHC can emit a lot of hole fits, this causes editors to be slow
-- or just crash, we limit the hole fits to 10. The number was chosen
-- arbirtarily by the author.
, maxRefHoleFits = maxRefHoleFits df <|> Just 10
, maxValidHoleFits = maxValidHoleFits df <|> Just 10
}


Expand Down
3 changes: 2 additions & 1 deletion plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/FillHole.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ processHoleSuggestions mm = (holeSuggestions, refSuggestions)
(mrAfter . (=~ t " *Valid (hole fits|substitutions) include"))
validHolesSection
let holeFit = T.strip $ T.takeWhile (/= ':') holeFitLine
guard (not $ T.null holeFit)
guard $ not $ holeFit =~ t "Some hole fits suppressed"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going on here? Why are we doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GHC prints this line where we expect a hole fit to be when the number of hole fits is limited. This line just ignores that. We do this in the refinement hole fits too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write it down!

guard $ not $ T.null holeFit
return holeFit
refSuggestions = do -- @[]
-- get the text indented under Valid refinement hole fits
Expand Down
16 changes: 8 additions & 8 deletions plugins/hls-refactor-plugin/test/Main.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2640,29 +2640,29 @@ fillTypedHoleTests = let
, testSession "postfix hole uses postfix notation of infix operator" $ do
let mkDoc x = T.unlines
[ "module Testing where"
, "test :: Int -> Int -> Int"
, "test a1 a2 = " <> x <> " a1 a2"
, "test :: Int -> Maybe Int -> Maybe Int"
, "test a ma = " <> x <> " (a +) ma"
]
doc <- createDoc "Test.hs" "haskell" $ mkDoc "_"
_ <- waitForDiagnostics
actions <- getCodeActions doc (Range (Position 2 13) (Position 2 14))
chosen <- pickActionWithTitle "replace _ with (+)" actions
chosen <- pickActionWithTitle "replace _ with (<$>)" actions
executeCodeAction chosen
modifiedCode <- documentContents doc
liftIO $ mkDoc "(+)" @=? modifiedCode
liftIO $ mkDoc "(<$>)" @=? modifiedCode
, testSession "filling infix type hole uses infix operator" $ do
let mkDoc x = T.unlines
[ "module Testing where"
, "test :: Int -> Int -> Int"
, "test a1 a2 = a1 " <> x <> " a2"
, "test :: Int -> Maybe Int -> Maybe Int"
, "test a ma = (a +) " <> x <> " ma"
]
doc <- createDoc "Test.hs" "haskell" $ mkDoc "`_`"
_ <- waitForDiagnostics
actions <- getCodeActions doc (Range (Position 2 16) (Position 2 19))
chosen <- pickActionWithTitle "replace _ with (+)" actions
chosen <- pickActionWithTitle "replace _ with (<$>)" actions
executeCodeAction chosen
modifiedCode <- documentContents doc
liftIO $ mkDoc "+" @=? modifiedCode
liftIO $ mkDoc "<$>" @=? modifiedCode
]

addInstanceConstraintTests :: TestTree
Expand Down
Loading