Skip to content

zend_ast: Wrap class names in parens during export when they are an expression #22389

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.

Sign up for GitHub

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
TimWolla merged 2 commits into from
Jul 6, 2026
Merged

zend_ast: Wrap class names in parens during export when they are an expression #22389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a1186b
zend_ast: Wrap class names in parens during export when they are an e…
TimWolla Jun 21, 2026
34502c1
Merge branch 'master' into zend-ast-export-class-name-expression
TimWolla Jul 6, 2026
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: 2 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ PHP NEWS

- Core:
. Sync Boost.Context assembly with 1.91.0. (kn1g78)
. Fixed bug GH-22387 (AST pretty-printing drops meaningful parentheses around
RHS of instanceof). (timwolla)

- DBA:
. Fixed OOB read on malformed length field in dba flatfile handler. (alhudz)
Expand Down
64 changes: 44 additions & 20 deletions Zend/zend_ast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1686,6 +1686,19 @@ static ZEND_COLD void zend_ast_export_ns_name(smart_str *str, zend_ast *ast, int
}
zend_ast_export_ex(str, ast, priority, indent);
}
static ZEND_COLD void zend_ast_export_ns_name_or_expression(smart_str *str, zend_ast *ast, int priority, int indent)
{
switch (ast->kind) {
case ZEND_AST_ZVAL:
case ZEND_AST_VAR:
zend_ast_export_ns_name(str, ast, priority, indent);
break;
default:
smart_str_appendc(str, '(');
zend_ast_export_ex(str, ast, priority, indent);
smart_str_appendc(str, ')');
}
}

static ZEND_COLD bool zend_ast_valid_var_name(const char *s, size_t len)
{
Expand Down Expand Up @@ -2523,25 +2536,12 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
zend_ast_export_var(str, ast->child[1], indent);
break;
case ZEND_AST_STATIC_PROP:
zend_ast_export_ns_name(str, ast->child[0], 0, indent);
zend_ast_export_ns_name_or_expression(str, ast->child[0], 0, indent);
smart_str_appends(str, "::$");
zend_ast_export_var(str, ast->child[1], indent);
break;
case ZEND_AST_CALL: {
zend_ast *left = ast->child[0];
switch (left->kind) {
/* ZEND_AST_ZVAL is a regular function call. */
case ZEND_AST_ZVAL:
/* ZEND_AST_VAR ($foo()) is unambiguous without parens. */
case ZEND_AST_VAR:
zend_ast_export_ns_name(str, left, 0, indent);
break;
default:
smart_str_appendc(str, '(');
zend_ast_export_ex(str, left, 0, indent);
smart_str_appendc(str, ')');
break;
}
zend_ast_export_ns_name_or_expression(str, ast->child[0], 0, indent);
smart_str_appendc(str, '(');
zend_ast_export_ex(str, ast->child[1], 0, indent);
smart_str_appendc(str, ')');
Expand All @@ -2553,7 +2553,7 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
goto simple_list;
}
case ZEND_AST_CLASS_CONST:
zend_ast_export_ns_name(str, ast->child[0], 0, indent);
zend_ast_export_ns_name_or_expression(str, ast->child[0], 0, indent);
smart_str_appends(str, "::");
zend_ast_export_name(str, ast->child[1], 0, indent);
break;
Expand All @@ -2570,7 +2570,7 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
default: ZEND_UNREACHABLE();
}
} else {
zend_ast_export_ns_name(str, ast->child[0], 0, indent);
zend_ast_export_ns_name_or_expression(str, ast->child[0], 0, indent);
}
smart_str_appends(str, "::class");
break;
Expand Down Expand Up @@ -2649,7 +2649,7 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
}
zend_ast_export_class_no_header(str, decl, indent);
} else {
zend_ast_export_ns_name(str, ast->child[0], 0, indent);
zend_ast_export_ns_name_or_expression(str, ast->child[0], 0, indent);
smart_str_appendc(str, '(');
zend_ast_export_ex(str, ast->child[1], 0, indent);
smart_str_appendc(str, ')');
Expand All @@ -2658,7 +2658,7 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
case ZEND_AST_INSTANCEOF:
zend_ast_export_ex(str, ast->child[0], 0, indent);
smart_str_appends(str, " instanceof ");
zend_ast_export_ns_name(str, ast->child[1], 0, indent);
zend_ast_export_ns_name_or_expression(str, ast->child[1], 0, indent);
break;
case ZEND_AST_YIELD:
if (priority > 70) smart_str_appendc(str, '(');
Expand Down Expand Up @@ -2851,7 +2851,12 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
smart_str_appendc(str, ')');
break;
case ZEND_AST_STATIC_CALL:
zend_ast_export_ns_name(str, ast->child[0], 0, indent);
if (zend_ast_is_parent_hook_call(ast)) {

@iluuu1994 iluuu1994 Jul 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume this is already tested elsewhere?

@TimWolla TimWolla Jul 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, Zend/tests/property_hooks/ast_printing.phpt fails without this check:

TEST 11/211 [Zend/tests/property_hooks/ast_printing.phpt]
========DIFF========
--
         }
         public $prop2 {
             get {
008-             return parent::$prop1::get();
008+             return (parent::$prop1)::get();
             }
             final set {
                 echo 'Foo';
--
========DONE========
FAIL Hook AST printing [Zend/tests/property_hooks/ast_printing.phpt]

zend_ast_export_ns_name(str, ast->child[0], 0, indent);
} else {
zend_ast_export_ns_name_or_expression(str, ast->child[0], 0, indent);
}

smart_str_appends(str, "::");
zend_ast_export_var(str, ast->child[1], indent);
smart_str_appendc(str, '(');
Expand Down Expand Up @@ -3082,3 +3087,22 @@ zend_ast * ZEND_FASTCALL zend_ast_call_get_args(zend_ast *ast)
ZEND_UNREACHABLE();
return NULL;
}

bool zend_ast_is_parent_hook_call(const zend_ast *ast)
{
ZEND_ASSERT(ast->kind == ZEND_AST_STATIC_CALL);

const zend_ast *class_ast = ast->child[0];
zend_ast *method_ast = ast->child[1];

return class_ast->kind == ZEND_AST_STATIC_PROP
&& !(class_ast->attr & ZEND_PARENTHESIZED_STATIC_PROP)
&& class_ast->child[0]->kind == ZEND_AST_ZVAL
&& Z_TYPE_P(zend_ast_get_zval(class_ast->child[0])) == IS_STRING
&& zend_get_class_fetch_type(zend_ast_get_str(class_ast->child[0])) == ZEND_FETCH_CLASS_PARENT
&& class_ast->child[1]->kind == ZEND_AST_ZVAL
&& method_ast->kind == ZEND_AST_ZVAL
&& Z_TYPE_P(zend_ast_get_zval(method_ast)) == IS_STRING
&& (zend_string_equals_literal_ci(zend_ast_get_str(method_ast), "get")
|| zend_string_equals_literal_ci(zend_ast_get_str(method_ast), "set"));
}
3 changes: 3 additions & 0 deletions Zend/zend_ast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,7 @@ zend_ast * ZEND_FASTCALL zend_ast_with_attributes(zend_ast *ast, zend_ast *attr)

zend_ast * ZEND_FASTCALL zend_ast_call_get_args(zend_ast *ast);

/* Recognize parent::$prop::get() pattern. */
bool zend_ast_is_parent_hook_call(const zend_ast *ast);

#endif
12 changes: 1 addition & 11 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5353,17 +5353,7 @@ static bool zend_compile_parent_property_hook_call(znode *result, const zend_ast
const zend_ast *class_ast = ast->child[0];
zend_ast *method_ast = ast->child[1];

/* Recognize parent::$prop::get() pattern. */
if (class_ast->kind != ZEND_AST_STATIC_PROP
|| (class_ast->attr & ZEND_PARENTHESIZED_STATIC_PROP)
|| class_ast->child[0]->kind != ZEND_AST_ZVAL
|| Z_TYPE_P(zend_ast_get_zval(class_ast->child[0])) != IS_STRING
|| zend_get_class_fetch_type(zend_ast_get_str(class_ast->child[0])) != ZEND_FETCH_CLASS_PARENT
|| class_ast->child[1]->kind != ZEND_AST_ZVAL
|| method_ast->kind != ZEND_AST_ZVAL
|| Z_TYPE_P(zend_ast_get_zval(method_ast)) != IS_STRING
|| (!zend_string_equals_literal_ci(zend_ast_get_str(method_ast), "get")
&& !zend_string_equals_literal_ci(zend_ast_get_str(method_ast), "set"))) {
if (!zend_ast_is_parent_hook_call(ast)) {
return false;
}

Expand Down
62 changes: 62 additions & 0 deletions ext/standard/tests/assert/gh22387.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
--TEST--
GH-22387: AST pretty-printing drops meaningful parentheses around RHS of instanceof

@iluuu1994 iluuu1994 Jul 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test does more than check instanceof.

@TimWolla TimWolla Jul 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, this is borrowing the title from the associated issue (and then testing some related logic).

--FILE--
<?php

class Foo {
public static $p = true;
public const C = true;

public static function m() {
return true;
}
}

$foo = new Foo();
const bar = 'Foo';
const baz = new stdClass();

try {
assert(!$foo instanceof (bar));
} catch (AssertionError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
assert(!new (bar)());
} catch (AssertionError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
assert(!(bar)::m());
} catch (AssertionError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
assert(!(bar)::$p);
} catch (AssertionError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
assert(!(bar)::C);
} catch (AssertionError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
assert((baz)::class !== 'stdClass');
} catch (AssertionError $e) {
echo $e->getMessage(), PHP_EOL;
}

?>
--EXPECT--
assert(!$foo instanceof (bar))
assert(!new (bar)())
assert(!(bar)::m())
assert(!(bar)::$p)
assert(!(bar)::C)
assert((baz)::class !== 'stdClass')
Loading