Skip to content

Conversation

@aschackmull
Copy link
Contributor

A minor refactor in the interface to the Guards library. This is necessary to facilitate instantiation in C# where a single expression might correspond to multiple equality checks. E.g. x.compareTo(y) == 0 is a single expression that both acts as an equality check between the comparison and 0, and x and y.

Copilot AI review requested due to automatic review settings June 26, 2025 08:29
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Jun 26, 2025
@aschackmull aschackmull requested a review from a team as a code owner June 26, 2025 08:29
@github-actions github-actions bot added the Java label Jun 26, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Refactors the equality-check interface in the Guards library by replacing the old EqualityTest class with a versatile predicate and updating all its usages.

  • Introduce predicate equalityTest(...) and remove the EqualityTest class in the shared QL signature.
  • Add equalityTestSymmetric and replace eqtestHasOperands calls in the shared controlflow module.
  • Remove Java-specific EqualityTest class and implement equalityTest predicate covering ==/!= and .equals(...) patterns.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
shared/controlflow/codeql/controlflow/Guards.qll Remove EqualityTest class, add equalityTest predicate, update to use equalityTestSymmetric
java/ql/lib/semmle/code/java/controlflow/Guards.qll Drop Java EqualityTest class, add equalityTest predicate to unify J::EqualityTest and equals calls
Comments suppressed due to low confidence (2)

shared/controlflow/codeql/controlflow/Guards.qll:353

  • [nitpick] The parameter name eqval is ambiguous; consider renaming it to polarity to match the fourth parameter of equalityTest and improve clarity.
  private predicate equalityTestSymmetric(Expr eqtest, Expr e1, Expr e2, boolean eqval) {

java/ql/lib/semmle/code/java/controlflow/Guards.qll:299

  • The predicate references class EqualityTest, but that class was removed earlier in this PR; this will cause a QL compilation error. You should refer directly to the built-in J::EqualityTest AST type (or reintroduce a local alias) before calling its getLeftOperand(), getRightOperand(), and polarity() methods.
    exists(EqualityTest eq | eq = eqtest |

@aschackmull aschackmull merged commit 7750f12 into github:main Jun 26, 2025
44 checks passed
@aschackmull aschackmull deleted the guards/eqtest-refactor branch June 26, 2025 09:05
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.

2 participants