Extend import suggestions for more than one option #913

gdevanla · 2020-11-17T14:14:23Z

Fixes #792.

…able

… suggestions.

pepeiborra

Thanks for the fix! I have been annoyed by this bug quite a few times.
Some minor changes needed before merging it

src/Development/IDE/Plugin/CodeAction.hs

test/exe/Main.hs

gdevanla · 2020-11-18T16:30:49Z

Question: I get different results from calling testSession.

#  ~/fsf/hls/ghcide on git:extend-import-suggestions-for-more-than-one-option x C:1
$ time stack test --test-arguments '--pattern "extend import list with"' --fast
ghcide> test (suite: ghcide-tests, args: --pattern "extend import list with")

ghcide> ghcide
ghcide>   code actions
ghcide>     extend import actions
ghcide>       extend import list with multiple choices:
ghcide> No of code actions = 0
ghcide> FAIL
ghcide>         Exception: test/exe/Main.hs:(1126,11)-(1127,82): Non-exhaustive patterns in CACodeAction action@CodeAction {_title = actionTitle} : _
ghcide>
ghcide> 1 out of 1 tests failed (2.46s)
ghcide> Test suite ghcide-tests failed
Test suite failure for package ghcide-0.5.0
    ghcide-tests:  exited with: ExitFailure 1
Logs printed to console

stack test --test-arguments '--pattern "extend import list with"' --fast  2.33s user 0.60s system 102% cpu 2.863 total

#  ~/fsf/hls/ghcide on git:extend-import-suggestions-for-more-than-one-option x C:1
$ time stack test --test-arguments '--pattern "extend import list with"' --fast
ghcide> test (suite: ghcide-tests, args: --pattern "extend import list with")

ghcide> ghcide
ghcide>   code actions
ghcide>     extend import actions
ghcide>       extend import list with multiple choices:
ghcide> No of code actions = 63
ghcide> OK (2.61s)
ghcide>
ghcide> All 1 tests passed (2.61s)
ghcide> Test suite ghcide-tests passed
stack test --test-arguments '--pattern "extend import list with"' --fast  2.65s user 0.57s system 106% cpu 3.022 total

Note the No of code actions output. They differ from different invocations. The results are arbitrary as I run this command multiple times.

pepeiborra · 2020-11-18T20:34:01Z

That's what the call to waitForDiagnostics is for. Did you remove it?

gdevanla · 2020-11-18T23:33:46Z

That's what the call to waitForDiagnostics is for. Did you remove it?

Nope. I have that whole piece of code intact.

  template contentA contentB range expectedAction expectedContentB = do                                                                                                                                           
      _docA <- createDoc "ModuleA.hs" "haskell" contentA                                                                                                                                           Add TODO2 Item   
      docB <- createDoc "ModuleB.hs" "haskell" contentB                                                                                                                                           Add TODO Item 1   
      _  <- waitForDiagnostics                                                                                                                                                                                      
      codeActions <- getCodeActions docB range                                                                                                                                                                      
      liftIO $ putStrLn $ "\nNo of code actions = " ++ (show $ length codeActions)                                                                                                                                  
      let CACodeAction action@CodeAction { _title = actionTitle } : _                                                                                                                                               
                  = sortOn (\(CACodeAction CodeAction{_title=x}) -> x) codeActions                                                                                                                                  
      liftIO $ expectedAction @=? actionTitle                                                                                                                                                                       
      executeCodeAction action                                                                                                                                                                                      
      contentAfterAction <- documentContents docB                                                                                                                                                                   
      liftIO $ expectedContentB @=? contentAfterAction

gdevanla · 2020-11-20T12:29:30Z

I ran the same command and printed out the diagnostics. Does this add any clue? The diagnostics returns with parse error the first time around.

$ time stack test --test-arguments '--pattern "extend import list"' --fast
ghcide-0.5.0: unregistering (local file changes: test/exe/Main.hs)
ghcide> configure (lib + exe + test)
ghcide> Configuring ghcide-0.5.0...
ghcide> build (lib + exe + test)
ghcide> Preprocessing executable 'ghcide-test-preprocessor' for ghcide-0.5.0..
ghcide> Building executable 'ghcide-test-preprocessor' for ghcide-0.5.0..
ghcide> Preprocessing library for ghcide-0.5.0..
ghcide> Building library for ghcide-0.5.0..
ghcide> Preprocessing executable 'ghcide' for ghcide-0.5.0..
ghcide> Building executable 'ghcide' for ghcide-0.5.0..
ghcide> Preprocessing test suite 'ghcide-tests' for ghcide-0.5.0..
ghcide> Building test suite 'ghcide-tests' for ghcide-0.5.0..
ghcide> [5 of 5] Compiling Main
ghcide> Linking .stack-work/dist/x86_64-linux/Cabal-3.2.0.0/build/ghcide-tests/ghcide-tests ...
ghcide> Preprocessing executable 'ghcide-bench' for ghcide-0.5.0..
ghcide> Building executable 'ghcide-bench' for ghcide-0.5.0..
ghcide> copy/register
ghcide> Installing executable ghcide-test-preprocessor in /home/devanla/fsf/hls/ghcide/.stack-work/install/x86_64-linux/d66a19e237aa809ac8cfd370cf12b025cd5720ba57179f663d7f8695a79ea3e5/8.10.1/bin
ghcide> Installing library in /home/devanla/fsf/hls/ghcide/.stack-work/install/x86_64-linux/d66a19e237aa809ac8cfd370cf12b025cd5720ba57179f663d7f8695a79ea3e5/8.10.1/lib/x86_64-linux-ghc-8.10.1/ghcide-0.5.0-BYH7o7yAusrAO60wzTJbHC
ghcide> Installing executable ghcide in /home/devanla/fsf/hls/ghcide/.stack-work/install/x86_64-linux/d66a19e237aa809ac8cfd370cf12b025cd5720ba57179f663d7f8695a79ea3e5/8.10.1/bin
ghcide> Installing executable ghcide-bench in /home/devanla/fsf/hls/ghcide/.stack-work/install/x86_64-linux/d66a19e237aa809ac8cfd370cf12b025cd5720ba57179f663d7f8695a79ea3e5/8.10.1/bin
ghcide> Registering library for ghcide-0.5.0..
ghcide> test (suite: ghcide-tests, args: --pattern "extend import list")

ghcide> ghcide
ghcide>   code actions
ghcide>     extend import actions
ghcide>       extend import list with multiple choices:
ghcide> Diag = [Diagnostic {_range = Range {_start = Position {_line = 0, _character = 21}, _end = Position {_line = 0, _character = 23}}, _severity = Just DsError, _code = Nothing, _source = Just "typecheck", _message = "Parse error: module header, import declaration\nor top-level declaration expected.", _tags = Nothing, _relatedInformation = Nothing}]
ghcide>
ghcide> No of code actions = 0
ghcide> FAIL
ghcide>         Exception: test/exe/Main.hs:(1127,11)-(1128,82): Non-exhaustive patterns in CACodeAction action@CodeAction {_title = actionTitle} : _
ghcide>
ghcide> 1 out of 1 tests failed (2.69s)
ghcide> Test suite ghcide-tests failed
Completed 2 action(s).
Test suite failure for package ghcide-0.5.0
    ghcide-tests:  exited with: ExitFailure 1
Logs printed to console

stack test --test-arguments '--pattern "extend import list"' --fast  12.24s user 3.58s system 94% cpu 16.714 total

Same command, run immediately but successful this time:

$ time stack test --test-arguments '--pattern "extend import list"' --fast
ghcide> test (suite: ghcide-tests, args: --pattern "extend import list")

ghcide> ghcide
ghcide>   code actions
ghcide>     extend import actions
ghcide>       extend import list with multiple choices:
ghcide> Diag = [Diagnostic {_range = Range {_start = Position {_line = 3, _character = 6}, _end = Position {_line = 3, _character = 14}}, _severity = Just DsError, _code = Nothing, _source = Just "typecheck", _message = "\8226 Variable not in scope: fromList :: [a0] -> t\n\8226 Perhaps you want to add \8216fromList\8217 to one of these import lists:\n    \8216Data.Map\8217 (/tmp/extra-dir-81931986100845/ModuleB.hs:2:1-18)\n    \8216Data.HashMap.Strict\8217 (/tmp/extra-dir-81931986100845/ModuleB.hs:3:1-29)", _tags = Nothing, _relatedInformation = Nothing}]
ghcide>
ghcide> No of code actions = 63
ghcide> OK (2.00s)
ghcide>
ghcide> All 1 tests passed (2.01s)
ghcide> Test suite ghcide-tests passed
stack test --test-arguments '--pattern "extend import list"' --fast  1.98s user 0.58s system 107% cpu 2.384 total

It turns out that the first case is the expected behavior since, there was a parsed error in the input I was feeding the testSession.But, it beats me why it succeeds the second time around.

Here is the input which is the cause of the parser error:

, testSession "extend import list with multiple choices" $ template
      (T.unlines                                                                                                                                                                                   Add TODO2 Item
            --  this is just a dummy module to help the arguments needed for this test                                                                                                            Add TODO Item 1
            [ "module ModuleA where ()"          # <------------- the source of parser error is here
               ])
      (T.unlines
            [ "module ModuleB where"
            , "import Data.Map ()"
            , "import Data.HashMap.Strict ()"
            , "foo = fromList []"
            ])
      (Range (Position 3 17) (Position 3 18))
      "Add fromList to the import list of Data.HashMap.Strict"
      (T.unlines
            [ "module ModuleB where"
            , "import Data.Map ()"
            , "import Data.HashMap.Strict (fromList)"
            , "foo = fromList []"
            ])

pepeiborra · 2020-11-20T21:00:27Z

Please take a look at the LSP transport messages to get a better understanding of the issue

…e-option

* Add support for extending import list when multiple options are available * Add function to module export list to make it available for testing * Fix typo * Add doc strings * Add tests for testing regex used to parse multiple choices for import suggestions. * Add test group * Remove trailing spaces * Hlint suggestions * Remove not used variable * Remove temporary code * Reuse matchRegExUnifySpaces * Fix test input. * Use testCase instead of testSession * Update extend import tests to assert on multiple actions. * Extend extendImports to use multiple modules for setup * Hlint changes

gdevanla added 10 commits November 17, 2020 05:17

Add support for extending import list when multiple options are avail…

c69aa9a

…able

Add function to module export list to make it available for testing

8f3ea08

Fix typo

99319b1

Add doc strings

62ec69a

Add tests for testing regex used to parse multiple choices for import…

171b191

… suggestions.

Add test group

1e03d8c

Remove trailing spaces

c3ccb7e

Hlint suggestions

fe273b3

Remove not used variable

983ebca

Remove temporary code

d751358

pepeiborra suggested changes Nov 17, 2020

View reviewed changes

src/Development/IDE/Plugin/CodeAction.hs Outdated Show resolved Hide resolved

src/Development/IDE/Plugin/CodeAction.hs Outdated Show resolved Hide resolved

test/exe/Main.hs Outdated Show resolved Hide resolved

test/exe/Main.hs Outdated Show resolved Hide resolved

Reuse matchRegExUnifySpaces

04da50e

gdevanla added 3 commits November 20, 2020 04:44

Fix test input.

fb0168b

Use testCase instead of testSession

a32e918

Update extend import tests to assert on multiple actions.

3ef045e

gdevanla mentioned this pull request Nov 21, 2020

Call to getCodeActions needs to wait - for tests lukel97/lsp-test#80

Open

gdevanla added 3 commits November 21, 2020 09:56

Extend extendImports to use multiple modules for setup

bf39775

Merge branch 'master' into extend-import-suggestions-for-more-than-on…

35252d4

…e-option

Hlint changes

de0e22e

pepeiborra approved these changes Nov 21, 2020

View reviewed changes

pepeiborra merged commit 863392b into haskell:master Nov 21, 2020

gdevanla mentioned this pull request Jan 1, 2021

Call to getCodeActions needs to wait - for tests haskell/haskell-language-server#819

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend import suggestions for more than one option #913

Extend import suggestions for more than one option #913

gdevanla commented Nov 17, 2020

pepeiborra left a comment

gdevanla commented Nov 18, 2020 •

edited

Loading

pepeiborra commented Nov 18, 2020

gdevanla commented Nov 18, 2020

gdevanla commented Nov 20, 2020 •

edited

Loading

pepeiborra commented Nov 20, 2020

Extend import suggestions for more than one option #913

Extend import suggestions for more than one option #913

Conversation

gdevanla commented Nov 17, 2020

pepeiborra left a comment

Choose a reason for hiding this comment

gdevanla commented Nov 18, 2020 • edited Loading

pepeiborra commented Nov 18, 2020

gdevanla commented Nov 18, 2020

gdevanla commented Nov 20, 2020 • edited Loading

pepeiborra commented Nov 20, 2020

gdevanla commented Nov 18, 2020 •

edited

Loading

gdevanla commented Nov 20, 2020 •

edited

Loading