fix(translator): validate abstract trait requirements, isolate alias flags

Address both review findings on the trait composition fix:

1. An abstract trait requirement was discarded without validating the
   concrete implementation. composeTraitAst now validates the
   implementation - the class's own method, a concrete method from
   another trait (in either collection order), or a class method
   matching an aliased abstract - against the abstract declaration
   before dropping it, following Zend's trait-composition rules:
   matching staticness, a kept by-reference return, no additional
   required parameters, contravariant parameter types, and a covariant
   return type. Type variance reuses the existing override-check
   machinery on the preprocessed definitions; late-bound self/static/
   parent keywords are unified through the recorded type keywords, and
   self-in-trait resolutions are remapped to the consuming class.
   Visibility is intentionally not restricted: Zend allows an
   implementation of any visibility to fulfill an abstract trait
   requirement (verified against Zend 8.4).

2. Multiple alias adaptations of the same method depended on source
   order because a same-name visibility change mutated the statement
   that later adaptations cloned. Every adaptation now derives its
   flags from the immutable original flags, and the original statement
   is only mutated after all adaptations are processed, so
   `value as protected; value as alias;` leaves `alias` public in both
   adaptation orders (matching Zend, where each adaptation derives from
   the original and the last same-name adaptation wins).

Both behaviors were pinned against Zend PHP 8.4 before implementing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

phpunit/code/trait_abstract_alias_incompatible.php

@@ -0,0 +1,24 @@
+<?php
+trait RequiresValue
+{
+    abstract public function value(int $value): string;
+}
+
+// Aliasing an abstract method creates the requirement under the new name;
+// the class method defined under that name must satisfy it.
+class AliasImplementation
+{
+    use RequiresValue { value as renamed; }
+
+    public function renamed(string $value): string
+    {
+        return $value;
+    }
+
+    public function value(int $value): string
+    {
+        return "$value";
+    }
+}
+
+function main() {}

phpunit/code/trait_abstract_class_incompatible.php

@@ -0,0 +1,17 @@
+<?php
+trait RequiresValue
+{
+    abstract public function value(int $value): string;
+}
+
+class InvalidImplementation
+{
+    use RequiresValue;
+
+    public function value(string $value): string
+    {
+        return $value;
+    }
+}
+
+function main() {}

phpunit/code/trait_abstract_extra_required_param.php

@@ -0,0 +1,17 @@
+<?php
+trait RequiresValue
+{
+    abstract public function value(int $value): string;
+}
+
+class GreedyImplementation
+{
+    use RequiresValue;
+
+    public function value(int $value, int $extra): string
+    {
+        return "$value:$extra";
+    }
+}
+
+function main() {}

phpunit/code/trait_abstract_return_widened.php

@@ -0,0 +1,17 @@
+<?php
+trait RequiresValue
+{
+    abstract public function value(): string;
+}
+
+class WideningImplementation
+{
+    use RequiresValue;
+
+    public function value(): string|int
+    {
+        return 'value';
+    }
+}
+
+function main() {}

phpunit/code/trait_abstract_static_mismatch.php

@@ -0,0 +1,17 @@
+<?php
+trait RequiresValue
+{
+    abstract public function value(): string;
+}
+
+class StaticImplementation
+{
+    use RequiresValue;
+
+    public static function value(): string
+    {
+        return 'value';
+    }
+}
+
+function main() {}

phpunit/code/trait_abstract_trait_concrete_first_incompatible.php

@@ -0,0 +1,22 @@
+<?php
+trait NeedsName
+{
+    abstract public function name(int $id): string;
+}
+
+trait HasName
+{
+    public function name(string $id): string
+    {
+        return $id;
+    }
+}
+
+// The concrete method is collected first; the later abstract requirement is
+// dropped in its favor but must still be satisfied by it.
+class ConcreteFirst
+{
+    use HasName, NeedsName;
+}
+
+function main() {}

phpunit/code/trait_abstract_trait_concrete_incompatible.php

@@ -0,0 +1,22 @@
+<?php
+trait NeedsName
+{
+    abstract public function name(int $id): string;
+}
+
+trait HasName
+{
+    public function name(string $id): string
+    {
+        return $id;
+    }
+}
+
+// The abstract requirement is collected first; the later concrete method
+// replaces it and must be validated against it.
+class AbstractFirst
+{
+    use NeedsName, HasName;
+}
+
+function main() {}

phpunit/code/trait_abstract_variance_ok.php

@@ -0,0 +1,51 @@
+<?php
+trait RequiresConversion
+{
+    abstract public function convert(int $value): iterable;
+
+    abstract public function label(int|string $value): string;
+}
+
+// A valid implementation does not need to be textually identical to the
+// requirement: parameters are contravariant, returns are covariant, extra
+// optional parameters are allowed, and Zend places no visibility constraint
+// on the implementation of an abstract trait requirement.
+class ValidImplementation
+{
+    use RequiresConversion;
+
+    public function convert(int|float $value, int $extra = 0): array
+    {
+        return [$value, $extra];
+    }
+
+    protected function label(int|string|float $value): string
+    {
+        return "$value";
+    }
+}
+
+trait NeedsMaker
+{
+    abstract public function make(int $value): iterable;
+}
+
+trait HasMaker
+{
+    public function make(int|string $value): array
+    {
+        return [$value];
+    }
+}
+
+class TraitFulfillsTrait
+{
+    use NeedsMaker, HasMaker;
+}
+
+class TraitFulfillsTraitReversed
+{
+    use HasMaker, NeedsMaker;
+}
+
+function main() {}

phpunit/src/TraitAbstractRequirementTest.php

@@ -0,0 +1,75 @@
+<?php
+
+/**
+ * Trait composition drops an abstract requirement once a concrete method is
+ * available for the same name. These tests cover the validation that must
+ * happen before the requirement is dropped: the implementation — whether the
+ * class's own method or a concrete method from another trait — has to satisfy
+ * the abstract declaration under PHP's method variance rules.
+ */
+class TraitAbstractRequirementTest extends BaseTest
+{
+    public function testClassMethodMustSatisfyAbstractTraitRequirement(): void
+    {
+        $this->exec(
+            'Declaration of `InvalidImplementation::value()` must be compatible with `RequiresValue::value()`',
+            'trait_abstract_class_incompatible.php'
+        );
+    }
+
+    public function testLaterConcreteTraitMethodMustSatisfyEarlierAbstract(): void
+    {
+        $this->exec(
+            'Declaration of `HasName::name()` must be compatible with `NeedsName::name()`',
+            'trait_abstract_trait_concrete_incompatible.php'
+        );
+    }
+
+    public function testEarlierConcreteTraitMethodMustSatisfyLaterAbstract(): void
+    {
+        $this->exec(
+            'Declaration of `HasName::name()` must be compatible with `NeedsName::name()`',
+            'trait_abstract_trait_concrete_first_incompatible.php'
+        );
+    }
+
+    public function testStaticnessMustMatchAbstractTraitRequirement(): void
+    {
+        $this->exec(
+            'Cannot make non static method `RequiresValue::value()` static in class `StaticImplementation`',
+            'trait_abstract_static_mismatch.php'
+        );
+    }
+
+    public function testImplementationCannotRequireMoreParameters(): void
+    {
+        $this->exec(
+            'Declaration of `GreedyImplementation::value()` must be compatible with `RequiresValue::value()`',
+            'trait_abstract_extra_required_param.php'
+        );
+    }
+
+    public function testReturnTypeCannotBeWidened(): void
+    {
+        $this->exec(
+            'Declaration of `WideningImplementation::value()` must be compatible with `RequiresValue::value()`',
+            'trait_abstract_return_widened.php'
+        );
+    }
+
+    public function testAliasedAbstractRequirementIsValidatedAgainstClassMethod(): void
+    {
+        $this->exec(
+            'Declaration of `AliasImplementation::renamed()` must be compatible with `RequiresValue::value()`',
+            'trait_abstract_alias_incompatible.php'
+        );
+    }
+
+    public function testValidVarianceIsAccepted(): void
+    {
+        // Contravariant parameters, covariant returns, extra optional
+        // parameters, and visibility changes are all valid ways to fulfill
+        // an abstract trait requirement.
+        $this->compile('trait_abstract_variance_ok.php');
+    }
+}

src/Translator.php

@@ -3065,7 +3065,8 @@ CODE;
         $traitMethods = [];
         $traitConstants = [];
         $traitProperties = [];
-        $classDef = $this->getClass($className->toString());
+        $consumingClass = $className->toString();
+        $classDef = $this->getClass($consumingClass);
 
         foreach ($stmt->stmts as $classStmt) {
             if ($classStmt instanceof Node\Stmt\ClassMethod) {
@@ -3124,22 +3125,44 @@ CODE;
                         // passes ZendVM's runtime signature-compatibility checks. The
                         // alias clones below inherit this rewrite.
                         $this->reresolveTraitMethodAstLateBoundTypes($classDef, $traitFullName, $traitStmt);
+                        // Every adaptation derives its flags from the original
+                        // method's flags: a same-name visibility change must not
+                        // leak into aliases of the same method that are processed
+                        // after it, so the original statement is only mutated once
+                        // all adaptations have been handled.
+                        $originalFlags = $traitStmt->flags;
+                        $adaptedFlags = $originalFlags;
                         foreach ($classDef->traitAliases[$fullMethodName] ?? [] as $alias) {
                             $aliasName = strtolower($alias['newName']);
                             if ($aliasName === $methodName) {
                                 if ($alias['newModifier']) {
-                                    $traitStmt->flags = $this->applyTraitAliasModifier($traitStmt->flags, $alias['newModifier']);
+                                    $adaptedFlags = $this->applyTraitAliasModifier($originalFlags, $alias['newModifier']);
                                 }
-                            } elseif (!isset($methods[$aliasName]) && !isset($traitMethods[$aliasName])) {
+                            } elseif (isset($methods[$aliasName])) {
+                                // The class defines the alias name itself. An
+                                // abstract source is still a requirement the
+                                // class method has to satisfy.
+                                if ($traitStmt->isAbstract()) {
+                                    [$requirementSource, $requirementDef] = $this->resolveTraitStmtMethodDef($traitStmt, $traitFullName);
+                                    $this->validateTraitAbstractImplementation(
+                                        $classStmt,
+                                        $traitStmt, $requirementSource, $requirementDef,
+                                        $methods[$aliasName], $consumingClass,
+                                        $classDef->methods[$aliasName] ?? $classDef->abstractMethodDefs[$aliasName] ?? null,
+                                        $consumingClass,
+                                    );
+                                }
+                            } elseif (!isset($traitMethods[$aliasName])) {
                                 $aliasStmt = clone $traitStmt;
                                 $aliasStmt->name = new Node\Identifier($alias['newName']);
                                 if ($alias['newModifier']) {
-                                    $aliasStmt->flags = $this->applyTraitAliasModifier($aliasStmt->flags, $alias['newModifier']);
+                                    $aliasStmt->flags = $this->applyTraitAliasModifier($originalFlags, $alias['newModifier']);
                                 }
                                 $aliasStmts[] = $aliasStmt;
                                 $traitMethods[$aliasName] = [$traitFullName, $aliasStmt];
                             }
                         }
+                        $traitStmt->flags = $adaptedFlags;
                         if (isset($classDef->traitIgnored[$fullMethodName])) {
                             unset($traitStmts[$k1]);
                             continue;
@@ -3147,7 +3170,18 @@ CODE;
                         if (isset($methods[$methodName])) {
                             // The class's own method always wins: suppressed
                             // trait copies must not take part in trait-vs-trait
-                            // conflict resolution.
+                            // conflict resolution. An abstract trait method is
+                            // still a requirement the class method must satisfy.
+                            if ($traitStmt->isAbstract()) {
+                                [$requirementSource, $requirementDef] = $this->resolveTraitStmtMethodDef($traitStmt, $traitFullName);
+                                $this->validateTraitAbstractImplementation(
+                                    $classStmt,
+                                    $traitStmt, $requirementSource, $requirementDef,
+                                    $methods[$methodName], $consumingClass,
+                                    $classDef->methods[$methodName] ?? $classDef->abstractMethodDefs[$methodName] ?? null,
+                                    $consumingClass,
+                                );
+                            }
                             unset($traitStmts[$k1]);
                             continue;
                         }
@@ -3168,15 +3202,33 @@ CODE;
                             }
 
                             if ($newAbstract && !$existingAbstract) {
-                                // Existing concrete wins over new abstract
+                                // Existing concrete wins over new abstract, but
+                                // it must satisfy the abstract requirement.
+                                [$requirementSource, $requirementDef] = $this->resolveTraitStmtMethodDef($traitStmt, $traitFullName);
+                                [$implementationSource, $implementationDef] = $this->resolveTraitStmtMethodDef($existingStmt, $existingTraitName);
+                                $this->validateTraitAbstractImplementation(
+                                    $classStmt,
+                                    $traitStmt, $requirementSource, $requirementDef,
+                                    $existingStmt, $implementationSource, $implementationDef,
+                                    $consumingClass,
+                                );
                                 unset($traitStmts[$k1]);
                                 continue;
                             }
 
                             if (!$newAbstract && $existingAbstract) {
                                 // The new concrete method fulfills the abstract
-                                // requirement: drop the already-collected
+                                // requirement: validate it against the
+                                // requirement, then drop the already-collected
                                 // abstract declaration and keep this one.
+                                [$requirementSource, $requirementDef] = $this->resolveTraitStmtMethodDef($existingStmt, $existingTraitName);
+                                [$implementationSource, $implementationDef] = $this->resolveTraitStmtMethodDef($traitStmt, $traitFullName);
+                                $this->validateTraitAbstractImplementation(
+                                    $classStmt,
+                                    $existingStmt, $requirementSource, $requirementDef,
+                                    $traitStmt, $implementationSource, $implementationDef,
+                                    $consumingClass,
+                                );
                                 foreach ($stmt->stmts as $k3 => $mergedStmt) {
                                     if ($mergedStmt === $existingStmt) {
                                         unset($stmt->stmts[$k3]);
@@ -3272,6 +3324,187 @@ CODE;
         return $flags | $newModifier;
     }
 
+    /**
+     * Resolve the trait a flattened method statement originated from and its
+     * preprocessed method definition. Recursive trait composition tags every
+     * copied statement with its true origin, which may be a nested trait
+     * rather than the trait currently being applied.
+     *
+     * @return array{string, ?MethodDef}
+     */
+    private function resolveTraitStmtMethodDef(Node\Stmt\ClassMethod $stmt, string $fallbackTrait): array
+    {
+        $origin = $stmt->getAttribute(self::TRAIT_ORIGIN_ATTRIBUTE);
+        if (!is_string($origin) || $origin === '') {
+            $origin = $fallbackTrait;
+        }
+        $originalName = $stmt->getAttribute(self::TRAIT_METHOD_ATTRIBUTE);
+        if (!is_string($originalName) || $originalName === '') {
+            $originalName = $stmt->name->toString();
+        }
+        $def = null;
+        if ($this->hasClass($origin)) {
+            $originDef = $this->getClass($origin);
+            $lower = strtolower($originalName);
+            $def = $originDef->methods[$lower] ?? $originDef->abstractMethodDefs[$lower] ?? null;
+        }
+        return [$origin, $def];
+    }
+
+    /**
+     * Validate that a concrete method satisfies an abstract requirement
+     * declared by a trait, following Zend's trait-composition rules: the
+     * static modifier must match, an abstract by-reference return must be
+     * kept, the implementation cannot require more parameters, parameter
+     * types are contravariant, and the return type is covariant. Visibility
+     * is deliberately not restricted — Zend allows an implementation of any
+     * visibility to fulfill an abstract trait requirement.
+     */
+    private function validateTraitAbstractImplementation(
+        Node $errorNode,
+        Node\Stmt\ClassMethod $requirement,
+        string $requirementSource,
+        ?MethodDef $requirementDef,
+        Node\Stmt\ClassMethod $implementation,
+        string $implementationSource,
+        ?MethodDef $implementationDef,
+        string $consumingClass,
+    ): void {
+        $requirementName = $requirement->name->toString();
+        $implementationName = $implementation->name->toString();
+
+        if ($requirement->isStatic() !== $implementation->isStatic()) {
+            $this->fatalError($errorNode, $requirement->isStatic()
+                ? "Cannot make static method `{$requirementSource}::{$requirementName}()` non static in class `{$consumingClass}`"
+                : "Cannot make non static method `{$requirementSource}::{$requirementName}()` static in class `{$consumingClass}`");
+        }
+
+        $incompatible = function () use ($errorNode, $implementationSource, $implementationName, $requirementSource, $requirementName): never {
+            $this->fatalError(
+                $errorNode,
+                "Declaration of `{$implementationSource}::{$implementationName}()` must be compatible " .
+                "with `{$requirementSource}::{$requirementName}()`"
+            );
+        };
+
+        // The requirement's by-reference return must be kept; the
+        // implementation may add one.
+        if ($requirement->byRef && !$implementation->byRef) {
+            $incompatible();
+        }
+
+        if ($this->countRequiredParams($implementation->params) > $this->countRequiredParams($requirement->params)) {
+            $incompatible();
+        }
+        $implParamCount = count($implementation->params);
+        $lastImplParam = $implParamCount > 0 ? $implementation->params[$implParamCount - 1] : null;
+        foreach ($requirement->params as $i => $requiredParam) {
+            // A trailing variadic accepts every remaining requirement position.
+            $implParam = $implementation->params[$i]
+                ?? ($lastImplParam?->variadic ? $lastImplParam : null);
+            if ($implParam === null
+                || $implParam->byRef !== $requiredParam->byRef
+                || ($requiredParam->variadic && !$implParam->variadic)
+            ) {
+                $incompatible();
+            }
+        }
+        foreach ($implementation->params as $i => $implParam) {
+            if ($i >= count($requirement->params) && !$implParam->default && !$implParam->variadic) {
+                $incompatible();
+            }
+        }
+
+        // Type variance is checked on the preprocessed definitions, whose
+        // names were resolved in each declaration's own lexical context.
+        $requirementFunc = $requirementDef?->functionDef;
+        $implementationFunc = $implementationDef?->functionDef;
+        if (!$requirementFunc || !$implementationFunc) {
+            return;
+        }
+
+        $implArgs = $implementationFunc->argInfoList;
+        $lastImplArg = $implArgs === [] ? null : $implArgs[count($implArgs) - 1];
+        foreach ($requirementFunc->argInfoList as $i => $requiredArg) {
+            $implArg = $implArgs[$i] ?? ($lastImplArg?->variadic ? $lastImplArg : null);
+            if ($implArg === null) {
+                continue;
+            }
+            // Late-bound keywords (`self`, `static`, `parent`) were resolved
+            // against different declaring types but denote the same type once
+            // both methods are flattened into the consuming class.
+            if ($requiredArg->typeKeyword !== '' && $requiredArg->typeKeyword === $implArg->typeKeyword) {
+                continue;
+            }
+            if (!$this->isParameterTypeOverrideCompatible($implArg, $requiredArg)) {
+                $incompatible();
+            }
+        }
+
+        if ($requirementFunc->returnTypeUndeclared) {
+            return;
+        }
+        if ($implementationFunc->returnTypeUndeclared) {
+            $incompatible();
+        }
+        if ($requirementFunc->returnTypeKeyword !== ''
+            && $requirementFunc->returnTypeKeyword === $implementationFunc->returnTypeKeyword
+        ) {
+            return;
+        }
+        $requirementTypes = $this->remapInstanceofClass(
+            $this->getReturnAcceptedTypes($requirementFunc, $consumingClass),
+            $requirementSource,
+            $consumingClass,
+        );
+        $implementationTypes = $this->remapInstanceofClass(
+            $this->getReturnAcceptedTypes($implementationFunc, $consumingClass),
+            $implementationSource,
+            $consumingClass,
+        );
+        foreach ($implementationTypes as $implementationType) {
+            if (!$this->isReturnTypeCoveredBy($implementationType, $requirementTypes)) {
+                $incompatible();
+            }
+        }
+    }
+
+    /**
+     * @param array<Node\Param> $params
+     */
+    private function countRequiredParams(array $params): int
+    {
+        $required = 0;
+        foreach (array_values($params) as $i => $param) {
+            if (!$param->default && !$param->variadic) {
+                $required = $i + 1;
+            }
+        }
+        return $required;
+    }
+
+    /**
+     * Replace `instanceof` references to a trait in a DNF type-check list with
+     * the consuming class. `self` (and `parent`) inside a trait were resolved
+     * to the trait itself during preprocessing, but once the method is
+     * flattened they denote the consuming class, and no object can ever be an
+     * instance of a trait.
+     */
+    private function remapInstanceofClass(array $types, string $from, string $to): array
+    {
+        if ($from === $to) {
+            return $types;
+        }
+        foreach ($types as &$type) {
+            if (($type['kind'] ?? null) === 'allOf') {
+                $type['types'] = $this->remapInstanceofClass($type['types'], $from, $to);
+            } elseif (($type['kind'] ?? null) === 'instanceof' && ($type['class'] ?? null) === $from) {
+                $type['class'] = $to;
+            }
+        }
+        return $types;
+    }
+
     private function cloneAstNode(Node $node): Node
     {
         $traverser = new NodeTraverser();

tests/compiler/trait/trait-composition-precedence.phpt

@@ -3,6 +3,21 @@ Trait composition: concrete fulfills abstract, alias keeps static, class method
 --FILE--
 <?php
 
+// Multiple adaptations of the same method each derive from the original
+// method: the same-name visibility change must not leak into the alias,
+// regardless of the order the adaptations are listed in.
+trait AliasSource {
+    public static function value(): string { return "value"; }
+}
+class AliasConsumer {
+    use AliasSource { value as protected; value as alias; }
+    public static function callValue(): string { return static::value(); }
+}
+class AliasConsumerReversed {
+    use AliasSource { value as alias; value as protected; }
+    public static function callValue(): string { return static::value(); }
+}
+
 // A concrete trait method fulfills an abstract requirement from another
 // trait, regardless of the order the traits are listed in.
 trait NeedsName { abstract public function name(): string; }
@@ -34,6 +49,14 @@ trait WhoA { public function who(): string { return "WhoA"; } }
 trait WhoB { public function who(): string { return "WhoB"; } }
 class Self1 { use WhoA, WhoB; public function who(): string { return "Self1"; } }
 
+// A method fulfilling an abstract requirement does not need to be textually
+// identical: contravariant parameters and covariant returns are valid.
+trait NeedsLabel { abstract public function label(int|string $v): iterable; }
+class WiderLabel {
+    use NeedsLabel;
+    public function label(int|string|float $v): array { return ["label:$v"]; }
+}
+
 function main(): void
 {
     echo (new AbstractFirst())->name(), "\n";
@@ -41,6 +64,9 @@ function main(): void
     echo Factory::build(), "\n";
     echo Stats::total(), "\n";
     echo (new Self1())->who(), "\n";
+    echo AliasConsumer::alias(), " ", AliasConsumer::callValue(), "\n";
+    echo AliasConsumerReversed::alias(), " ", AliasConsumerReversed::callValue(), "\n";
+    foreach ((new WiderLabel())->label(1) as $v) { echo $v, "\n"; }
 }
 ?>
 --EXPECT--
@@ -49,3 +75,6 @@ HasName
 made
 7
 Self1
+value value
+value value
+label:1