You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "neilcsmith-net (via GitHub)" <gi...@apache.org> on 2026/03/11 11:14:56 UTC
[PR] WIP : Fix for reformatting issue with unnamed vars GH9255 [netbeans]
neilcsmith-net opened a new pull request, #9258:
URL: https://github.com/apache/netbeans/pull/9258
Work in progress look at #9255 This change appears to fix usage in the IDE. However, the test passes whether the formatter is changed here or not. Something appears to be different in the context of how these tests run.
Also it looks like the added Diff here is one of the factors that causes the file to be modified on reformat even when there is no actual change to the text.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "ebarboni (via GitHub)" <gi...@apache.org>.
ebarboni commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2919406030
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java:
##########
@@ -6328,7 +6325,52 @@ public void testForVar2() throws Exception {
+ " }\n"
+ "}\n";
reformat(doc, content, golden);
- JavaSourceTest.SourceLevelQueryImpl.sourceLevel = oldLevel;
+ }
+
+ public void testForVarUnnamed() throws Exception {
+ sourceLevel = "22";
+ testFile = new File(getWorkDir(), "Test.java");
+ TestUtilities.copyStringToFile(testFile, "");
+ FileObject testSourceFO = FileUtil.toFileObject(testFile);
+ DataObject testSourceDO = DataObject.find(testSourceFO);
+ EditorCookie ec = (EditorCookie) testSourceDO.getCookie(EditorCookie.class);
+ final Document doc = ec.openDocument();
+ doc.putProperty(Language.class, JavaTokenId.language());
+ String content
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for ( var _:orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ String golden
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for (var _ : orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ //check no change then formatted
+ reformat(doc, golden, golden);
+ reformat(doc, content, golden);
Review Comment:
maybe reformat shoud reformat twice,because any reformat in this file should converge to same golden
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] WIP : Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2917732356
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java:
##########
@@ -6331,6 +6331,46 @@ public void testForVar2() throws Exception {
JavaSourceTest.SourceLevelQueryImpl.sourceLevel = oldLevel;
}
+ public void testForVarUnnamed() throws Exception {
+ testFile = new File(getWorkDir(), "Test.java");
+ TestUtilities.copyStringToFile(testFile, "");
+ FileObject testSourceFO = FileUtil.toFileObject(testFile);
+ DataObject testSourceDO = DataObject.find(testSourceFO);
+ EditorCookie ec = (EditorCookie) testSourceDO.getCookie(EditorCookie.class);
+ String oldLevel = JavaSourceTest.SourceLevelQueryImpl.sourceLevel;
+ JavaSourceTest.SourceLevelQueryImpl.sourceLevel = "22";
+ final Document doc = ec.openDocument();
+ doc.putProperty(Language.class, JavaTokenId.language());
+ String content
+ = "package hierbas.del.litoral;\n\n"
+ + "public class Test {\n\n"
Review Comment:
can we use multi-line Strings for new test code?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2918481633
##########
java/java.lexer/src/org/netbeans/lib/java/lexer/JavaLexer.java:
##########
@@ -1196,7 +1196,9 @@ else if ('0' <= c && c <= '9') { // float literal
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);
Review 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] WIP : Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2917762876
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java:
##########
@@ -6331,6 +6331,46 @@ public void testForVar2() throws Exception {
JavaSourceTest.SourceLevelQueryImpl.sourceLevel = oldLevel;
}
+ public void testForVarUnnamed() throws Exception {
+ testFile = new File(getWorkDir(), "Test.java");
+ TestUtilities.copyStringToFile(testFile, "");
+ FileObject testSourceFO = FileUtil.toFileObject(testFile);
+ DataObject testSourceDO = DataObject.find(testSourceFO);
+ EditorCookie ec = (EditorCookie) testSourceDO.getCookie(EditorCookie.class);
+ String oldLevel = JavaSourceTest.SourceLevelQueryImpl.sourceLevel;
+ JavaSourceTest.SourceLevelQueryImpl.sourceLevel = "22";
+ final Document doc = ec.openDocument();
+ doc.putProperty(Language.class, JavaTokenId.language());
+ String content
+ = "package hierbas.del.litoral;\n\n"
+ + "public class Test {\n\n"
Review Comment:
Probably. This was mostly copy-pasta for now, but will update if it ends up testing anything useful!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "ebarboni (via GitHub)" <gi...@apache.org>.
ebarboni commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2919815931
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java:
##########
@@ -6328,7 +6325,52 @@ public void testForVar2() throws Exception {
+ " }\n"
+ "}\n";
reformat(doc, content, golden);
- JavaSourceTest.SourceLevelQueryImpl.sourceLevel = oldLevel;
+ }
+
+ public void testForVarUnnamed() throws Exception {
+ sourceLevel = "22";
+ testFile = new File(getWorkDir(), "Test.java");
+ TestUtilities.copyStringToFile(testFile, "");
+ FileObject testSourceFO = FileUtil.toFileObject(testFile);
+ DataObject testSourceDO = DataObject.find(testSourceFO);
+ EditorCookie ec = (EditorCookie) testSourceDO.getCookie(EditorCookie.class);
+ final Document doc = ec.openDocument();
+ doc.putProperty(Language.class, JavaTokenId.language());
+ String content
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for ( var _:orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ String golden
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for (var _ : orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ //check no change then formatted
+ reformat(doc, golden, golden);
+ reformat(doc, content, golden);
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2918481633
##########
java/java.lexer/src/org/netbeans/lib/java/lexer/JavaLexer.java:
##########
@@ -1196,7 +1196,9 @@ else if ('0' <= c && c <= '9') { // float literal
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);
Review 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` ?
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2919505846
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java:
##########
@@ -6328,7 +6325,52 @@ public void testForVar2() throws Exception {
+ " }\n"
+ "}\n";
reformat(doc, content, golden);
- JavaSourceTest.SourceLevelQueryImpl.sourceLevel = oldLevel;
+ }
+
+ public void testForVarUnnamed() throws Exception {
+ sourceLevel = "22";
+ testFile = new File(getWorkDir(), "Test.java");
+ TestUtilities.copyStringToFile(testFile, "");
+ FileObject testSourceFO = FileUtil.toFileObject(testFile);
+ DataObject testSourceDO = DataObject.find(testSourceFO);
+ EditorCookie ec = (EditorCookie) testSourceDO.getCookie(EditorCookie.class);
+ final Document doc = ec.openDocument();
+ doc.putProperty(Language.class, JavaTokenId.language());
+ String content
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for ( var _:orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ String golden
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for (var _ : orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ //check no change then formatted
+ reformat(doc, golden, golden);
+ reformat(doc, content, golden);
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2919837202
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java:
##########
@@ -6328,7 +6325,52 @@ public void testForVar2() throws Exception {
+ " }\n"
+ "}\n";
reformat(doc, content, golden);
- JavaSourceTest.SourceLevelQueryImpl.sourceLevel = oldLevel;
+ }
+
+ public void testForVarUnnamed() throws Exception {
+ sourceLevel = "22";
+ testFile = new File(getWorkDir(), "Test.java");
+ TestUtilities.copyStringToFile(testFile, "");
+ FileObject testSourceFO = FileUtil.toFileObject(testFile);
+ DataObject testSourceDO = DataObject.find(testSourceFO);
+ EditorCookie ec = (EditorCookie) testSourceDO.getCookie(EditorCookie.class);
+ final Document doc = ec.openDocument();
+ doc.putProperty(Language.class, JavaTokenId.language());
+ String content
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for ( var _:orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ String golden
+ = """
+ package hierbas.del.litoral;
+
+ public class Test {
+
+ public static void main(String[] args) {
+ int[] orderIDs = {34, 45, 23, 27, 15};
+ int total = 0;
+ for (var _ : orderIDs) {
+ total++;
+ }
+ }
+ }
+ """;
+
+ //check no change then formatted
+ reformat(doc, golden, golden);
+ reformat(doc, content, golden);
Review Comment:
Ah, yes, OK, that would make a lot of sense!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net merged PR #9258:
URL: https://github.com/apache/netbeans/pull/9258
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] WIP : Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#issuecomment-4038731274
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Fix for reformatting issue with unnamed vars GH9255 [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #9258:
URL: https://github.com/apache/netbeans/pull/9258#discussion_r2918191450
##########
java/java.lexer/src/org/netbeans/lib/java/lexer/JavaLexer.java:
##########
@@ -1196,7 +1196,9 @@ else if ('0' <= c && c <= '9') { // float literal
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);
Review 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: https://github.com/matthiasblaesing/netbeans/commit/0f4f15cbf3ec1ab30e19032288d8efccf39834f8 (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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists