JS-769 Restrict nested node_modules lookups and remove ruling node_modules cleanup

its/ruling/src/test/java/org/sonar/javascript/it/RulingTest.java

@@ -29,12 +29,9 @@
 import com.sonar.orchestrator.util.Version;
 import java.io.File;
 import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.stream.Stream;
@@ -167,7 +164,6 @@ private static Arguments project(
 
   @BeforeAll
   public static void setUp() throws Exception {
-    cleanRootNodeModules();
     ProfileGenerator.RulesConfiguration jsRulesConfiguration =
       new ProfileGenerator.RulesConfiguration()
         .add(
@@ -234,32 +230,6 @@ public static void setUp() throws Exception {
     installScanner();
   }
 
-  /**
-   * Method to remove SonarJS root node_modules to avoid typescript picking up typings from SonarJS,
-   * as they are not available during CI and the results with and without node_modules are different.
-   *
-   * @throws IOException
-   */
-  private static void cleanRootNodeModules() throws IOException {
-    var nodeModules = Path.of("../../node_modules");
-    if (Files.exists(nodeModules)) {
-      var start = System.currentTimeMillis();
-      LOG.info("Cleaning node_modules");
-      try (var dirStream = Files.walk(nodeModules)) {
-        dirStream.sorted(Comparator.reverseOrder()).forEachOrdered(RulingTest::deleteUnchecked);
-      }
-      LOG.info("Done cleaning node_modules in {}ms", System.currentTimeMillis() - start);
-    }
-  }
-
-  private static void deleteUnchecked(Path path) {
-    try {
-      Files.delete(path);
-    } catch (IOException e) {
-      throw new UncheckedIOException(e);
-    }
-  }
-
   private static void installScanner() {
     var installer = new SonarScannerInstaller(orchestrator.getLocators());
     installer.install(Version.create(SCANNER_VERSION), Path.of("target").toFile());

packages/jsts/src/program/compilerHost.ts

@@ -23,7 +23,7 @@ import {
   setCachedSourceFile,
   invalidateParsedSourceFile,
 } from './cache/sourceFileCache.js';
-import type { NormalizedAbsolutePath } from '../rules/helpers/files.js';
+import { normalizeToAbsolutePath, type NormalizedAbsolutePath } from '../rules/helpers/files.js';
 
 interface FsCall {
   op: string;
@@ -120,7 +120,11 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
   // CompilerHost implementation - intercept file reads
 
   readFile(fileName: string): string | undefined {
-    const normalized = path.normalize(fileName);
+    const normalized = normalizeToAbsolutePath(fileName, this.baseDir);
+    if (this.shouldSkipNodeModulesOutsideBaseDir(normalized)) {
+      this.trackFsCall('readFile-node_modules-skip', fileName);
+      return undefined;
+    }
     const cache = getSourceFileContentCache();
     const filesContext = getCurrentFilesContext();
 
@@ -158,7 +162,11 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
   }
 
   fileExists(fileName: string): boolean {
-    const normalized = path.normalize(fileName);
+    const normalized = normalizeToAbsolutePath(fileName, this.baseDir);
+    if (this.shouldSkipNodeModulesOutsideBaseDir(normalized)) {
+      this.trackFsCall('fileExists-node_modules-skip', fileName);
+      return false;
+    }
     const cache = getSourceFileContentCache();
 
     // 1. Check global cache
@@ -186,13 +194,17 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     onError?: (message: string) => void,
     shouldCreateNewSourceFile?: boolean,
   ): ts.SourceFile | undefined {
-    const normalized = path.normalize(fileName);
+    const normalized = normalizeToAbsolutePath(fileName, this.baseDir);
+    if (this.shouldSkipNodeModulesOutsideBaseDir(normalized)) {
+      this.trackFsCall('getSourceFile-node_modules-skip', fileName);
+      return undefined;
+    }
 
     // For files explicitly present in the current analysis context, make the
     // request content authoritative before looking up cached parsed ASTs.
     const contextContent = getCurrentFilesContext()?.[fileName]?.fileContent;
     if (contextContent !== undefined) {
-      this.updateFile(normalized as NormalizedAbsolutePath, contextContent);
+      this.updateFile(normalized, contextContent);
     }
 
     const currentVersion = this.fileVersions.get(normalized) || 0;
@@ -245,6 +257,14 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     return sourceFile;
   }
 
+  private shouldSkipNodeModulesOutsideBaseDir(fileName: string): boolean {
+    if (fileName.startsWith(this.baseHost.getDefaultLibLocation!())) {
+      return false;
+    }
+    const baseDirPrefix = `${this.baseDir}/`;
+    return fileName.includes('/node_modules/') && !fileName.startsWith(baseDirPrefix);
+  }
+
   private trackFsCall(op: string, file: string): void {
     this.fsCallTracker.push({ op, file, timestamp: Date.now() });
   }
@@ -288,6 +308,13 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     includes: readonly string[],
     depth?: number,
   ): string[] {
+    const normalizedRootDir = normalizeToAbsolutePath(rootDir, this.baseDir);
+    if (this.shouldSkipNodeModulesOutsideBaseDir(normalizedRootDir)) {
+      this.trackFsCall('readDirectory-node_modules-skip', rootDir);
+      return [];
+    }
+
+    this.trackFsCall('readDirectory-disk', rootDir);
     return this.baseHost.readDirectory!(rootDir, extensions, excludes, includes, depth);
   }
 
@@ -302,4 +329,12 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
   getNewLine(): string {
     return this.baseHost.getNewLine();
   }
+
+  private shouldSkipNodeModulesOutsideBaseDir(fileName: string): boolean {
+    if (fileName.startsWith(this.getDefaultLibLocation())) {
+      return false;
+    }
+    const baseDirPrefix = `${this.baseDir}/`;
+    return fileName.includes('/node_modules/') && !fileName.startsWith(baseDirPrefix);
+  }
 }

packages/jsts/tests/program/compilerHost.test.ts

@@ -24,7 +24,7 @@ import {
   clearSourceFileContentCache,
   getCachedSourceFile,
 } from '../../src/program/cache/sourceFileCache.js';
-import { normalizeToAbsolutePath } from '../../src/rules/helpers/files.js';
+import { joinPaths, normalizeToAbsolutePath } from '../../src/rules/helpers/files.js';
 
 describe('IncrementalCompilerHost', () => {
   const baseDir = normalizeToAbsolutePath('/project');
@@ -170,6 +170,17 @@ describe('IncrementalCompilerHost', () => {
 
       expect(cache.get(normalizeToAbsolutePath('/project/src/index.ts'))).toBe(content);
     });
+
+    it('should not skip node_modules reads nested under baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+      const nestedNodeModulesFile = joinPaths(baseDir, 'src', 'node_modules', 'pkg', 'index.d.ts');
+
+      host.readFile(nestedNodeModulesFile);
+
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'readFile-node_modules-skip')).toBe(false);
+      expect(calls.some(c => c.op === 'readFile-disk')).toBe(true);
+    });
   });
 
   describe('fileExists', () => {
@@ -197,6 +208,73 @@ describe('IncrementalCompilerHost', () => {
       const calls = host.getTrackedFsCalls();
       expect(calls.some(c => c.op === 'fileExists-context')).toBe(true);
     });
+
+    it('should not skip node_modules lookups nested under baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+      const nestedNodeModulesFile = joinPaths(baseDir, 'src', 'node_modules', 'pkg', 'index.d.ts');
+
+      host.fileExists(nestedNodeModulesFile);
+
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'fileExists-node_modules-skip')).toBe(false);
+      expect(calls.some(c => c.op === 'fileExists-disk')).toBe(true);
+    });
+
+    it('should not skip node_modules lookups under baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+      const fileInsideBaseNodeModules = joinPaths(baseDir, 'node_modules', 'pkg', 'index.d.ts');
+
+      host.fileExists(fileInsideBaseNodeModules);
+
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'fileExists-node_modules-skip')).toBe(false);
+      expect(calls.some(c => c.op === 'fileExists-disk')).toBe(true);
+    });
+
+    it('should skip node_modules lookups outside baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+      const outsideNodeModulesFile = normalizeToAbsolutePath(
+        '/external/node_modules/pkg/index.d.ts',
+      );
+
+      expect(host.fileExists(outsideNodeModulesFile)).toBe(false);
+
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'fileExists-node_modules-skip')).toBe(true);
+      expect(calls.some(c => c.op === 'fileExists-disk')).toBe(false);
+    });
+  });
+
+  describe('readDirectory', () => {
+    const extensions = ['.ts', '.d.ts'];
+    const includes = ['**/*'];
+
+    it('should skip node_modules lookups outside baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+
+      const files = host.readDirectory(
+        '/external/node_modules/pkg',
+        extensions,
+        undefined,
+        includes,
+      );
+
+      expect(files).toEqual([]);
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'readDirectory-node_modules-skip')).toBe(true);
+      expect(calls.some(c => c.op === 'readDirectory-disk')).toBe(false);
+    });
+
+    it('should not skip node_modules lookups under baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+      const nodeModulesUnderBaseDir = joinPaths(baseDir, 'node_modules');
+
+      host.readDirectory(nodeModulesUnderBaseDir, extensions, undefined, includes);
+
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'readDirectory-node_modules-skip')).toBe(false);
+      expect(calls.some(c => c.op === 'readDirectory-disk')).toBe(true);
+    });
   });
 
   describe('getSourceFile', () => {
@@ -264,6 +342,19 @@ describe('IncrementalCompilerHost', () => {
 
       expect(sf?.languageVersion).toBe(ts.ScriptTarget.ES2020);
     });
+
+    it('should not skip source files from nested node_modules under baseDir', () => {
+      const host = new IncrementalCompilerHost(compilerOptions, baseDir);
+      const nestedNodeModulesFile = joinPaths(baseDir, 'src', 'node_modules', 'pkg', 'index.d.ts');
+
+      const sf = host.getSourceFile(nestedNodeModulesFile, ts.ScriptTarget.ESNext);
+
+      expect(sf).toBeUndefined();
+
+      const calls = host.getTrackedFsCalls();
+      expect(calls.some(c => c.op === 'getSourceFile-node_modules-skip')).toBe(false);
+      expect(calls.some(c => c.op === 'getSourceFile-disk-fallback')).toBe(true);
+    });
   });
 
   describe('getSourceFileByPath', () => {