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
This commit is contained in:
Martin Probst 2019-09-09 14:08:00 +02:00 committed by Daniel Rosenwasser
parent 2f8832cccc
commit 6bb7e5c086
13 changed files with 64 additions and 18 deletions

View File

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

View File

@ -25705,7 +25705,7 @@ namespace ts {
for (const member of node.members) {
if (member.kind === SyntaxKind.Constructor) {
for (const param of (member as ConstructorDeclaration).parameters) {
if (isParameterPropertyDeclaration(param) && !isBindingPattern(param.name)) {
if (isParameterPropertyDeclaration(param, member) && !isBindingPattern(param.name)) {
addName(instanceNames, param.name, param.name.escapedText, DeclarationMeaning.GetOrSetAccessor);
}
}
@ -27469,7 +27469,7 @@ namespace ts {
const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
const name = local.valueDeclaration && getNameOfDeclaration(local.valueDeclaration);
if (parameter && name) {
if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) {
if (!isParameterPropertyDeclaration(parameter, parameter.parent) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) {
addDiagnostic(parameter, UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local)));
}
}

View File

@ -337,7 +337,7 @@ namespace ts {
if (constructor && constructor.body) {
let parameterPropertyDeclarationCount = 0;
for (let i = indexOfFirstStatement; i < constructor.body.statements.length; i++) {
if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]))) {
if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]), constructor)) {
parameterPropertyDeclarationCount++;
}
else {

View File

@ -135,7 +135,7 @@ namespace ts {
return getReturnTypeVisibilityError;
}
else if (isParameter(node)) {
if (isParameterPropertyDeclaration(node) && hasModifier(node.parent, ModifierFlags.Private)) {
if (isParameterPropertyDeclaration(node, node.parent) && hasModifier(node.parent, ModifierFlags.Private)) {
return getVariableDeclarationTypeVisibilityError;
}
return getParameterDeclarationTypeVisibilityError;

View File

@ -895,7 +895,7 @@ namespace ts {
const members: ClassElement[] = [];
const constructor = getFirstConstructorWithBody(node);
const parametersWithPropertyAssignments = constructor &&
filter(constructor.parameters, isParameterPropertyDeclaration);
filter(constructor.parameters, p => isParameterPropertyDeclaration(p, constructor));
if (parametersWithPropertyAssignments) {
for (const parameter of parametersWithPropertyAssignments) {
@ -1907,7 +1907,7 @@ namespace ts {
function transformConstructorBody(body: Block, constructor: ConstructorDeclaration) {
const parametersWithPropertyAssignments = constructor &&
filter(constructor.parameters, isParameterPropertyDeclaration);
filter(constructor.parameters, p => isParameterPropertyDeclaration(p, constructor));
if (!some(parametersWithPropertyAssignments)) {
return visitFunctionBody(body, visitor, context);
}

View File

@ -1009,7 +1009,7 @@ namespace ts {
}
export function isDeclarationReadonly(declaration: Declaration): boolean {
return !!(getCombinedModifierFlags(declaration) & ModifierFlags.Readonly && !isParameterPropertyDeclaration(declaration));
return !!(getCombinedModifierFlags(declaration) & ModifierFlags.Readonly && !isParameterPropertyDeclaration(declaration, declaration.parent));
}
export function isVarConst(node: VariableDeclaration | VariableDeclarationList): boolean {
@ -4983,8 +4983,8 @@ namespace ts {
}
export type ParameterPropertyDeclaration = ParameterDeclaration & { parent: ConstructorDeclaration, name: Identifier };
export function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration {
return hasModifier(node, ModifierFlags.ParameterPropertyModifier) && node.parent.kind === SyntaxKind.Constructor;
export function isParameterPropertyDeclaration(node: Node, parent: Node): node is ParameterPropertyDeclaration {
return hasModifier(node, ModifierFlags.ParameterPropertyModifier) && parent.kind === SyntaxKind.Constructor;
}
export function isEmptyBindingPattern(node: BindingName): node is BindingPattern {

View File

@ -1145,7 +1145,7 @@ namespace ts.FindAllReferences.Core {
}
export function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined {
const symbol = isParameterPropertyDeclaration(definition.parent)
const symbol = isParameterPropertyDeclaration(definition.parent, definition.parent.parent)
? first(checker.getSymbolsOfParameterPropertyDeclaration(definition.parent, definition.text))
: checker.getSymbolAtLocation(definition);
if (!symbol) return undefined;
@ -1886,7 +1886,7 @@ namespace ts.FindAllReferences.Core {
const res = fromRoot(symbol);
if (res) return res;
if (symbol.valueDeclaration && isParameterPropertyDeclaration(symbol.valueDeclaration)) {
if (symbol.valueDeclaration && isParameterPropertyDeclaration(symbol.valueDeclaration, symbol.valueDeclaration.parent)) {
// For a parameter property, now try on the other symbol (property if this was a parameter, parameter if this was a property).
const paramProps = checker.getSymbolsOfParameterPropertyDeclaration(cast(symbol.valueDeclaration, isParameter), symbol.name);
Debug.assert(paramProps.length === 2 && !!(paramProps[0].flags & SymbolFlags.FunctionScopedVariable) && !!(paramProps[1].flags & SymbolFlags.Property)); // is [parameter, property]

View File

@ -197,7 +197,7 @@ namespace ts.NavigationBar {
// Parameter properties are children of the class, not the constructor.
for (const param of ctr.parameters) {
if (isParameterPropertyDeclaration(param)) {
if (isParameterPropertyDeclaration(param, ctr)) {
addLeafNode(param);
}
}

View File

@ -92,7 +92,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
}
function isAcceptedDeclaration(node: Node): node is AcceptedDeclaration {
return isParameterPropertyDeclaration(node) || isPropertyDeclaration(node) || isPropertyAssignment(node);
return isParameterPropertyDeclaration(node, node.parent) || isPropertyDeclaration(node) || isPropertyAssignment(node);
}
function createPropertyName (name: string, originalName: AcceptedNameType) {
@ -214,7 +214,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
}
function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AcceptedDeclaration, container: ContainerDeclaration) {
isParameterPropertyDeclaration(declaration) ? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor) :
isParameterPropertyDeclaration(declaration, declaration.parent) ? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor) :
isPropertyAssignment(declaration) ? changeTracker.insertNodeAfterComma(file, declaration, accessor) :
changeTracker.insertNodeAfter(file, declaration, accessor);
}

View File

@ -300,6 +300,34 @@ namespace ts {
});
});
// https://github.com/microsoft/TypeScript/issues/33295
testBaseline("transformParameterProperty", () => {
return transpileModule("", {
transformers: {
before: [transformAddParameterProperty],
},
compilerOptions: {
target: ScriptTarget.ES5,
newLine: NewLineKind.CarriageReturnLineFeed,
}
}).outputText;
function transformAddParameterProperty(_context: TransformationContext) {
return (sourceFile: SourceFile): SourceFile => {
return visitNode(sourceFile);
};
function visitNode(sf: SourceFile) {
// produce `class Foo { constructor(@Dec private x) {} }`;
// The decorator is required to trigger ts.ts transformations.
const classDecl = createClassDeclaration([], [], "Foo", /*typeParameters*/ undefined, /*heritageClauses*/ undefined, [
createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, [
createParameter(/*decorators*/ [createDecorator(createIdentifier("Dec"))], /*modifiers*/ [createModifier(SyntaxKind.PrivateKeyword)], /*dotDotDotToken*/ undefined, "x")], createBlock([]))
]);
return updateSourceFileNode(sf, [classDecl]);
}
}
});
function baselineDeclarationTransform(text: string, opts: TranspileOptions) {
const fs = vfs.createFromFileSystem(Harness.IO, /*caseSensitive*/ true, { documents: [new documents.TextDocument("/.src/index.ts", text)] });
const host = new fakes.CompilerHost(fs, opts.compilerOptions);
@ -389,7 +417,7 @@ class Clazz {
}
`, {
transformers: {
before: [addSyntheticComment(n => isPropertyDeclaration(n) || isParameterPropertyDeclaration(n) || isClassDeclaration(n) || isConstructorDeclaration(n))],
before: [addSyntheticComment(n => isPropertyDeclaration(n) || isParameterPropertyDeclaration(n, n.parent) || isClassDeclaration(n) || isConstructorDeclaration(n))],
},
compilerOptions: {
target: ScriptTarget.ES2015,

View File

@ -3274,7 +3274,7 @@ declare namespace ts {
parent: ConstructorDeclaration;
name: Identifier;
};
function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration;
function isParameterPropertyDeclaration(node: Node, parent: Node): node is ParameterPropertyDeclaration;
function isEmptyBindingPattern(node: BindingName): node is BindingPattern;
function isEmptyBindingElement(node: BindingElement): boolean;
function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration;

View File

@ -3274,7 +3274,7 @@ declare namespace ts {
parent: ConstructorDeclaration;
name: Identifier;
};
function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration;
function isParameterPropertyDeclaration(node: Node, parent: Node): node is ParameterPropertyDeclaration;
function isEmptyBindingPattern(node: BindingName): node is BindingPattern;
function isEmptyBindingElement(node: BindingElement): boolean;
function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration;

View File

@ -0,0 +1,18 @@
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
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;
return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __param = (this && this.__param) || function (paramIndex, decorator) {
return function (target, key) { decorator(target, key, paramIndex); }
};
var Foo = /** @class */ (function () {
function Foo(x) {
this.x = x;
}
Foo = __decorate([
__param(0, Dec)
], Foo);
return Foo;
}());