Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse multiple Patterns in a case statement #9

Conversation

datho7561
Copy link

@datho7561 datho7561 commented Jan 18, 2024

Second part of JEP 443.

  • Change the parser so that it can parse a list of Patterns in a case.
  • Change GuardedPattern AST (internal and and API) so that it has a list of patterns as its child instead of just one.
  • Modify some tests that use record patterns with a variable (eg. MyPoint(int x, int y) p vs MyPoint(int x, int y)), since this syntax was only ever used in the preview version of the feature and is no longer allowed.
  • Add code generation for all the above
  • Include Fix nested pattern code generation eclipse-jdt/eclipse.jdt.core#1816 to fix some bugs in code generation

Signed-off-by: David Thompson [email protected]

@datho7561 datho7561 marked this pull request as draft January 18, 2024 16:43
@datho7561
Copy link
Author

datho7561 commented Jan 18, 2024

See eclipse-jdt#1742 (comment) I nearly forgot that I still need to write the code for code generation.

Code generation is added

@rgrunber
Copy link

rgrunber commented Feb 5, 2024

Can you rebase this PR onto develop ? You don't need to carry "Releng changes for jdtls" as you would be requesting to be rebase on top of that & the JEP 445 PR.

@datho7561 datho7561 force-pushed the pattern-lists-in-switch-squashed branch from 80a8db4 to 804d22c Compare February 6, 2024 16:04
@datho7561
Copy link
Author

Please don't merge yet I still need to test it with JDT-LS

@datho7561
Copy link
Author

datho7561 commented Feb 6, 2024

I tried editing and running this class in jdt.ls, and it seems to be working as expected:

public class MyClass {

	public static void main(String... args) {
		MyRecord<?> my_record = new MyRecord(null);

		switch (my_record) {
			case MyRecord(Chartreuse _), MyRecord(Licorice _):
				System.out.println("sweet");
				break;
			case MyRecord(Cilantro _):
				System.out.println("savory");
				break;
			default:
				System.out.println("yikes");
				break;
		}
	}
}


record MyRecord<T extends Flavour>(T f){}

sealed abstract class Flavour permits Cilantro, Licorice, Chartreuse {
}
final class Cilantro extends Flavour {}
final class Licorice extends Flavour {}
final class Chartreuse extends Flavour {}

I'll do another rebase.

Second part of JEP 443.
Change the parser so that it can parse a list of Patterns in a case.
Change GuardedPattern AST (internal and and API) so that it has a list
of patterns as its child instead of just one.
Modify some tests that use record patterns with a variable
(eg. `MyPoint(int x, int y) p` vs `MyPoint(int x, int y)`),
since this syntax was only ever used in the preview version
of the feature and is no longer allowed.

Signed-off-by: David Thompson <[email protected]>
@datho7561
Copy link
Author

Pushing the rebase soon. I have the following 3 failures and 3 errors when running this change in the jdt.ls test suite:

Failures:
  ProjectUtilsTest.testGetMaxProjectProblemSeverity:51 expected:<0> but was:<2>
  ProtobufSupportTest.testGenerateProtobufSources:64
  GradleProjectImporterTest.testNameConflictProject:632 expected:<3> but was:<2>
Errors:
  ProtobufSupportTest.testAfterImportsForProtobuf:53 NullPointer Cannot invoke "java.util.List.size()" because "notifications" is null
  GradleProjectImporterTest.testProtoBufSupport:591 » JavaModel protobuf does not exist
  GradleProjectImporterTest.testProtoBufSupportChanged:609 » JavaModel protobuf does not exist

@datho7561
Copy link
Author

@rgrunber FYI, one of the commits I include in this PR is a PR upstream, but that PR became stale due to changes in Pattern code generation, so I completely rewrote the upstream PR. eclipse-jdt#2016

@datho7561
Copy link
Author

We should use Srikanth's PR from upstream, which contains many bug fixes that Srikanth and I have written to this feature.

I'll open a PR here, so that if we want to include it in incubator, we can.

@datho7561 datho7561 closed this Feb 27, 2024
@datho7561 datho7561 deleted the pattern-lists-in-switch-squashed branch January 2, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants