Skip to content

Conversation

aschackmull
Copy link
Contributor

Follow-up for #19573 to fix the small issue with asserts.

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

@Copilot 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

This PR adds a dedicated CFG node for failing assert statements and updates tests accordingly.

  • Remove stale expected warnings for assert-false paths in Nullness tests
  • Update test comment to reflect that the correlated conditions are now handled
  • Introduce TAssertThrowNode and AssertThrowNode in the CFG and wire up assert failures

Reviewed Changes

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

File Description
java/ql/test/query-tests/Nullness/NullMaybe.expected Remove three outdated expected-warning lines for xs.
java/ql/test/query-tests/Nullness/C.java Change comment on xs[0] = 42; from “FP...” to “OK”.
java/ql/lib/semmle/code/java/ControlFlowGraph.qll Add TAssertThrowNode type and AssertThrowNode class; update CFG rules for assert.
Comments suppressed due to low confidence (1)

java/ql/lib/semmle/code/java/ControlFlowGraph.qll:103

  • [nitpick] The new TAssertThrowNode(AssertStmt s) case in the TNode newtype lacks a brief comment. Please add documentation describing its purpose (i.e., modeling a failing assert).
    TExitNode(Callable c) { exists(c.getBody()) } or

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Great to see a new CFG node being added.

@aschackmull aschackmull merged commit 321a4af into github:main Jun 26, 2025
15 checks passed
@aschackmull aschackmull deleted the java/fix-assert-cfg branch June 26, 2025 09:43
@aschackmull
Copy link
Contributor Author

Great to see a new CFG node being added.

I've got more: #19885

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