Skip to content

Java Automodel extraction: fix extracted meta information by using Object for the type of generic parameters #14818

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

Merged

Conversation

kaeluka
Copy link

@kaeluka kaeluka commented Nov 16, 2023

@atorralba thanks for pointing me in the right direction. Could you give this a quick review? I think it should not be surprising any more 👍

@kaeluka kaeluka requested a review from a team as a code owner November 16, 2023 15:08
@github-actions github-actions bot added the Java label Nov 16, 2023
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as long as the tests pass! Speaking of which, I thought that there were two expectation rows affected by this, but I only see one in the diff 🤔

@kaeluka
Copy link
Author

kaeluka commented Nov 16, 2023

LGTM as long as the tests pass! Speaking of which, I thought that there were two expectation rows affected by this, but I only see one in the diff 🤔

Well spotted!

The reason: I recreated the changes on a fresh branch to avoid a complicated history due to merge conflicts with another fix I merged today; when recreating, I decided I don't need a NEW test case because there was already an example of such a class in the existing code.

I've offered to fix the occurrence in ModelEditor.qll as well - depending on the secexp team's response I might push this as a drive-by change to this PR as well. Otherwise, I'll merge soon.

@kaeluka kaeluka merged commit cb7213d into main Nov 16, 2023
@kaeluka kaeluka deleted the kaeluka/application-mode-erase-type-signatures-of-generic-types branch November 16, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants