Skip to content
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

8.4 - Property hooks #1143

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions src/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ const Position = require("./ast/position");
* - [Namespace](#namespace)
* - [PropertyStatement](#propertystatement)
* - [Property](#property)
* - [PropertyHook](#propertyhook)
* - [Declaration](#declaration)
* - [Class](#class)
* - [Interface](#interface)
Expand Down Expand Up @@ -552,6 +553,7 @@ AST.prototype.checkNodes = function () {
require("./ast/print"),
require("./ast/program"),
require("./ast/property"),
require("./ast/propertyhook"),
require("./ast/propertylookup"),
require("./ast/propertystatement"),
require("./ast/reference"),
Expand Down
3 changes: 3 additions & 0 deletions src/ast/property.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const KIND = "property";
* @property {boolean} readonly
* @property {boolean} nullable
* @property {Identifier|Array<Identifier>|null} type
* @propert {PropertyHook[]} hooks
* @property {AttrGroup[]} attrGroups
*/
module.exports = Statement.extends(
Expand All @@ -28,6 +29,7 @@ module.exports = Statement.extends(
readonly,
nullable,
type,
hooks,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would probably make sense to put hooks last as this would be less breaking of a change for existing code. Also making the default [] would make the type signature simpler, and reduce ambiguity between an empty list and null, as an empty property list is not valid syntax.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding the hooks parameter at the end of the list would be inconsistent with how things have been done so far.

The PR introducing support for typed class constant added type "in the middle". https://github.com/glayzzle/php-parser/pull/1136/files#diff-33d9ef64a624f7abad7378a94559475184322ac87b7c864326beea672657adbcR34

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cseufert I do not think it is possible to put hooks as the last parameters: when we create a node, the parameter docs and location are passed in by some other layer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@czosel any opinion on this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yer not last last, every node has docs and location last, this is automatically populated.

However if 3rd party code is replacing/creating new AST nodes, then this will cause some undefined behavior.

attrGroups,
docs,
location,
Expand All @@ -38,6 +40,7 @@ module.exports = Statement.extends(
this.readonly = readonly;
this.nullable = nullable;
this.type = type;
this.hooks = hooks;
this.attrGroups = attrGroups;
},
);
33 changes: 33 additions & 0 deletions src/ast/propertyhook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Copyright (C) 2024 Glayzzle (BSD3 License)
* @authors https://github.com/glayzzle/php-parser/graphs/contributors
* @url http://glayzzle.com
*/
"use strict";

const Node = require("./node");
const KIND = "propertyhook";

/**
* Defines a class property hook getter & setter
*
* @constructor PropertyHook
* @memberOf module:php-parser
* @extends {Node}
* @property {string} name
* @property {Boolean} isFinal
* @property {Boolean} byref
* @property {Parameter|null} parameter
* @property {Block|Statement} body
*/
module.exports = Node.extends(
KIND,
function PropertyHook(name, isFinal, byref, parameter, body, docs, location) {
Node.apply(this, [KIND, docs, location]);
this.name = name;
this.byref = byref;
this.parameter = parameter;
this.body = body;
this.isFinal = isFinal;
},
);
2 changes: 2 additions & 0 deletions src/ast/propertystatement.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ PropertyStatement.prototype.parseFlags = function (flags) {
}

this.isStatic = flags[1] === 1;
this.isAbstract = flags[2] === 1;
this.isFinal = flags[2] === 2;
};

module.exports = PropertyStatement;
127 changes: 109 additions & 18 deletions src/parser/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ module.exports = {
// reads a variable
const variables = this.read_variable_list(flags, attrs);
attrs = [];
this.expect(";");
this.next();
result = result.concat(variables);
} else {
// raise an error
Expand All @@ -178,7 +176,7 @@ module.exports = {
* ```
*/
read_variable_list: function (flags, attrs) {
const result = this.node("propertystatement");
let property_statement = this.node("propertystatement");

const properties = this.read_list(
/*
Expand All @@ -201,28 +199,119 @@ module.exports = {
const name = this.text().substring(1); // ignore $
this.next();
propName = propName(name);
if (this.token === ";" || this.token === ",") {
return result(propName, null, readonly, nullable, type, attrs || []);
} else if (this.token === "=") {

let value = null;
let property_hooks = [];

this.expect([",", ";", "=", "{"]);

// Property has a value
if (this.token === "=") {
// https://github.com/php/php-src/blob/master/Zend/zend_language_parser.y#L815
return result(
propName,
this.next().read_expr(),
readonly,
nullable,
type,
attrs || [],
);
value = this.next().read_expr();
}

// Property is using a hook to define getter/setters
if (this.token === "{") {
property_hooks = this.read_property_hooks();
} else {
this.expect([",", ";", "="]);
return result(propName, null, nullable, type, attrs || []);
this.expect([";", ","]);
}

return result(
propName,
value,
readonly,
nullable,
type,
property_hooks,
attrs || [],
);
},
",",
);

return result(null, properties, flags);
property_statement = property_statement(null, properties, flags);

// semicolons are found only for regular properties definitions.
// Property hooks are terminated by a closing curly brace, }.
// property_statement is instanciated before this check to avoid including the semicolon in the AST end location of the property.
if (this.token === ";") {
this.next();
}
return property_statement;
},

/**
* Reads property hooks
*
* @returns {PropertyHook[]}
*/
read_property_hooks: function () {
if (this.version < 804) {
this.raiseError("Parse Error: Typed Class Constants requires PHP 8.4+");
}
this.expect("{");
this.next();

const hooks = [];

while (this.token !== "}") {
hooks.push(this.read_property_hook());
}

if (this.token === "}") {
this.next();
return hooks;
}
return [];
},

read_property_hook: function () {
const property_hooks = this.node("propertyhook");

const is_final = this.token === this.tok.T_FINAL;
if (is_final) this.next();

const is_reference = this.token === "&";
if (is_reference) this.next();

const method_name = this.text();

if (method_name !== "get" && method_name !== "set") {
this.raiseError(
"Parse Error: Property hooks must be either 'get' or 'set'",
);
}
this.next();

let parameter = null;
let body = null;
this.expect([this.tok.T_DOUBLE_ARROW, "{", "(", ";"]);

// interface or abstract definition
if (this.token === ";") {
this.next();
}

if (this.token === "(") {
this.next();
parameter = this.read_parameter(false);
this.expect(")");
this.next();
}

if (this.token === this.tok.T_DOUBLE_ARROW) {
this.next();
body = this.read_expr();
this.next();
} else if (this.token === "{") {
body = this.read_code_block();
}

return property_hooks(method_name, is_final, is_reference, parameter, body);
},

/*
* Reads constant list
* ```ebnf
Expand Down Expand Up @@ -460,9 +549,11 @@ module.exports = {
this.next();
}
attrs = [];
} else if (this.token === this.tok.T_STRING) {
result.push(this.read_variable_list(flags, attrs));
} else {
// raise an error
this.error([this.tok.T_CONST, this.tok.T_FUNCTION]);
this.error([this.tok.T_CONST, this.tok.T_FUNCTION, this.tok.T_STRING]);
this.next();
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/snapshot/__snapshots__/acid.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,8 @@ Program {
"visibility": "",
},
PropertyStatement {
"isAbstract": false,
"isFinal": false,
"isStatic": false,
"kind": "propertystatement",
"loc": Location {
Expand All @@ -852,6 +854,7 @@ Program {
"properties": [
Property {
"attrGroups": [],
"hooks": [],
"kind": "property",
"loc": Location {
"end": Position {
Expand Down
12 changes: 12 additions & 0 deletions test/snapshot/__snapshots__/attributes.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,8 @@ Program {
"attrGroups": [],
"body": [
PropertyStatement {
"isAbstract": false,
"isFinal": false,
"isStatic": false,
"kind": "propertystatement",
"properties": [
Expand All @@ -381,6 +383,7 @@ Program {
"kind": "attrgroup",
},
],
"hooks": [],
"kind": "property",
"name": Identifier {
"kind": "identifier",
Expand All @@ -399,6 +402,8 @@ Program {
"visibility": "public",
},
PropertyStatement {
"isAbstract": false,
"isFinal": false,
"isStatic": false,
"kind": "propertystatement",
"properties": [
Expand All @@ -415,6 +420,7 @@ Program {
"kind": "attrgroup",
},
],
"hooks": [],
"kind": "property",
"name": Identifier {
"kind": "identifier",
Expand All @@ -433,6 +439,8 @@ Program {
"visibility": "private",
},
PropertyStatement {
"isAbstract": false,
"isFinal": false,
"isStatic": false,
"kind": "propertystatement",
"properties": [
Expand All @@ -449,6 +457,7 @@ Program {
"kind": "attrgroup",
},
],
"hooks": [],
"kind": "property",
"name": Identifier {
"kind": "identifier",
Expand Down Expand Up @@ -2566,6 +2575,8 @@ Program {
"attrGroups": [],
"body": [
PropertyStatement {
"isAbstract": false,
"isFinal": false,
"isStatic": false,
"kind": "propertystatement",
"properties": [
Expand Down Expand Up @@ -2652,6 +2663,7 @@ Program {
"kind": "attrgroup",
},
],
"hooks": [],
"kind": "property",
"name": Identifier {
"kind": "identifier",
Expand Down
Loading
Loading