Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

Add code action for remove all redundant imports #867

Merged
merged 6 commits into from
Oct 18, 2020

Conversation

berberman
Copy link
Contributor

This PR implements haskell/haskell-language-server#339. The code removing single redundant import can be reused.

src/Development/IDE/Plugin/CodeAction.hs Outdated Show resolved Hide resolved
src/Development/IDE/Plugin/CodeAction.hs Outdated Show resolved Hide resolved
src/Development/IDE/Plugin/CodeAction.hs Outdated Show resolved Hide resolved
@pepeiborra
Copy link
Collaborator

Does it work? Can you add a test please?

@berberman
Copy link
Contributor Author

Any instruction of tests? I'm not sure what to add in removeImportTests.

@pepeiborra
Copy link
Collaborator

All the tests live in test/exe/Main.hs
You can find the tests for the redundant imports code action there too

@berberman
Copy link
Contributor Author

berberman commented Oct 17, 2020

removeImportTests :: TestTree
removeImportTests = testGroup "remove import actions"
  [ testSession "redundant" $ do
      let contentA = T.unlines
            [ "module ModuleA where"
            ]
      _docA <- createDoc "ModuleA.hs" "haskell" contentA
      let contentB = T.unlines
            [ "{-# OPTIONS_GHC -Wunused-imports #-}"
            , "module ModuleB where"
            , "import ModuleA"
            , "stuffB :: Integer"
            , "stuffB = 123"
            ]
      docB <- createDoc "ModuleB.hs" "haskell" contentB
      _ <- waitForDiagnostics
      -- two code actions are expected
      -- one for remove the single redundant import in range, the other for remove all
      [CACodeAction action@CodeAction { _title = actionTitle }] 
          <- getCodeActions docB (Range (Position 2 0) (Position 2 5))
      liftIO $ "Remove import" @=? actionTitle
      executeCodeAction action
      -- the import has been removed so far...
      contentAfterAction <- documentContents docB
      let expectedContentAfterAction = T.unlines
            [ "{-# OPTIONS_GHC -Wunused-imports #-}"
            , "module ModuleB where"
            , "stuffB :: Integer"
            , "stuffB = 123"
            ]
      liftIO $ expectedContentAfterAction @=? contentAfterAction
  , -- ......

executeCodeAction here applies the edit which removes the single redundant import to the document, so I have no where to place the test of remove all redundant imports. Can I extract TextEdit from CodeAction then use applyEdit to avoid modifying the document in session directly, or I have to create separate tests?

@pepeiborra
Copy link
Collaborator

Please create a separate test

@berberman
Copy link
Contributor Author

Ok. For removeImportTests, we have to modify each pattern matching on getCodeActions, because there will be a new code action. I think [CACodeAction action@CodeAction { _title = actionTitle }, _] would be enough, since we know that "Remove all redundant imports" always appears later than "Remove import".

@berberman berberman requested a review from pepeiborra October 18, 2020 02:28
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

Looks good with some minor suggestions. Thank you for adding a test!

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
@berberman
Copy link
Contributor Author

Thanks for the review and suggestions! I enum more cases in the test.

@berberman berberman requested a review from pepeiborra October 18, 2020 11:24
@pepeiborra pepeiborra merged commit cf143ea into haskell:master Oct 18, 2020
@pepeiborra
Copy link
Collaborator

Merged! Thanks again, this is an awesome contribution I've been wanting for a long time.

pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Add code action for remove all redundant imports

* Call suggestRemoveRedundantImport only once

* Adjust tests for code action removing all redundant imports

* Update src/Development/IDE/Plugin/CodeAction.hs

Co-authored-by: Pepe Iborra <[email protected]>

* Refactor removeAll

* Update the test of remove all redundant imports

Co-authored-by: Pepe Iborra <[email protected]>
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Add code action for remove all redundant imports

* Call suggestRemoveRedundantImport only once

* Adjust tests for code action removing all redundant imports

* Update src/Development/IDE/Plugin/CodeAction.hs

Co-authored-by: Pepe Iborra <[email protected]>

* Refactor removeAll

* Update the test of remove all redundant imports

Co-authored-by: Pepe Iborra <[email protected]>
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Add code action for remove all redundant imports

* Call suggestRemoveRedundantImport only once

* Adjust tests for code action removing all redundant imports

* Update src/Development/IDE/Plugin/CodeAction.hs

Co-authored-by: Pepe Iborra <[email protected]>

* Refactor removeAll

* Update the test of remove all redundant imports

Co-authored-by: Pepe Iborra <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants