Sitelet https://web.archive.org/web/20260614194708/https://github.com/github/codeql/pull/7988
Skip to content

Java: Add predicates for sealed classes#7988

Merged
atorralba merged 2 commits into
github:mainfrom
Marcono1234:marcono1234/sealed-types-predicates
Feb 15, 2022
Merged

Java: Add predicates for sealed classes#7988
atorralba merged 2 commits into
github:mainfrom
Marcono1234:marcono1234/sealed-types-predicates

Conversation

@Marcono1234

@Marcono1234 Marcono1234 commented Feb 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adds two predicates to ClassOrInterface to support sealed classes (Java 17 feature; previously a preview feature). This is based on #6178.

I have also restructured the type tests a bit to be all under the common types folder. Please let me know if I should revert that.

Feel free to close this pull request in case you have your own changes for this planned internally, or want to implement this yourself.

@Marcono1234 Marcono1234 requested a review from a team as a code owner February 12, 2022 00:07
@github-actions github-actions Bot added the Java label Feb 12, 2022
@Marcono1234 Marcono1234 force-pushed the marcono1234/sealed-types-predicates branch from a654e7c to 483f482 Compare February 12, 2022 16:25
@smowton

smowton commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

Please add a change note and remove the (Java 17 feature) parenthetical, otherwise lgtm

@Marcono1234

Marcono1234 commented Feb 14, 2022

Copy link
Copy Markdown
Contributor Author

remove the (Java 17 feature) parenthetical

My intention with this was to make it a bit more obvious what these predicates do, because not everyone might be up to date with the latest Java features, and for users of older Java versions these predicates would be irrelevant. For StringLiteral I added "(Java 15 feature)" in the past for text blocks and it was merged. Should I remove it there as well?

atorralba
atorralba reviewed Feb 14, 2022
View reviewed changes
Comment thread java/ql/lib/semmle/code/java/Type.qll Outdated
@owen-mc

owen-mc commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

(Note that the way we format change notes has recently changed. The instructions are here.)

@smowton

smowton commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

I don't mind much either way re: the Java 17 feature text

@Marcono1234 Marcono1234 force-pushed the marcono1234/sealed-types-predicates branch from 483f482 to a496b1d Compare February 14, 2022 20:05
@Marcono1234

Marcono1234 commented Feb 14, 2022

Copy link
Copy Markdown
Contributor Author

Thanks! I have addressed the review comment, added a change note (I hope the format is correct) and force pushed the changes.

@atorralba atorralba merged commit 5f0ab52 into github:main Feb 15, 2022
@Marcono1234 Marcono1234 deleted the marcono1234/sealed-types-predicates branch February 15, 2022 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smowton smowton smowton approved these changes

+1 more reviewer

@atorralba atorralba atorralba left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

documentation Java

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Marcono1234 @smowton @owen-mc @atorralba