-
Notifications
You must be signed in to change notification settings - Fork 1.8k
JS: Add overlay support to extractor #20307
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 implements initial overlay support for JavaScript and TypeScript extractors to enable incremental analysis of codebases. It adds the necessary infrastructure for tracking file changes, discarding outdated entities, and managing database metadata while preparing for future overlay evaluation capabilities.
- Database schema updates with overlay-specific relations and support version
- Java extractor changes to filter extraction based on changed files and write overlay metadata
- CodeQL library support for identifying and discarding obsolete entities in overlay scenarios
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
javascript/resources/codeql-extractor.yml | Adds overlay support version configuration |
javascript/ql/lib/upgrades/ccefb5e2d49318eea4aeafd4c6ae2af9f94ac72a/upgrade.properties | Defines upgrade metadata for overlay relations |
javascript/ql/lib/upgrades/ccefb5e2d49318eea4aeafd4c6ae2af9f94ac72a/semmlecode.javascript.dbscheme | New database schema with overlay support |
javascript/ql/lib/upgrades/ccefb5e2d49318eea4aeafd4c6ae2af9f94ac72a/old.dbscheme | Previous database schema for upgrade comparison |
javascript/ql/lib/semmlecode.javascript.dbscheme.stats | Statistics for overlay relations |
javascript/ql/lib/semmlecode.javascript.dbscheme | Main schema with overlay metadata and changed files tables |
javascript/ql/lib/semmle/javascript/internal/Overlay.qll | Overlay logic for discarding outdated entities |
javascript/ql/lib/javascript.qll | Imports overlay support module |
javascript/extractor/src/com/semmle/js/extractor/OverlayChanges.java | Data class for overlay change information |
javascript/extractor/src/com/semmle/js/extractor/AutoBuild.java | Extractor modifications for overlay file filtering and metadata writing |
javascript/downgrades/76a926a00d5f3bc199c203a1437796fd7b2835ba/upgrade.properties | Downgrade configuration for overlay relations |
javascript/downgrades/76a926a00d5f3bc199c203a1437796fd7b2835ba/semmlecode.javascript.dbscheme | Downgrade schema without overlay support |
@@ -465,10 +465,11 @@ public int run() throws IOException { | |||
try { | |||
CompletableFuture<?> sourceFuture = extractSource(); | |||
sourceFuture.join(); // wait for source extraction to complete | |||
if (hasSeenCode()) { // don't bother with the externs if no code was seen | |||
if (hasSeenCode() && !isOverlayChangeMode()) { // don't bother with the externs if no code was seen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The comment should clarify why externs are skipped in overlay change mode, not just when no code is seen. Consider: '// don't bother with the externs if no code was seen or in overlay change mode'
if (hasSeenCode() && !isOverlayChangeMode()) { // don't bother with the externs if no code was seen | |
if (hasSeenCode() && !isOverlayChangeMode()) { // don't bother with the externs if no code was seen or in overlay change mode |
Copilot uses AI. Check for mistakes.
// Write an empty string to the file as we currently have no metadata to emit. | ||
// The file must be created for the database to recognized as an overlay base. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a grammatical error in the comment. It should be 'to be recognized' instead of 'to recognized'.
// Write an empty string to the file as we currently have no metadata to emit. | |
// The file must be created for the database to recognized as an overlay base. | |
// The file must be created for the database to be recognized as an overlay base. |
Copilot uses AI. Check for mistakes.
int before = filesToExtract.size(); | ||
filesToExtract.retainAll(changedFiles); | ||
int after = filesToExtract.size(); | ||
System.out.println("Overlay filter removed " + (before - after) + " out of " + before + " files from extraction."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider using a proper logger instead of System.out.println for this informational message to maintain consistency with other logging in the codebase.
System.out.println("Overlay filter removed " + (before - after) + " out of " + before + " files from extraction."); | |
DiagnosticWriter.info("Overlay filter removed " + (before - after) + " out of " + before + " files from extraction."); |
Copilot uses AI. Check for mistakes.
This contains some of the initial changes needed for overlay support for JavaScript and TypeScript:
It does not include
compileForOverlayEval
, to be enabled separately when it does not cause a slowdown