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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
38 changes: 35 additions & 3 deletions javascript/extractor/src/com/semmle/js/extractor/AutoBuild.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
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.

}
}
// 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.

filesToExtract.removeIf(f -> outDirs.stream().anyMatch(od -> f.startsWith(od)));

DependencyInstallationResult dependencyInstallationResult = DependencyInstallationResult.empty;
if (!tsconfigFiles.isEmpty()) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public void extractTypeScriptFiles(
FileExtractors extractors) {
for (Path f : files) {
actual.add(f.toString());
extractedFiles.add(f);
}
}

Expand Down Expand Up @@ -175,7 +176,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)

@Test
public void basicTest() throws IOException {
addFile(true, LGTM_SRC, "tst.js");
addFile(false, LGTM_SRC, "tst.js");
addFile(true, LGTM_SRC, "tst.ts");
addFile(true, LGTM_SRC, "tst.html");
addFile(true, LGTM_SRC, "tst.xsjs");
Expand Down Expand Up @@ -203,6 +204,43 @@ public void typescriptWrongConfig() throws IOException {
runTest();
}

@Test
public void skipJsFilesDerivedFromTypeScriptFiles() throws IOException {
// JS-derived files (.js, .cjs, .mjs, .jsx, .cjsx, .mjsx) should be skipped when TS indexing
envVars.put("LGTM_INDEX_TYPESCRIPT", "basic");
// Add TypeScript sources
addFile(true, LGTM_SRC, "foo.ts");
addFile(true, LGTM_SRC, "bar.tsx");
// Add derived JS variants (should be skipped)
addFile(false, LGTM_SRC, "foo.js");
addFile(false, LGTM_SRC, "bar.jsx");
addFile(false, LGTM_SRC, "foo.cjs");
addFile(false, LGTM_SRC, "foo.mjs");
addFile(false, LGTM_SRC, "bar.cjsx");
addFile(false, LGTM_SRC, "bar.mjsx");
// A normal JS file without TS counterpart should be extracted
addFile(true, LGTM_SRC, "normal.js");
runTest();
}

@Test
public void skipFilesInTsconfigOutDir() throws IOException {
envVars.put("LGTM_INDEX_TYPESCRIPT", "basic");
// Files under outDir in tsconfig.json should be excluded
// Create tsconfig.json with outDir set to "dist"
addFile(true, LGTM_SRC, "tsconfig.json");
Path config = Paths.get(LGTM_SRC.toString(), "tsconfig.json");
Files.write(config,
"{\"compilerOptions\":{\"outDir\":\"dist\"}}".getBytes(StandardCharsets.UTF_8));
// Add files outside outDir (should be extracted)
addFile(true, LGTM_SRC, "src", "app.ts");
addFile(true, LGTM_SRC, "main.js");
// Add files under dist/outDir (should be skipped)
addFile(false, LGTM_SRC, "dist", "generated.js");
addFile(false, LGTM_SRC, "dist", "sub", "x.js");
runTest();
}

@Test
public void includeFile() throws IOException {
envVars.put("LGTM_INDEX_INCLUDE", "tst.js");
Expand Down
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.