changes from review

Author: vdiez

packages/jsts/src/analysis/projectAnalysis/analyzeFile.ts

@@ -34,7 +34,7 @@ import ts from 'typescript';
  * Analyzes a single file, optionally with a TypeScript program for type-checking.
  * This is the common entry point for all analysis paths (with program, without program, with cache).
  */
-export async function analyzeSingleFile(
+export async function analyzeFile(
   fileName: string,
   file: JsTsFiles[string],
   program: ts.Program | undefined,
@@ -53,7 +53,7 @@ export async function analyzeSingleFile(
   };
 
   // Analyze the file (with error handling)
-  const result = await analyzeFile(input);
+  const result = await analyzeInput(input);
 
   if (pendingFiles) {
     pendingFiles.delete(fileName);
@@ -65,7 +65,7 @@ export async function analyzeSingleFile(
  * Safely analyze a JavaScript/TypeScript file wrapping raised exceptions in the output format
  * @param input JsTsAnalysisInput object containing all the data necessary for the analysis
  */
-async function analyzeFile(input: JsTsAnalysisInput): Promise<FileResult> {
+async function analyzeInput(input: JsTsAnalysisInput): Promise<FileResult> {
   try {
     return await getAnalyzerForFile(input.filePath)(input);
   } catch (e) {

packages/jsts/src/analysis/projectAnalysis/analyzeWithIncrementalProgram.ts

@@ -21,7 +21,7 @@ import { ProgressReport } from '../../../../shared/src/helpers/progress-report.j
 import { WsIncrementalResult } from '../../../../bridge/src/request.js';
 import { isAnalysisCancelled } from './analyzeProject.js';
 import { error, info, warn } from '../../../../shared/src/helpers/logging.js';
-import { analyzeSingleFile } from './analyzeFile.js';
+import { analyzeFile } from './analyzeFile.js';
 import { dirname } from 'node:path/posix';
 import { sanitizeReferences } from '../../program/tsconfig/utils.js';
 import ts from 'typescript';
@@ -68,7 +68,7 @@ export async function analyzeWithIncrementalProgram(
       programOptionsFromClosestTsconfig(filename, results, foundProgramOptions, pendingFiles),
     );
 
-    await analyzeSingleFile(
+    await analyzeFile(
       filename,
       files[filename],
       program,
@@ -135,12 +135,14 @@ function programOptionsFromClosestTsconfig(
   try {
     info('No tsconfig found for files, using default options');
     // Fallback: use default options if no tsconfig found
+    // TODO(JS-1138): File order can affect program combinations - improve strategy
     return createProgramOptionsFromJson(defaultCompilerOptions, [...pendingFiles], getBaseDir());
   } catch (e) {
     error(`Failed to generate program from merged config: ${e}`);
   }
 }
 
+// TODO(JS-1139): Optimize by only checking tsconfigs in ancestor directories
 function pickBestMatchTsConfig(tsconfigs: string[], file: string) {
   let bestTsConfig: string | undefined = undefined;
   for (const tsconfig of tsconfigs) {

packages/jsts/src/analysis/projectAnalysis/analyzeWithProgram.ts

@@ -15,7 +15,7 @@
  * along with this program; if not, see https://sonarsource.com/license/ssal/
  */
 import type { JsTsFiles, ProjectAnalysisOutput } from './projectAnalysis.js';
-import { analyzeSingleFile } from './analyzeFile.js';
+import { analyzeFile } from './analyzeFile.js';
 import { error, info, warn } from '../../../../shared/src/helpers/logging.js';
 import { tsConfigStore } from './file-stores/index.js';
 import ts from 'typescript';
@@ -59,8 +59,9 @@ export async function analyzeWithProgram(
   const processedTSConfigs: Set<string> = new Set();
   const tsconfigs = tsConfigStore.getTsConfigs();
 
-  // Process tsconfigs, discovering references as we go
-  // Iterator calls next() on each iteration, so newly added references are automatically processed
+  // Process tsconfigs, discovering project references as we go.
+  // When a tsconfig has project references, we add them via addDiscoveredTsConfig(),
+  // and they will be included in this iteration since getTsConfigs() returns a live iterable.
   for (const tsConfig of tsconfigs) {
     if (isAnalysisCancelled()) {
       return;
@@ -98,7 +99,8 @@ export async function analyzeWithProgram(
   if (foundProgramOptions.some(options => options.missingTsConfig)) {
     results.meta.warnings.push(MISSING_EXTENDED_TSCONFIG);
   }
-  // Clear caches after SonarQube analysis (single-run, don't persist)
+  // Clear caches after SonarQube analysis to free memory (single-run, don't persist).
+  // Note: analyzeWithIncrementalProgram (SonarLint) intentionally keeps caches for subsequent requests.
   const cacheManager = getProgramCacheManager();
   const stats = cacheManager.getCacheStats();
   if (stats.size > 0) {
@@ -140,7 +142,7 @@ async function analyzeFilesFromEntryPoint(
       return;
     }
 
-    await analyzeSingleFile(
+    await analyzeFile(
       fileName,
       files[fileName],
       tsProgram,
@@ -213,7 +215,7 @@ async function analyzeFilesFromTsConfig(
       return;
     }
 
-    await analyzeSingleFile(
+    await analyzeFile(
       fileName,
       files[fileName],
       tsProgram,

packages/jsts/src/analysis/projectAnalysis/analyzeWithoutProgram.ts

@@ -15,13 +15,13 @@
  * along with this program; if not, see https://sonarsource.com/license/ssal/
  */
 import { JsTsFiles, ProjectAnalysisOutput } from './projectAnalysis.js';
-import { getBaseDir } from '../../../../shared/src/helpers/configuration.js';
-import { debug } from '../../../../shared/src/helpers/logging.js';
+import { getBaseDir, isJsTsFile } from '../../../../shared/src/helpers/configuration.js';
+import { debug, warn } from '../../../../shared/src/helpers/logging.js';
 import { relative } from 'node:path/posix';
 import { ProgressReport } from '../../../../shared/src/helpers/progress-report.js';
 import { WsIncrementalResult } from '../../../../bridge/src/request.js';
 import { isAnalysisCancelled } from './analyzeProject.js';
-import { analyzeSingleFile } from './analyzeFile.js';
+import { analyzeFile } from './analyzeFile.js';
 
 /**
  * Analyzes files without type-checking.
@@ -43,8 +43,15 @@ export async function analyzeWithoutProgram(
     if (isAnalysisCancelled()) {
       return;
     }
-    debug(`File not part of any tsconfig.json: ${relative(getBaseDir(), filename)}`);
-    await analyzeSingleFile(
+    const relativePath = relative(getBaseDir(), filename);
+    if (isJsTsFile(filename)) {
+      warn(
+        `JS/TS file analyzed without type-checking (not part of any tsconfig.json): ${relativePath}`,
+      );
+    } else {
+      debug(`File not part of any tsconfig.json: ${relativePath}`);
+    }
+    await analyzeFile(
       filename,
       files[filename],
       undefined,

packages/jsts/src/program/cache/programOptionsCache.ts

@@ -19,22 +19,33 @@ import type { ProgramOptions } from '../tsconfig/options.js';
 
 /**
  * Cache for createProgramOptions() results
- * Key: tsConfig path and hash of tsconfigContents (if provided)
+ * Key: tsConfig path + tsconfigContents (if provided)
  */
 const programOptionsCache = new Map<string, ProgramOptions>();
 
+function buildCacheKey(tsConfig: string, tsconfigContents?: string): string {
+  return `${tsConfig}:${tsconfigContents}`;
+}
+
 /**
  * Get cached ProgramOptions from createProgramOptions()
  */
-export function getCachedProgramOptions(cacheKey: string): ProgramOptions | undefined {
-  return programOptionsCache.get(cacheKey);
+export function getCachedProgramOptions(
+  tsConfig: string,
+  tsconfigContents?: string,
+): ProgramOptions | undefined {
+  return programOptionsCache.get(buildCacheKey(tsConfig, tsconfigContents));
 }
 
 /**
  * Store ProgramOptions in cache for createProgramOptions()
  */
-export function setCachedProgramOptions(cacheKey: string, options: ProgramOptions): void {
-  programOptionsCache.set(cacheKey, options);
+export function setCachedProgramOptions(
+  tsConfig: string,
+  options: ProgramOptions,
+  tsconfigContents?: string,
+): void {
+  programOptionsCache.set(buildCacheKey(tsConfig, tsconfigContents), options);
 }
 
 /**

packages/jsts/src/program/cache/sourceFileCache.ts

@@ -26,7 +26,8 @@ const sourceFileContentCache = new Map<string, string>();
 /**
  * Global cache for parsed TypeScript SourceFile ASTs
  *
- * This cache stores parsed ASTs keyed by (fileName, scriptTarget, contentHash).
+ * This cache stores parsed ASTs keyed by (fileName, scriptTarget).
+ * The contentHash is stored as a value to validate cache freshness.
  * Multiple programs can share the same parsed AST if they have the same target.
  *
  * IMPLEMENTATION NOTE: We cache SourceFiles per ScriptTarget to preserve the
@@ -95,6 +96,9 @@ export function getCachedSourceFile(
   }
 
   const cached = targetCache.get(scriptTarget);
+  // If hash doesn't match, return undefined (cache miss). The caller will parse
+  // the file and call setCachedSourceFile, which overwrites the stale entry.
+  // This makes the cache self-healing - stale entries cause one miss, then get fixed.
   if (cached?.contentHash === contentHash) {
     return cached.sourceFile;
   }
@@ -125,11 +129,11 @@ export function setCachedSourceFile(
 }
 
 /**
- * Invalidate cached SourceFile for a specific file (all targets)
+ * Invalidate parsed SourceFile AST cache for a specific file (all targets)
  * Used when file content changes
  * @param fileName Normalized file path
  */
-export function invalidateCachedSourceFile(fileName: string): void {
+export function invalidateParsedSourceFile(fileName: string): void {
   parsedSourceFileCache.delete(fileName);
 }
 

packages/jsts/src/program/compilerHost.ts

@@ -21,7 +21,7 @@ import {
   getCurrentFilesContext,
   getCachedSourceFile,
   setCachedSourceFile,
-  invalidateCachedSourceFile,
+  invalidateParsedSourceFile,
 } from './cache/sourceFileCache.js';
 
 interface FsCall {
@@ -38,6 +38,8 @@ interface FsCall {
  * - Incremental file updates for builder programs
  */
 export class IncrementalCompilerHost implements ts.CompilerHost {
+  // Map from normalized file path to version number (incremented on file changes).
+  // Used by TypeScript's builder programs for change detection.
   private readonly fileVersions = new Map<string, number>();
   private readonly modifiedFiles = new Set<string>();
   private fsCallTracker: FsCall[] = [];
@@ -71,7 +73,7 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     this.modifiedFiles.add(normalized);
 
     // Invalidate global parsed SourceFile cache for this file (all targets)
-    invalidateCachedSourceFile(normalized);
+    invalidateParsedSourceFile(normalized);
 
     return true; // File was updated
   }
@@ -141,7 +143,9 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     if (content) {
       cache.set(normalized, content);
 
-      // Update files object if entry exists (keep cache and files in sync)
+      // Update files object if entry already exists (keep cache and files in sync).
+      // We only update existing entries - we don't create new ones as the filesContext
+      // represents files explicitly provided by the caller.
       if (filesContext?.[fileName]) {
         filesContext[fileName].fileContent = content;
       }
@@ -167,6 +171,7 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     }
 
     // 3. Fallback to filesystem
+    // TODO: Consider caching negative results to avoid repeated filesystem checks for missing files
     this.trackFsCall('fileExists-disk', fileName);
     return this.baseHost.fileExists(fileName);
   }
@@ -200,8 +205,6 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
     const content = this.readFile(fileName);
     let sourceFile: (ts.SourceFile & { version?: string }) | undefined;
     if (content) {
-      this.trackFsCall('getSourceFile', fileName);
-
       // Parse the file
       sourceFile = ts.createSourceFile(
         fileName,
@@ -213,7 +216,8 @@ export class IncrementalCompilerHost implements ts.CompilerHost {
       // Store in global cache for reuse across programs
       setCachedSourceFile(normalized, scriptTarget, contentHash, sourceFile);
     } else {
-      // Fallback to base host for files we don't have
+      // Fallback to base host for files not in our cache/context (e.g., TypeScript lib files).
+      // readFile returns undefined for these, but baseHost can resolve them.
       this.trackFsCall('getSourceFile-disk-fallback', fileName);
       sourceFile = this.baseHost.getSourceFile(
         fileName,

packages/jsts/src/program/tsconfig/options.ts

@@ -124,7 +124,7 @@ export function createProgramOptionsFromJson(
  */
 export function createProgramOptions(tsConfig: string, tsconfigContents?: string): ProgramOptions {
   // Check cache first
-  const cached = getCachedProgramOptions(`${tsConfig}:${tsconfigContents}`);
+  const cached = getCachedProgramOptions(tsConfig, tsconfigContents);
   if (cached) {
     return cached;
   }
@@ -157,7 +157,7 @@ export function createProgramOptions(tsConfig: string, tsconfigContents?: string
         return cachedTsConfig.contents;
       }
 
-      // 3. Read from disk
+      // 3. Read from filesystem or sourceFileStore (depending on canAccessFileSystem())
       const fileContents = defaultParseConfigHost.readFile(file);
 
       // 4. Handle missing extended tsconfig (return empty config)
@@ -226,7 +226,7 @@ export function createProgramOptions(tsConfig: string, tsconfigContents?: string
     [PROGRAM_OPTIONS_BRAND]: true,
   } as const;
 
-  setCachedProgramOptions(`${tsConfig}:${tsconfigContents}`, result);
+  setCachedProgramOptions(tsConfig, result, tsconfigContents);
 
   return result;
 }

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

@@ -36,17 +36,16 @@ describe('programOptionsCache', () => {
 
   describe('getCachedProgramOptions', () => {
     it('should return undefined for cache miss', () => {
-      const result = getCachedProgramOptions('/project/tsconfig.json:undefined');
+      const result = getCachedProgramOptions('/project/tsconfig.json');
       expect(result).toBeUndefined();
     });
 
     it('should return cached options for cache hit', () => {
       const tsConfig = path.join(fixtures, 'tsconfig.json');
       const options = createProgramOptions(tsConfig);
-      const cacheKey = `${tsConfig}:custom`;
 
-      setCachedProgramOptions(cacheKey, options);
-      const result = getCachedProgramOptions(cacheKey);
+      setCachedProgramOptions(tsConfig, options, 'custom');
+      const result = getCachedProgramOptions(tsConfig, 'custom');
 
       expect(result).toBe(options);
     });
@@ -56,11 +55,10 @@ describe('programOptionsCache', () => {
     it('should store options in cache', () => {
       const tsConfig = path.join(fixtures, 'tsconfig.json');
       const options = createProgramOptions(tsConfig);
-      const cacheKey = `${tsConfig}:custom`;
 
-      setCachedProgramOptions(cacheKey, options);
+      setCachedProgramOptions(tsConfig, options, 'custom');
 
-      expect(getCachedProgramOptions(cacheKey)).toBe(options);
+      expect(getCachedProgramOptions(tsConfig, 'custom')).toBe(options);
     });
 
     it('should handle different cache keys for same tsconfig with different contents', () => {
@@ -72,14 +70,11 @@ describe('programOptionsCache', () => {
 
       const options2 = createProgramOptions(tsConfig);
 
-      const cacheKey1 = `${tsConfig}:content1`;
-      const cacheKey2 = `${tsConfig}:content2`;
+      setCachedProgramOptions(tsConfig, options1, 'content1');
+      setCachedProgramOptions(tsConfig, options2, 'content2');
 
-      setCachedProgramOptions(cacheKey1, options1);
-      setCachedProgramOptions(cacheKey2, options2);
-
-      expect(getCachedProgramOptions(cacheKey1)).toBe(options1);
-      expect(getCachedProgramOptions(cacheKey2)).toBe(options2);
+      expect(getCachedProgramOptions(tsConfig, 'content1')).toBe(options1);
+      expect(getCachedProgramOptions(tsConfig, 'content2')).toBe(options2);
     });
 
     it('should overwrite existing cache entry', () => {
@@ -91,12 +86,11 @@ describe('programOptionsCache', () => {
       clearTsConfigContentCache();
 
       const options2 = createProgramOptions(tsConfig2);
-      const cacheKey = 'shared-key';
 
-      setCachedProgramOptions(cacheKey, options1);
-      setCachedProgramOptions(cacheKey, options2);
+      setCachedProgramOptions('shared-key', options1);
+      setCachedProgramOptions('shared-key', options2);
 
-      expect(getCachedProgramOptions(cacheKey)).toBe(options2);
+      expect(getCachedProgramOptions('shared-key')).toBe(options2);
     });
   });