Feat/barrel enhancements

[Coderrob]

This pull request introduces several improvements and safeguards to the barrel file generation system, focusing on preserving direct definitions in index.ts, optimizing performance and memory usage, and enhancing robustness against large files and deep directory structures. It also updates tests and linting rules to support these changes.

Barrel file generation and content preservation:

• The BarrelFileGenerator now preserves direct definitions (such as functions, types, and constants) in index.ts files when updating, instead of overwriting or removing them. Only re-exports are sanitized and regenerated as needed. [1] [2] [3] [4]
• Added a new test to ensure that direct definitions are correctly preserved during barrel file updates.

Performance and memory optimizations:

• Introduced batching and concurrency control when processing TypeScript files for export extraction, preventing excessive memory usage and improving performance on large projects. [1] [2]
• Implemented an LRU-style cache for parsed exports, keyed by file modification time, to avoid redundant parsing and further reduce processing time. [1] [2]

Robustness and safeguards:

• Added a maximum recursion depth (20) for processing subdirectories to prevent infinite loops or stack overflows in deeply nested directory structures. [1] [2] [3]
• The file system service now checks file size before reading and rejects files larger than 10MB, protecting against out-of-memory errors. [1] [2] [3] [4]
• Added a method to retrieve file stats with error handling, supporting the caching and size-checking features.

Testing and linting improvements:

• Test patterns updated to include new test files under dist/types and dist/src/types.
• ESLint config now warns against inline object types in type annotations, encouraging use of named interfaces or type aliases.

These changes enhance the reliability, maintainability, and scalability of the barrel file generation system, especially for large or complex codebases.

[Copilot]

Pull request overview

This pull request enhances the barrel file generation system with performance optimizations, robustness safeguards, and improved content preservation. The changes focus on making the system more reliable and scalable for large codebases.

Changes:

• Added preservation of direct definitions (functions, types, constants) in index.ts files when updating barrel files, ensuring only re-exports are sanitized
• Implemented batching, concurrency control, and LRU-style caching for export parsing to optimize performance and memory usage
• Added safeguards including recursion depth limits (20 levels) and file size checks (10MB maximum) to prevent infinite loops and memory issues

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/types/contracts.test.ts	New comprehensive contract validation tests for enums, types, and behavioral contracts
src/extension.ts	Added BarrelCommandQueue to prevent concurrent barrel generation operations
src/core/io/file-system.service.ts	Added file size validation and new getFileStats method for caching support
src/core/io/file-system.service.test.ts	Added tests for file size validation and updated existing tests for stat calls
src/core/barrel/barrel-file.generator.ts	Major refactoring with caching, batching, concurrency control, recursion depth limiting, and definition preservation logic
src/core/barrel/barrel-file.generator.test.ts	Added test for preserving direct definitions during barrel file updates
scripts/run-tests.cjs	Added test patterns for dist/types/*.test.js and dist/src/types/*.test.js
eslint.config.mjs	Added linting rule to discourage inline object types in type annotations

Comments suppressed due to low confidence (1)

src/core/io/file-system.service.ts:142

• The error handling at lines 139-142 will catch and re-wrap errors from the file size check at lines 131-136. This means if a file is too large, the original descriptive error message about the file size will be wrapped in a generic "Failed to read file" message, making it less clear to the user what the actual problem is.

Consider checking the error type and re-throwing the size error directly, or move the size check outside the try-catch block to avoid double-wrapping the error message.

    } catch (error) {
      const errorMessage = getErrorMessage(error);
      throw new Error(`Failed to read file ${filePath}: ${errorMessage}`);
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The extractExportPath method (called at line 488) only recognizes re-exports in the format export * from '...'. However, TypeScript also supports other re-export patterns like export { foo, bar } from './module', export { default } from './module', and export { foo as bar } from './module'.

These other re-export patterns will return null from extractExportPath, causing them to be treated as direct definitions and preserved at line 497, even though they are re-exports that should be sanitized and validated. This could lead to stale or invalid re-exports being retained in the barrel file.

Consider extending the extractExportPath logic to handle all TypeScript re-export patterns, or document this limitation clearly.

[Copilot]

At line 497, the original line (not the trimmed line) is preserved when it's not a re-export. This is correct for maintaining the original formatting, including indentation. However, at line 485, the code checks if trimmedLine.length > 0, which means empty lines in the original content will be skipped. This is intentional according to line 202 which filters out empty lines anyway.

However, there's an inconsistency: if the original line has only whitespace, it won't be added at line 497 (because trimmedLine.length === 0), but if it had mixed content, the original spacing is preserved. This could lead to inconsistent whitespace handling. Consider documenting this behavior or normalizing the whitespace handling approach.

Suggested change

	// This is not a re-export (direct definition, comment, etc.), preserve it
	// This is not a re-export (direct definition, comment, etc.), preserve it.
	// Note: Lines that are only whitespace are intentionally skipped to normalize
	// empty lines, while lines with content keep their original indentation/spacing.

[Copilot]

The recursion depth check at line 308 uses >= which means processing stops when depth equals maxDepth. However, the initial call at line 99 starts with depth=0, so the maximum actual depth processed is 19, not 20 as suggested by the comment at line 306.

For example: depth 0, 1, 2, ..., 19 are processed (20 levels), and depth 20 is rejected. This is correct for 20 levels, but the comment could be clearer that this allows 20 levels starting from 0. Consider clarifying the comment to say "Maximum 20 levels of nesting" or adjust the check to > maxDepth if you want to allow 21 levels (0-20).

Suggested change

	const maxDepth = 20; // Prevent infinite recursion in deeply nested directory structures
	const maxDepth = 20; // Maximum 20 levels of nesting (depth 0–19) to prevent infinite recursion

[Copilot]

The Semaphore implementation has a subtle ordering issue in the release method. At line 668, it increments permits, then at lines 669-672, if there are waiters, it decrements permits again. This means if a waiter exists, the permit count temporarily increases by 1 then decreases by 1, resulting in no net change. However, if release() is called and there's no waiter initially, the permit count increases. Then if acquire() is called before the next release(), it will correctly decrement.

The issue is that this creates a race condition: the permits are incremented at line 668 before checking for waiters, which means there's a brief moment where permits is higher than it should be. If another acquire() call happens between line 668 and 670, it might pass the check at line 654 when it shouldn't.

A safer implementation would check for waiters first before incrementing permits.

[Copilot]

The error handling in the BarrelCommandQueue has a problem. When an error occurs in the operation at line 52-53, it calls reject(error) which correctly rejects the promise returned by enqueue(). However, at line 77, if an error occurs during operation execution in the while loop, it's caught and logged but the error has already been propagated via reject at line 53. This creates a double error handling path.

The bigger issue is that at line 74, await operation() is called, and if that operation throws, it will be caught at line 75-78. But the operation itself (lines 48-55) already has a try-catch that will call reject(). So if the operation's inner code throws, it will:

1. Be caught at line 52, call reject(error) at line 53
2. Then propagate up to line 74, get caught at line 75, and logged at line 77

This means errors are handled twice, which could lead to confusion. The processQueue catch at line 75-78 is redundant since each operation already has its own try-catch.

Suggested change

	try {
	await operation();
	} catch (error) {
	// Log error but continue processing queue
	console.error('Barrel command failed:', error);
	}
	await operation();

[Copilot]

The cache eviction strategy is not truly LRU (Least Recently Used). The current implementation simply deletes the first entry in the Map when the size exceeds 1000. JavaScript Maps maintain insertion order, but this approach removes the oldest inserted entry, not the least recently accessed entry.

To implement true LRU, the cache should track access order and remove the least recently accessed item. The current implementation will remove entries that were added first, even if they were recently used. This could lead to cache thrashing where frequently accessed files that were cached early get evicted while less frequently accessed newer entries remain.

Consider using a proper LRU cache implementation or updating the access order when cache hits occur at line 585-586.

[Copilot]

The code uses console.warn for logging when the maximum recursion depth is reached. The codebase has a proper logging infrastructure (OutputChannelLogger) that should be used instead of console methods for consistency and proper integration with VS Code's output channel.

Consider injecting the logger into BarrelFileGenerator or using the OutputChannelLogger to ensure warnings are visible in the extension's output channel rather than just the console.

[Copilot]

The code uses console.warn for logging file processing errors. The codebase has a proper logging infrastructure (OutputChannelLogger) that should be used instead for consistency and to ensure errors are properly visible in the VS Code output channel.

Consider injecting the logger into BarrelFileGenerator or using the OutputChannelLogger to log these warnings.

[Copilot]

The code uses console.error for logging queue processing errors. The codebase has a proper logging infrastructure (OutputChannelLogger) that should be used instead for consistency.

Consider using the OutputChannelLogger to ensure errors are properly captured in the VS Code output channel.

[Copilot]

The processConcurrently method has redundant code. Lines 619 and 632 are unnecessary. The method creates an empty results array at line 619, then at line 631 resolves all promises into resolvedResults, and at line 632 pushes all of them into results before returning it at line 633.

This can be simplified to just return the result of Promise.all() directly without the intermediate results array, i.e., return await Promise.all(promises);. The current code adds unnecessary operations and memory allocation.