Skip to content

Commit 34b17f8

Browse files
committed
Clean up native parser follow-up comments
1 parent c43113d commit 34b17f8

3 files changed

Lines changed: 17 additions & 23 deletions

File tree

packages/mysql-on-sqlite/src/sqlite/class-wp-pdo-mysql-on-sqlite.php

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,17 +1160,14 @@ public function get_insert_id() {
11601160
* @return WP_MySQL_Parser A parser initialized for the MySQL query.
11611161
*/
11621162
public function create_parser( string $query ): WP_MySQL_Parser {
1163-
$lexer = new WP_MySQL_Lexer(
1163+
$lexer = new WP_MySQL_Lexer(
11641164
$query,
11651165
80038,
11661166
$this->active_sql_modes
11671167
);
1168-
if ( $lexer instanceof WP_MySQL_Native_Lexer ) {
1169-
$tokens = $lexer->native_token_stream();
1170-
return $this->reset_or_create_parser( $tokens );
1171-
}
1172-
1173-
$tokens = $lexer->remaining_tokens();
1168+
$tokens = $lexer instanceof WP_MySQL_Native_Lexer
1169+
? $lexer->native_token_stream()
1170+
: $lexer->remaining_tokens();
11741171
return $this->reset_or_create_parser( $tokens );
11751172
}
11761173

packages/mysql-on-sqlite/tests/mysql/native/WP_MySQL_Native_Parser_Node_Identity_Tests.php

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
/**
66
* Regression tests for the per-AST identity map on native parser nodes.
77
*
8-
* The native extension constructs a fresh PHP wrapper for every accessor
9-
* call. Without interning, two reads of the same logical node would yield
10-
* distinct objects, and a mutation made through a still-live wrapper would be
11-
* invisible through the second. WP_Parser_Node exposes public mutators and
12-
* stable child identity, so the native wrapper must preserve both.
8+
* The native extension materializes PHP wrappers from Rust-owned arena nodes.
9+
* Without interning, two reads of the same logical node would yield distinct
10+
* objects, and a mutation made through a still-live wrapper would be invisible
11+
* through the second. WP_Parser_Node exposes public mutators and stable child
12+
* identity, so the native wrapper must preserve both.
1313
*
14-
* Skipped when the native extension is not loaded the pure-PHP code
14+
* Skipped when the native extension is not loaded; the pure-PHP code
1515
* path already has stable identity by construction.
1616
*/
1717
class WP_MySQL_Native_Parser_Node_Identity_Tests extends TestCase {
@@ -96,11 +96,9 @@ public function test_mutation_on_child_survives_re_read(): void {
9696
$child = $tree->get_first_child_node();
9797
$this->assertNotNull( $child );
9898

99-
// Mutate via the public WP_Parser_Node API — this is exactly the
100-
// kind of state the reviewer worried would be lost when accessors
101-
// hand back fresh wrappers. rule_name is a declared public property
102-
// that the parser itself sets, so PHP 8.2's dynamic-property
103-
// deprecation does not apply here.
99+
// Mutate via the public WP_Parser_Node API. This catches regressions
100+
// where accessors hand back fresh wrappers and lose state written
101+
// through a previously returned child.
104102
$child->rule_name = 'mutated-rule';
105103

106104
$same_child = $tree->get_first_child_node();

packages/mysql-on-sqlite/tests/mysql/native/WP_MySQL_Parser_Instanceof_Tests.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@
55
/**
66
* `WP_MySQL_Parser instanceof WP_Parser` must hold in both modes.
77
*
8-
* Pre-PR, the native-mode `WP_MySQL_Parser` was `extends WP_MySQL_Native_Parser`
9-
* (a Rust-registered class with no `WP_Parser` in its chain), so existing
10-
* downstream code doing `if ($parser instanceof WP_Parser)` silently
11-
* skipped the parser when the extension was loaded. This test pins the
12-
* contract for both modes.
8+
* The native-mode `WP_MySQL_Parser` must not expose the Rust-registered
9+
* parser directly. Existing downstream code may rely on
10+
* `if ($parser instanceof WP_Parser)`, so this test pins the contract for
11+
* both modes.
1312
*/
1413
class WP_MySQL_Parser_Instanceof_Tests extends TestCase {
1514

0 commit comments

Comments
 (0)