Skip to content

Share XML discard predicates - #20873

Merged
asgerf merged 3 commits into
mainfrom
shared-xml-discard
Dec 1, 2025
Merged

Share XML discard predicates#20873
asgerf merged 3 commits into
mainfrom
shared-xml-discard

Conversation

@asgerf

@asgerfasgerf commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

Shares the discard predicates for XML entities.

This happens via synchronized-files since parameterised modules and qlpacks can't be used in this case.

@asgerf
asgerf marked this pull request as ready for review November 27, 2025 09:00
@asgerf
asgerf requested review from a team as code ownersNovember 27, 2025 09:00
CopilotAI review requested due to automatic review settings November 27, 2025 09:00
@asgerf
asgerf requested review from a team as code ownersNovember 27, 2025 09:00
@asgerfasgerf added the no-change-note-required This PR does not need a change note label Nov 27, 2025

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors XML discard predicates by extracting them into shared OverlayXml.qll modules across five languages (Python, JavaScript, Java, Go, and C#). The shared code is synchronized via the identical-files.json configuration since parameterized modules and qlpacks cannot be used in this case.

  • Introduces identical OverlayXml.qll files in each language's internal directory
  • Removes duplicate XML handling code from existing Overlay.qll files
  • Adds imports to the new shared modules

Reviewed changes

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

Show a summary per file
FileDescription
python/ql/lib/semmle/python/internal/OverlayXml.qllNew shared module containing XML discard predicates for overlay analysis
python/ql/lib/semmle/python/Overlay.qllRemoves duplicate XML handling code and imports the new OverlayXml module
javascript/ql/lib/semmle/javascript/internal/OverlayXml.qllNew shared module containing XML discard predicates for overlay analysis
javascript/ql/lib/semmle/javascript/internal/Overlay.qllRemoves xmllocations reference and imports the new OverlayXml module
java/ql/lib/semmle/code/xml/XML.qllRemoves unnecessary Overlay import and discardable XML classes
java/ql/lib/semmle/code/java/internal/OverlayXml.qllNew shared module containing XML discard predicates for overlay analysis
java/ql/lib/semmle/code/java/Overlay.qllRemoves duplicate XML handling code, updates getRawFile to exclude xmllocations, and imports the new OverlayXml module
go/ql/lib/semmle/go/internal/OverlayXml.qllNew shared module containing XML discard predicates for overlay analysis
go/ql/lib/semmle/go/Overlay.qllRemoves duplicate XML predicates and imports the new OverlayXml module
csharp/ql/lib/semmle/code/csharp/internal/OverlayXml.qllNew shared module containing XML discard predicates for overlay analysis
csharp/ql/lib/semmle/code/csharp/internal/Overlay.qllRemoves duplicate XML handling code and imports the new OverlayXml module
config/identical-files.jsonRegisters all five OverlayXml.qll files as synchronized identical files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

michaelnebel
michaelnebel previously approved these changes Nov 27, 2025

@michaelnebelmichaelnebel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

C# LGTM!

owen-mc
owen-mc previously approved these changes Nov 27, 2025

@owen-mcowen-mc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Go 👍🏻

tausbn
tausbn previously approved these changes Nov 27, 2025

@tausbntausbn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Python 👍

*/
private predicate isOverlay() { databaseMetadata("isOverlay", "true") }

private @file getXmlFile(@xmllocatable locatable) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do all languages extract @file using named trap ids? Because if not, then this identification change from string to @file won't work. I'm guessing that they do, but this is a subtle change, so I figured it would be best to check.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

If string is the more robust option, let's go with that.

@asgerf
asgerf dismissed stale reviews from tausbn, owen-mc, and michaelnebel via 6257bedNovember 28, 2025 08:24
@asgerf
asgerf merged commit b8cff77 into mainDec 1, 2025
48 checks passed
@asgerf
asgerf deleted the shared-xml-discard branch December 1, 2025 09:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C#GoJavaJSno-change-note-requiredThis PR does not need a change notePython

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@asgerf@tausbn@michaelnebel@aschackmull@owen-mc