Skip to content

Commit 6bb7e5c

Browse files
mprobstDanielRosenwasser
mprobst
authored and
DanielRosenwasser
committed
Handle parentless nodes in isParameterPropertyDeclaration
Fixes #33295. This follows a similar pattern as in #20314 by requiring an explicit `parent` parameter. Where possible, it uses the appopriate variable at the call sites. In several locations there is no context available though (e.g. inspecting `valueDeclarations`) and we access `.parent` as the code previously did. From a cursory inspection this seems correct, these callpaths originate in phases where there must be a `parent` (i.e. in checker, binder, etc). Change-Id: I28e4726777b57237bec776e4001e9e69ac591b11
1 parent 2f8832c commit 6bb7e5c

File tree

13 files changed

+64
-18
lines changed

13 files changed

+64
-18
lines changed

‎src/compiler/binder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2859,7 +2859,7 @@ namespace ts {
28592859

28602860
// If this is a property-parameter, then also declare the property symbol into the
28612861
// containing class.
2862-
if (isParameterPropertyDeclaration(node)) {
2862+
if (isParameterPropertyDeclaration(node, node.parent)) {
28632863
const classDeclaration = <ClassLikeDeclaration>node.parent.parent;
28642864
declareSymbol(classDeclaration.symbol.members!, classDeclaration.symbol, node, SymbolFlags.Property | (node.questionToken ? SymbolFlags.Optional : SymbolFlags.None), SymbolFlags.PropertyExcludes);
28652865
}

‎src/compiler/checker.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25705,7 +25705,7 @@ namespace ts {
2570525705
for (const member of node.members) {
2570625706
if (member.kind === SyntaxKind.Constructor) {
2570725707
for (const param of (member as ConstructorDeclaration).parameters) {
25708-
if (isParameterPropertyDeclaration(param) && !isBindingPattern(param.name)) {
25708+
if (isParameterPropertyDeclaration(param, member) && !isBindingPattern(param.name)) {
2570925709
addName(instanceNames, param.name, param.name.escapedText, DeclarationMeaning.GetOrSetAccessor);
2571025710
}
2571125711
}
@@ -27469,7 +27469,7 @@ namespace ts {
2746927469
const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
2747027470
const name = local.valueDeclaration && getNameOfDeclaration(local.valueDeclaration);
2747127471
if (parameter && name) {
27472-
if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) {
27472+
if (!isParameterPropertyDeclaration(parameter, parameter.parent) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) {
2747327473
addDiagnostic(parameter, UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local)));
2747427474
}
2747527475
}

‎src/compiler/transformers/classFields.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ namespace ts {
337337
if (constructor && constructor.body) {
338338
let parameterPropertyDeclarationCount = 0;
339339
for (let i = indexOfFirstStatement; i < constructor.body.statements.length; i++) {
340-
if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]))) {
340+
if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]), constructor)) {
341341
parameterPropertyDeclarationCount++;
342342
}
343343
else {

‎src/compiler/transformers/declarations/diagnostics.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ namespace ts {
135135
return getReturnTypeVisibilityError;
136136
}
137137
else if (isParameter(node)) {
138-
if (isParameterPropertyDeclaration(node) && hasModifier(node.parent, ModifierFlags.Private)) {
138+
if (isParameterPropertyDeclaration(node, node.parent) && hasModifier(node.parent, ModifierFlags.Private)) {
139139
return getVariableDeclarationTypeVisibilityError;
140140
}
141141
return getParameterDeclarationTypeVisibilityError;

‎src/compiler/transformers/ts.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ namespace ts {
895895
const members: ClassElement[] = [];
896896
const constructor = getFirstConstructorWithBody(node);
897897
const parametersWithPropertyAssignments = constructor &&
898-
filter(constructor.parameters, isParameterPropertyDeclaration);
898+
filter(constructor.parameters, p => isParameterPropertyDeclaration(p, constructor));
899899

900900
if (parametersWithPropertyAssignments) {
901901
for (const parameter of parametersWithPropertyAssignments) {
@@ -1907,7 +1907,7 @@ namespace ts {
19071907

19081908
function transformConstructorBody(body: Block, constructor: ConstructorDeclaration) {
19091909
const parametersWithPropertyAssignments = constructor &&
1910-
filter(constructor.parameters, isParameterPropertyDeclaration);
1910+
filter(constructor.parameters, p => isParameterPropertyDeclaration(p, constructor));
19111911
if (!some(parametersWithPropertyAssignments)) {
19121912
return visitFunctionBody(body, visitor, context);
19131913
}

‎src/compiler/utilities.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,7 @@ namespace ts {
10091009
}
10101010

10111011
export function isDeclarationReadonly(declaration: Declaration): boolean {
1012-
return !!(getCombinedModifierFlags(declaration) & ModifierFlags.Readonly && !isParameterPropertyDeclaration(declaration));
1012+
return !!(getCombinedModifierFlags(declaration) & ModifierFlags.Readonly && !isParameterPropertyDeclaration(declaration, declaration.parent));
10131013
}
10141014

10151015
export function isVarConst(node: VariableDeclaration | VariableDeclarationList): boolean {
@@ -4983,8 +4983,8 @@ namespace ts {
49834983
}
49844984

49854985
export type ParameterPropertyDeclaration = ParameterDeclaration & { parent: ConstructorDeclaration, name: Identifier };
4986-
export function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration {
4987-
return hasModifier(node, ModifierFlags.ParameterPropertyModifier) && node.parent.kind === SyntaxKind.Constructor;
4986+
export function isParameterPropertyDeclaration(node: Node, parent: Node): node is ParameterPropertyDeclaration {
4987+
return hasModifier(node, ModifierFlags.ParameterPropertyModifier) && parent.kind === SyntaxKind.Constructor;
49884988
}
49894989

49904990
export function isEmptyBindingPattern(node: BindingName): node is BindingPattern {

‎src/services/findAllReferences.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,7 +1145,7 @@ namespace ts.FindAllReferences.Core {
11451145
}
11461146

11471147
export function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined {
1148-
const symbol = isParameterPropertyDeclaration(definition.parent)
1148+
const symbol = isParameterPropertyDeclaration(definition.parent, definition.parent.parent)
11491149
? first(checker.getSymbolsOfParameterPropertyDeclaration(definition.parent, definition.text))
11501150
: checker.getSymbolAtLocation(definition);
11511151
if (!symbol) return undefined;
@@ -1886,7 +1886,7 @@ namespace ts.FindAllReferences.Core {
18861886
const res = fromRoot(symbol);
18871887
if (res) return res;
18881888

1889-
if (symbol.valueDeclaration && isParameterPropertyDeclaration(symbol.valueDeclaration)) {
1889+
if (symbol.valueDeclaration && isParameterPropertyDeclaration(symbol.valueDeclaration, symbol.valueDeclaration.parent)) {
18901890
// For a parameter property, now try on the other symbol (property if this was a parameter, parameter if this was a property).
18911891
const paramProps = checker.getSymbolsOfParameterPropertyDeclaration(cast(symbol.valueDeclaration, isParameter), symbol.name);
18921892
Debug.assert(paramProps.length === 2 && !!(paramProps[0].flags & SymbolFlags.FunctionScopedVariable) && !!(paramProps[1].flags & SymbolFlags.Property)); // is [parameter, property]

‎src/services/navigationBar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ namespace ts.NavigationBar {
197197

198198
// Parameter properties are children of the class, not the constructor.
199199
for (const param of ctr.parameters) {
200-
if (isParameterPropertyDeclaration(param)) {
200+
if (isParameterPropertyDeclaration(param, ctr)) {
201201
addLeafNode(param);
202202
}
203203
}

‎src/services/refactors/generateGetAccessorAndSetAccessor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
9292
}
9393

9494
function isAcceptedDeclaration(node: Node): node is AcceptedDeclaration {
95-
return isParameterPropertyDeclaration(node) || isPropertyDeclaration(node) || isPropertyAssignment(node);
95+
return isParameterPropertyDeclaration(node, node.parent) || isPropertyDeclaration(node) || isPropertyAssignment(node);
9696
}
9797

9898
function createPropertyName (name: string, originalName: AcceptedNameType) {
@@ -214,7 +214,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
214214
}
215215

216216
function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AcceptedDeclaration, container: ContainerDeclaration) {
217-
isParameterPropertyDeclaration(declaration) ? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor) :
217+
isParameterPropertyDeclaration(declaration, declaration.parent) ? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor) :
218218
isPropertyAssignment(declaration) ? changeTracker.insertNodeAfterComma(file, declaration, accessor) :
219219
changeTracker.insertNodeAfter(file, declaration, accessor);
220220
}

‎src/testRunner/unittests/transform.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,34 @@ namespace ts {
300300
});
301301
});
302302

303+
// https://github.com/microsoft/TypeScript/issues/33295
304+
testBaseline("transformParameterProperty", () => {
305+
return transpileModule("", {
306+
transformers: {
307+
before: [transformAddParameterProperty],
308+
},
309+
compilerOptions: {
310+
target: ScriptTarget.ES5,
311+
newLine: NewLineKind.CarriageReturnLineFeed,
312+
}
313+
}).outputText;
314+
315+
function transformAddParameterProperty(_context: TransformationContext) {
316+
return (sourceFile: SourceFile): SourceFile => {
317+
return visitNode(sourceFile);
318+
};
319+
function visitNode(sf: SourceFile) {
320+
// produce `class Foo { constructor(@Dec private x) {} }`;
321+
// The decorator is required to trigger ts.ts transformations.
322+
const classDecl = createClassDeclaration([], [], "Foo", /*typeParameters*/ undefined, /*heritageClauses*/ undefined, [
323+
createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, [
324+
createParameter(/*decorators*/ [createDecorator(createIdentifier("Dec"))], /*modifiers*/ [createModifier(SyntaxKind.PrivateKeyword)], /*dotDotDotToken*/ undefined, "x")], createBlock([]))
325+
]);
326+
return updateSourceFileNode(sf, [classDecl]);
327+
}
328+
}
329+
});
330+
303331
function baselineDeclarationTransform(text: string, opts: TranspileOptions) {
304332
const fs = vfs.createFromFileSystem(Harness.IO, /*caseSensitive*/ true, { documents: [new documents.TextDocument("/.src/index.ts", text)] });
305333
const host = new fakes.CompilerHost(fs, opts.compilerOptions);
@@ -389,7 +417,7 @@ class Clazz {
389417
}
390418
`, {
391419
transformers: {
392-
before: [addSyntheticComment(n => isPropertyDeclaration(n) || isParameterPropertyDeclaration(n) || isClassDeclaration(n) || isConstructorDeclaration(n))],
420+
before: [addSyntheticComment(n => isPropertyDeclaration(n) || isParameterPropertyDeclaration(n, n.parent) || isClassDeclaration(n) || isConstructorDeclaration(n))],
393421
},
394422
compilerOptions: {
395423
target: ScriptTarget.ES2015,

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3274,7 +3274,7 @@ declare namespace ts {
32743274
parent: ConstructorDeclaration;
32753275
name: Identifier;
32763276
};
3277-
function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration;
3277+
function isParameterPropertyDeclaration(node: Node, parent: Node): node is ParameterPropertyDeclaration;
32783278
function isEmptyBindingPattern(node: BindingName): node is BindingPattern;
32793279
function isEmptyBindingElement(node: BindingElement): boolean;
32803280
function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration;

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3274,7 +3274,7 @@ declare namespace ts {
32743274
parent: ConstructorDeclaration;
32753275
name: Identifier;
32763276
};
3277-
function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration;
3277+
function isParameterPropertyDeclaration(node: Node, parent: Node): node is ParameterPropertyDeclaration;
32783278
function isEmptyBindingPattern(node: BindingName): node is BindingPattern;
32793279
function isEmptyBindingElement(node: BindingElement): boolean;
32803280
function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration;

‎tests/baselines/reference/transformApi/transformsCorrectly.transformParameterProperty.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
2+
var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
3+
if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
4+
else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
5+
return c > 3 && r && Object.defineProperty(target, key, r), r;
6+
};
7+
var __param = (this && this.__param) || function (paramIndex, decorator) {
8+
return function (target, key) { decorator(target, key, paramIndex); }
9+
};
10+
var Foo = /** @class */ (function () {
11+
function Foo(x) {
12+
this.x = x;
13+
}
14+
Foo = __decorate([
15+
__param(0, Dec)
16+
], Foo);
17+
return Foo;
18+
}());

0 commit comments

Comments
 (0)