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

Discard diagnostics from type check in suggestModuleTypo #943

Closed
wants to merge 1 commit into from

Conversation

berberman
Copy link
Contributor

@pepeiborra
Copy link
Collaborator

The code change doesn't do what the PR title says. It filters out a code action, not a diagnostic. My concern is that this could end up filtering out suggestModuleTypo code actions even in situations where there is no duplicate.

Moreover, since the duplicate diagnostic has not actually been filtered, the underlying problem still exists. Ghcide still reports 2 diagnostics for the same problem to the LSP client. And what about other code actions the could be triggered by the duplicate diagnostic?

@berberman berberman marked this pull request as draft December 8, 2020 01:24
@berberman
Copy link
Contributor Author

IIUC, two diagnostics are reported from locateModule and typecheckModule respectively. We always calculate the graph of modules, which uses import declarations, before parsing and typechecking a specific module. Thus the diagnostic from "typecheck" populates later than that from "not found". I haven't encountered the case that the diagnostic from "typecheck" showed up without the other's company.

Currently, only suggestModuleTypo uses this kind of diagnostic to correct the import list, so I think we can filter it out in typecheckModule, keeping the one from locateModule .

@pepeiborra
Copy link
Collaborator

Good analysis! Your findings suggest that locateModule should not be reporting any diagnostics, and the result type should change to m (Maybe Import). What do you think?

@berberman
Copy link
Contributor Author

It should be OK to remove diagnostics from locateModule. I think the only advantage of using them is that we can fix the wrong import before the typecheck. But this would require us to filter typecheck warnings, which is not graceful.

@berberman
Copy link
Contributor Author

Hmmm, It seemed that when loading a file first time, if it contained a wrong import, ghcide won't go ahead. So there were no hints after I changed the type of locateModule to m (Maybe Import):
image

@pepeiborra
Copy link
Collaborator

Hmm, you are right, I can repro. Will look into it this weekend

@pepeiborra
Copy link
Collaborator

I tracked this down to a tricky issue with stale data. A fix is included in #952

@berberman berberman closed this Dec 13, 2020
@berberman berberman deleted the patch-module branch December 13, 2020 11:41
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.

Two code actions for import module name; one of them is broken
2 participants