Skip to content

Explain files port #1566

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Aug 12, 2025
Merged

Explain files port #1566

merged 7 commits into from
Aug 12, 2025
+1,097 −245

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Aug 12, 2025 •
edited
Loading

Porting of --explainFiles
And some of the porting for explaining diagnsotics with file include reason.

TODOs that can be done in another PR

  • caching for fileIncludeReason chain to diagnostic / related information
  • some of the file explaining diagnostics i have marked as todos so we have them tracked

@sheetalkamat sheetalkamat marked this pull request as ready for review August 12, 2025 05:38
Copilot
Copilot AI reviewed Aug 12, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a 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 the --explainFiles feature by porting the corresponding functionality from the TypeScript compiler. The implementation adds file inclusion reasoning and diagnostic capabilities to explain why specific files are included in the compilation, along with supporting infrastructure for explaining diagnostics with file include reasons.

  • File inclusion tracking: Adds comprehensive tracking of why each file is included (root file, import, reference, etc.)
  • Explain files output: Implements --explainFiles flag to show file inclusion reasons and diagnostic explanations
  • Diagnostic infrastructure: Creates processing diagnostics system to handle file inclusion explanations

Reviewed Changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/compiler/program.go Adds ExplainFiles method and file inclusion processor integration
internal/compiler/includeprocessor.go New file inclusion processing system with caching for diagnostic generation
internal/compiler/fileInclude.go Core file inclusion reason tracking and diagnostic generation logic
internal/compiler/processingDiagnostic.go Processing diagnostic infrastructure for file inclusion explanations
internal/compiler/fileloader.go Updates file loading to track inclusion reasons for each file
internal/compiler/filesparser.go Integrates inclusion reason tracking into file parsing workflow
internal/execute/tsc.go Updates CLI to call ExplainFiles when --explainFiles flag is used
internal/tsoptions/parsedcommandline.go Adds methods to get matched file and include specs for diagnostic generation
internal/tsoptions/tsconfigparsing.go Refactors config parsing to support file spec tracking before substitution
internal/modulespecifiers/specifiers.go Updates module specifier generation to use project reference source tracking
Test baselines Updated baselines showing new explain files output in various test scenarios
Comments suppressed due to low confidence (1)

internal/tsoptions/tsconfigparsing.go:17

  • The scanner package import was removed but is still being used in line 1188. This will cause a compilation error.
	"github.com/microsoft/typescript-go/internal/parser"

jakebailey
jakebailey reviewed Aug 12, 2025
View reviewed changes
Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM though with https://chromewebstore.google.com/detail/codecov/gedikamndpbemklijjkncpnolildpbgo installed looking at the PR, it seems like a bunch of this code is uncovered so has no tests.

jakebailey
jakebailey approved these changes Aug 12, 2025
View reviewed changes
Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but will need a merge from main (maybe after the trace resolutions code goes in too)

@sheetalkamat
Copy link
Member Author

LGTM but will need a merge from main (maybe after the trace resolutions code goes in too)

yep waitng on that one to do merge

jakebailey
jakebailey reviewed Aug 12, 2025
View reviewed changes
@sheetalkamat sheetalkamat enabled auto-merge August 12, 2025 20:18
@jakebailey
Copy link
Member

jakebailey commented Aug 12, 2025 •
edited
Loading

Ah, smoke test has pointed to a bug:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xcc5d7b]

goroutine 1 [running]:
github.com/microsoft/typescript-go/internal/compiler.(*parseTask).isRoot(...)
	/home/runner/work/typescript-go/typescript-go/internal/compiler/filesparser.go:54
github.com/microsoft/typescript-go/internal/compiler.(*filesParser).start.func1()
	/home/runner/work/typescript-go/typescript-go/internal/compiler/filesparser.go:207 +0x1bb
github.com/microsoft/typescript-go/internal/core.(*singleThreadedWorkGroup).RunAndWait(0xc0002d7e90)
	/home/runner/work/typescript-go/typescript-go/internal/core/workgroup.go:73 +0x93
github.com/microsoft/typescript-go/internal/compiler.(*filesParser).parse(0xc000140e60, 0xc000236308, {0xc00025d408, 0x4e, 0x9f})
	/home/runner/work/typescript-go/typescript-go/internal/compiler/filesparser.go:175 +0x77
github.com/microsoft/typescript-go/internal/compiler.processAllProgramFiles({{0x12fdc50, 0xc000140e10}, 0xc0002121e0, 0x0, 0x0, 0x0, {0x0, 0x0}, {0x0, 0x0}, ...}, ...)
	/home/runner/work/typescript-go/typescript-go/internal/compiler/fileloader.go:132 +0xd1e
github.com/microsoft/typescript-go/internal/compiler.NewProgram({{0x12fdc50, 0xc000140e10}, 0xc0002121e0, 0x0, 0x0, 0x0, {0x0, 0x0}, {0x0, 0x0}, ...})
	/home/runner/work/typescript-go/typescript-go/internal/compiler/program.go:202 +0x246
github.com/microsoft/typescript-go/internal/execute.performIncrementalCompilation({0x12ff908, 0xc000176c60}, 0xc0002121e0, 0xc00011f840, 0xc0001219e0, 0xba335c, 0x0)
	/home/runner/work/typescript-go/typescript-go/internal/execute/tsc.go:246 +0x318
github.com/microsoft/typescript-go/internal/execute.tscCompilation({0x12ff908, 0xc000176c60}, 0xc0002120f0, 0x0)
	/home/runner/work/typescript-go/typescript-go/internal/execute/tsc.go:194 +0x1372
github.com/microsoft/typescript-go/internal/execute.CommandLine({0x12ff908, 0xc000176c60}, {0xc000128090, 0x3, 0x3}, 0x0)
	/home/runner/work/typescript-go/typescript-go/internal/execute/tsc.go:63 +0x1e6
main.runMain()
	/home/runner/work/typescript-go/typescript-go/cmd/tsgo/main.go:23 +0x173
main.main()
	/home/runner/work/typescript-go/typescript-go/cmd/tsgo/main.go:10 +0x1d

Not sure how this isn't caught in tests. It was, smoke test is just our fastest job.

jakebailey
jakebailey reviewed Aug 12, 2025
View reviewed changes
@jakebailey
Copy link
Member

It seems weird that isForAutomaticTypeDirective would ever be set for the CLI or most tests. Or is there another crash expected to appear?

@jakebailey
Copy link
Member

Oh, addAutomaticTypeDirectiveTasks is unconditional, huh...

@sheetalkamat
Copy link
Member Author

It seems weird that isForAutomaticTypeDirective would ever be set for the CLI or most tests. Or is there another crash expected to appear?

That task is to discover and add types from @types added either default or from "compilerOptions.types"

Would have caught it but i have so many terminals open that i thought previous successful run was the run so didnt notice it right away

@jakebailey
Copy link
Member

I confused it for ATA, my mistake!

@sheetalkamat sheetalkamat added this pull request to the merge queue Aug 12, 2025
Merged via the queue into main with commit 1ee2ba9 Aug 12, 2025
22 checks passed
@sheetalkamat sheetalkamat deleted the explainFiles branch August 12, 2025 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@jakebailey jakebailey jakebailey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sheetalkamat @jakebailey