[JEP 440] Add support for record patterns by johannescoetzee · Pull Request #4432 · javaparser/javaparser

johannescoetzee · 2024-05-23T16:05:20Z

Implements #4385. I've already described the general approach I've taken in that issue, so won't repeat that here.

The diff is huge, but I think all of this is necessary to show that the implementation works. In an attempt to make this more manageable, I used the commits to document all the steps I took while implementing this. This means the commit list is also very long, but this will also be useful as a reference for improving the documentation on adding nodes and code generation in general (which I intend to do soon).

A lot of the tests added here are just boilerplate tests which either test generated code, or are copies of similar tests for TypePatternExpr. I've left comments on the tests that test more interesting functionality.

This PR doesn't include a ResolvedRecordPatternDeclaration class. I could implement one similar to ResolvedTypePatternDeclaration, but it's not obvious to me how this would be used (and the type pattern declaration one contains a comment suggesting the author of that wasn't too sure either). Any insight on how such a class could be useful would be much appreciated!

…ationTest

…onTest

…methods for RecordPatternExpr

…llow unit tests to pass

…as well

codecov · 2024-05-23T16:24:54Z

Codecov Report

Attention: Patch coverage is 54.89130% with 83 lines in your changes are missing coverage. Please review.

Project coverage is 51.999%. Comparing base (58a4063) to head (33b200f).
Report is 2 commits behind head on master.

Additional details and impacted files

@@              Coverage Diff              @@
##            master     #4432       +/-   ##
=============================================
+ Coverage   51.943%   51.999%   +0.056%     
=============================================
  Files          505       507        +2     
  Lines        28481     28654      +173     
  Branches      4939      4969       +30     
=============================================
+ Hits         14794     14900      +106     
- Misses       11638     11685       +47     
- Partials      2049      2069       +20

Flag	Coverage Δ
AlsoSlowTests	`51.999% <54.891%> (+0.056%)`	⬆️
javaparser-core	`51.999% <54.891%> (+0.056%)`	⬆️
javaparser-symbol-solver	`51.999% <54.891%> (+0.056%)`	⬆️
jdk-10	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-11	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-12	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-13	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-14	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-15	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-16	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-17	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-18	`51.996% <54.891%> (+0.056%)`	⬆️
jdk-8	`51.994% <54.891%> (+0.056%)`	⬆️
jdk-9	`51.996% <54.891%> (+0.056%)`	⬆️
macos-latest	`51.992% <54.891%> (+0.056%)`	⬆️
ubuntu-latest	`51.992% <54.891%> (+0.056%)`	⬆️
windows-latest	`51.978% <54.891%> (+0.056%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...va/com/github/javaparser/ast/expr/PatternExpr.java	`50.000% <ø> (+6.250%)`	⬆️
...ub/javaparser/ast/observer/ObservableProperty.java	`89.017% <100.000%> (+0.063%)`	⬆️
...or/language_level_validations/Java21Validator.java	`100.000% <100.000%> (ø)`
...om/github/javaparser/ast/visitor/CloneVisitor.java	`39.479% <100.000%> (+1.726%)`	⬆️
...parser/ast/visitor/GenericVisitorWithDefaults.java	`98.076% <100.000%> (+0.018%)`	⬆️
...vaparser/ast/visitor/NoCommentHashCodeVisitor.java	`67.592% <100.000%> (+0.302%)`	⬆️
...arser/ast/visitor/ObjectIdentityEqualsVisitor.java	`99.038% <100.000%> (+0.009%)`	⬆️
...ser/ast/visitor/ObjectIdentityHashCodeVisitor.java	`97.115% <100.000%> (+0.028%)`	⬆️
...hub/javaparser/ast/visitor/VoidVisitorAdapter.java	`99.054% <100.000%> (+0.011%)`	⬆️
...avaparser/ast/visitor/VoidVisitorWithDefaults.java	`98.048% <100.000%> (+0.019%)`	⬆️
... and 17 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c52a57a...33b200f. Read the comment docs.

johannescoetzee · 2024-05-23T16:33:16Z

javaparser-core-testing/src/test/java/com/github/javaparser/ast/expr/SwitchExprTest.java

        assertFalse(Navigator.findNameExpression(entry, "b").isPresent());
    }
+
+    @Test


Simple AST test

johannescoetzee · 2024-05-23T16:35:44Z

javaparser-core-testing/src/test/java/com/github/javaparser/printer/PrettyPrinterTest.java

        assertEqualsStringIgnoringEol(code, new DefaultPrettyPrinter().print(cu));
    }
+
+    @Test


PrettyPrinter test

johannescoetzee · 2024-05-23T16:35:59Z

...arser/printer/lexicalpreservation/transformations/ast/body/StatementTransformationsTest.java

        assertTransformedToString(code, stmt);
    }
+
+    @Test


johannescoetzee · 2024-05-23T16:36:46Z

javaparser-core/src/main/java/com/github/javaparser/ast/expr/PatternExpr.java

    }

    @Override
+    @Generated("com.github.javaparser.generator.core.node.ReplaceMethodGenerator")


I don't know why these @Generated annotations appeared suddenly, but this code was, in fact, generated.

johannescoetzee · 2024-05-23T16:39:25Z

javaparser-core/src/main/java/com/github/javaparser/printer/ConcreteSyntaxModel.java

        concreteSyntaxModelByClass.put(SimpleName.class, attribute(ObservableProperty.IDENTIFIER));
        concreteSyntaxModelByClass.put(SingleMemberAnnotationExpr.class, sequence(comment(), token(GeneratedJavaParserConstants.AT), child(ObservableProperty.NAME), token(GeneratedJavaParserConstants.LPAREN), child(ObservableProperty.MEMBER_VALUE), token(GeneratedJavaParserConstants.RPAREN)));
        concreteSyntaxModelByClass.put(StringLiteralExpr.class, sequence(comment(), stringToken(ObservableProperty.VALUE)));
-        concreteSyntaxModelByClass.put(TextBlockLiteralExpr.class, sequence(comment(), textBlockToken(ObservableProperty.VALUE)));


I seem to have rearranged some items here to maintain alphabetic order. I can't remember why I chose to do that for this PR, so can undo it if that's preferred.

johannescoetzee · 2024-05-23T16:45:25Z

...ain/java/com/github/javaparser/symbolsolver/javaparsermodel/contexts/SwitchEntryContext.java

-        // look for declaration in a pattern label for this entry
-        for (Expression e : wrappedNode.getLabels()) {
-            if (!e.isTypePatternExpr()) {
-                continue;
-            }
-
-            TypePatternExpr typePatternExpr = e.asTypePatternExpr();
-            if (typePatternExpr.getNameAsString().equals(name)) {
-                JavaParserPatternDeclaration decl = JavaParserSymbolDeclaration.patternVar(typePatternExpr, typeSolver);
-                return SymbolReference.solved(decl);
-            }
-        }
-


I added this logic while adding support for switch pattern expressions. The possibility of having record patterns as well complicates this, however, so I moved the logic for finding declarations in patterns to AbstractJavaParserContext to avoid duplicating that. For that to work, I added the typePatternExprsExposedToChild method below which just finds all pattern exprs in the entry label and returns them (since they're all exposed to the children of this entry)

johannescoetzee · 2024-05-23T16:46:37Z

...a/com/github/javaparser/symbolsolver/javaparsermodel/contexts/AbstractJavaParserContext.java


        // First check if there are any pattern expressions available to this node.
        Context parentContext = optionalParentContext.get();
-        if(parentContext instanceof BinaryExprContext || parentContext instanceof IfStatementContext) {


The changes in this method are all to keep type solving logic for patterns contained. See a comment about this below in SwitchEntryContext

johannescoetzee · 2024-05-23T16:48:19Z

...a/com/github/javaparser/symbolsolver/javaparsermodel/contexts/AbstractJavaParserContext.java

        return wrappedNode;
    }
+
+    public List<TypePatternExpr> typePatternExprsDiscoveredInPattern(PatternExpr patternExpr) {


In the symbol solver, we're generally only interested in TypePatternExprs since these contain the variable declarations needed. This method just does a breadth-first traversal of the pattern tree to discover all TypePatternExprs contained in the nested RecordPatternExpr structure.

johannescoetzee · 2024-05-23T16:49:50Z

...-solver-testing/src/test/java/com/github/javaparser/symbolsolver/resolution/ContextTest.java

    }

+    @Nested
+    class RecordPatternExprTests {


Tests to ensure declarations in nested patterns are correctly exposed to children

johannescoetzee · 2024-05-23T16:50:34Z

...lver-testing/src/test/java/com/github/javaparser/symbolsolver/resolution/InstanceOfTest.java


    }

+    @Nested


Test to ensure symbols in record patterns can be solved in instanceof expressions

johannescoetzee · 2024-05-23T16:50:44Z

...lver-testing/src/test/java/com/github/javaparser/symbolsolver/resolution/SwitchExprTest.java


        assertThrows(UnsolvedSymbolException.class, resolveS);
    }
+


Test to ensure symbols in record patterns can be solved in switch expressions

johannescoetzee · 2024-05-23T16:54:15Z

The codecov bot messages suggest I missed some boilerplate tests. I'll add those either tomorrow or early next week

jlerbsc · 2024-05-29T14:02:32Z

...a/com/github/javaparser/symbolsolver/javaparsermodel/contexts/AbstractJavaParserContext.java

    public N getWrappedNode() {
        return wrappedNode;
    }
+


Comments in the code would be welcome

jlerbsc · 2024-05-29T14:22:25Z

Everything seems fine to me. If you have nothing to add, I will merge your PR. Thank you for your excellent work.

jlerbsc · 2024-05-29T14:37:39Z

Can you resolve the checkstyle violations?

[INFO] There are 2 errors reported by Checkstyle 10.17.0 with dev-files/JavaParser-CheckStyle.xml ruleset.
Error: src/main/java/com/github/javaparser/resolution/Context.java:[26,8] (imports) UnusedImports: Unused import - com.github.javaparser.ast.expr.PatternExpr.
Error: src/main/java/com/github/javaparser/ast/expr/RecordPatternExpr.java:[11,8] (imports) UnusedImports: Unused import - java.util.regex.Pattern.

johannescoetzee · 2024-05-30T09:25:41Z

I've updated the pr with a comment explaining the type pattern discovery method, tests that should satisfy some of the codecov issues (but not all) and which fixed checkstyle violations.

@jlerbsc if the remaining codecov violations are acceptable (they're all for generated boilerplate, as far as I can tell, except for the deprecated PrettyPrinterVisitor method which I added for anyone still using that, but which is tested in the new DefaultPrettyPrintVisitor), then I don't have anything further to add to this PR.

I think fixing #4440 should then be the only remaining task for java 21 support, but please let me know if you can think of anything I missed!

Edit: looks like I added enough tests to get the coverage above average and therefore codecov bot is happy, but I could still add some more if that's preferred :)

johannescoetzee added 22 commits May 23, 2024 10:42

Rename JavaParserPatternDeclaration to JavaParserTypePatternDeclaration

dd3f9b0

Rename JavaParserTypPatternDeclarationTest to JavaParserPatternDeclar…

4b5f48c

…ationTest

Rename ResolvedTypePatternDeclarationTest to ResolvedPatternDeclarati…

b958b35

…onTest

Rename PatternSymbolDeclarator to TypePatternSymbolDeclarator

0d92188

Rename instances of Pattern to TypePattern in ResolvedPatternDeclaration

f509f85

Add RecordPatternExpr outline

602c461

Add RecordPatternExpr to MetaModelGenerator

fc056bc

Run core metamodel generator

014ca6a

Add first pass core generator output

ae6ecb2

Add initial PrettyPrintVisitor and DefaultPrettyPrinterVisitor visit …

1ad0452

…methods for RecordPatternExpr

Add DefaultVisitorAdapter visit method for RecordPatternExpr

15f1c49

Run core generators second pass

86b0174

Add placeholder for RecordPatternExpr to the ConcreteSyntaxModel to a…

3aeb87c

…llow unit tests to pass

Add record pattern support to java.jj

c4555cc

Add AST test for RecordPatternExpr

0b1f572

Add PrettyPrinter test for RecordPatterns

7523720

Update RecordPatternExpr constructors after codegen

f7bb1db

Add RecordPatternExpr tests for generated visitors

08145e1

Update patternExprExposedTo/FromChildren to traverse record patterns …

20c8753

…as well

Add record pattern support to concrete syntax model with tests

5242142

Fix resolution for record patterns in scope with tests

ac7d2ef

Add validators and tests for record patterns

181b471

johannescoetzee commented May 23, 2024

View reviewed changes

johannescoetzee marked this pull request as ready for review May 23, 2024 16:53

jlerbsc reviewed May 29, 2024

View reviewed changes

Merge branch 'master' into johannes/record-pattern-impl

15aeb1d

johannescoetzee added 2 commits May 30, 2024 11:20

Add comment, some tests and fix unused imports

00c016e

Merge branch 'master' into johannes/record-pattern-impl

33b200f

jlerbsc merged commit 61d3619 into javaparser:master May 30, 2024

jlerbsc added this to the next release milestone May 30, 2024

jlerbsc added the PR: Added A PR that introduces new behaviour (e.g. functionality, tests) label May 30, 2024

johannescoetzee deleted the johannes/record-pattern-impl branch May 30, 2024 12:51

This was referenced Jun 4, 2024

Refactor: Do some missed Pattern to TypePattern renamings #4418

Closed

Add link to the guide to adding nodes in CONTRIBUTING.md #4453

Merged

coderabbitai bot mentioned this pull request Mar 27, 2026

Updating javaparser parent 3.28.0 typetools/checker-framework#7585

Open

Uh oh!

Conversation

johannescoetzee commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johannescoetzee May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johannescoetzee commented May 23, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlerbsc commented May 29, 2024

Uh oh!

jlerbsc commented May 29, 2024

Uh oh!

johannescoetzee commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

johannescoetzee commented May 23, 2024 •

edited

Loading

codecov bot commented May 23, 2024 •

edited

Loading

johannescoetzee May 23, 2024 •

edited

Loading

johannescoetzee commented May 30, 2024 •

edited

Loading