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

Java: Document which assignment type is covered by which class #15451

Merged

Conversation

Marcono1234
Copy link
Contributor

Resolves #3266 (see discussion there)

This change tries to document the current behavior, reducing some confusion regarding which CodeQL class covers which assignment type.

I assume this requires no change note since these are only documentation changes, is that correct?

However, personally I think maybe Assignment should actually cover UnaryAssignExpr (at least the name UnaryAssignExpr suggests this). For example if you currently want to check for any assignments to array elements you still have to write custom code for this because as mentioned Assignment does not cover UnaryAssignExpr, and all the Var... classes don't work either because the assignment is not directly on a variable, but instead on an ArrayAccess.

@Marcono1234 Marcono1234 requested a review from a team as a code owner January 28, 2024 18:05
@github-actions github-actions bot added the Java label Jan 28, 2024
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Feb 13, 2024
Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the contribution.

@aschackmull
Copy link
Contributor

However, personally I think maybe Assignment should actually cover UnaryAssignExpr (at least the name UnaryAssignExpr suggests this)

That's certainly a fair point, but I'm not sure the effort involved in terms of judging the need for and handling deprecation, or assessing the impact of a potentially breaking change is worth the gain.

@aschackmull aschackmull merged commit fb2d36d into github:main Feb 14, 2024
14 of 15 checks passed
@Marcono1234 Marcono1234 deleted the marcono1234/java-assignment-doc branch February 14, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java: VariableAssign (LocalVariableDeclExpr) is not an Assignment
2 participants