-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
14f5088
8829f78
619256e
281ccf7
b8772bc
f08c2fa
e3d9d92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package com.semmle.js.dependencies.tsconfig; | ||
|
||
public class CompilerOptions { | ||
private String outDir; | ||
|
||
public String getOutDir() { | ||
return outDir; | ||
} | ||
|
||
public void setOutDir(String outDir) { | ||
this.outDir = outDir; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package com.semmle.js.dependencies.tsconfig; | ||
|
||
public class TsConfigJson { | ||
private CompilerOptions compilerOptions; | ||
|
||
public CompilerOptions getCompilerOptions() { | ||
return compilerOptions; | ||
} | ||
|
||
public void setCompilerOptions(CompilerOptions compilerOptions) { | ||
this.compilerOptions = compilerOptions; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -39,6 +39,8 @@ | |||||
|
||||||
import com.google.gson.Gson; | ||||||
import com.google.gson.JsonParseException; | ||||||
import com.semmle.js.dependencies.tsconfig.TsConfigJson; | ||||||
import com.semmle.js.dependencies.tsconfig.CompilerOptions; | ||||||
import com.semmle.js.dependencies.AsyncFetcher; | ||||||
import com.semmle.js.dependencies.DependencyResolver; | ||||||
import com.semmle.js.dependencies.packument.PackageJson; | ||||||
|
@@ -745,6 +747,26 @@ private CompletableFuture<?> extractSource() throws IOException { | |||||
.filter(p -> !isFileTooLarge(p)) | ||||||
.sorted(PATH_ORDERING) | ||||||
.collect(Collectors.toCollection(() -> new LinkedHashSet<>())); | ||||||
// exclude files in output directories as configured in tsconfig.json | ||||||
final List<Path> outDirs = new ArrayList<>(); | ||||||
for (Path cfg : tsconfigFiles) { | ||||||
try { | ||||||
String txt = new WholeIO().read(cfg); | ||||||
TsConfigJson root = new Gson().fromJson(txt, TsConfigJson.class); | ||||||
if (root != null && root.getCompilerOptions() != null) { | ||||||
if (root.getCompilerOptions().getOutDir() == null) { | ||||||
// no outDir specified, so skip this tsconfig.json | ||||||
continue; | ||||||
} | ||||||
Path odir = cfg.getParent().resolve(root.getCompilerOptions().getOutDir()).toAbsolutePath().normalize(); | ||||||
outDirs.add(odir); | ||||||
} | ||||||
} catch (Exception e) { | ||||||
// ignore malformed tsconfig or missing fields | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
} | ||||||
} | ||||||
// exclude files in output directories as configured in tsconfig.json | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
filesToExtract.removeIf(f -> outDirs.stream().anyMatch(od -> f.startsWith(od))); | ||||||
|
||||||
DependencyInstallationResult dependencyInstallationResult = DependencyInstallationResult.empty; | ||||||
if (!tsconfigFiles.isEmpty()) { | ||||||
|
@@ -796,9 +818,19 @@ private CompletableFuture<?> extractFiles( | |||||
*/ | ||||||
private boolean isFileDerivedFromTypeScriptFile(Path path, Set<Path> extractedFiles) { | ||||||
String name = path.getFileName().toString(); | ||||||
if (!name.endsWith(".js")) | ||||||
// only skip JS variants when a corresponding TS/TSX file was already extracted | ||||||
if (!(name.endsWith(".js") | ||||||
|| name.endsWith(".cjs") | ||||||
|| name.endsWith(".mjs") | ||||||
|| name.endsWith(".jsx") | ||||||
|| name.endsWith(".cjsx") | ||||||
|| name.endsWith(".mjsx"))) { | ||||||
return false; | ||||||
String stem = name.substring(0, name.length() - ".js".length()); | ||||||
} | ||||||
// strip off extension | ||||||
int dot = name.lastIndexOf('.'); | ||||||
String stem = dot != -1 ? name.substring(0, dot) : name; | ||||||
// if a TS/TSX file with same base name was extracted, skip this file | ||||||
for (String ext : FileType.TYPESCRIPT.getExtensions()) { | ||||||
if (extractedFiles.contains(path.getParent().resolve(stem + ext))) { | ||||||
return true; | ||||||
|
@@ -1154,7 +1186,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) | |||||
} | ||||||
|
||||||
// extract TypeScript projects from 'tsconfig.json' | ||||||
if (typeScriptMode == TypeScriptMode.FULL | ||||||
if (typeScriptMode != TypeScriptMode.NONE | ||||||
&& treatAsTSConfig(file.getFileName().toString()) | ||||||
&& !excludes.contains(file) | ||||||
&& isFileIncluded(file)) { | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
--- | ||
category: minorAnalysis | ||
--- | ||
* The JavaScript extractor now skips generated JavaScript files if the original TypeScript files are already present. It also skips any files in the output directory specified in the `compilerOptions` part of the `tsconfig.json` file. |
Uh oh!
There was an error while loading. Please reload this page.