-
Notifications
You must be signed in to change notification settings - Fork 744
Handle file casing when creating program #2159
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
base: main
Are you sure you want to change the base?
Conversation
…nostics is reported
…already startedSubTasks with earlier seen different casing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements file casing handling for case-insensitive file systems to prevent duplicate files that only differ in casing and adds checks for the forceConsistentCasingInFileNames compiler option.
- Adds a new utility function
GetNormalizedAbsolutePathWithoutRootto strip root paths for casing comparison - Refactors the file parser to track files by both path and casing, allowing detection of duplicate files with different casings
- Implements diagnostic reporting for files that differ only in casing (TS1149 and TS1261)
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/baselines/reference/tsc/incremental/Compile-incremental-with-case-insensitive-file-names.js | New baseline test for incremental compilation with case-insensitive file names |
| testdata/baselines/reference/tsc/forceConsistentCasingInFileNames/*.js | New baseline tests covering various forceConsistentCasingInFileNames scenarios |
| internal/tspath/path_test.go | Adds test for GetNormalizedAbsolutePathWithoutRoot function |
| internal/tspath/path.go | Implements GetNormalizedAbsolutePathWithoutRoot to strip root from absolute paths |
| internal/execute/tsctests/tsc_test.go | Adds comprehensive test cases for forceConsistentCasingInFileNames feature |
| internal/execute/incremental/snapshottobuildinfo.go | Changes MapNonNil to Map and removes duplicate file filtering logic |
| internal/compiler/processingDiagnostic.go | Prevents duplicate related info when the preferred location is the same as the include reason |
| internal/compiler/includeprocessor.go | Adds addProcessingDiagnosticsForFileCasing to generate appropriate casing error diagnostics |
| internal/compiler/filesparser.go | Major refactor to track files by casing, detect duplicates, and generate casing diagnostics |
| internal/compiler/fileloader.go | Moves file collection logic into filesParser.getProcessedFiles() |
| for i, task := range tasks { | ||
| taskIsFromExternalLibrary := isFromExternalLibrary || task.fromExternalLibrary | ||
| newTask := &queuedParseTask{task: task, lowestDepth: math.MaxInt} | ||
| loadedTask, loaded := w.tasksByFileName.LoadOrStore(task.FileName(), newTask) | ||
| task = loadedTask.task | ||
| if loaded { | ||
| tasks[i].loadedTask = task | ||
| // Add in the loaded task's external-ness. | ||
| taskIsFromExternalLibrary = taskIsFromExternalLibrary || task.fromExternalLibrary | ||
| } | ||
| task.path = loader.toPath(task.normalizedFilePath) | ||
| data, loaded := w.taskDataByPath.LoadOrStore(task.path, &parseTaskData{ | ||
| tasks: map[string]*parseTask{task.normalizedFilePath: task}, | ||
| lowestDepth: math.MaxInt, | ||
| }) | ||
|
|
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loop variable capture issue: The task and i variables are captured in the closure passed to w.wg.Queue(), but the loop continues to modify these variables. This means all goroutines will see the final values of task and i from the loop, not the values from when the closure was created.
To fix this, capture the loop variables in the loop scope:
for i, task := range tasks {
i := i
task := task
task.path = loader.toPath(task.normalizedFilePath)
// ... rest of the code
}|
|
||
| assert.Equal(t, GetNormalizedAbsolutePathWithoutRoot("/a/b/c.txt", "/a/b"), "a/b/c.txt") | ||
| assert.Equal(t, GetNormalizedAbsolutePathWithoutRoot("c:/work/hello.txt", "c:/work"), "work/hello.txt") | ||
| assert.Equal(t, GetNormalizedAbsolutePathWithoutRoot("c:/work/hello.txt", "d:/worspaces"), "work/hello.txt") |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in test case: "worspaces" should be "workspaces".
| assert.Equal(t, GetNormalizedAbsolutePathWithoutRoot("c:/work/hello.txt", "d:/worspaces"), "work/hello.txt") | |
| assert.Equal(t, GetNormalizedAbsolutePathWithoutRoot("c:/work/hello.txt", "d:/workspaces"), "work/hello.txt") |
| // ensure we only walk each task once | ||
| if checkedName, ok := seen[data]; ok { | ||
| if !loader.opts.Config.CompilerOptions().ForceConsistentCasingInFileNames.IsFalse() { | ||
| // Check if it differs only in drive letters its ok to ignore that error: |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar issue in comment: Missing semicolon/period, and "its" should be "it's".
| // Check if it differs only in drive letters its ok to ignore that error: | |
| // Check if it differs only in drive letters; it's ok to ignore that error. |
...sistentCasingInFileNames/when-file-is-included-from-multiple-places-with-different-casing.js
Show resolved
Hide resolved
.../tsc/forceConsistentCasingInFileNames/two-files-exist-on-disk-that-differs-only-in-casing.js
Show resolved
Hide resolved
...ence/tsc/forceConsistentCasingInFileNames/with-relative-and-non-relative-file-resolutions.js
Show resolved
Hide resolved
.../baselines/reference/tsc/forceConsistentCasingInFileNames/with-triple-slash-ref-from-file.js
Show resolved
Hide resolved
testdata/baselines/reference/tsc/forceConsistentCasingInFileNames/with-type-ref-from-file.js
Show resolved
Hide resolved
.../baselines/reference/tsc/incremental/Compile-incremental-with-case-insensitive-file-names.js
Show resolved
Hide resolved
| seen[data] = task.normalizedFilePath | ||
| } | ||
|
|
||
| if tasksSeenByNameIgnoreCase != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make sure I understand this, why can't this check be made above along with the check for when ForceConsistentCasingInFileNames is true?
Handle file casing when creating program so that we dont add duplicate files that only differ in casing on case insesitive file system.
This also adds checks for
forceConsistentCasingInFileNamesFixes #1549
Fixes #1619