durkiewicz / elm-plugin

Elm language support plugin for IntelliJ IDEA.
MIT License
136 stars 15 forks source link

References are being resolved without going through import clauses #54

Closed klazuka closed 7 years ago

klazuka commented 7 years ago

@durkiewicz commit 8d247e8 introduced a pretty big regression in the PsiReference system. The plugin is incorrectly marking qualified references ("absolute references") as resolved regardless of whether the module that exposes the target value/type is actually imported.

Example

This can be seen with a very simple program (assuming that you have already install the Html package):

main = Html.text "foo"

Prior to commit 8d247e8, Html was correctly marked as unresolved (because the developer forgot to import the Html module). But after that commit, the reference is no longer unresolved, and the user will not find out about the problem until they go to compile (at which point elm-make will complain that Html has not been imported).

Scope of the Problem

It affects qualified references to both values and types (or what you call AbsoluteValueReference and AbsoluteTypeReference) when the target module is present in the project, but is not actually imported in the reference's source file. It does not affect unqualified references (what you call an Exposed reference).

Root Cause

It appears that the problem is that ElmMixedCasePathImpl and ElmUpperCasePathImpl no longer provide the (now-deleted) ElmPartOfPathModuleReference in their reference stream. Instead they use the new ElmContainingModuleReference, which resolves the reference that it contains using the project module index (ElmModuleIndex.getFilesByModuleName())—thereby ignoring whether the user has actually imported the module into the source file that contains the referencing element. As best I can tell, the code which was deleted in ElmPartOfPathModuleReference does look for import clauses when resolving a reference.

Solution

This code is too tricky for me to attempt a fix. I wish I had a better understanding of the reference system architecture. (Perhaps you can write up some quick notes and check them in to a docs folder?) If you have time to fix the root cause, that'd be great. Otherwise, I would recommend reverting this change as it breaks a very common scenario (basic development using qualified references) while fixing a much rarer scenario (issue #30).

As always, thank you for the work you've done on this plugin.

durkiewicz commented 7 years ago

@klazuka Thanks for finding that out. I'll try to fix that as it's definitely a major issue. I don't use the plugin anymore - that's why I haven't noticed that.

klazuka commented 7 years ago

@durkiewicz do you think it's likely that you will return to Elm? (apparently 0.19 will have a faster compiler, which I believe was the problem you were running into)

I intend to do a bunch more work on the plugin (find usages, rename module, structure tool window, etc.), so perhaps I should just do it in my own fork?