Port TS PR #62418: Add diagnostic when rootDir inference changes output layout#2819
Port TS PR #62418: Add diagnostic when rootDir inference changes output layout#2819
Conversation
…n directory Add diagnostic 5011 to warn when rootDir inference changes output layout. Export GetComputedCommonSourceDirectory from outputpaths package. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
|
@copilot For the tsbuild tests that had their diffs changed, look into the submodule and find the equivalent tests, and see if the Go versions should be updated to match to avoid extra errors in the output. |
I checked all 10 tsbuild/tsc baselines that changed and compared them against the TS submodule equivalents. All TS5011 error counts match exactly between Go and the submodule baselines (verified all 10 files). The TS PR #62418 only changed 2 test definition files:
For the resolveJsonModule non-composite, Windows paths, and pnpm layout tests — the TS submodule's test definitions were NOT changed by the PR, and both Go and TS submodule baselines intentionally include the TS5011 errors. No Go test updates are needed. |
There was a problem hiding this comment.
Pull request overview
Ports the missing TS5011 diagnostic from TypeScript PR #62418 to warn when rootDir inference (via configFilePath) changes the output file layout compared to the common source directory computed from emittable inputs.
Changes:
- Exported
outputpaths.GetComputedCommonSourceDirectoryto compute a common source directory from a list of emittable file names (ignoringconfigFilePath-based inference). - Added a new TS5011 diagnostic in
Program.verifyCompilerOptions()for non-composite builds whenrootDiris not set and output layout-affecting options are present. - Updated multiple
tsc/tsbuild/submodule baselines to reflect the new diagnostic output.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/outputpaths/commonsourcedirectory.go | Adds an exported helper to compute common source dir from file names without configFilePath inference. |
| internal/compiler/program.go | Emits TS5011 (with migration link) when config-inferred common dir differs from computed common dir for emittable inputs. |
| testdata/baselines/reference/tsc/moduleResolution/pnpm-style-layout.js | Baseline updated to include TS5011 and adjusted error summary. |
| testdata/baselines/reference/tsc/declarationEmit/when-using-Windows-paths-and-uppercase-letters.js | Baseline updated to include TS5011 output. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/sourcemap-non-composite.js | Baseline updated for TS5011, exit status, and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-with-json-without-rootDir-but-outside-configDirectory-non-composite.js | Baseline updated for TS5011 and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-non-composite.js | Baseline updated for TS5011 and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/include-of-json-along-with-other-include-non-composite.js | Baseline updated for TS5011 and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/include-of-json-along-with-other-include-and-file-name-matches-ts-file-non-composite.js | Baseline updated for TS5011 and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/include-and-files-non-composite.js | Baseline updated for TS5011 and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/resolveJsonModule/files-containing-json-file-non-composite.js | Baseline updated for TS5011 and buildinfo errors flag behavior. |
| testdata/baselines/reference/tsbuild/outputPaths/when-rootDir-is-not-specified.js | Baseline updated for TS5011, exit status, and rebuild behavior when errors must be reported. |
| testdata/baselines/reference/submodule/compiler/declarationEmitToDeclarationDirWithDeclarationOption.js.diff | Diff baseline reduced/adjusted to account for TS5011 in DtsFileErrors output. |
| testdata/baselines/reference/submodule/compiler/declarationEmitToDeclarationDirWithDeclarationOption.js | Baseline updated to include TS5011 in DtsFileErrors output. |
| testdata/baselines/reference/submodule/compiler/commonSourceDirectory_dts.errors.txt.diff | Deleted/reduced because behavior now matches TS reference output. |
| testdata/baselines/reference/submodule/compiler/commonSourceDirectory_dts.errors.txt | Baseline updated to include TS5011 diagnostic output. |
| testdata/baselines/reference/submodule/compiler/commonSourceDirectory.js.diff | Diff baseline reduced/adjusted to account for TS5011 in DtsFileErrors output. |
| testdata/baselines/reference/submodule/compiler/commonSourceDirectory.js | Baseline updated to include TS5011 in DtsFileErrors output. |
| func GetComputedCommonSourceDirectory(emittedFiles []string, currentDirectory string, useCaseSensitiveFileNames bool) string { | ||
| commonSourceDirectory := computeCommonSourceDirectoryOfFilenames(emittedFiles, currentDirectory, useCaseSensitiveFileNames) | ||
| if len(commonSourceDirectory) > 0 { | ||
| commonSourceDirectory = tspath.EnsureTrailingDirectorySeparator(commonSourceDirectory) | ||
| } | ||
| return commonSourceDirectory |
There was a problem hiding this comment.
GetComputedCommonSourceDirectory’s parameter name emittedFiles is misleading because the function is computing a directory from input/emittable source file names, not from the actual emitted output file paths. Renaming the parameter (and/or adding a short comment clarifying the expected inputs and how it differs from GetCommonSourceDirectory) would make misuse less likely.
| // Check if rootDir inferred changed and issue diagnostic | ||
| dir := p.CommonSourceDirectory() | ||
| var emittedFiles []string | ||
| for _, file := range p.files { | ||
| if !file.IsDeclarationFile && sourceFileMayBeEmitted(file, p, false) { | ||
| emittedFiles = append(emittedFiles, file.FileName()) | ||
| } | ||
| } | ||
| dir59 := outputpaths.GetComputedCommonSourceDirectory(emittedFiles, p.GetCurrentDirectory(), p.UseCaseSensitiveFileNames()) | ||
| if dir59 != "" && tspath.GetCanonicalFileName(dir, p.UseCaseSensitiveFileNames()) != tspath.GetCanonicalFileName(dir59, p.UseCaseSensitiveFileNames()) { |
There was a problem hiding this comment.
Variable names in this block are hard to interpret (e.g. dir59 and emittedFiles), and !file.IsDeclarationFile is redundant since sourceFileMayBeEmitted already excludes declaration files. Consider renaming to something like “configInferredCommonDir” vs “computedCommonDirFromEmittableFiles”, and (optionally) derive the file list from the existing getSourceFilesToEmit logic to avoid duplicating emit eligibility rules in multiple places.
Ports the missing piece from microsoft/TypeScript#62418. The core behavioral change (
getCommonSourceDirectoryusingconfigFilePathwithout requiringcomposite) was already ported. What was missing is the helpful TS5011 diagnostic that warns users when their inferred rootDir differs from what the computed common source directory would produce.Changes
outputpaths/commonsourcedirectory.go: ExportGetComputedCommonSourceDirectory— computes common source directory from actual emitted filenames without considering config file pathcompiler/program.go: Add diagnostic check inverifyCompilerOptions()— when!noEmit && !composite && !rootDir && configFilePath && (outDir || declarationDir || outFile), compare config-inferred dir against file-computed dir and emit TS5011 with migration link if they differBaseline changes
commonSourceDirectory_dts.errors.txt.diffdeleted (Go now matches TS output)commonSourceDirectory.js.diffanddeclarationEmitToDeclarationDirWithDeclarationOption.js.diffreduced (DtsFileErrors section now produced; remaining diff is pre-existing test harness formatting difference)💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.