Merge pull request #85 from Ardenexal/feat/phpstan-model-fixes

feat/phpstan-model-fixes

Author: Ardenexal

.github/workflows/pr.yml

@@ -78,7 +78,7 @@ jobs:
         strategy:
             fail-fast: false
             matrix:
-                component: [ bundle, codegen, fhir-path, validation ]
+                component: [ bundle, codegen, fhir-path, validation, models ]
 
         steps:
             -   uses: actions/checkout@v4

composer.json

@@ -114,6 +114,7 @@
         "phpstan:serialization": "php ./vendor/bin/phpstan.phar analyse src/Component/Serialization/ --configuration=phpstan.neon --memory-limit=-1 -v",
         "phpstan:fhir-path": "php ./vendor/bin/phpstan.phar analyse src/Component/FHIRPath/ --configuration=phpstan.neon --memory-limit=-1 -v",
         "phpstan:validation": "php ./vendor/bin/phpstan.phar analyse src/Component/Validation/ --configuration=phpstan.neon --memory-limit=-1 -v",
+        "phpstan:models": "php ./vendor/bin/phpstan.phar analyse src/Component/Models/src/ --configuration=phpstan.neon --memory-limit=-1 -v",
         "phpstan-ai": "php scripts/ai-phpstan-runner.php",
         "phpstan-ai:bundle": "php scripts/ai-phpstan-runner.php src/Bundle/",
         "phpstan-ai:codegen": "php scripts/ai-phpstan-runner.php src/Component/CodeGeneration/",

src/Component/CodeGeneration/src/Generator/FHIRExtensionGenerator.php

@@ -245,6 +245,7 @@ private function buildSimpleConstructor(
                     ->setType('array')
                     ->setNullable(false)
                     ->setDefaultValue([]);
+                $constructor->addComment("@param list<\\Ardenexal\\FHIRTools\\Component\\Models\\{$version}\\DataType\\Extension> \$extension");
 
                 $constructor->setBody(
                     "parent::__construct(\n" .
@@ -275,6 +276,7 @@ private function buildSimpleConstructor(
             ->setType('array')
             ->setNullable(false)
             ->setDefaultValue([]);
+        $constructor->addComment("@param list<\\Ardenexal\\FHIRTools\\Component\\Models\\{$version}\\DataType\\Extension> \$extension");
         $constructor->setBody(
             "parent::__construct(\n" .
             "    id: \$id,\n" .
@@ -349,6 +351,7 @@ private function buildMultiTypeValueConstructor(
             ->setType('array')
             ->setNullable(false)
             ->setDefaultValue([]);
+        $constructor->addComment("@param list<\\Ardenexal\\FHIRTools\\Component\\Models\\{$version}\\DataType\\Extension> \$extension");
         $constructor->setBody(
             "parent::__construct(\n" .
             "    id: \$id,\n" .
@@ -490,9 +493,17 @@ private function buildFromSubExtensionsMethod(
     ): void {
         $slices = $this->collectSlices($elements, $version, $context, $namespace, $errorCollector);
 
+        // Match the constructor's required-first parameter order so $args aligns positionally.
+        usort($slices, static function(array $a, array $b): int {
+            $aRequired = $a['isRequired'] && !$a['isArray'];
+            $bRequired = $b['isRequired'] && !$b['isArray'];
+
+            return ($bRequired ? 1 : 0) <=> ($aRequired ? 1 : 0);
+        });
+
         $method = $class->addMethod('fromSubExtensions')
             ->setStatic(true)
-            ->setReturnType('static')
+            ->setReturnType('self')
             ->setVisibility('public');
 
         $method->addParameter('subExtensions')->setType('array');
@@ -502,7 +513,7 @@ private function buildFromSubExtensionsMethod(
             ->setDefaultValue(null);
 
         $method->addComment('Reconstruct from an array of already-denormalized sub-extension objects.');
-        $method->addComment('@param array<\Ardenexal\FHIRTools\Component\Metadata\Contract\FHIRExtensionInterface> $subExtensions');
+        $method->addComment("@param array<\\Ardenexal\\FHIRTools\\Component\\Models\\{$version}\\DataType\\Extension> \$subExtensions");
         $method->addComment('@param string|null $id');
 
         // Initialise parameter variables
@@ -555,7 +566,7 @@ private function buildFromSubExtensionsMethod(
 
         $args    = array_map(static fn (array $s): string => "\${$s['paramName']}", $slices);
         $args[]  = '$id';
-        $lines[] = 'return new static(' . implode(', ', $args) . ');';
+        $lines[] = 'return new self(' . implode(', ', $args) . ');';
 
         $method->setBody(implode("\n", $lines));
     }

src/Component/CodeGeneration/src/Generator/FHIRModelGenerator.php

@@ -388,19 +388,33 @@ public function generateModelClass(array $structureDefinition, string $version,
         $class->addMethod('__construct');
         $parentParameters = [];
 
+        // Build a set of param names the PHP parent constructor actually accepts.
+        // This prevents passing CanonicalResource-inherited params to DomainResourceResource
+        // when a FHIR type (e.g. MetadataResource) lists both as ancestors via base.path.
+        $validParentParamNames = [];
+        if (isset($parentClass) && $parentClass->class instanceof ClassType) {
+            try {
+                $validParentParamNames = array_keys($parentClass->class->getMethod('__construct')->getParameters());
+            } catch (\Throwable) {
+                // Parent has no __construct — pass all params as before
+            }
+        }
+
         if (isset($structureDefinition['snapshot']) === true) {
             $elements = $this->nestElements($structureDefinition['snapshot']['element']);
 
             foreach ($elements['_properties'] as $property) {
-                $element = $property['_element'];
+                $element      = $property['_element'];
+                $derivedParam = $this->convertToMethodName($element['base']['path']);
                 if (
                     $element['path'] !== $element['base']['path']
                     && ! in_array($element['path'], $parentParameters, true)
                     && $element['max'] !== '0'
+                    && ($validParentParamNames === [] || in_array($derivedParam, $validParentParamNames, true))
                 ) {
-                    $parentParameters[] = $this->convertToMethodName($element['base']['path']);
+                    $parentParameters[] = $derivedParam;
                 }
-                $this->createForElement($class, $property['_element'], $property['_properties'], $version, $builderContext, $sdUrl);
+                $this->createForElement($class, $property['_properties'], $version, $builderContext, $sdUrl, $validParentParamNames);
             }
         }
 
@@ -409,14 +423,15 @@ public function generateModelClass(array $structureDefinition, string $version,
 
     /**
      * @param ClassType                          $classType
-     * @param array<string, mixed>               $classElement
      * @param array<string,array<string, mixed>> $propertyElements
      * @param string                             $version
      * @param BuilderContextInterface            $builderContext
+     * @param list<string>                       $validParentParamNames when non-empty, only elements whose derived parameter
+     *                                                                  name appears in this list are added to the parent::__construct() call
      *
      * @return ClassType
      */
-    public function createForElement(ClassType $classType, array $classElement, array $propertyElements, string $version, BuilderContextInterface $builderContext, ?string $sdUrl = null): ClassType
+    public function createForElement(ClassType $classType, array $propertyElements, string $version, BuilderContextInterface $builderContext, ?string $sdUrl = null, array $validParentParamNames = []): ClassType
     {
         $constructor      = $classType->getMethod('__construct');
         $parentParameters = [];
@@ -437,14 +452,16 @@ public function createForElement(ClassType $classType, array $classElement, arra
                 // Track ValueSet dependencies for primitive elements with bindings
                 $this->trackValueSetDependencies($element, $builderContext);
 
+                $derivedParam = $this->convertToMethodName($element['base']['path']);
                 if (
                     $element['path'] !== $element['base']['path']
                     && ! in_array($element['path'], $parentParameters, true)
                     && $element['max'] !== '0'
+                    && ($validParentParamNames === [] || in_array($derivedParam, $validParentParamNames, true))
                 ) {
-                    $parentParameters[] = $this->convertToMethodName($element['base']['path']);
+                    $parentParameters[] = $derivedParam;
                 }
-                $this->addElementAsProperty($propertyElement['_element'], $constructor, $version, $builderContext, $classNamespace);
+                $this->addElementAsProperty($propertyElement['_element'], $constructor, $version, $builderContext);
             } else {
                 $element = $propertyElement['_element'];
 
@@ -515,14 +532,16 @@ public function createForElement(ClassType $classType, array $classElement, arra
                 if (isset($element['definition'])) {
                     $childClass->addComment('@description ' . $element['definition']);
                 }
+                $derivedParam = $this->convertToMethodName($element['base']['path']);
                 if (
                     $element['path'] !== $element['base']['path']
                     && ! in_array($element['path'], $parentParameters, true)
                     && $element['max'] !== '0'
+                    && ($validParentParamNames === [] || in_array($derivedParam, $validParentParamNames, true))
                 ) {
-                    $parentParameters[] = $this->convertToMethodName($element['base']['path']);
+                    $parentParameters[] = $derivedParam;
                 }
-                $this->addElementAsProperty($element, $constructor, $version, $builderContext, $classNamespace);
+                $this->addElementAsProperty($element, $constructor, $version, $builderContext);
 
                 // Emit FHIRPathInvariant attributes on child classes from their element constraints.
                 foreach ($element['constraint'] ?? [] as $constraint) {
@@ -544,7 +563,7 @@ public function createForElement(ClassType $classType, array $classElement, arra
 
                 if (isset($propertyElement['_properties'])) {
                     // Recursively process nested elements for ValueSet dependencies
-                    $this->createForElement($childClass, $element, $propertyElement['_properties'], $version, $builderContext, $sdUrl);
+                    $this->createForElement($childClass, $propertyElement['_properties'], $version, $builderContext, $sdUrl);
                 }
             }
         }
@@ -693,10 +712,9 @@ private function trackProfileBindings(string $profileUrl, BuilderContextInterfac
      * @param Method                  $method
      * @param string                  $version
      * @param BuilderContextInterface $builderContext
-     * @param PhpNamespace            $namespace
      * @param EnumType|null           $enum
      */
-    private function addElementAsProperty(array $element, Method $method, string $version, BuilderContextInterface $builderContext, PhpNamespace $namespace, ?EnumType $enum = null): void
+    private function addElementAsProperty(array $element, Method $method, string $version, BuilderContextInterface $builderContext, ?EnumType $enum = null): void
     {
         $types = [];
         if (! isset($element['type']) && isset($element['contentReference'])) {
@@ -711,7 +729,7 @@ private function addElementAsProperty(array $element, Method $method, string $ve
             $relatedNamespace = $relatedClass->namespace;
             $types[]          = '\\' . $relatedNamespace . '\\' . $relatedClass->asClassType()->getName();
         } elseif (isset($element['type'])) {
-            $types = $this->resolveClassFromType($element, $builderContext, $version, $types, $enum, $namespace);
+            $types = $this->resolveClassFromType($element, $builderContext, $version, $types, $enum);
         }
 
         $parameterName = $this->convertToMethodName($element['path']);
@@ -1225,13 +1243,9 @@ private function getNamespaceForFhirType(string $code, string $version, BuilderC
             'ElementDefinition',
             'ProductShelfLife',
             'MarketingStatus',
-        ];
-
-        // Types that remain in Resource namespace across all versions
-        $typesAlwaysInResource = [
-            'SubstanceAmount',    // Stays in Resource even in R5
-            'ProdCharacteristic', // Stays in Resource even in R5
-            'Population',         // Stays in Resource even in R5
+            'SubstanceAmount',    // complex-type in all FHIR versions; generated into DataType
+            'ProdCharacteristic', // complex-type in all FHIR versions; generated into DataType
+            'Population',         // complex-type in all FHIR versions; generated into DataType
         ];
 
         if (in_array($code, $typesInDataTypeNamespace, true)) {
@@ -1243,11 +1257,6 @@ private function getNamespaceForFhirType(string $code, string $version, BuilderC
             }
         }
 
-        if (in_array($code, $typesAlwaysInResource, true)) {
-            // These types always stay in Resource namespace
-            return $builderContext->getElementNamespace($version)->getName();
-        }
-
         // List of known FHIR primitive types
         $primitiveTypes = [
             'boolean',
@@ -1398,18 +1407,16 @@ private function shouldEmitBindingAttribute(string $bindingStrength): bool
      * Handles ValueSet resolution with versioned URLs, fallback to string type
      * when ValueSet cannot be resolved, and proper enum/code type generation.
      *
-     * @param string                  $valueSetUrl            The ValueSet URL (may include version)
-     * @param BuilderContextInterface $builderContext         The builder context
-     * @param string                  $version                The FHIR version
-     * @param string                  $targetElementNamespace The target element namespace
+     * @param string                  $valueSetUrl    The ValueSet URL (may include version)
+     * @param BuilderContextInterface $builderContext The builder context
+     * @param string                  $version        The FHIR version
      *
      * @return string The resolved code type (class name or 'string')
      */
     private function resolveValueSetCodeType(
         string $valueSetUrl,
         BuilderContextInterface $builderContext,
         string $version,
-        string $targetElementNamespace
     ): string {
         // Code type wrappers are in DataType namespace since they extend FHIRCode
         $dataTypeNamespace = $builderContext->getDatatypeNamespace($version)->getName();
@@ -1596,17 +1603,15 @@ private function nestElements(array $elements): array
      * @param string                  $version
      * @param array<string>           $types
      * @param EnumType|null           $enum
-     * @param PhpNamespace            $namespace
      *
      * @return array<string>
      */
-    public function resolveClassFromType(array $element, BuilderContextInterface $builderContext, string $version, array $types, ?EnumType $enum, PhpNamespace $namespace): array
+    public function resolveClassFromType(array $element, BuilderContextInterface $builderContext, string $version, array $types, ?EnumType $enum): array
     {
         foreach ($element['type'] as $type) {
             $code = $type['code'];
 
-            $targetElementNamespace = $builderContext->getElementNamespace($version)->getName();
-            $targetEnumNamespace    = $builderContext->getEnumNamespace($version)->getName();
+            $targetEnumNamespace = $builderContext->getEnumNamespace($version)->getName();
             if ($code === 'http://hl7.org/fhirpath/System.String') {
                 if (isset($element['base']['path']) && $element['base']['path'] === 'integer.value') {
                     $types[] = 'int';
@@ -1696,7 +1701,7 @@ public function resolveClassFromType(array $element, BuilderContextInterface $bu
                 // Only generate enums for required binding strength
                 if ($this->shouldGenerateEnumForBinding($bindingStrength)) {
                     $valueSetUrl = $element['binding']['valueSet'];
-                    $codeType    = $this->resolveValueSetCodeType($valueSetUrl, $builderContext, $version, $targetElementNamespace);
+                    $codeType    = $this->resolveValueSetCodeType($valueSetUrl, $builderContext, $version);
                 } else {
                     // For extensible, preferred, and example bindings, use string type
                     $codeType = 'string';

src/Component/CodeGeneration/tests/Unit/Generator/FHIRExtensionGeneratorTest.php

@@ -332,7 +332,7 @@ public function testFromSubExtensionsBodyChecksSliceUrls(): void
         self::assertStringContainsString("'value'", $body, 'fromSubExtensions body should check slice URL "value"');
         self::assertStringContainsString("'period'", $body, 'fromSubExtensions body should check slice URL "period"');
         self::assertStringContainsString("'comment'", $body, 'fromSubExtensions body should check slice URL "comment"');
-        self::assertStringContainsString('return new static(', $body, 'fromSubExtensions must return new static(...)');
+        self::assertStringContainsString('return new self(', $body, 'fromSubExtensions must return new self(...)');
     }
 
     public function testFromSubExtensionsBodyUsesInstanceofForClassTypes(): void

src/Component/Metadata/src/Contract/FHIRComplexExtensionInterface.php

@@ -34,5 +34,5 @@ interface FHIRComplexExtensionInterface extends FHIRExtensionInterface
      * @param array<FHIRExtensionInterface> $subExtensions Denormalized sub-extension objects
      * @param string|null                   $id            Optional element id
      */
-    public static function fromSubExtensions(array $subExtensions, ?string $id = null): static;
+    public static function fromSubExtensions(array $subExtensions, ?string $id = null): self;
 }

src/Component/Models/src/R4/Extension/ADUseExtension.php

@@ -21,6 +21,9 @@
 #[FHIRExtensionContext(type: 'element', expression: 'Address')]
 class ADUseExtension extends Extension
 {
+    /**
+     * @param list<Extension> $extension
+     */
     public function __construct(
         /** @var CodePrimitive|null valueCode Value of extension */
         #[FhirProperty(fhirType: 'code', propertyKind: 'primitive')]

src/Component/Models/src/R4/Extension/ADXPAdditionalLocatorExtension.php

@@ -21,6 +21,9 @@
 #[FHIRExtensionContext(type: 'element', expression: 'Address.line')]
 class ADXPAdditionalLocatorExtension extends Extension
 {
+    /**
+     * @param list<Extension> $extension
+     */
     public function __construct(
         /** @var StringPrimitive|null valueString Value of extension */
         #[FhirProperty(fhirType: 'string', propertyKind: 'primitive')]

src/Component/Models/src/R4/Extension/ADXPBuildingNumberSuffixExtension.php

@@ -21,6 +21,9 @@
 #[FHIRExtensionContext(type: 'element', expression: 'Address.line')]
 class ADXPBuildingNumberSuffixExtension extends Extension
 {
+    /**
+     * @param list<Extension> $extension
+     */
     public function __construct(
         /** @var StringPrimitive|null valueString Value of extension */
         #[FhirProperty(fhirType: 'string', propertyKind: 'primitive')]

src/Component/Models/src/R4/Extension/ADXPCareOfExtension.php

@@ -21,6 +21,9 @@
 #[FHIRExtensionContext(type: 'element', expression: 'Address.line')]
 class ADXPCareOfExtension extends Extension
 {
+    /**
+     * @param list<Extension> $extension
+     */
     public function __construct(
         /** @var StringPrimitive|null valueString Value of extension */
         #[FhirProperty(fhirType: 'string', propertyKind: 'primitive')]