Add support for extensionless documents to the parametric language server

[urbanfly]

When there is only one language contributed and the document has no extension, use the single language.

Fixes #862
Fixes #868

[DavyLandman]

I'm impressed how little changes there are on the java side. So let's indeed make sure our tests have some coverage of this feature, such that we know it works properly.

[DavyLandman]

If the test succeed, I think this is ready to merge.

[urbanfly]

If the test succeed, I think this is ready to merge.

The new test fails on MacOS. The steps were basically copied from has highlighting and parse errors except:

1. a couple of commands are executed
   • other tests also use commands
2. InputBox is used to type/choose the language
   • that step apparently worked b/c I can see this in the logs

     [debug] [parametric-1] org.rascalmpl.vscode.lsp.parametric.ParametricTextDocumentService Did Change file: untitled:Untitled-1
     

[DavyLandman]

In general we've found that there are cases where the test need some extra retries (ignore failures and friends).

We've worked hard to make sure that the current tests succeed in every run, if you're stuck on this, please let me know, I'll ask someone to take a look and see if they can figure out why it's failing.

[urbanfly]

if you're stuck on this, please let me know, I'll ask someone to take a look and see if they can figure out why it's failing.

Please have someone else take a look.

[rodinaarssen]

Hi @urbanfly.

What a remarkably small PR for this feature! Your test issues are caused by a combination of two factors:

• The parametric text document service did not properly clear out diagnostics of "transient files", such as the unsaved untitled files you are playing with. I've submitted a PR (#902) to fix that.
• I think the special-casing of extensionless files you added in this PR should be applied at another method; I took the liberty of adding a suggestion in this review.

With these two things fixed, it "works on my machine". After the merge of the aforementioned PR, could you update your branch and see whether the test issues are resolved? Thanks!

[urbanfly]

Thanks for the help, @rodinaarssen. Can you help me understand how "not clearing diagnostics" was causing my test to fail on MacOS?

---

Also, why is the change to isLanguageRegistered necessary? I don't like making it "lie" about whether a language is registered. It is used:

1. when a file is deleted, to publish empty diags

   • But why even check isLanguageRegistered here? If the client is informing the language server that a file was deleted, then that document was already (manually?) associated with that language. What would be the harm in just clearing diags without the check?

2. to skip triggerAnalyzer, when files are opened or changed (or when a lang. is registered "late")

   • The check here appears to be trying to avoid an UnsupportedOperationException when facts(loc) calls language(loc).
   • Wouldn't it be better to make facts(loc) use safeLanguage(loc) and fail gracefully if no language is provided? maybe adding safeFacts(loc)?

3. to skip certain renamed files

   • Used within bundleRenamesByContribution, but wouldn't it be better to use safeLanguage and continue; if there is no language?

[rodinaarssen]

Thanks for the help, @rodinaarssen. Can you help me understand how "not clearing diagnostics" was causing my test to fail on MacOS?

The stack traces in the CI logs and the screenshots indicated to me that the uncleared diagnostics were causing UX issues in the IDE. In this case, a parse error would not be cleared upon closing the untitled file, and it would be reinstated by VS Code when a new untitled file (with the same "name") would be opened, and it would even persist with correct pico.

I hope that solving this issue will make the test succeed, but if not, we'll have to dive deeper. At the very least then, this attempt to fix your test will have fixed an actual bug.

Also, why is the change to isLanguageRegistered necessary? I don't like making it "lie" about whether a language is registered.

Playing the devil's advocate a bit: isn't it, in essence, the same lie as in safeLanguage?

1. when a file is deleted, to publish empty diags
   • But why even check isLanguageRegistered here? If the client is informing the language server that a file was deleted, then that document was already (manually?) associated with that language. What would be the harm in just clearing diags without the check?

I don't think there's any guarantee that the argument of didDeleteFiles only contains files that we're managing.

2. to skip triggerAnalyzer, when files are opened or changed (or when a lang. is registered "late")
   • The check here appears to be trying to avoid an UnsupportedOperationException when facts(loc) calls language(loc).
   • Wouldn't it be better to make facts(loc) use safeLanguage(loc) and fail gracefully if no language is provided? maybe adding safeFacts(loc)?
3. to skip certain renamed files
   • Used within bundleRenamesByContribution, but wouldn't it be better to use safeLanguage and continue; if there is no language?

I agree that isLanguageRegistered is not essential and that safeLanguage could be used instead.

[urbanfly]

uncleared diagnostics were causing UX issues

The test doesn't assert "no red squiggles", though.

isn't it, in essence, the same lie as in safeLanguage

One is asking an explicit question "Is this language registered?"; the other is saying "give me the language for this document - if you can figure it out". I don't see the change to safeLanguage as a "lie", just that it can provide a language in more cases now.

I don't think there's any guarantee that the argument of didDeleteFiles only contains files that we're managing.

@DavyLandman and I experimented with this a bit and I think we agreed that removing the condition is fine. It will be called for unmanaged files, but there should be no harm in telling VS Code "we don't have any diags for this file"

[rodinaarssen]

I updated the code after discussing with @DavyLandman and made this branch up-to-date.

isLanguageRegistered has been removed

[rodinaarssen]

Unfortunately, it seems to have been a red herring (although the logs identified an actual bug). We'll dig deeper to see whether we can pinpoint what's causing this particular test to fail on macOS; we know that the test framework sometimes behaves differently on macOS than on Windows/Ubuntu.