Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

#"Sign up for GitHub">

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

tausbn
Copy link
Contributor

In#19680 we added support for automatically ignoring files in theoutDir directory as specified in the TSconfig compiler options (as these files were likely duplicates of.ts file we were already scanning).

However, in some cases people putoutDir: "." or evenoutDir: ".." in their configuration, which had the side effect of excludingall files, leading to a failed extraction.

With the changes in this PR, we now ignore anyoutDirs that are not properly contained within the source root of the code being scanned. This should prevent the files from being extracted, while still allowing us to not double-scan files in, say, a.github directory, as seen in some Actions workflows.

In#19680 we added support for automatically ignoring files in the`outDir` directory as specified in the TSconfig compiler options (asthese files were likely duplicates of `.ts` file we were alreadyscanning).However, in some cases people put `outDir: "."` or even `outDir: ".."`in their configuration, which had the side effect of excluding _all_files, leading to a failed extraction.With the changes in this PR, we now ignore any `outDir`s that are notproperly contained within the source root of the code being scanned.This should prevent the files from being extracted, while still allowingus to not double-scan files in, say, a `.github` directory, as seen insome Actions workflows.
@tausbntausbn marked this pull request as ready for reviewJuly 11, 2025 13:53
@CopilotCopilotAI review requested due to automatic review settingsJuly 11, 2025 13:53
@tausbntausbn requested a review froma team as acode ownerJuly 11, 2025 13:53
Copy link
Contributor

@CopilotCopilotAI 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 fixes an issue where the JavaScript extractor would incorrectly exclude all source files whenoutDir intsconfig.json points to the source root directory (.) or parent directory (..). The fix ensures that onlyoutDir configurations that are proper subdirectories of the source root are used for file exclusion.

  • Added validation to check ifoutDir is properly contained within the source root before excluding files
  • Enhanced test coverage with two new test cases for edge cases withoutDir configuration
  • Updated change notes to document the fix

Reviewed Changes

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

FileDescription
javascript/extractor/src/com/semmle/js/extractor/AutoBuild.javaAdds logic to validateoutDir is a proper subdirectory before excluding files
javascript/extractor/test/com/semmle/js/extractor/test/AutoBuildTests.javaAdds test cases foroutDir pointing to parent and source root directories
javascript/ql/lib/change-notes/2025-07-11-ignore-outdirs-that-would-exclude-everything.mdDocuments the fix in release notes

Napalys
Napalys previously approved these changesJul 11, 2025
Copy link
Contributor

@NapalysNapalys left a comment•
edited
Loading

Choose a reason for hiding this comment

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

It might be silly but would it make sense to also add a test case whereoutDir would besomeDir/../../ or similar?

Assuming theDCA looks good and you don't think we need such test case:shipit:

@tausbn
Copy link
ContributorAuthor

It might be silly but would it make sense to also add a test case whereoutDir would besomeDir/../../ or similar?

That's a good idea. I'll add an extra test. (And assuming it doesn't reveal a bug in the implementation, I'll refrain from rerunning DCA as I'm only changing a test.)

@jketemajketema merged commitd33cd71 intomainJul 14, 2025
12 checks passed
@jketemajketema deleted the tausbn/javascript-ignore-tsconfig-outdirs-that-exclude-everything branchJuly 14, 2025 15:36
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@NapalysNapalysNapalys approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@tausbn@Napalys@jketema
[8]ページ先頭
©2009-2025 Movatter.jp