Fix for reformatting issue with unnamed vars GH9255#9258
Fix for reformatting issue with unnamed vars GH9255#9258neilcsmith-net wants to merge 1 commit intoapache:masterfrom
Conversation
neilcsmith-net
added
Java
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java
Outdated
Show resolved
Hide resolved
|
OK, difference in test behaviour was due to the incorrect way of setting the source level copied from other tests. Updated all. Test now fails in same way as IDE without this change. |
Update JavaLexer to generate correct token type for `var _`. Add lexer and formatting tests.
neilcsmith-net
changed the title
| if ((c = nextChar()) == 'r') { | ||
| c = nextChar(); | ||
| // Check whether the given char is non-ident and if so then return keyword | ||
| if (c != EOF && !Character.isJavaIdentifierPart(c = translateSurrogates(c)) && | ||
| version >= 10) { | ||
| // For surrogate 2 chars must be backed up | ||
| backup((c >= Character.MIN_SUPPLEMENTARY_CODE_POINT) ? 2 : 1); | ||
|
|
||
| int len = input.readLength(); | ||
|
|
||
| Token next = nextToken(); | ||
| boolean varKeyword = false; | ||
|
|
||
| if (AFTER_VAR_TOKENS.contains(next.id())) { | ||
| do { | ||
| next = nextToken(); | ||
| } while (next != null && AFTER_VAR_TOKENS.contains(next.id())); | ||
|
|
||
| varKeyword = next != null && next.id() == JavaTokenId.IDENTIFIER; | ||
| varKeyword = next != null | ||
| && (next.id() == JavaTokenId.IDENTIFIER | ||
| || next.id() == JavaTokenId.UNDERSCORE); | ||
| } | ||
|
|
||
| input.backup(input.readLengthEOF()- len); |
There was a problem hiding this comment.
Just an observation and not intended to stop this:
This whole code is problematic. Its existence breaks JShell support for var. The nested language infrastructure breaks when backtracking is used in this manner. I suspect the issue here is that this is not just backtracking, but backtracking over token boundaries.
You can test this by opening "Tools" -> "Open Java Platform Shell", input var s = "String"; and try to let that be evaluated. "Enter" is not accepted and you can observe exception on command line.
With the backtracking code removed (see: matthiasblaesing@0f4f15c (first commit of https://github.com/matthiasblaesing/netbeans/commits/jshell_lexer2/ and inspired by @lahodaj approach in a different PR) execution is at least done (there are still problems there).
There was a problem hiding this comment.
Makes sense (not that I use the JShell support, so hadn't noticed). This ended up being a minimal fix after a long trawl into the reformatter. I didn't really look further at the lexer.
I note that your linked commit shows the same issue in TreeUtilities::isVarType and Reformatter::isVarIdentifier ?
I wonder if it would make sense or be less problematic to always return a var token type, and then check whether it's valid in circumstances where that's necessary?
|
|
||
| //check no change then formatted | ||
| reformat(doc, golden, golden); | ||
| reformat(doc, content, golden); |
There was a problem hiding this comment.
maybe reformat shoud reformat twice,because any reformat in this file should converge to same golden
There was a problem hiding this comment.
Not sure what you mean? But the tests are isolated and don't (shouldn't) affect each other. The entire contents of doc is set to the second String parameter, reformatted, and tested against the third String parameter.
The first check is there to make sure golden does not change.
There was a problem hiding this comment.
exactly ensure golden is golden but for every use of reformat in the test.
More for the future of course (because it break 2 tests):
private void reformat(Document doc, String content, String golden) throws Exception {
reformat(doc, golden, golden, 0, golden.length());
reformat(doc, content, golden, 0, content.length());
}
Tested for fun (only 2 golden not golden),
test135210 generate javadoc line
testJavadoc reformat is in 2 step
There was a problem hiding this comment.
Ah, yes, OK, that would make a lot of sense!
4 participants
Update JavaLexer to return token type of
VARwhen followed by underscore as invar _. Fixes formatter issues seen in #9255 Also ensures correct token colouring for this usage.