Skip to content

Handle parentless nodes in isParameterPropertyDeclaration #33321

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
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
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2843,7 +2843,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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25659,7 +25659,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);
}
}
Expand Down Expand Up @@ -27420,7 +27420,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)));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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 @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,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) {
Expand Down Expand Up @@ -1904,7 +1904,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);
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -4959,8 +4959,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 {
Expand Down
4 changes: 2 additions & 2 deletions src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,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;
Expand Down Expand Up @@ -1888,7 +1888,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]
Expand Down
2 changes: 1 addition & 1 deletion src/services/navigationBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,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);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/services/refactors/generateGetAccessorAndSetAccessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -214,7 +214,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
}

function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AcceptedDeclaration, container: ContainerDeclaration) {
isParameterPropertyDeclaration(declaration)
isParameterPropertyDeclaration(declaration, declaration.parent)
? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor)
: isPropertyAssignment(declaration)
? changeTracker.insertNodeAfterComma(file, declaration, accessor)
Expand Down
30 changes: 29 additions & 1 deletion src/testRunner/unittests/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3264,7 +3264,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;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3264,7 +3264,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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}());