nwolverson / purescript-language-server

MIT License
183 stars 41 forks source link

Allow lookup of Constructors again #190

Closed i-am-the-slime closed 1 year ago

i-am-the-slime commented 1 year ago

The filter for identifiers was too strict. Because if you're looking for Just it won't appear in the list of identifiers for an import as Just or (Just) but rather Maybe(..).

That kind of means in the current shape, this could lead to false positives when names overlap. I think this could be improved by either returning an exploded list of identifiers from psc-ide (Maybe, Just, Nothing) instead of (Maybe) for these kinds of imports. Maybe also by being more type-safe about the types of the identifiers (is it a type alias, or a constructor, or a type name or a newtype or whatever) and matching this info with what we get from the CST parser.

Closes #145

nwolverson commented 1 year ago

There will be a proper fix for this with the next PureScript compiler release, so it's 100% not worth trying to be more accurate. The worst case is actually something like goto-definition, because that should find only 1 match, but with this change you may go to the wrong place (and without it, you may go nowhere).

I'm undecided given that if this should be merged; but I certainly don't want to do that before getting the changes to work with the upcoming compiler in, version dependent, and checking that this code path is then obsoleted. (I have the start of those locally)

i-am-the-slime commented 1 year ago

I think I'll close it. It does indeed go to the wrong place on conflicts.

nwolverson commented 1 year ago

Well presumably it sometimes goes to the right place and sometimes the wrong... to be exactly wrong all the time would require the same information as to be right :D