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

Syntax highlighting is broken with array syntax #728

Closed
Yanpas opened this issue Nov 28, 2018 · 8 comments · Fixed by atom/language-java#215
Closed

Syntax highlighting is broken with array syntax #728

Yanpas opened this issue Nov 28, 2018 · 8 comments · Fixed by atom/language-java#215

Comments

@Yanpas
Copy link

Yanpas commented Nov 28, 2018

[provide a description of the issue]

Environment
  • Operating System: linux
  • JDK version: 11
  • Visual Studio Code version: 1.29.1
  • Java extension version: 0.34
Steps To Reproduce

Snippet:

        for (int i = 0; i==0;){
            
        }
        String[] aaa = new String[] {
            String.class.toString(),
            "aaaa",
            "bbb",
        };
        for (int i = 0; i==0;){

        }

image

@fbricon
Copy link
Collaborator

fbricon commented Nov 29, 2018

Looks like this is caused by this issue upstream: atom/language-java#172

@fbricon
Copy link
Collaborator

fbricon commented Nov 29, 2018

@snjeza please keep an eye on this one. You can provide feedback on the proposed fix upstream, if it can help getting things moved forward.

@Vigilans
Copy link
Contributor

Vigilans commented Oct 9, 2019

Also reproduced when initialized with new operator:
image

After new keyword gets removed:
image

sadikovi pushed a commit to atom/language-java that referenced this issue Mar 25, 2020
### Description of the Change

Similar to #212, it uses a two-scope pattern to distinguish `inner-class` block and `array-initializer` block. it is based on #214 to provide correct end regex for `inner-class` and `array-initializer`.

#### Detailed description

Conside how a syntax scope terminates:
```
new Class() <end of new expression>
            ^~~~~ terminate here
new Class[] <end of new expression>
            ^~~~~ terminate here
new Class() {} <end of new expression>
               ^~~~~ terminate here
new Class[] {} <end of new expression>
               ^~~~~ terminate here
```
Previous new-expr grammar handles following ways of termination:
* inner-class: `(?<!\\])\\s*({)`...`(})`
* parens (square): `[`...`]`
* parens (round): `(`...`)`
* parens (square + curly): `[`...`]`...`{`...`}`

The usage of `\s*` suffers from new-line and comment-in-between pattern, and this should be relaxed using a two-scope model.

Besides, `inner-class` and `array-initializer` do not share same syntax (`class-body` and `code`), so they should fall in different scope.

So, this PR:
* Differentiate `object-new` and `array-declaration` using two different scopes.
* Put `inner-class` and `array-initializer` in different scopes, with similar names.

### Alternate Designs

This PR repetitively uses the end regex of new expression:
```
'end': '(?=;|\\)|\\]|\\.|,|\\?|:|}|\\+|\\-|\\*|\\/(?!\\/|\\*)|%|!|&|\\||\\^|=)'
```
to determine when a scope terminates. It would be better if we could find a way to reduce its usage. 

One possible alternative is: match when inner end token reached (`)`, `]` or `}`), not the outer token that ends it (the complex end regex).

But just like #214, it is hard to tell when `)` or `]` is met, whether there will be a `{}` block next to it. I (maybe) tried such silly regexes (cannot remember them correctly):
* outer - `(?<=\\)`...`}`, inner: `{`...`(?=}`
   The problem with it: if there's no `}` after `)`, it won't ends, causing the whole `new-expr` scope won't ends.
   e.g. `A[] arr = new A[]{ new A(), new A() };` - the comma won't ends first `new A()` because inner scope eats until `}` is met.

### Benefits

* Can correctly highlight `array-initializer` when there's any distance of spaces between ']' and '{'.
* Can correctly highlight `array-initializer` when '{' is placed at new line or with a comment in between.
* Introduced a new scope `array-initializer`, in correspondence with `inner-class`.
* Patterns are more neat and uniform.

### Possible Drawbacks

* Undetected possible regression issues.

### Applicable Issues

Fix #172
Fix #199
Fix redhat-developer/vscode-java#728
@nkibbey
Copy link

nkibbey commented Apr 8, 2020

Found that some array syntax still breaks some highlighting

using
vscode 1.43.2
java extension 0.59.1
java 8

poorHighlighting

@Eskibear
Copy link
Contributor

@nkibbey It has been fixed in upstream, and has been adopted in vscode-insiders. I just tried in latest vscode-insiders, and it worked. So it's expected to be shipped in the next stable release of vscode.

@fbricon
Copy link
Collaborator

fbricon commented Apr 27, 2020

@nkibbey's case works in stable vscode 1.44.0
@Yanpas's case is fixed with semantic highlighting enabled in vscode-java 0.61.0

@fbricon fbricon closed this as completed Apr 27, 2020
@fbricon fbricon added this to the End April 2020 milestone Apr 27, 2020
@Yanpas
Copy link
Author

Yanpas commented Apr 27, 2020

@fbricon Vscode 1.44.2, the following snippet isn't highlighted correctly:

class A {
public static void name() {
	
	for (int i = 0; i==0;){
		
	}
	String[] aaa = new String[] {
		String.class.toString(),
		"aaaa",
		"bbb",
	};
	for (int i = 0; i==0;){
		
	}
}
}

Maybe it's https://github.com/microsoft/vscode/blob/master/extensions/java/syntaxes/java.tmLanguage.json problem

@fbricon
Copy link
Collaborator

fbricon commented Apr 27, 2020

@Yanpas sorry I did update my previous comment. Your issue is fixed in upcoming 0.61.0, with semantic highlighting turned on:

Screen Shot 2020-04-27 at 11 48 29 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants