Skip to content

Conversation

kaspersv
Copy link
Contributor

@kaspersv kaspersv commented Sep 23, 2025

This PR improves the Java config and XML discard predicates to avoid apparent bqrs accuracy regressions under the current non-incremental Java property and XML extraction. Under non-incremental extraction, the Java property and XML extractors may extract files outside of overlayChangedFiles. To account for these files and properly discard from base, this PR extends the discard predicates to discard Java config and XML base entities in all overlay extracted files, even if they do not appear in overlayChangedFiles. The use of overlayChangedFiles is still necessary to discard from deleted Java property and XML files.

DCA experiments (variant vs baseline) shows the expected accuracy improvements for java/maven/non-https-url, java/maven/dependency-upon-bintray and java/spring-boot-exposed-actuators-config.

@github-actions github-actions bot added the Java label Sep 23, 2025
@kaspersv kaspersv force-pushed the kaspersv/future-proof-java-discarding2 branch 3 times, most recently from 9122500 to c70ad9b Compare September 23, 2025 08:57
@kaspersv kaspersv force-pushed the kaspersv/future-proof-java-discarding2 branch from c70ad9b to f02da68 Compare September 23, 2025 10:28
@kaspersv kaspersv requested a review from alexet September 24, 2025 06:47
@kaspersv kaspersv marked this pull request as ready for review September 24, 2025 06:48
@kaspersv kaspersv requested a review from a team as a code owner September 24, 2025 06:48
@Copilot Copilot AI review requested due to automatic review settings September 24, 2025 06:48
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 improves overlay database handling by extending discard predicates for Java config and XML entities. The change addresses accuracy regressions under non-incremental extraction where files outside of overlayChangedFiles may still be extracted.

  • Introduces new predicates to track overlay-extracted config and XML files
  • Updates discard logic to handle all overlay-extracted files, not just changed files
  • Creates a new abstract class DiscardableXmlLocatable for XML entity management

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
java/ql/lib/semmle/code/java/Overlay.qll Adds new overlay predicates and discard logic for config and XML files
java/ql/lib/semmle/code/xml/XML.qll Updates XML discardable classes to extend new DiscardableXmlLocatable base class

string toString() { none() }
}

overlay[local]
Copy link
Preview

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

The condition not files(el, _) and not xmlNs(el, _, _, _) appears to be filtering logic but lacks documentation explaining why these specific exclusions are necessary. Consider adding a comment to clarify the purpose of these filters.

Suggested change
overlay[local]
overlay[local]
/**
* Returns true if the given file is extracted in the overlay variant for an `@xmllocatable`
* that is neither a file entity nor an XML namespace declaration.
* The conditions `not files(el, _)` and `not xmlNs(el, _, _, _)` ensure that only relevant
* XML locatable entities are considered, excluding files and namespace declarations which
* are not subject to overlay extraction.
*/

Copilot uses AI. Check for mistakes.

@kaspersv kaspersv added the no-change-note-required This PR does not need a change note label Sep 24, 2025
@kaspersv kaspersv merged commit b52fff2 into github:main Sep 29, 2025
18 of 19 checks passed
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