typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

Add missing TS6504 diagnostic for JavaScript files without allowJs

Open Copilot opened this issue 1 month ago • 1 comments

tsgo was silently ignoring JavaScript files when allowJs was not enabled, while tsc reports error TS6504. This made tsgo less helpful for users accidentally compiling .js files.

Changes

Modified internal/compiler/filesparser.go:

  • Added diagnostic check in parseTask.load() to detect JS files without allowJs for all file inclusion paths
  • Check now applies to root files, triple-slash references, type references, and all other file loading paths
  • Uses existing processingDiagnosticKindExplainingFileInclude infrastructure to report the error with file inclusion context

Modified internal/compiler/fileloader.go:

  • Updated addRootTask to include files with extensions even if unsupported, so they can be properly diagnosed in the common load path

The diagnostic check is now placed in the common file loading path (parseTask.load()), similar to how TypeScript's getSourceFileFromReferenceWorker is called by processSourceFile for all file types.

Test baselines updated to match TypeScript compiler output.

  • Fixes microsoft/typescript-go#2084
Original prompt

This section details on the original issue you should resolve

<issue_title>Missing error when JS file compiled without allowJs on</issue_title> <issue_description>## Steps to reproduce


D:\Throwaway\corsarepro>tsc a.js
error TS6504: File 'a.js' is a JavaScript file. Did you mean to enable the 'allowJs' option?
  The file is in the program because:
    Root file specified for compilation


Found 1 error.


D:\Throwaway\corsarepro>tsgo a.js

D:\Throwaway\corsarepro>
```</issue_description>

<agent_instructions>Use print debugging and `go test -v` to trace why the file is not being included or errored on for the test case diff mentioned in the issue thread. Print debugging with the tsgo binary on the example in the issue itself may also work.</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@jakebailey</author><body>
I assume this is https://github.com/microsoft/typescript-go/blob/05f4d69f81d416946ce46a374d91b78c0fbd37c8/testdata/baselines/reference/submodule/compiler/checkJsFiles6.errors.txt.diff#L5</body></comment_new>
</comments>

  • Fixes microsoft/typescript-go#2084

💡 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.

Copilot avatar Nov 15 '25 19:11 Copilot

This introduces a data race.

I'm not sure includeProcessor is safe for concurrent use, yet the existing code appears to already touch this concurrently and I think that is likely wrong.

This PR has:

==================
WARNING: DATA RACE
Read at 0x00c016a3e8c8 by goroutine 113684:
  github.com/microsoft/typescript-go/internal/compiler.(*includeProcessor).addProcessingDiagnostic()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/includeprocessor.go:59 +0x998
  github.com/microsoft/typescript-go/internal/compiler.(*parseTask).load()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:89 +0x8b8
  github.com/microsoft/typescript-go/internal/compiler.(*filesParser).start.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:253 +0x344
  github.com/microsoft/typescript-go/internal/core.(*parallelWorkGroup).Queue.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/core/workgroup.go:40 +0x2e
  sync.(*WaitGroup).Go.func1()
      /home/cloudtest/.local/share/microsoft-go/go1.25.4-2/src/sync/waitgroup.go:239 +0x5d

Previous write at 0x00c016a3e8c8 by goroutine 113687:
  github.com/microsoft/typescript-go/internal/compiler.(*includeProcessor).addProcessingDiagnostic()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/includeprocessor.go:59 +0xa31
  github.com/microsoft/typescript-go/internal/compiler.(*parseTask).load()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:89 +0x8b8
  github.com/microsoft/typescript-go/internal/compiler.(*filesParser).start.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:253 +0x344
  github.com/microsoft/typescript-go/internal/core.(*parallelWorkGroup).Queue.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/core/workgroup.go:40 +0x2e
  sync.(*WaitGroup).Go.func1()
      /home/cloudtest/.local/share/microsoft-go/go1.25.4-2/src/sync/waitgroup.go:239 +0x5d

Which is obviously wrong, but resolveTypeReferenceDirectives today already does p.includeProcessor.addProcessingDiagnostic without any locking, so I think we have just been lucky and not hit this.

jakebailey avatar Dec 02 '25 00:12 jakebailey

@copilot See above. Make includeProcessor safe for concurrent use.

jakebailey avatar Dec 03 '25 03:12 jakebailey

@copilot See above. Make includeProcessor safe for concurrent use.

Fixed in d1c8f3b. Added mutex protection to includeProcessor to prevent concurrent access to processingDiagnostics and fileIncludeReasons. All reads and writes to these fields are now protected by the mutex, and helper methods addFileIncludeReason() and getFileIncludeReasons() were added for safe concurrent access. Tests pass with race detector enabled.

Copilot avatar Dec 03 '25 03:12 Copilot

@copilot Address the comments

jakebailey avatar Dec 03 '25 19:12 jakebailey

Cleaned this up; PTAL 🙂

jakebailey avatar Dec 03 '25 20:12 jakebailey