Fix missing TS2725 error for class named 'Object' in non-ES2015 modules#3268
Fix missing TS2725 error for class named 'Object' in non-ES2015 modules#3268
Conversation
…named Object in non-ES2015 modules Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/590500f5-62e3-4972-831c-f52d6b3a2fa0 Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com>
|
Why not fix #2087 and fix all of these? |
There was a problem hiding this comment.
Pull request overview
This PR aligns the Go checker’s name-collision diagnostics with TypeScript by ensuring TS2725 is reported when a non-ambient class is named Object in files that emit to a non-ES2015 module format (e.g. CommonJS), and updates baseline outputs accordingly.
Changes:
- Added
checkClassNameCollisionWithObjectand invoked it fromcheckCollisionsForDeclarationNamefor non-ambient class-like nodes. - Updated reference
.errors.txtbaselines to include TS2725 where Go output now matches the TypeScript reference. - Removed now-unnecessary
.errors.txt.difffiles for the affected baseline tests.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/checker/checker.go | Adds TS2725 reporting for class Object under non-ES2015 emit module formats and wires it into collision checks. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=nodenext).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=nodenext).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=node20).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=node20).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=node18).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=node18).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=node16).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesGeneratedNameCollisions(module=node16).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=nodenext).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=nodenext).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file (allowJs). |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=node20).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=node20).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file (allowJs). |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=node18).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=node18).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file (allowJs). |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=node16).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsGeneratedNameCollisions(module=node16).errors.txt | Updates expected diagnostics to include TS2725 for the CJS-format file (allowJs). |
| testdata/baselines/reference/submodule/conformance/exportDefaultClassNameWithObject(target=es2015).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/exportDefaultClassNameWithObject(target=es2015).errors.txt | Adds/updates expected TS2725 output for default-exported class named Object under CommonJS. |
| testdata/baselines/reference/submodule/conformance/exportClassNameWithObjectCommonJS(target=es2015).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/conformance/exportClassNameWithObjectCommonJS(target=es2015).errors.txt | Adds/updates expected TS2725 output for exported class named Object under CommonJS. |
| testdata/baselines/reference/submodule/compiler/instanceofOperator(target=es2015).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/compiler/instanceofOperator(target=es2015).errors.txt | Updates expected diagnostics to include TS2725 in this scenario. |
| testdata/baselines/reference/submodule/compiler/checkForObjectTooStrict(target=es2015).errors.txt.diff | Removes obsolete baseline diff after outputs converged. |
| testdata/baselines/reference/submodule/compiler/checkForObjectTooStrict(target=es2015).errors.txt | Updates expected diagnostics to include TS2725 in this scenario. |
| func (c *Checker) checkClassNameCollisionWithObject(name *ast.Node) { | ||
| if name.Text() == "Object" && c.program.GetEmitModuleFormatOfFile(ast.GetSourceFileOfNode(name)) < core.ModuleKindES2015 { | ||
| c.error(name, diagnostics.Class_name_cannot_be_Object_when_targeting_ES5_and_above_with_module_0, c.moduleKind.String()) | ||
| } |
There was a problem hiding this comment.
checkClassNameCollisionWithObject currently reports TS2725 solely based on the file’s emit module format. The diagnostic text is specific to “targeting ES5 and above”, but this helper doesn’t check the compilation target (c.languageVersion). Consider gating the error on c.languageVersion >= core.ScriptTargetES5 (or the equivalent target threshold used by the emitter) so we don’t report TS2725 for ES3 targets while still emitting an ES5+ specific message.
In CommonJS (and other non-ES2015 module formats), declaring
class Object {}should produce error TS2725 because the emitted code for object spread depends on the globalObject. The Go implementation was missing thecheckClassNameCollisionWithObjectcall incheckCollisionsForDeclarationName.Changes
internal/checker/checker.go: AddedcheckClassNameCollisionWithObject— reports TS2725 when a non-ambient class is named"Object"and the file's emit module format is below ES2015. Wired it intocheckCollisionsForDeclarationNamefor non-ambient class-like nodes, matching TypeScript's behavior.Test baselines: Removed
.difffiles and updated/added.errors.txtbaselines for all affected tests whose Go output now matches the TypeScript reference:exportClassNameWithObjectCommonJS,exportDefaultClassNameWithObject,checkForObjectTooStrict,instanceofOperatornodeModulesGeneratedNameCollisionsandnodeModulesAllowJsGeneratedNameCollisions(node16/18/20/nodenext variants) — correctly emits the error only for CJS-format files, not ESM-format files in the same project💡 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.