Skip to content

Lock down computed names in object literals and classes under --isolatedDeclarations #58596

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

Merged
merged 6 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1504,7 +1504,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
var syntacticNodeBuilder = createSyntacticTypeNodeBuilder(compilerOptions, {
isEntityNameVisible,
isExpandoFunctionDeclaration,
isNonNarrowedBindableName,
getAllAccessorDeclarations: getAllAccessorDeclarationsForDeclaration,
requiresAddingImplicitUndefined,
isUndefinedIdentifierExpression(node: Identifier) {
Expand Down Expand Up @@ -49111,25 +49110,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
return false;
}
function isNonNarrowedBindableName(node: ComputedPropertyName) {
if (!hasBindableName(node.parent)) {
return false;
}

const expression = node.expression;
if (!isEntityNameExpression(expression)) {
return true;
}

const type = getTypeOfExpression(expression);
const symbol = getSymbolAtLocation(expression);
if (!symbol) {
return false;
}
// Ensure not type narrowing
const declaredType = getTypeOfSymbol(symbol);
return declaredType === type;
}

function literalTypeToNode(type: FreshableType, enclosing: Node, tracker: SymbolTracker): Expression {
const enumResult = type.flags & TypeFlags.EnumLike ? nodeBuilder.symbolToExpression(type.symbol, SymbolFlags.Value, enclosing, /*flags*/ undefined, tracker)
Expand Down Expand Up @@ -49255,7 +49235,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return node && getExternalModuleFileFromDeclaration(node);
},
isLiteralConstDeclaration,
isNonNarrowedBindableName,
isLateBound: (nodeIn: Declaration): nodeIn is LateBoundDeclaration => {
const node = getParseTreeNode(nodeIn, isDeclaration);
const symbol = node && getSymbolOfDeclaration(node);
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -7014,6 +7014,10 @@
"category": "Error",
"code": 9037
},
"Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9038
},
"JSX attributes must only be assigned a non-empty 'expression'.": {
"category": "Error",
"code": 17000
Expand Down
1 change: 0 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,6 @@ export const notImplementedResolver: EmitResolver = {
isArgumentsLocalBinding: notImplemented,
getExternalModuleFileFromDeclaration: notImplemented,
isLiteralConstDeclaration: notImplemented,
isNonNarrowedBindableName: notImplemented,
getJsxFactoryEntity: notImplemented,
getJsxFragmentFactoryEntity: notImplemented,
isBindingCapturedByNode: notImplemented,
Expand Down
15 changes: 1 addition & 14 deletions src/compiler/expressionToTypeNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
isBlock,
isConstTypeReference,
isDeclarationReadonly,
isEntityNameExpression,
isGetAccessor,
isIdentifier,
isJSDocTypeAssertion,
Expand Down Expand Up @@ -55,7 +54,6 @@ import {
PropertySignature,
SetAccessorDeclaration,
SignatureDeclaration,
SymbolAccessibility,
SyntacticTypeNodeBuilderContext,
SyntacticTypeNodeBuilderResolver,
SyntaxKind,
Expand Down Expand Up @@ -351,7 +349,7 @@ export function createSyntacticTypeNodeBuilder(options: CompilerOptions, resolve
}
else if (prop.name.kind === SyntaxKind.ComputedPropertyName) {
const expression = prop.name.expression;
if (!isPrimitiveLiteralValue(expression, /*includeBigInt*/ false) && !isEntityNameExpression(expression)) {
if (!isPrimitiveLiteralValue(expression, /*includeBigInt*/ false)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also code in typeFromObjectLiteral that is not effectively unused. isNonNarrowedBindableName also probably becomes useless since we can now syntactically determine if the computed property is valid so it can probably be removed from the resolver.

context.tracker.reportInferenceFallback(prop.name);
result = false;
}
Expand All @@ -367,17 +365,6 @@ export function createSyntacticTypeNodeBuilder(options: CompilerOptions, resolve
Debug.assert(!isShorthandPropertyAssignment(prop) && !isSpreadAssignment(prop));

const name = prop.name;
if (prop.name.kind === SyntaxKind.ComputedPropertyName) {
if (!resolver.isNonNarrowedBindableName(prop.name)) {
context.tracker.reportInferenceFallback(prop.name);
}
else if (isEntityNameExpression(prop.name.expression)) {
const visibilityResult = resolver.isEntityNameVisible(prop.name.expression, context.enclosingDeclaration!, /*shouldComputeAliasToMakeVisible*/ false);
if (visibilityResult.accessibility !== SymbolAccessibility.Accessible) {
context.tracker.reportInferenceFallback(prop.name);
}
}
}
switch (prop.kind) {
case SyntaxKind.MethodDeclaration:
canInferObjectLiteral = !!typeFromObjectLiteralMethod(prop, name, context) && canInferObjectLiteral;
Expand Down
19 changes: 12 additions & 7 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ import {
isTupleTypeNode,
isTypeAliasDeclaration,
isTypeElement,
isTypeLiteralNode,
isTypeNode,
isTypeParameterDeclaration,
isTypeQueryNode,
Expand Down Expand Up @@ -995,16 +996,20 @@ export function transformDeclarations(context: TransformationContext) {
if (shouldStripInternal(input)) return;
if (isDeclaration(input)) {
if (isDeclarationAndNotVisible(input)) return;
if (hasDynamicName(input) && !resolver.isLateBound(getParseTreeNode(input) as Declaration)) {
if (hasDynamicName(input)) {
if (
isolatedDeclarations
// Classes usually elide properties with computed names that are not of a literal type
// Classes and object literals usually elide properties with computed names that are not of a literal type
// In isolated declarations TSC needs to error on these as we don't know the type in a DTE.
&& isClassDeclaration(input.parent)
&& isEntityNameExpression(input.name.expression)
// If the symbol is not accessible we get another TS error no need to add to that
&& resolver.isEntityNameVisible(input.name.expression, input.parent).accessibility === SymbolAccessibility.Accessible
&& !resolver.isNonNarrowedBindableName(input.name)
&& (isClassDeclaration(input.parent) || isObjectLiteralExpression(input.parent))
) {
context.addDiagnostic(createDiagnosticForNode(input, Diagnostics.Computed_property_names_on_class_or_object_literals_cannot_be_inferred_with_isolatedDeclarations));
}
if (
isolatedDeclarations
// Type declarations just need to double-check that the input computed name is an entity name expression
&& (isInterfaceDeclaration(input.parent) || isTypeLiteralNode(input.parent))
&& !isEntityNameExpression(input.name.expression)
) {
context.addDiagnostic(createDiagnosticForNode(input, Diagnostics.Computed_properties_must_be_number_or_string_literals_variables_or_dotted_expressions_with_isolatedDeclarations));
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/declarations/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ export function createGetIsolatedDeclarationErrors(resolver: EmitResolver) {
[SyntaxKind.VariableDeclaration]: Diagnostics.Variable_must_have_an_explicit_type_annotation_with_isolatedDeclarations,
[SyntaxKind.PropertyDeclaration]: Diagnostics.Property_must_have_an_explicit_type_annotation_with_isolatedDeclarations,
[SyntaxKind.PropertySignature]: Diagnostics.Property_must_have_an_explicit_type_annotation_with_isolatedDeclarations,
[SyntaxKind.ComputedPropertyName]: Diagnostics.Computed_properties_must_be_number_or_string_literals_variables_or_dotted_expressions_with_isolatedDeclarations,
[SyntaxKind.ComputedPropertyName]: Diagnostics.Computed_property_names_on_class_or_object_literals_cannot_be_inferred_with_isolatedDeclarations,
[SyntaxKind.SpreadAssignment]: Diagnostics.Objects_that_contain_spread_assignments_can_t_be_inferred_with_isolatedDeclarations,
[SyntaxKind.ShorthandPropertyAssignment]: Diagnostics.Objects_that_contain_shorthand_properties_can_t_be_inferred_with_isolatedDeclarations,
[SyntaxKind.ArrayLiteralExpression]: Diagnostics.Only_const_arrays_can_be_inferred_with_isolatedDeclarations,
Expand Down
2 changes: 0 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5752,7 +5752,6 @@ export enum TypeReferenceSerializationKind {

/** @internal */
export interface EmitResolver {
isNonNarrowedBindableName(node: ComputedPropertyName): boolean;
hasGlobalName(name: string): boolean;
getReferencedExportContainer(node: Identifier, prefixLocals?: boolean): SourceFile | ModuleDeclaration | EnumDeclaration | undefined;
getReferencedImportDeclaration(node: Identifier): Declaration | undefined;
Expand Down Expand Up @@ -10288,7 +10287,6 @@ export interface SyntacticTypeNodeBuilderContext {
/** @internal */
export interface SyntacticTypeNodeBuilderResolver {
isUndefinedIdentifierExpression(name: Identifier): boolean;
isNonNarrowedBindableName(name: ComputedPropertyName): boolean;
isExpandoFunctionDeclaration(name: FunctionDeclaration | VariableDeclaration): boolean;
getAllAccessorDeclarations(declaration: AccessorDeclaration): AllAccessorDeclarations;
isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node, shouldComputeAliasToMakeVisible?: boolean): SymbolVisibilityResult;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ const errorCodes = [
Diagnostics.Property_must_have_an_explicit_type_annotation_with_isolatedDeclarations.code,
Diagnostics.Expression_type_can_t_be_inferred_with_isolatedDeclarations.code,
Diagnostics.Binding_elements_can_t_be_exported_directly_with_isolatedDeclarations.code,
Diagnostics.Computed_property_names_on_class_or_object_literals_cannot_be_inferred_with_isolatedDeclarations.code,
Diagnostics.Computed_properties_must_be_number_or_string_literals_variables_or_dotted_expressions_with_isolatedDeclarations.code,
Diagnostics.Enum_member_initializers_must_be_computable_without_references_to_external_symbols_with_isolatedDeclarations.code,
Diagnostics.Extends_clause_can_t_contain_an_expression_with_isolatedDeclarations.code,
Expand Down
38 changes: 25 additions & 13 deletions tests/baselines/reference/computedPropertiesNarrowed.errors.txt
Original file line number Diff line number Diff line change
@@ -1,26 +1,32 @@
computedPropertiesNarrowed.ts(5,5): error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
computedPropertiesNarrowed.ts(18,20): error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
computedPropertiesNarrowed.ts(22,5): error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
computedPropertiesNarrowed.ts(26,5): error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
computedPropertiesNarrowed.ts(37,5): error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
computedPropertiesNarrowed.ts(47,5): error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
computedPropertiesNarrowed.ts(5,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(11,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(18,20): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(22,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(26,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(31,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(37,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(42,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
computedPropertiesNarrowed.ts(47,5): error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.


==== computedPropertiesNarrowed.ts (6 errors) ====
==== computedPropertiesNarrowed.ts (9 errors) ====
const x: 0 | 1 = Math.random()? 0: 1;
declare function assert(n: number): asserts n is 1;
assert(x);
export let o = {
[x]: 1 // error narrow type !== declared type
~~~
!!! error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:4:12: Add a type annotation to the variable o.
}


const y: 0 = 0
export let o2 = {
[y]: 1 // ok literal computed type
~~~
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:10:12: Add a type annotation to the variable o2.
}

// literals are ok
Expand All @@ -29,48 +35,54 @@ computedPropertiesNarrowed.ts(47,5): error TS9014: Computed properties must be n

export let o32 = { [1-1]: 1 } // error number
~~~~~
!!! error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:18:12: Add a type annotation to the variable o32.

let u = Symbol();
export let o4 = {
[u]: 1 // Should error, nut a unique symbol
~~~
!!! error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:21:12: Add a type annotation to the variable o4.
}

export let o5 ={
[Symbol()]: 1 // Should error
~~~~~~~~~~
!!! error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:25:12: Add a type annotation to the variable o5.
}

const uu: unique symbol = Symbol();
export let o6 = {
[uu]: 1 // Should be ok
~~~~
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:30:12: Add a type annotation to the variable o6.
}


function foo (): 1 { return 1; }
export let o7 = {
[foo()]: 1 // Should error
~~~~~~~
!!! error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:36:12: Add a type annotation to the variable o7.
};

let E = { A: 1 } as const
export const o8 = {
[E.A]: 1 // Fresh
~~~~~
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:41:14: Add a type annotation to the variable o8.
}

function ns() { return { v: 0 } as const }
export const o9 = {
[ns().v]: 1
~~~~~~~~
!!! error TS9014: Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.
!!! error TS9038: Computed property names on class or object literals cannot be inferred with --isolatedDeclarations.
!!! related TS9027 computedPropertiesNarrowed.ts:46:14: Add a type annotation to the variable o9.
}

Loading