nwolverson / purescript-language-server

MIT License
184 stars 42 forks source link

Apply all suggestions broken for certain imports #67

Closed nwolverson closed 4 years ago

nwolverson commented 4 years ago

Seems like the suggestion to remove data constructor imports when only the type is used

  1. may appear multiple times, with a single fix
  2. breaks apply all (import) suggestions until resolved
i-am-the-slime commented 4 years ago

I think this happens in the following combination:

There's both an unnecessary import for a data constructor and some unused functions on the same line ((..) and fromMaybe are unnecessary in this example):

import Data.Maybe (Maybe(..), fromMaybe)

example :: Maybe String -> Maybe String
example = identity

🔝 breaks.

When they occur in isolation it works (only fromMaybe is redundant here):

import Data.Maybe (Maybe(..), fromMaybe)
import PscIdeClient (compileCode)

example :: Maybe String -> Maybe String
example _ = Just ""

as well as (only (..) is redundant here)

import Data.Maybe (Maybe(..), fromMaybe)

example :: Maybe String -> Maybe String
example = identity

example2 = fromMaybe

work.

i-am-the-slime commented 4 years ago

After more investigation something surprising happens for the first example when specifying: "Apply suggestion" on the line in question. It does what it should and leaves us with: import Data.Maybe (Maybe) the same is true for "Remove unused reference". It also produces the complete correct result.

nwolverson commented 4 years ago

PR #78 fixes this case for me, but the following fails:

import Data.Maybe (Maybe(..), fromMaybe)
import Data.Maybe (Maybe(..), fromMaybe)

example :: Maybe String -> Maybe String
example = identity

I think this is just due to overlaps being prohibited, in this case they are still present due to not being simple duplicates. It's clear in the LSP spec that overlaps are not permitted, but I think either earlier versions of vscode allowed this or the other form of edit in the earlier LSP spec did so.

nwolverson commented 4 years ago

I think I have it in 3fb5f186be945fc62f81ca56df8764944b1e2103