Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should enrich semantic highlighting. #9058

Open
sunix opened this issue Feb 12, 2021 · 6 comments
Open

should enrich semantic highlighting. #9058

sunix opened this issue Feb 12, 2021 · 6 comments
Labels
java issues related to the java language monaco issues related to monaco vscode issues related to VSCode compatibility

Comments

@sunix
Copy link
Contributor

sunix commented Feb 12, 2021

Bug Description:

Got that issue with vscode-java: when enabled, semantic highlighting is overriding existing syntax highlighting. It should enrich it.

Steps to Reproduce:

  1. Start plain Theia on a Java11 environment
  2. Install the latest vscode-java from openvsix
  3. clone a java project for instance: https://github.com/che-samples/console-java-simple.git
  4. make sure that "editor.semanticHighlighting.enabled": true and "java.semanticHighlighting.enabled": true
  5. open a java file

Selection_220

Additional Information

  • Operating System: kubernetes
  • Theia Version: master branch
@sunix
Copy link
Contributor Author

sunix commented Feb 12, 2021

Maybe this is related to monaco version ?

@vince-fugnitto vince-fugnitto added java issues related to the java language monaco issues related to monaco vscode issues related to VSCode compatibility labels Feb 12, 2021
@azatsarynnyy
Copy link
Member

Maybe this is related to monaco version ?

That's unlikely.
I see the error from Java LS each time when I'm changing Java's semantic highlighting preference:

jdt

Here's the error from Java LS:

[Error - 8:43:11 PM] Feb 12, 2021, 8:43:11 PM Failed to resolve user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
java.net.URISyntaxException: Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
	at java.base/java.net.URI$Parser.fail(URI.java:2938)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3109)
	at java.base/java.net.URI$Parser.parse(URI.java:3135)
	at java.base/java.net.URI.<init>(URI.java:623)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.toURI(JDTUtils.java:875)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.resolveCompilationUnit(JDTUtils.java:159)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceEventsHandler.didChangeWatchedFiles(WorkspaceEventsHandler.java:82)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.didChangeWatchedFiles(JDTLanguageServer.java:487)
	at jdk.internal.reflect.GeneratedMethodAccessor16.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:65)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.notify(GenericEndpoint.java:152)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleNotification(RemoteEndpoint.java:220)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:187)
	at org.eclipse.jdt.ls.core.internal.ParentProcessWatcher.lambda$0(ParentProcessWatcher.java:123)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)

[Error - 8:43:11 PM] Feb 12, 2021, 8:43:11 PM Failed to resolve user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
java.net.URISyntaxException: Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
	at java.base/java.net.URI$Parser.fail(URI.java:2938)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3109)
	at java.base/java.net.URI$Parser.parse(URI.java:3135)
	at java.base/java.net.URI.<init>(URI.java:623)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.toURI(JDTUtils.java:875)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.findFile(JDTUtils.java:802)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.getFileOrFolder(JDTUtils.java:968)
	at org.eclipse.jdt.ls.core.internal.managers.StandardProjectsManager.fileChanged(StandardProjectsManager.java:216)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceEventsHandler.didChangeWatchedFiles(WorkspaceEventsHandler.java:108)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.didChangeWatchedFiles(JDTLanguageServer.java:487)
	at jdk.internal.reflect.GeneratedMethodAccessor16.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:65)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.notify(GenericEndpoint.java:152)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleNotification(RemoteEndpoint.java:220)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:187)
	at org.eclipse.jdt.ls.core.internal.ParentProcessWatcher.lambda$0(ParentProcessWatcher.java:123)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)



I hope it helps to investigate the root cause of the problem.

@sunix
Copy link
Contributor Author

sunix commented Feb 12, 2021

I don't have any error, following the steps above, and still have the syntax highlighting issue:
image

So I am not sure, the java ls error is related to this problem.

@nils-mosbach
Copy link

Same here. That actually appeared after upgrading the vscode-java extension to version 0.69.0. (JDK 15 support)
I've also tried to use the latest version (0.74.0) without any success.

The good news: after disabling both options (semantic highlighting) everything works as expected.

I also had an older quarkus plugin with vscode-java 0.63.0 and that still works fine. So I guess it must have been broken somewhere in between 0.63.0 and 0.69.0.

@sunix
Copy link
Contributor Author

sunix commented Feb 13, 2021

@0dinD
Copy link

0dinD commented Mar 12, 2021

Hey, I just saw this issue and thought I'd do a quick investigation since I've contributed a large part of the new semantic highlighting implementation with semantic tokens in the Java language server used by the extension.

I see the error from Java LS each time when I'm changing Java's semantic highlighting preference

From reading the log (and experimenting on my own) the error doesn't seem to have anything to do with the semantic tokens. See #8412, which is about the same exact exception.

Maybe this is related to monaco version ?

It's definitely related to the semantic tokens support in Monaco. The semantic tokens data arrives in correct shape to Theia, but the tokens aren't applied correctly (see microsoft/monaco-editor#1833). These are the main problems with how semantic tokens are applied in Theia/Monaco:

  • Semantic tokens always override TextMate tokens, which means that semantic tokens remove the styling of tokens in the code unless the current color theme contains semantic token rules.
    • In VS Code, if a semantic token doesn't have an applicable styling rule, it is ignored and the TextMate styling still shows up.
  • Themeing support for semantic tokens in monaco is currently using a temporary workaround, which doesn't support all of the theme rules for semantic tokens defined by VS Code (such as the token type wildcard *.readonly, or language specifiers keyword:java). Additionally, semantic token rules are applied from monaco.editor.ITokenThemeRule#token, the same place as for normal TextMate token rules (see the Monaco playground example).

With this information in mind, I noticed it's actually possible to style the problematic semantic tokens in Theia by putting the semantic token rule in a TextMate rule, like so (from a VS Code color theme):

{
    "tokenColors": [
        {
            "scope": "class.declaration", // note, this is a semantic token rule, not really a TextMate scope
            "settings": {
                "foreground": "#FF0000"
            }
        }
    ]
}

But again, this comes with limitations, and won't work for existing themes which will have all of their semantic token rules defined in semanticTokenColors, not in tokenColors. Although sometimes a TextMate scope will be a valid semantic token rule, and get applied even though that's not what the theme author intended.

What Theia and/or Monaco should probably do is (in order of importance):

  1. Not override TextMate coloring unless there's actually a matching semantic token rule for the token in the current color theme.
  2. Support the semanticTokenColors property in VS Code themes (relevant code seems to be here). Also avoid reading semantic token rules from TextMate rules (the tokenColors property). This will ensure all VS Code color themes using semantic token rules are working correctly.
  3. Make sure that semantic token rules are parsed and applied the same way as in VS Code, to add support for rules like *.readonly or keyword:java.
  4. Register scope mapping for the standard token types defined in LSP, as is done in VS Code (relevant code here). Scope mapping maps semantic token rules to TextMate scopes, which means that old themes using only TextMate rules can still apply styling to semantic tokens (relevant documentation here).

Unless I'm missing something this problem shouldn't be unique to the Java extension, there are plenty of extensions providing semantic highlighting nowadays, such as the builtin TypeScript extension, rust-analyzer and the C# extension. But I'm guessing those extensions haven't been updated to the latest versions in Theia, which would explain why the problem hasn't occured (yet).

I hope this can be of help, let me know if you have further questions about the semantic tokens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java issues related to the java language monaco issues related to monaco vscode issues related to VSCode compatibility
Projects
None yet
Development

No branches or pull requests

5 participants