Skip to content

JavaScript: Don't extract obviously generated files #19680

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tausbn
Copy link
Contributor

@tausbn tausbn commented Jun 5, 2025

Extends the JavaScript extractor so that certain files are skipped automatically during extraction.

Specifically, it will skip

  • Files that are in the output directory specified in the tsconfig.json file (if any).
  • Files that have the same base name (but a JavaScript extension) as a file with a TypeScript extension in the same directory.

tausbn added 5 commits June 5, 2025 14:56
Fixes two things:
- The basic test should no longer extract `tst.js` (as `tst.ts` is
  present)
- The `AutoBuild` mock did not populate `extractedFiles` correctly,
  which broke the logic that looks for TypeScript files with the same
  basename.
This is needed for the logic that skips files inside the directory
specified in the `tsconfig.json` `outDir` compiler option.
@tausbn tausbn marked this pull request as ready for review June 5, 2025 15:33
@Copilot Copilot AI review requested due to automatic review settings June 5, 2025 15:33
@tausbn tausbn requested a review from a team as a code owner June 5, 2025 15:33
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 extends the JavaScript extractor to automatically skip obviously generated files, specifically:

  • Skips JS variants when the corresponding TypeScript source exists.
  • Skips any files under the outDir defined in tsconfig.json.

Reviewed Changes

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

Show a summary per file
File Description
change-notes/2025-06-05-skip-obviously-generated-files.md Documents new skip behavior for generated JS and tsconfig outDir
javascript/extractor/test/com/semmle/js/extractor/test/AutoBuildTests.java Adds tests for skipping derived JS files and files under outDir
javascript/extractor/src/com/semmle/js/extractor/AutoBuild.java Implements filters for tsconfig outDir and TS-derived JS files
javascript/extractor/src/com/semmle/js/dependencies/tsconfig/TsConfigJson.java Introduces JSON binding for tsconfig root object
javascript/extractor/src/com/semmle/js/dependencies/tsconfig/CompilerOptions.java Introduces JSON binding for compilerOptions.outDir
Comments suppressed due to low confidence (2)

javascript/extractor/test/com/semmle/js/extractor/test/AutoBuildTests.java:179

  • [nitpick] The test method basicTest now skips JS files by default—consider renaming the test or updating its comment to reflect this changed behavior.
    addFile(false, LGTM_SRC, "tst.js");

javascript/extractor/src/com/semmle/js/dependencies/tsconfig/TsConfigJson.java:4

  • [nitpick] Indentation here uses 2 spaces, while the rest of the project uses 4; align this class with the existing formatting conventions.
  private CompilerOptions compilerOptions;

// ignore malformed tsconfig or missing fields
}
}
// exclude files in output directories as configured in tsconfig.json
Copy link
Preview

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

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

[nitpick] This comment duplicates the one above; consider removing it to reduce redundancy and keep the code concise.

Suggested change
// exclude files in output directories as configured in tsconfig.json

Copilot uses AI. Check for mistakes.

outDirs.add(odir);
}
} catch (Exception e) {
// ignore malformed tsconfig or missing fields
Copy link
Preview

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

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

[nitpick] Silently swallowing all exceptions during tsconfig parsing can obscure errors; consider logging a warning or including the exception message.

Suggested change
// ignore malformed tsconfig or missing fields
System.err.println("Warning: Failed to parse tsconfig.json at " + cfg + ": " + e.getMessage());

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant