fix(record): valid assignments in record compact constructors #4389
Conversation
…e4377 � Conflicts: � src/test/java/spoon/test/record/CtRecordTest.java � src/test/resources/records/CompactConstructor.java
|
Please review. This fixes #4377. |
xzel23
changed the title
|
@SirYwell can you review this? @xzel23 Could you please add "fix #4377" to the PR body (first post)? That will automatically close the issue when this is merged. See this line of the contributing guidelines
|
xzel23
changed the title
There was a problem hiding this comment.
I left two comments for possible improvements. The fix itself looks good.
I also figured out why I wasn't able to reproduce it, as I even checked out your commit before you added the fix. I ran the tests with Java 11 locally, and as JDT can't resolve java.lang.Record there, it does not add the assignments... :)
| if (e instanceof CtAssignment) { | ||
| e.setImplicit((node.bits & ASTNode.IsImplicit) != 0); | ||
| } |
There was a problem hiding this comment.
Not sure if this is the best place for this, I think it could also be added to the visit(Assignment, BlockScope) method in JDTTreeBuilder (similar to how it is done here). WDYT?
There was a problem hiding this comment.
Yes, that's of course much better.
|
|
||
| import org.junit.Ignore; | ||
| import org.junit.jupiter.api.Disabled; |
There was a problem hiding this comment.
The annotations aren't used anymore, you can remove them again.
There was a problem hiding this comment.
removed the imports, will update the PR once I'm done with the other comments and unit tests are green
monperrus
changed the title
I have created a second test case for record compact constructors to demostrate the problem. I have created a PR even if I don't have a fix yet to make reproducing easier: either run CtRecordTest.testCompactConstructor2() directly from your IDE (in IntelliJ it's possible to run disabled tests from the IDE, don't know about eclipse), or remove the "@disabled" annotation from said method.