Skip to content

Unify the way anonymous class symbols are printed in error messages #61943

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

@Copilot Copilot AI review requested due to automatic review settings June 25, 2025 08:09
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Jun 25, 2025
@typescript-bot typescript-bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jun 25, 2025
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

1 similar comment
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Copy link

@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 unifies the way anonymous class symbols are printed in error messages to ensure consistency across the compiler error outputs.

  • Replaces calls to factory-based naming routines with symbolToString for both source and target symbols.
  • Updates baseline tests to reflect the new naming format for anonymous classes in types, symbols, and error messages.

Reviewed Changes

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

Show a summary per file
File Description
tests/cases/compiler/privateNamesUnique-6.ts Adds a test case for anonymous classes with private fields.
tests/cases/compiler/privateNameMethodClassExpression2.ts Adds tests for anonymous class expressions with private methods and fields.
tests/baselines/reference/*.types Updates expected type outputs to the new anonymous class naming.
tests/baselines/reference/*.symbols Updates expected symbol outputs to reflect unified naming.
tests/baselines/reference/*.errors.txt Updates error messages to use consistent symbol representation.
src/compiler/checker.ts Replaces existing naming functions with symbolToString for uniformity in error reporting.

@@ -34681,7 +34679,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
right,
Diagnostics.Property_0_is_not_accessible_outside_class_1_because_it_has_a_private_identifier,
diagName,
diagnosticName(typeClass.name || anon),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think anon can now be completely removed. Only checkNoTypeArguments relies on it - but I can't see how it would actually end up in that branch (and there are no tests proving otherwise). I have refrained myself from doing changes there though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this proposed patch makes it a little bit more clear why I think this anon branch there is essentially never hit:

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 7716a39688..ef89daf9b9 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -1141,7 +1141,6 @@ import * as moduleSpecifiers from "./_namespaces/ts.moduleSpecifiers.js";
 import * as performance from "./_namespaces/ts.performance.js";
 
 const ambientModuleSymbolRegex = /^".+"$/;
-const anon = "(anonymous)" as __String & string;
 
 const enum ReferenceHint {
     Unspecified,
@@ -17161,9 +17160,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         return !!(node.flags & NodeFlags.JSDoc) && (node.kind === SyntaxKind.TypeReference || node.kind === SyntaxKind.ImportType);
     }
 
-    function checkNoTypeArguments(node: NodeWithTypeArguments, symbol?: Symbol) {
+    function checkNoTypeArguments(node: NodeWithTypeArguments, symbol: Symbol): boolean 
+    function checkNoTypeArguments(node: TypeReferenceNode): boolean 
+    function checkNoTypeArguments(node: TypeReferenceNode | NodeWithTypeArguments, symbol?: Symbol) {
         if (node.typeArguments) {
-            error(node, Diagnostics.Type_0_is_not_generic, symbol ? symbolToString(symbol) : (node as TypeReferenceNode).typeName ? declarationNameToString((node as TypeReferenceNode).typeName) : anon);
+            let name
+            if (isTypeReferenceNode(node)) {
+                name = declarationNameToString(node.typeName);
+            } else {
+                Debug.assertIsDefined(symbol);
+                name = symbolToString(symbol);
+            }
+            error(node, Diagnostics.Type_0_is_not_generic, name);
             return false;
         }
         return true;

This function is always called with a symbol when the argument is NodeWithTypeArguments and it's never called with a symbol when the argument is TypeReferenceNode. And TypeReferenceNode always has a typeName property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

2 participants