Skip to content

Commit 29dffc3

Browse files
uhyouhyo
authored
Forbid unused property renaming in destructuring binding in function types (#41044)
* Forbid renaming a propertyin function type parameters * add tests * Remove renaming from declaration output * accept baseline * accept baseline * renew tests (not very right now) * get correct result * update diagnostic text * accept baseline * add declaration emit test * fix declaration emit * fix formatting * revert unnecessary change * accept baseline * extend tests * Revert "revert unnecessary change" This reverts commit 17a29ff. * accept baseline * Rename and refactor potentialAlways... stuff * add non-identifier names * extend check to non-identifier original property names * update diagnostic message * add related span * accept baseline * add symbol-keyed test case * oops? * workaround for unstable test * fix suggested name * add comment about non-identifier property names * simplify isReferenced check * accept baseline * move it one step further
1 parent dbab6eb commit 29dffc3

File tree

41 files changed

+1691
-73
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1691
-73
lines changed

‎src/compiler/checker.ts‎

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,7 @@ namespace ts {
10151015
const potentialNewTargetCollisions: Node[] = [];
10161016
const potentialWeakMapSetCollisions: Node[] = [];
10171017
const potentialReflectCollisions: Node[] = [];
1018+
const potentialUnusedRenamedBindingElementsInTypes: BindingElement[] = [];
10181019
const awaitedTypeStack: number[] = [];
10191020

10201021
const diagnostics = createDiagnosticCollection();
@@ -5945,19 +5946,29 @@ namespace ts {
59455946
return parameterNode;
59465947

59475948
function cloneBindingName(node: BindingName): BindingName {
5948-
return elideInitializerAndSetEmitFlags(node) as BindingName;
5949-
function elideInitializerAndSetEmitFlags(node: Node): Node {
5949+
return elideInitializerAndPropertyRenamingAndSetEmitFlags(node) as BindingName;
5950+
function elideInitializerAndPropertyRenamingAndSetEmitFlags(node: Node): Node {
59505951
if (context.tracker.trackSymbol && isComputedPropertyName(node) && isLateBindableName(node)) {
59515952
trackComputedName(node.expression, context.enclosingDeclaration, context);
59525953
}
5953-
let visited = visitEachChild(node, elideInitializerAndSetEmitFlags, nullTransformationContext, /*nodesVisitor*/ undefined, elideInitializerAndSetEmitFlags)!;
5954+
let visited = visitEachChild(node, elideInitializerAndPropertyRenamingAndSetEmitFlags, nullTransformationContext, /*nodesVisitor*/ undefined, elideInitializerAndPropertyRenamingAndSetEmitFlags)!;
59545955
if (isBindingElement(visited)) {
5955-
visited = factory.updateBindingElement(
5956-
visited,
5957-
visited.dotDotDotToken,
5958-
visited.propertyName,
5959-
visited.name,
5960-
/*initializer*/ undefined);
5956+
if (visited.propertyName && isIdentifier(visited.propertyName) && isIdentifier(visited.name)) {
5957+
visited = factory.updateBindingElement(
5958+
visited,
5959+
visited.dotDotDotToken,
5960+
/* propertyName*/ undefined,
5961+
visited.propertyName,
5962+
/*initializer*/ undefined);
5963+
}
5964+
else {
5965+
visited = factory.updateBindingElement(
5966+
visited,
5967+
visited.dotDotDotToken,
5968+
visited.propertyName,
5969+
visited.name,
5970+
/*initializer*/ undefined);
5971+
}
59615972
}
59625973
if (!nodeIsSynthesized(visited)) {
59635974
visited = factory.cloneNode(visited);
@@ -37432,6 +37443,24 @@ namespace ts {
3743237443
});
3743337444
}
3743437445

37446+
function checkPotentialUncheckedRenamedBindingElementsInTypes() {
37447+
for (const node of potentialUnusedRenamedBindingElementsInTypes) {
37448+
if (!getSymbolOfNode(node)?.isReferenced) {
37449+
const wrappingDeclaration = walkUpBindingElementsAndPatterns(node);
37450+
Debug.assert(isParameterDeclaration(wrappingDeclaration), "Only parameter declaration should be checked here");
37451+
const diagnostic = createDiagnosticForNode(node.name, Diagnostics._0_is_an_unused_renaming_of_1_Did_you_intend_to_use_it_as_a_type_annotation, declarationNameToString(node.name), declarationNameToString(node.propertyName));
37452+
if (!wrappingDeclaration.type) {
37453+
// entire parameter does not have type annotation, suggest adding an annotation
37454+
addRelatedInfo(
37455+
diagnostic,
37456+
createFileDiagnostic(getSourceFileOfNode(wrappingDeclaration), wrappingDeclaration.end, 1, Diagnostics.We_can_only_write_a_type_for_0_by_adding_a_type_for_the_entire_parameter_here, declarationNameToString(node.propertyName))
37457+
);
37458+
}
37459+
diagnostics.add(diagnostic);
37460+
}
37461+
}
37462+
}
37463+
3743537464
function bindingNameText(name: BindingName): string {
3743637465
switch (name.kind) {
3743737466
case SyntaxKind.Identifier:
@@ -37773,6 +37802,19 @@ namespace ts {
3777337802
}
3777437803

3777537804
if (isBindingElement(node)) {
37805+
if (
37806+
node.propertyName &&
37807+
isIdentifier(node.name) &&
37808+
isParameterDeclaration(node) &&
37809+
nodeIsMissing((getContainingFunction(node) as FunctionLikeDeclaration).body)) {
37810+
// type F = ({a: string}) => void;
37811+
// ^^^^^^
37812+
// variable renaming in function type notation is confusing,
37813+
// so we forbid it even if noUnusedLocals is not enabled
37814+
potentialUnusedRenamedBindingElementsInTypes.push(node);
37815+
return;
37816+
}
37817+
3777637818
if (isObjectBindingPattern(node.parent) && node.dotDotDotToken && languageVersion < ScriptTarget.ES2018) {
3777737819
checkExternalEmitHelpers(node, ExternalEmitHelpers.Rest);
3777837820
}
@@ -41617,6 +41659,7 @@ namespace ts {
4161741659
clear(potentialNewTargetCollisions);
4161841660
clear(potentialWeakMapSetCollisions);
4161941661
clear(potentialReflectCollisions);
41662+
clear(potentialUnusedRenamedBindingElementsInTypes);
4162041663

4162141664
forEach(node.statements, checkSourceElement);
4162241665
checkSourceElement(node.endOfFileToken);
@@ -41636,6 +41679,9 @@ namespace ts {
4163641679
}
4163741680
});
4163841681
}
41682+
if (!node.isDeclarationFile) {
41683+
checkPotentialUncheckedRenamedBindingElementsInTypes();
41684+
}
4163941685
});
4164041686

4164141687
if (compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error &&

‎src/compiler/diagnosticMessages.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3487,6 +3487,14 @@
34873487
"category": "Error",
34883488
"code": 2841
34893489
},
3490+
"'{0}' is an unused renaming of '{1}'. Did you intend to use it as a type annotation?": {
3491+
"category": "Error",
3492+
"code": 2842
3493+
},
3494+
"We can only write a type for '{0}' by adding a type for the entire parameter here.": {
3495+
"category": "Error",
3496+
"code": 2843
3497+
},
34903498

34913499
"Import declaration '{0}' is using private name '{1}'.": {
34923500
"category": "Error",

‎src/compiler/transformers/declarations.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ namespace ts {
453453
return ret;
454454
}
455455

456-
function filterBindingPatternInitializers(name: BindingName) {
456+
function filterBindingPatternInitializersAndRenamings(name: BindingName) {
457457
if (name.kind === SyntaxKind.Identifier) {
458458
return name;
459459
}
@@ -471,7 +471,23 @@ namespace ts {
471471
if (elem.kind === SyntaxKind.OmittedExpression) {
472472
return elem;
473473
}
474-
return factory.updateBindingElement(elem, elem.dotDotDotToken, elem.propertyName, filterBindingPatternInitializers(elem.name), shouldPrintWithInitializer(elem) ? elem.initializer : undefined);
474+
if (elem.propertyName && isIdentifier(elem.propertyName) && isIdentifier(elem.name) && !elem.symbol.isReferenced) {
475+
// Unnecessary property renaming is forbidden in types, so remove renaming
476+
return factory.updateBindingElement(
477+
elem,
478+
elem.dotDotDotToken,
479+
/* propertyName */ undefined,
480+
elem.propertyName,
481+
shouldPrintWithInitializer(elem) ? elem.initializer : undefined
482+
);
483+
}
484+
return factory.updateBindingElement(
485+
elem,
486+
elem.dotDotDotToken,
487+
elem.propertyName,
488+
filterBindingPatternInitializersAndRenamings(elem.name),
489+
shouldPrintWithInitializer(elem) ? elem.initializer : undefined
490+
);
475491
}
476492
}
477493

@@ -485,7 +501,7 @@ namespace ts {
485501
p,
486502
maskModifiers(p, modifierMask),
487503
p.dotDotDotToken,
488-
filterBindingPatternInitializers(p.name),
504+
filterBindingPatternInitializersAndRenamings(p.name),
489505
resolver.isOptionalParameter(p) ? (p.questionToken || factory.createToken(SyntaxKind.QuestionToken)) : undefined,
490506
ensureType(p, type || p.type, /*ignorePrivate*/ true), // Ignore private param props, since this type is going straight back into a param
491507
ensureNoInitializer(p)

‎tests/baselines/reference/contextuallyTypedBindingInitializerNegative.types‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ interface Show {
55
>x : number
66
}
77
function f({ show: showRename = v => v }: Show) {}
8-
>f : ({ show: showRename }: Show) => void
8+
>f : ({ show }: Show) => void
99
>show : any
1010
>showRename : (x: number) => string
1111
>v => v : (v: number) => number
@@ -32,7 +32,7 @@ interface Nested {
3232
>nested : Show
3333
}
3434
function ff({ nested: nestedRename = { show: v => v } }: Nested) {}
35-
>ff : ({ nested: nestedRename }: Nested) => void
35+
>ff : ({ nested }: Nested) => void
3636
>nested : any
3737
>nestedRename : Show
3838
>{ show: v => v } : { show: (v: number) => number; }

‎tests/baselines/reference/declarationEmitBindingPatterns.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ function f(_a, _b, _c) {
1818

1919

2020
//// [declarationEmitBindingPatterns.d.ts]
21-
declare const k: ({ x: z }: {
21+
declare const k: ({ x }: {
2222
x?: string;
2323
}) => void;
2424
declare var a: any;

‎tests/baselines/reference/declarationEmitBindingPatterns.types‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
=== tests/cases/compiler/declarationEmitBindingPatterns.ts ===
22
const k = ({x: z = 'y'}) => { }
3-
>k : ({ x: z }: { x?: string; }) => void
4-
>({x: z = 'y'}) => { } : ({ x: z }: { x?: string; }) => void
3+
>k : ({ x }: { x?: string; }) => void
4+
>({x: z = 'y'}) => { } : ({ x }: { x?: string; }) => void
55
>x : any
66
>z : string
77
>'y' : "y"

‎tests/baselines/reference/declarationsAndAssignments.types‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ function f13() {
417417
}
418418

419419
function f14([a = 1, [b = "hello", { x, y: c = false }]]) {
420-
>f14 : ([a, [b, { x, y: c }]]: [number, [string, { x: any; y?: boolean; }]]) => void
420+
>f14 : ([a, [b, { x, y }]]: [number, [string, { x: any; y?: boolean; }]]) => void
421421
>a : number
422422
>1 : 1
423423
>b : string
@@ -438,7 +438,7 @@ function f14([a = 1, [b = "hello", { x, y: c = false }]]) {
438438
}
439439
f14([2, ["abc", { x: 0, y: true }]]);
440440
>f14([2, ["abc", { x: 0, y: true }]]) : void
441-
>f14 : ([a, [b, { x, y: c }]]: [number, [string, { x: any; y?: boolean; }]]) => void
441+
>f14 : ([a, [b, { x, y }]]: [number, [string, { x: any; y?: boolean; }]]) => void
442442
>[2, ["abc", { x: 0, y: true }]] : [number, [string, { x: number; y: true; }]]
443443
>2 : 2
444444
>["abc", { x: 0, y: true }] : [string, { x: number; y: true; }]
@@ -451,7 +451,7 @@ f14([2, ["abc", { x: 0, y: true }]]);
451451

452452
f14([2, ["abc", { x: 0 }]]);
453453
>f14([2, ["abc", { x: 0 }]]) : void
454-
>f14 : ([a, [b, { x, y: c }]]: [number, [string, { x: any; y?: boolean; }]]) => void
454+
>f14 : ([a, [b, { x, y }]]: [number, [string, { x: any; y?: boolean; }]]) => void
455455
>[2, ["abc", { x: 0 }]] : [number, [string, { x: number; }]]
456456
>2 : 2
457457
>["abc", { x: 0 }] : [string, { x: number; }]
@@ -462,7 +462,7 @@ f14([2, ["abc", { x: 0 }]]);
462462

463463
f14([2, ["abc", { y: false }]]); // Error, no x
464464
>f14([2, ["abc", { y: false }]]) : void
465-
>f14 : ([a, [b, { x, y: c }]]: [number, [string, { x: any; y?: boolean; }]]) => void
465+
>f14 : ([a, [b, { x, y }]]: [number, [string, { x: any; y?: boolean; }]]) => void
466466
>[2, ["abc", { y: false }]] : [number, [string, { y: false; }]]
467467
>2 : 2
468468
>["abc", { y: false }] : [string, { y: false; }]

‎tests/baselines/reference/destructuringInFunctionType.errors.txt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
tests/cases/conformance/es6/destructuring/destructuringInFunctionType.ts(12,18): error TS2842: 'b' is an unused renaming of 'a'. Did you intend to use it as a type annotation?
2+
tests/cases/conformance/es6/destructuring/destructuringInFunctionType.ts(12,28): error TS2842: 'a' is an unused renaming of 'b'. Did you intend to use it as a type annotation?
3+
4+
5+
==== tests/cases/conformance/es6/destructuring/destructuringInFunctionType.ts (2 errors) ====
6+
interface a { a }
7+
interface b { b }
8+
interface c { c }
9+
10+
type T1 = ([a, b, c]);
11+
type F1 = ([a, b, c]) => void;
12+
13+
type T2 = ({ a });
14+
type F2 = ({ a }) => void;
15+
16+
type T3 = ([{ a: b }, { b: a }]);
17+
type F3 = ([{ a: b }, { b: a }]) => void;
18+
~
19+
!!! error TS2842: 'b' is an unused renaming of 'a'. Did you intend to use it as a type annotation?
20+
!!! related TS2843 tests/cases/conformance/es6/destructuring/destructuringInFunctionType.ts:12:32: We can only write a type for 'a' by adding a type for the entire parameter here.
21+
~
22+
!!! error TS2842: 'a' is an unused renaming of 'b'. Did you intend to use it as a type annotation?
23+
!!! related TS2843 tests/cases/conformance/es6/destructuring/destructuringInFunctionType.ts:12:32: We can only write a type for 'b' by adding a type for the entire parameter here.
24+
25+
type T4 = ([{ a: [b, c] }]);
26+
type F4 = ([{ a: [b, c] }]) => void;
27+
28+
type C1 = new ([{ a: [b, c] }]) => void;
29+
30+
var v1 = ([a, b, c]) => "hello";
31+
var v2: ([a, b, c]) => string;
32+

‎tests/baselines/reference/destructuringInFunctionType.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ declare type T3 = ([{
5252
}, {
5353
b: a;
5454
}]);
55-
declare type F3 = ([{ a: b }, { b: a }]: [{
55+
declare type F3 = ([{ a }, { b }]: [{
5656
a: any;
5757
}, {
5858
b: any;

‎tests/baselines/reference/destructuringInFunctionType.types‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type T3 = ([{ a: b }, { b: a }]);
3131
>b : a
3232

3333
type F3 = ([{ a: b }, { b: a }]) => void;
34-
>F3 : ([{ a: b }, { b: a }]: [{ a: any; }, { b: any; }]) => void
34+
>F3 : ([{ a }, { b }]: [{ a: any; }, { b: any; }]) => void
3535
>a : any
3636
>b : any
3737
>b : any

0 commit comments

Comments
 (0)