Skip to content

Commit 79c01c3

Browse files
Boshenclaude
authored andcommitted
fix: address PR review comments for tsconfig file matching
- Fix critical bug in files array matching: normalize files in constructor and use exact match only (prevents false positives) - Fix include empty check: None should default to **/* not be treated as empty - Remove duplicate empty pattern check - Add tests for fixture files in dist/ and node_modules/ - Fix test assertions for extends inheritance and paths outside root - Update character set pattern test (TypeScript doesn't support [A-Z] syntax) - Fix outdir_exclude fixture to include .js and .d.ts files - Add test case verifying excluded dist directory cannot use path mappings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e08648f commit 79c01c3

File tree

6 files changed

+18
-20
lines changed

6 files changed

+18
-20
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"include": ["[A-Z]*.ts"]
2+
"include": ["*.ts"]
33
}

fixtures/tsconfig/cases/outdir_exclude/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"include": ["**/*.ts"],
2+
"include": ["**/*.ts", "**/*.js", "**/*.d.ts"],
33
"compilerOptions": {
44
"outDir": "dist"
55
}

src/tests/tsconfig_file_matcher.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,12 @@ fn test_wildcard_patterns() {
106106
}
107107

108108
#[test]
109-
fn test_character_set_patterns() {
109+
fn test_simple_wildcard_patterns() {
110110
let (matcher, fixture_dir) = create_matcher_from_fixture("character_set_patterns");
111111

112112
let test_cases = [
113-
("App.ts", true, "character set matches uppercase start"),
114-
("Button.ts", true, "character set matches uppercase start"),
113+
("App.ts", true, "wildcard matches .ts files"),
114+
("Button.ts", true, "wildcard matches .ts files"),
115115
];
116116

117117
for (file_path, should_match, comment) in test_cases {

src/tests/tsconfig_include_exclude.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,19 @@ fn tsconfig_include_exclude_patterns() {
1919
// Files in src/ can use path mappings, files outside cannot
2020
("include_basic", "src/index.ts", "@/utils/helper", true, "file in src/ can use path mapping"),
2121
("include_basic", "test.ts", "@/utils/helper", false, "file outside include pattern cannot use path mapping"),
22+
("include_basic", "dist/output.js", "@/utils/helper", false, "file in dist/ cannot use path mapping"),
2223

2324
// Exclude basic - Include: **/*.ts, Exclude: **/*.test.ts
2425
// Test files are excluded from using path mappings
2526
("exclude_basic", "src/index.ts", "@/helper", true, "non-test file can use path mapping"),
2627
("exclude_basic", "src/index.test.ts", "@/helper", false, "test file excluded from using path mapping"),
28+
("exclude_basic", "node_modules/foo.ts", "@/helper", false, "node_modules excluded by default"),
2729

2830
// Default include (no include specified, defaults to **/*) - Exclude: [dist]
2931
// All files except dist/ can use path mappings
3032
("with_baseurl", "index.ts", "~/log", true, "file in root can use path mapping"),
3133
("with_baseurl", "index.ts", "log", true, "file in root can use baseUrl"),
34+
("with_baseurl", "dist/output.js", "~/log", false, "file in excluded dist cannot use path mapping"),
3235
];
3336

3437
for (fixture, importer, specifier, should_resolve, comment) in test_cases {
@@ -127,8 +130,7 @@ fn test_extends_include_exclude_inheritance() {
127130
assert!(!tsconfig.matches_file(&f.join("lib/utils.ts")));
128131

129132
// Test whether exclude from parent applies to child's include
130-
// (behavior to be determined by implementation)
131-
let _is_test_file_included = tsconfig.matches_file(&f.join("src/index.test.ts"));
133+
assert!(!tsconfig.matches_file(&f.join("src/index.test.ts")));
132134
}
133135

134136
/// Test project references with include/exclude
@@ -181,9 +183,8 @@ fn test_paths_outside_root() {
181183
// Files in my-app should be included (default include)
182184
assert!(tsconfig.matches_file(&my_app_dir.join("index.ts")));
183185

184-
// Files outside the tsconfig directory - behavior depends on implementation
185-
// Whether include/exclude patterns can reach outside the tsconfig directory
186-
let _is_outside_file_included = tsconfig.matches_file(&my_utils_dir.join("log.ts"));
186+
// Files outside the tsconfig directory
187+
assert!(!tsconfig.matches_file(&my_utils_dir.join("log.ts")));
187188
}
188189

189190
/// Test case sensitivity on Unix systems

src/tsconfig/file_matcher.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ impl TsconfigFileMatcher {
9797
}
9898

9999
Self {
100-
files,
100+
files: files.map(|f| Self::normalize_patterns(f, &tsconfig_dir)),
101101
include_patterns: Self::normalize_patterns(include_patterns, &tsconfig_dir),
102102
exclude_patterns: Self::normalize_patterns(exclude_patterns, &tsconfig_dir),
103103
tsconfig_dir,
@@ -200,8 +200,7 @@ impl TsconfigFileMatcher {
200200
// 1. Check files array first (absolute priority)
201201
if let Some(files) = &self.files {
202202
for file in files {
203-
// Check both exact match and ends_with
204-
if normalized == *file || normalized.ends_with(file) {
203+
if normalized == *file {
205204
return true; // Files overrides exclude
206205
}
207206
}
@@ -210,14 +209,12 @@ impl TsconfigFileMatcher {
210209
if self.include_patterns.is_empty() {
211210
return false;
212211
}
213-
}
214-
215-
// 2. Check if empty patterns (match nothing case)
216-
if self.include_patterns.is_empty() {
212+
} else if self.include_patterns.is_empty() {
213+
// No files array and empty patterns (match nothing case)
217214
return false;
218215
}
219216

220-
// 3. Test against include patterns
217+
// 2. Test against include patterns
221218
let mut included = false;
222219
for pattern in &self.include_patterns {
223220
if fast_glob::glob_match(pattern, &normalized) {
@@ -230,7 +227,7 @@ impl TsconfigFileMatcher {
230227
return false;
231228
}
232229

233-
// 4. Test against exclude patterns
230+
// 3. Test against exclude patterns
234231
for pattern in &self.exclude_patterns {
235232
if fast_glob::glob_match(pattern, &normalized) {
236233
return false;

src/tsconfig/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl TsConfig {
336336

337337
// SPECIAL CASE: empty files + no include = skip file matching
338338
let is_empty_case = self.files.as_ref().is_some_and(std::vec::Vec::is_empty)
339-
&& self.include.as_ref().is_none_or(std::vec::Vec::is_empty);
339+
&& self.include.as_ref().is_some_and(std::vec::Vec::is_empty);
340340

341341
if is_empty_case {
342342
self.file_matcher = Some(TsconfigFileMatcher::empty());

0 commit comments

Comments
 (0)