Community
Participate
Working Groups
This would help solve bug 562818. Currently the ASTParser returns an emtpy node with following problems: Pb(240) Syntax error, insert "Identifier (" to complete MethodHeaderName, Pb(240) Syntax error, insert ")" to complete MethodDeclaration
(In reply to Mateusz Matela from comment #0) > This would help solve bug 562818. > > Currently the ASTParser returns an emtpy node with following problems: > Pb(240) Syntax error, insert "Identifier (" to complete MethodHeaderName, > Pb(240) Syntax error, insert ")" to complete MethodDeclaration Can you elaborate, which call returns the empty node and the errors? Tried the sample from Bug 560575, but did not understand which call to parser is returning error.
Bug 560575 is about a new feature that is not pushed in yet, so you can't reproduce the problem directly. Based on description in bug 562818 I added the following test in FormatterBugsTest: public void testBug562818() { String source = "public Record {}\n"; formatSource(source, "public Record {\n" + "}", CodeFormatter.K_CLASS_BODY_DECLARATIONS); } Then the parser gets called from DefaultCodeFormatter.parseSourceCode()
Moving to RC1 and hoping it is not urgent and required for RC1?
Will check in M3.
Bulk move out of 4.19 M1
@Manoj, Investigated this, org.eclipse.jdt.internal.core.util.CodeSnippetParsingUtil.parseClassBodyDeclarations(char[], int, int, Map<String, String>, boolean, boolean) calls org.eclipse.jdt.internal.compiler.parser.Parser.parseClassBodyDeclarations(char[], int, int, CompilationUnitDeclaration) and gets empty set of ASTNodes for record. Can you look into this?
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179512
(In reply to Sarika Sinha from comment #6) > @Manoj, > Investigated this, > org.eclipse.jdt.internal.core.util.CodeSnippetParsingUtil. > parseClassBodyDeclarations(char[], int, int, Map<String, String>, boolean, > boolean) calls > org.eclipse.jdt.internal.compiler.parser.Parser. > parseClassBodyDeclarations(char[], int, int, CompilationUnitDeclaration) > > and gets empty set of ASTNodes for record. > > Can you look into this? @Sarika: sure. If you look into the call, you can see that the call is parseClassBodyDeclarations while the expectation is to get a compact constructor. Now that is not possible since, classBodyDeclarations are a subset of RecordBodyDeclarations [ref grammar - java.g]. So this method is insufficient and we need something like parseRecordBodyDeclarations() to get the special case of compact constructor. I have added the required grammar in java.g, generated the files and created the new parseRecordBodyDeclarations() in the attached gerrit (comment 7). This is from the Parser side. Dom side (alongwith CodeFormatter code) need to be augmented with a new K_declaration and then parseRecordBodyDeclarations() need to called with the new parameter K_RECORD_BODY_DECLARATIONS(or something like that) -ref bug 562818 comment 2 also. You need take the gerrit attached and then add the dom code on top of it along with all the bells and whistles (-read API change - hence documentation/since etc). Let me know whether you can take this forward from here.
(In reply to Manoj Palat from comment #8) > (In reply to Sarika Sinha from comment #6) > > @Manoj, > > Investigated this, > > org.eclipse.jdt.internal.core.util.CodeSnippetParsingUtil. > > parseClassBodyDeclarations(char[], int, int, Map<String, String>, boolean, > > boolean) calls > > org.eclipse.jdt.internal.compiler.parser.Parser. > > parseClassBodyDeclarations(char[], int, int, CompilationUnitDeclaration) > > > > and gets empty set of ASTNodes for record. > > > > Can you look into this? > > @Sarika: sure. > > If you look into the call, you can see that the call is > parseClassBodyDeclarations while the expectation is to get a compact > constructor. Now that is not possible since, classBodyDeclarations are a > subset of RecordBodyDeclarations [ref grammar - java.g]. So this method is > insufficient and we need something like parseRecordBodyDeclarations() to get > the special case of compact constructor. > > I have added the required grammar in java.g, generated the files and created > the new parseRecordBodyDeclarations() in the attached gerrit (comment 7). > This is from the Parser side. > > Dom side (alongwith CodeFormatter code) need to be augmented with a new > K_declaration and then parseRecordBodyDeclarations() need to called with the > new parameter K_RECORD_BODY_DECLARATIONS(or something like that) -ref bug > 562818 comment 2 also. > > You need take the gerrit attached and then add the dom code on top of it > along with all the bells and whistles (-read API change - hence > documentation/since etc). > Let me know whether you can take this forward from here. Sure, Will take it fwd.
(In reply to Sarika Sinha from comment #9) > > Sure, Will take it fwd. @Sarika: Thanks, reassigning to you. Do ping if any further info/help required from the parser side.
(In reply to Manoj Palat from comment #10) > (In reply to Sarika Sinha from comment #9) > > > > Sure, Will take it fwd. > > @Sarika: Thanks, reassigning to you. Do ping if any further info/help > required from the parser side. Added the changes to the gerrit to use the new parser method and the Formatter test fails properly with junit.framework.ComparisonFailure: Different number of length. ----------- Expected ------------ public Record {\n } ------------ but was ------------ public Record {}\n
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179512 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=ea65681e2c5e1f609892c6ae4cd2b46da5c0122d
@Mateusz, Let us know if any other change is required.
(In reply to Eclipse Genie from comment #12) > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179512 was > merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=ea65681e2c5e1f609892c6ae4cd2b46da5c0122d As I just saw this commit fly by I have some questions: The values of ASTParser.K_* look strange: * previous values are 0x01, 0x02, 0x04, 0x08 as if they are meant to be or-ed into a bitset, but then the kind is always checked using a switch rather than bit comparison * the new value 0x16 doesn't seem to follow any system :) Does anyone know if there was reason behind the bitset-like values? OTOH, we cannot change existing values anyway as those are API. Just the new value should be reasonable. The same applies to the correpsonding constant in CodeFormatter. Also javadoc of the new constants look like copy-paste needing an update :) More severely, I couldn't find a way how ASTParser.astKind could ever receive this new value, see that ASTParser.setKind() is very picky about the argument. Finally, DefaultCodeFormatter.FORMAT_TO_PARSER_KIND will need an update, right?
Haven't looked at this earlier. It would it make usage a lot simpler if it could be handled without adding a new constant. Manoj, you explained that classBodyDeclarations are a subset of RecordBodyDeclarations. So maybe it would make sense to actually assume RecordBodyDeclarations when kind is K_CLASS_BODY_DECLARATIONS? (In reply to Stephan Herrmann from comment #14) > Finally, DefaultCodeFormatter.FORMAT_TO_PARSER_KIND will need an update, > right? Not only that, the new constant in DefaultCodeFormatter can't be 0x16, because it has common bits with K_COMMENTS_MASK and thus the formatter will only tries to format comments. And I'm not even sure if there should be a new constant at the formatter level. The formatter could just try to invoke parser with both kinds and use the result that doesn't have errors instead of burdening the client with deciding which kind it is.
Reopening in the light of new comments. (In reply to Mateusz Matela from comment #15) > Manoj, you explained that classBodyDeclarations are a subset of > RecordBodyDeclarations. So maybe it would make sense to actually assume > RecordBodyDeclarations when kind is K_CLASS_BODY_DECLARATIONS? @Mateusz: yes, RecordBodyDeclarations take in all of class plus the new method structure of CompactConstructor. So, I think your suggestion makes sense to consider these as class body declarations. @Sarika: Could you please take a re-look - From a grammar point of view, the changes are still required since we are looking for Records now - so the change will be in the non-grammar part of the fix.
(In reply to Manoj Palat from comment #16) > Reopening in the light of new comments. > > (In reply to Mateusz Matela from comment #15) > > > Manoj, you explained that classBodyDeclarations are a subset of > > RecordBodyDeclarations. So maybe it would make sense to actually assume > > RecordBodyDeclarations when kind is K_CLASS_BODY_DECLARATIONS? > > > @Mateusz: yes, RecordBodyDeclarations take in all of class plus the new > method structure of CompactConstructor. So, I think your suggestion makes > sense to consider these as class body declarations. > @Sarika: Could you please take a re-look - From a grammar point of view, the > changes are still required since we are looking for Records now - so the > change will be in the non-grammar part of the fix. Sure!
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180554
Changes are not looking safe for RC1 and putting up a test case to reach the recovery of record is not straight forward, will continue to work on this for M1.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180956
(In reply to Eclipse Genie from comment #20) > New Gerrit change created: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180956 +1
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180956 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1787e34296d2a15591701831f0465d0832adc1f9
(In reply to Eclipse Genie from comment #22) > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180956 was > merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=1787e34296d2a15591701831f0465d0832adc1f9 Reverting the addition of new K_RECORD_BODY_DECLARATIONS
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
