From 441091fbc5a37d566b5dcbc43a165fcb1a4d9453 Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Fri, 10 Jul 2026 14:38:51 +0800 Subject: [PATCH] refactor(compiler): standardize comment formatting in compiler output - Replace direct string concatenation with formatCppLineComment method - Add proper line wrapping for multi-line comments in generated code - Apply consistent comment formatting across function calls, expressions and method calls - Introduce formatCppLineComment helper to handle comment formatting logic - Update all comment generation points to use the new standardized approach - Add test cases for ThinkPHP cookie, environment and service patterns - Add test case for trait method return by reference functionality --- src/CompilerBase.php | 34 +++- tests/aot/ref/return-by-ref-trait-method.phpt | 39 +++++ ...app-service-filter-dynamic-instanceof.phpt | 66 ++++++++ .../cookie-destructure-options-save.phpt | 147 ++++++++++++++++++ .../env-arrayaccess-magic-convert.phpt | 136 ++++++++++++++++ 5 files changed, 415 insertions(+), 7 deletions(-) create mode 100644 tests/aot/ref/return-by-ref-trait-method.phpt create mode 100644 tests/aot/thinkphp/app-service-filter-dynamic-instanceof.phpt create mode 100644 tests/aot/thinkphp/cookie-destructure-options-save.phpt create mode 100644 tests/aot/thinkphp/env-arrayaccess-magic-convert.phpt diff --git a/src/CompilerBase.php b/src/CompilerBase.php index a1922694..bc8feb77 100644 --- a/src/CompilerBase.php +++ b/src/CompilerBase.php @@ -3532,7 +3532,7 @@ class CompilerBase implements PropertyAccessContext } $placeHolder = $this->identifierToStr($expr->name); $fn = $this->getFuncPtr($name); - $this->context->beforeStmtLines[] = '// Func Call: ' . $name . '()'; + $this->context->beforeStmtLines[] = $this->formatCppLineComment('Func Call: ', $name . '()'); } else { $tmpVar = $this->addTmpVar(self::TYPE_VAR); $this->context->beforeStmtLines[] = $tmpVar . ' = ' . $this->parseExpr($expr->name) . ';'; @@ -4654,7 +4654,7 @@ class CompilerBase implements PropertyAccessContext $tmpVar = $this->addTmpVar(self::TYPE_VAR); if ($rightBeforeStmts || $rightAfterStmts) { - $code = '// Expr: ' . $this->printer->prettyPrintExpr($expr) . PHP_EOL . + $code = $this->formatCppLineComment('Expr: ', $this->printer->prettyPrintExpr($expr)) . PHP_EOL . 'if (' . $condExpr . ') {' . PHP_EOL . $this->getIndent() . $tmpVar . ' = ' . $leftExpr . ';' . PHP_EOL . '} else {' . PHP_EOL; @@ -4672,7 +4672,7 @@ class CompilerBase implements PropertyAccessContext $code .= '}'; $this->context->beforeStmtLines[] = $code; } else { - $this->context->beforeStmtLines[] = '// Expr: ' . $this->printer->prettyPrintExpr($expr) . PHP_EOL . + $this->context->beforeStmtLines[] = $this->formatCppLineComment('Expr: ', $this->printer->prettyPrintExpr($expr)) . PHP_EOL . $tmpVar . ' = ' . $condExpr . ' ? ' . $leftExpr . ' : ' . $rightExpr . ';'; } $expr->setAttribute('replace', $tmpVar); @@ -4680,6 +4680,17 @@ class CompilerBase implements PropertyAccessContext return $tmpVar; } + protected function formatCppLineComment(string $label, string $text): string + { + $lines = explode("\n", str_replace(["\r\n", "\r"], "\n", $text)); + $padding = str_repeat(' ', strlen($label)); + $comments = []; + foreach ($lines as $i => $line) { + $comments[] = '// ' . ($i === 0 ? $label : $padding) . $line; + } + return implode(PHP_EOL, $comments); + } + protected function packData(string $bytes): string { $out = ''; @@ -6612,7 +6623,10 @@ class CompilerBase implements PropertyAccessContext } $this->fatalError($expr, "Cannot call method `{$methodName}()` on variable of type {$type}"); } - $this->context->beforeStmtLines[] = '// Method Call: ' . $object . '->' . $this->parseIdentifier($expr->name) . '()'; + $this->context->beforeStmtLines[] = $this->formatCppLineComment( + 'Method Call: ', + $object . '->' . $this->parseIdentifier($expr->name) . '()' + ); try { $nativeFunc = $this->findNativeMethod($expr, $object, $this->parseIdentifier($expr->name)); if ($nativeFunc) { @@ -6733,7 +6747,10 @@ class CompilerBase implements PropertyAccessContext } elseif ($this->isNameExpr($expr->class) and $class === 'static') { $methodPtr = $this->identifierToStr($expr->name, literal: true); $fn = Symbol::getCalledCe() . ', php::getMethod(' . Symbol::getCalledCe() . ', ' . $methodPtr . ')'; - $this->context->beforeStmtLines[] = '// Static Method Call: static::' . $this->parseIdentifier($expr->name) . '()'; + $this->context->beforeStmtLines[] = $this->formatCppLineComment( + 'Static Method Call: ', + 'static::' . $this->parseIdentifier($expr->name) . '()' + ); $placeHolder = $this->genArray([Symbol::getCalledClass(), $methodPtr]); } elseif ($this->isNameExpr($expr->class)) { if ($class === 'self') { @@ -6749,7 +6766,10 @@ class CompilerBase implements PropertyAccessContext _do_call: $method = $this->parseIdentifier($expr->name); $dynamicCall = false; - $this->context->beforeStmtLines[] = '// Static Method Call: ' . $class . '::' . $method . '()'; + $this->context->beforeStmtLines[] = $this->formatCppLineComment( + 'Static Method Call: ', + $class . '::' . $method . '()' + ); if ($this->isNameExpr($expr->class) and $this->isIdExpr($expr->name)) { $callScope = [$this->genCharPtr($class, true), $this->genCharPtr($method)]; @@ -8001,7 +8021,7 @@ class CompilerBase implements PropertyAccessContext ): string { $list = []; - $comment = '// Nullsafe Operator: ' . $this->printer->prettyPrint([$expr]); + $comment = $this->formatCppLineComment('Nullsafe Operator: ', $this->printer->prettyPrint([$expr])); while (1) { if ($expr instanceof Expr\NullsafePropertyFetch) { diff --git a/tests/aot/ref/return-by-ref-trait-method.phpt b/tests/aot/ref/return-by-ref-trait-method.phpt new file mode 100644 index 00000000..927a3b38 --- /dev/null +++ b/tests/aot/ref/return-by-ref-trait-method.phpt @@ -0,0 +1,39 @@ +--TEST-- +Return value by trait (method) +--FILE-- +value; + } +} + +class Test +{ + use T1; + + private $value = 1; + + public function getValue() + { + return $this->value; + } +} + +function main() +{ + $test = new Test; + var_dump($test->getValue()); + var_dump($refValue = &$test->getRefValue()); + $refValue = 2; + var_dump($test->getValue()); + var_dump($test->getRefValue()); +} +?> +--EXPECT-- +int(1) +int(1) +int(2) +int(2) diff --git a/tests/aot/thinkphp/app-service-filter-dynamic-instanceof.phpt b/tests/aot/thinkphp/app-service-filter-dynamic-instanceof.phpt new file mode 100644 index 00000000..49099c8a --- /dev/null +++ b/tests/aot/thinkphp/app-service-filter-dynamic-instanceof.phpt @@ -0,0 +1,66 @@ +--TEST-- +ThinkPHP App pattern: array_filter with dynamic instanceof service lookup +--FILE-- +services[] = $service; + } + + public function getService(object|string $service): ?object + { + $name = is_string($service) ? $service : $service::class; + return array_values(array_filter($this->services, function ($value) use ($name) { + return $value instanceof $name; + }, ARRAY_FILTER_USE_BOTH))[0] ?? null; + } + + public function boot(): array + { + $booted = []; + array_walk($this->services, function ($service) use (&$booted) { + $booted[] = $service::class; + }); + return $booted; + } +} + +function main(): void +{ + $app = new ThinkAppServiceLike(); + $app->register(new ThinkLoggerService()); + $app->register(new ThinkCacheService()); + + var_dump($app->getService(ThinkCacheService::class)::class); + var_dump($app->getService(new ThinkLoggerService())::class); + var_dump($app->getService(DateTimeImmutable::class)); + var_dump($app->boot()); +} +?> +--EXPECT-- +string(17) "ThinkCacheService" +string(18) "ThinkLoggerService" +NULL +array(2) { + [0]=> + string(18) "ThinkLoggerService" + [1]=> + string(17) "ThinkCacheService" +} diff --git a/tests/aot/thinkphp/cookie-destructure-options-save.phpt b/tests/aot/thinkphp/cookie-destructure-options-save.phpt new file mode 100644 index 00000000..80f97385 --- /dev/null +++ b/tests/aot/thinkphp/cookie-destructure-options-save.phpt @@ -0,0 +1,147 @@ +--TEST-- +ThinkPHP Cookie pattern: option normalization, destructuring and trailing call args +--FILE-- +cookie[$name] = $value; + } +} + +class ThinkCookieLike +{ + private array $config = [ + 'expire' => 0, + 'path' => '/', + 'domain' => '', + 'secure' => false, + 'httponly' => false, + 'samesite' => '', + ]; + private array $cookie = []; + public array $saved = []; + + public function __construct(private ThinkCookieRequestLike $request, array $config = []) + { + $this->config = array_merge($this->config, array_change_key_case($config)); + } + + public function set(string $name, string $value, mixed $option = null): void + { + if ($option !== null) { + if (is_numeric($option) || $option instanceof DateTimeInterface) { + $option = ['expire' => $option]; + } + $config = array_merge($this->config, array_change_key_case($option)); + } else { + $config = $this->config; + } + + if ($config['expire'] instanceof DateTimeInterface) { + $expire = $config['expire']->getTimestamp(); + } else { + $expire = !empty($config['expire']) ? 1000 + intval($config['expire']) : 0; + } + + $this->setCookie($name, $value, $expire, $config); + $this->request->setCookie($name, $value); + } + + public function forever(string $name, string $value = '', mixed $option = null): void + { + if (is_null($option) || is_numeric($option)) { + $option = []; + } + + $option['expire'] = 315360000; + $this->set($name, $value, $option); + } + + private function setCookie(string $name, string $value, int $expire, array $option = []): void + { + $this->cookie[$name] = [$value, $expire, $option]; + } + + public function save(): void + { + foreach ($this->cookie as $name => $val) { + [$value, $expire, $option] = $val; + $this->saveCookie( + (string) $name, + $value, + $expire, + $option['path'], + $option['domain'], + (bool) $option['secure'], + (bool) $option['httponly'], + $option['samesite'], + ); + } + } + + private function saveCookie(string $name, string $value, int $expire, string $path, string $domain, bool $secure, bool $httponly, string $samesite): void + { + $this->saved[$name] = compact('value', 'expire', 'path', 'domain', 'secure', 'httponly', 'samesite'); + } +} + +function main(): void +{ + $request = new ThinkCookieRequestLike(); + $cookie = new ThinkCookieLike($request, ['SameSite' => 'lax', 'Secure' => true]); + $cookie->set('token', 'abc', new DateTimeImmutable('@42')); + $cookie->forever('remember', 'yes', ['HttpOnly' => true]); + $cookie->save(); + + var_dump($request->cookie); + var_dump($cookie->saved); +} +?> +--EXPECT-- +array(2) { + ["token"]=> + string(3) "abc" + ["remember"]=> + string(3) "yes" +} +array(2) { + ["token"]=> + array(7) { + ["value"]=> + string(3) "abc" + ["expire"]=> + int(42) + ["path"]=> + string(1) "/" + ["domain"]=> + string(0) "" + ["secure"]=> + bool(true) + ["httponly"]=> + bool(false) + ["samesite"]=> + string(3) "lax" + } + ["remember"]=> + array(7) { + ["value"]=> + string(3) "yes" + ["expire"]=> + int(315361000) + ["path"]=> + string(1) "/" + ["domain"]=> + string(0) "" + ["secure"]=> + bool(true) + ["httponly"]=> + bool(true) + ["samesite"]=> + string(3) "lax" + } +} diff --git a/tests/aot/thinkphp/env-arrayaccess-magic-convert.phpt b/tests/aot/thinkphp/env-arrayaccess-magic-convert.phpt new file mode 100644 index 00000000..518a4bad --- /dev/null +++ b/tests/aot/thinkphp/env-arrayaccess-magic-convert.phpt @@ -0,0 +1,136 @@ +--TEST-- +ThinkPHP Env pattern: ArrayAccess, magic accessors and nested env flattening +--FILE-- + true, + 'false' => false, + 'off' => false, + 'on' => true, + ]; + + public function get(?string $name = null, mixed $default = null): mixed + { + if ($name === null) { + return $this->data; + } + + $name = strtoupper(str_replace('.', '_', $name)); + if (isset($this->data[$name])) { + $result = $this->data[$name]; + if (is_string($result) && isset($this->convert[$result])) { + return $this->convert[$result]; + } + return $result; + } + + return $default; + } + + public function set(mixed $env, mixed $value = null): void + { + if (is_array($env)) { + $env = array_change_key_case($env, CASE_UPPER); + foreach ($env as $key => $val) { + if (is_array($val)) { + foreach ($val as $k => $v) { + if (is_string($k)) { + $this->data[$key . '_' . strtoupper($k)] = $v; + } else { + $this->data[$key][$k] = $v; + } + } + } else { + $this->data[$key] = $val; + } + } + } else { + $name = strtoupper(str_replace('.', '_', $env)); + $this->data[$name] = $value; + } + } + + public function has(string $name): bool + { + return !is_null($this->get($name)); + } + + public function __set(string $name, mixed $value): void + { + $this->set($name, $value); + } + + public function __get(string $name): mixed + { + return $this->get($name); + } + + public function __isset(string $name): bool + { + return $this->has($name); + } + + public function offsetSet(mixed $name, mixed $value): void + { + $this->set($name, $value); + } + + public function offsetExists(mixed $name): bool + { + return $this->__isset($name); + } + + public function offsetUnset(mixed $name): void + { + throw new Exception('not support: unset'); + } + + public function offsetGet(mixed $name): mixed + { + return $this->get($name); + } +} + +function main(): void +{ + $env = new ThinkEnvLike(); + $env->set([ + 'app' => ['debug' => 'true', 'hosts' => ['a', 'b']], + 'feature' => 'off', + ]); + $env['database.host'] = 'localhost'; + $env->cache_enabled = 'on'; + + var_dump($env->get('app.debug')); + var_dump($env['feature']); + var_dump($env->database_host); + var_dump(isset($env->cache_enabled)); + var_dump($env->get()); +} +?> +--EXPECT-- +bool(true) +bool(false) +string(9) "localhost" +bool(true) +array(5) { + ["APP_DEBUG"]=> + string(4) "true" + ["APP_HOSTS"]=> + array(2) { + [0]=> + string(1) "a" + [1]=> + string(1) "b" + } + ["FEATURE"]=> + string(3) "off" + ["DATABASE_HOST"]=> + string(9) "localhost" + ["CACHE_ENABLED"]=> + string(2) "on" +}