Add support for match-all patterns#4867
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4867 +/- ##
===============================================
- Coverage 58.354% 58.330% -0.024%
- Complexity 2532 2535 +3
===============================================
Files 671 677 +6
Lines 38861 39038 +177
Branches 7064 7088 +24
===============================================
+ Hits 22677 22771 +94
- Misses 13293 13368 +75
- Partials 2891 2899 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
It looks like the codecov misses are just in the generated boilerplate code. If this is an issue, let me know and I'll add tests targeting this specifically. |
|
It's excellent work. It is indeed difficult to keep track of the changes, but overall it seems correct to me. |
Resolves #4713
This PR mainly introduces support for match-all patterns, but does contain a small change to the grammar involving identifiers which was required to disambiguate match-all pattern parsing.
Major changes
MatchAllPatternExpr added
The main point of this PR is to add support for
MatchAllPatternExprs. This includes a grammar change injava.jj, a new node type, and support in theConcreteSyntaxModel. I initially went back-and-forth on the naming, but settled forMatchAllPatternExprin the end since this is consistent with the JLS and is a good description of what it actually is. The other option wasUnnamedPatternExpr, as it is often referred to in text, but in my opinion this could easily be confused with an unnamed pattern variable which is actually contained in a TypePatternExpr, e.g.String _.While the actual
MatchAllPatternExprclass and syntax are very simple, fairly large changes to the pattern type hierarchy are required. Previously, the assumption was made that (1) all patterns can be used as top-level patterns in instanceof expressions and switch entry labels, and (2) all pattern expressions have a type. While this was true forTypePatternExprandRecordPatternExpr, this no longer holds, leading to the next major change.Pattern type hierarchy changed
To account for the fact that not all patterns can be used as top-level patterns and not all patterns are typed, a new abstract base class
ComponentPatternExpris introduced. Following the structure in the updated Java syntax, aComponentPatternExprcan be either aMatchAllPatternExpr, or aPatternExpr.PatternExprremains unchanged from before and can still be aTypePatternExpror aRecordPatternExpr.For reference, the updated syntax is:
Java 22 validator added
I've added a Java 22 validator class with 2 new validators: a check to ensure that match-all patterns aren't used as top-level patterns and a check to ensure that unnamed variables are only used where allowed by JEP456. The logic is quite particular, so I've added tests for all of the cases mentioned in JEP456 that should serve as decent documentation.
Other notes
Grammar change for identifiers
I had to make a change to the
IDENTIFIERtoken to getMatchAllPatternExprto parse correctly. TheIDENTIFIERtoken now excludes the identifier_. It must either be an underscore followed by a non-zero number of characters, or a non-underscore character followed by zero or more other characters. In order to continue parsing Java <= 8 code where_is an allowed identifier, I added"_"as an explicit case in theIdentifierproduction, similar to other keywords. This disambiguates parsing forMatchAllPatternExprwithout affecting identifier parsing.Note on testing
I've added tests for everything I could think of that should be affected by this change. This includes the
IfStatmentContextTestsandSwitchEntryContextTests. Some of the tests added in these suites aren't for examples that should be affected by the changes here, but there was a gap in testing so I thought it good to add them. If you can think of anything that I missed, please let me know and I'll add tests for that.Note on commits
The early commits are a bit of a mess since I didn't settle on the current naming scheme/pattern hierarchy until I'd done a significant amount of work. I initially decided to use
TypedPatternExpras the common ancestor forTypePatternExprandRecordPatternExpr, andPatternExpras the common ancestor forTypedPatternExprandMatchAllPatternExpr. I later changed my mind, sinceTypedPatternExprandTypePatternExprwere too close together and none of this matched the JLS. At this point it was easier to rename the classes than to redo everything, so apologies for the messy back-and-forth in the commits.