Fixed declaration emit crash related to enum entity name expressions by Andarist · Pull Request #58786

Fixed declaration emit crash related to enum entity name expressions by Andarist · Pull Request #58786 · microsoft/TypeScript

		}
		else {
		const evaluated = evaluateEntityNameExpression(node.expression);
		const literalNode = typeof evaluated.value === 'string' ? factory.createStringLiteral(evaluated.value, /isSingleQuote/ undefined) :

Choose a reason for hiding this comment

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

creating those objects is wasteful here but it feels like it keeps the code cleaner, I can change this if you request that change

		literal = computedPropertyNameType.literal;
		}
		else {
		const evaluated = evaluateEntityNameExpression(node.expression);

Choose a reason for hiding this comment

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

the result of this matches the 5.4 behavior, that's why I've reached for evaluating this but perhaps there are reasons why this is wrong

		@@ -8856,8 +8856,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
		else {
		const type = getWidenedType(getRegularTypeOfExpression(node.expression));
		const computedPropertyNameType = typeToTypeNodeHelper(type, context);

Choose a reason for hiding this comment

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

this can return ImportType in the enum's case, that's the reason why it crashed - the code here assumed that a literal type node would always be returned but that wasn't true

		const literalNode = typeof evaluated.value === "string" ? factory.createStringLiteral(evaluated.value, /isSingleQuote/ undefined) :
		typeof evaluated.value === "number" ? factory.createNumericLiteral(evaluated.value, /numericLiteralFlags/ 0) :
		undefined;
		Debug.assert(literalNode);

Choose a reason for hiding this comment

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

I don't think this assertion is legit - unless you make assumptions about the computed property name being "correct" in some way, its' expression could resolve to any type (as the issue prompting this showed). Notably, evaluateEntityNameExpression will just return undefined if the entity name doesn't resolve (or resolves to a non-literal), which'll in turn trigger this assertion. In such a case, we probably want to retain the whole computed name as-is, but mark an error?

Choose a reason for hiding this comment

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

I couldn't cause a crash using invalid computed property names but I managed to find a crashing case with symbols. Could you take another look?

Not sure how many of these repos have declaration emit, but

@typescript-bot test top400

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`test top400`	✅ Started	✅ Results

@andrewbranch Here are the results of running the top 400 repos with tsc comparing main and refs/pull/58786/merge:

Everything looks good!

Choose a reason for hiding this comment

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

No longer incorrectly asserts, has fallback behaviors when each attempt fails to produce something usable here, seems good to me~

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`cherry-pick this to release-5.5`	✅ Started	✅ Results

DanielRosenwasser pushed a commit that referenced this pull request

Jun 15, 2024

…e-5.5 (#58853)

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

microsoft locked as resolved and limited conversation to collaborators

Oct 16, 2025