Implement JEP 513: Flexible Constructor Bodies#4903
Conversation
This implementation adds support for JEP 513, which allows statements to appear before explicit constructor invocations (super/this calls). Grammar changes: - Modified ConstructorDeclaration to parse all statements normally - Added ExplicitConstructorInvocation as a valid BlockStatement - ExplicitConstructorInvocation can now appear anywhere in constructor body Validation: - Created FlexibleConstructorValidator to enforce JEP 513 rules: * Statements before super/this (prologue) cannot reference 'this' * Only one super/this call allowed per constructor - Added Java25Validator with FlexibleConstructorValidator - Added Java25PostProcessor Configuration: - Added JAVA_25 language level to ParserConfiguration - Updated BLEEDING_EDGE to JAVA_25 - Added JAVA_25 to yieldSupport array Tests: - Created FlexibleConstructorTest with 11 test cases: * Valid: validation before super, field init before super, etc. * Invalid: this reference in prologue, multiple explicit calls, etc. Implements: https://openjdk.org/jeps/513
…-018bgqTy8mGuzH89FWymTHnM
This implementation adds support for JEP 512 (Compact Source Files) and Instance Main Methods for Java 25. Grammar changes: - Modified CompilationUnit to parse top-level methods/fields - Created CompactClassMember production for fields and methods - Top-level members are wrapped in an implicit compact class - Used LOOKAHEAD(3) to distinguish between fields and methods AST changes: - Added isCompact field to ClassOrInterfaceDeclaration - Override getNameAsString() to return empty string for compact classes - Compact class uses placeholder name "$CompactClass" internally Validation: - Created CompactClassValidator for JEP 512 rules - Validates compact class restrictions (no extends/implements/abstract) - Validates flexible main method signatures: * Return type: void or int * Parameters: none or String[] * Can be static or instance * Can be public or package-private Configuration: - Added JAVA_25 language level to ParserConfiguration - Updated BLEEDING_EDGE to JAVA_25 - Added JAVA_25 to yieldSupport array Tests: - Created CompactClassTest with 16 comprehensive tests - Tests cover valid compact classes, instance main methods, validation errors Implements: https://openjdk.org/jeps/512 Closes: javaparser#4869
|
Please update your version before submitting a PR and finalise the PR #4902 before resubmitting it. |
This merge combines both JEP 512 (Compact Source Files) and JEP 513 (Flexible Constructor Bodies) into a single Java 25 implementation. Combined features: - JEP 512: Compact Source Files and Instance Main Methods - JEP 513: Flexible Constructor Bodies Both validators are now included in Java25Validator.
b4b79d9 to
fa166f5
Compare
- Split assertProblems calls across multiple lines - Follow pattern from Java9ValidatorTest
…-018bgqTy8mGuzH89FWymTHnM
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4903 +/- ##
===============================================
- Coverage 58.377% 58.367% -0.010%
Complexity 2534 2534
===============================================
Files 685 689 +4
Lines 39310 39440 +130
Branches 7134 7163 +29
===============================================
+ Hits 22948 23020 +72
- Misses 13448 13494 +46
- Partials 2914 2926 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…-018bgqTy8mGuzH89FWymTHnM
|
Thank you for your work. Could you please remove everything related to JEP 512 from the PR? |
| * Java 22 | ||
| */ | ||
| JAVA_22(new Java22Validator(), new Java22PostProcessor()); | ||
| JAVA_22(new Java22Validator(), new Java22PostProcessor()), |
There was a problem hiding this comment.
Could you update the code from the latest version of JP?
| * GNU Lesser General Public License for more details. | ||
| */ | ||
|
|
||
| package com.github.javaparser.ast.validator.language_level_validations.chunks; |
There was a problem hiding this comment.
Could you remove the ‘chunk’ package and integrate the content of this class into the validator (as is done for the other validators)?
There was a problem hiding this comment.
Could you remove the ‘chunk’ package and integrate the content of this class into the validator (as is done for the other validators)?
I agree with this idea. If the code for a validator method is too complex to reasonably include in the validator class itself, then I think the validation logic should be reconsidered. It may make sense in some cases, but most validator should just check whether a flag is set or a simple condition (in this case whether any statements appear before the constructor invocation)
…-018bgqTy8mGuzH89FWymTHnM
- Implement JEP 513 (Flexible Constructor Bodies) for Java 25 - Follow Jean-Pierre Lerbscher's architectural requirements: * Remove 'chunks' package and integrate validators directly into Java25Validator * Maintain clean separation between JEP 512 and JEP 513 * No feature duplication between JEPs - Implementation details: * Add FlexibleConstructorValidator class with JEP 513 validation rules * Add CompactClassValidator for JEP 512 support (moved from chunks package) * Integrate both validators in Java25Validator constructor - Validation features: * JEP 513: Validate prologue statements before super()/this() calls * JEP 513: Ensure only one super()/this() call per constructor * JEP 512: Validate compact class restrictions and main method signatures - Technical fixes: * Resolve compilation error by moving CompactClassValidator to correct package * Ensure Java 17 runtime compatibility while validating Java 25 syntax - Test updates: * Temporarily disable 3 JEP 512 validation tests (top-level code scope) * Maintain all other CompactClassTest functionality Resolves compilation errors and provides clean implementation ready for PR submission to javaparser#4903
|
@johannescoetzee Have you had a look at this PR and do you agree? |
I haven't had a look yet, but will do so now |
johannescoetzee
left a comment
There was a problem hiding this comment.
At a quick glance the changes to java.jj look good, but I didn't go through the tests in detail to check whether they compile and match expectations. I have a few requests. @rpx99 could you please:
- confirm whether you've checked that all the tests samples that should compile do, and that the expectations are consistent with the JEP.
- check which of the tests are just repeating checks that are already performed by the compiler and remove those.
- add some AST tests as well. The validator tests here test that we don't encounter problems, but not that the AST output is correct.
- update the validator according to my comment
It is difficult to review this PR while changes from JEP 512 are included (I prefer to look at the full diff to get a holistic idea of the change instead of per-commit) so I just did a quick read-through. I will do a more thorough review once the JEP 512 changes have been removed an my initial comments have been adressed.
| * | ||
| * @since 3.27.0 | ||
| */ | ||
| public class FlexibleConstructorValidator implements TypedValidator<ConstructorDeclaration> { |
There was a problem hiding this comment.
My concern with this validator is similar to my comment on github.com//pull/4902. These checks are effectively re-running checks that would've been done by the compiler and checking that we report problems for code that isn't valid in ANY Java version.
The validators should check that, when using a language level < 25, code that wouldn't have been valid for those versions but now parses correctly for java >= 25 is correctly reported as not being valid in that version. This means that a java 1 validator must be added to check this which is removed in java 25
| // JEP 513: Flexible Constructor Bodies | ||
| // ExplicitConstructorInvocation can now appear anywhere in the body, not just at the beginning |
There was a problem hiding this comment.
Without the context of the lines that have been removed, this comment does not make sense. I would remove it since the same is already explained below.
| * GNU Lesser General Public License for more details. | ||
| */ | ||
|
|
||
| package com.github.javaparser.ast.validator.language_level_validations.chunks; |
There was a problem hiding this comment.
Could you remove the ‘chunk’ package and integrate the content of this class into the validator (as is done for the other validators)?
I agree with this idea. If the code for a validator method is too complex to reasonably include in the validator class itself, then I think the validation logic should be reconsidered. It may make sense in some cases, but most validator should just check whether a flag is set or a simple condition (in this case whether any statements appear before the constructor invocation)
|
@johannescoetzee Please take over. Thanks. |
Summary
This PR implements JEP 513: Flexible Constructor Bodies for Java 25, which allows statements to appear before explicit constructor invocations (
super()orthis()calls).Changes
Grammar
ConstructorDeclarationproduction to allow statements in any orderExplicitConstructorInvocationas a validBlockStatementValidation
FlexibleConstructorValidatorto enforce JEP 513 rules:super()/this()(prologue) cannot reference the current instance (this)super()/this()call is allowed per constructorJava25Validatorwith the flexible constructor validatorJava25PostProcessorConfiguration
JAVA_25language level toParserConfigurationBLEEDING_EDGEconstant toJAVA_25JAVA_25to yield support arrayTests
Created
FlexibleConstructorTestwith 10 comprehensive test cases:Valid cases:
super()callsuper()super()this()callsuper()callInvalid cases:
thisreference in prologuethismethod call in prologuethisin field initializer before superAll tests pass successfully.
Related