HP-2508 Working on typing for Psalm #101
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis change introduces and refines generic type annotations and return types throughout the billing and product modules, focusing on interfaces and collections. Fluent setter methods are updated to return Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TariffTypeDefinition
participant PriceTypeDefinition
participant BehaviorCollection
Client->>TariffTypeDefinition: ofProduct(ProductInterface)
TariffTypeDefinition-->>Client: static
Client->>PriceTypeDefinition: unit(UnitInterface)
PriceTypeDefinition-->>Client: static
Client->>BehaviorCollection: attach(BehaviorInterface)
BehaviorCollection-->>Client: static
Client->>PriceTypeDefinition: end()
PriceTypeDefinition-->>Client: TParentCollection (generic)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/product/price/PriceTypeDefinitionCollectionInterface.php (1)
32-32: Fix typos in existing DocBlock comments.There are typos in the existing comments:
@plsam-return Mshould be@psalm-returnwith a proper type annotation.- * @plsam-return M + * @psalm-return TariffTypeDefinitionInterfaceApply the same fix to line 41.
Also applies to: 41-41
src/product/price/PriceTypeDefinition.php (1)
128-134: Remove or implement the validation mentioned in the comment.The comment on line 132 mentions "Validate the PriceType" but no validation is performed. Either implement the validation or remove the misleading comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/product/BillingRegistryInterface.php(1 hunks)src/product/TariffTypeDefinition.php(3 hunks)src/product/TariffTypeDefinitionInterface.php(2 hunks)src/product/behavior/BehaviorCollection.php(2 hunks)src/product/behavior/BehaviorCollectionInterface.php(2 hunks)src/product/behavior/BehaviorPriceTypeDefinitionCollection.php(1 hunks)src/product/behavior/BehaviorTariffTypeCollection.php(1 hunks)src/product/behavior/HasBehaviorsInterface.php(1 hunks)src/product/behavior/TariffTypeBehaviorRegistry.php(1 hunks)src/product/invoice/RepresentationCollection.php(4 hunks)src/product/price/PriceTypeDefinition.php(10 hunks)src/product/price/PriceTypeDefinitionCollection.php(1 hunks)src/product/price/PriceTypeDefinitionCollectionInterface.php(3 hunks)src/product/price/PriceTypeDefinitionInterface.php(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/product/behavior/BehaviorCollection.php (2)
src/product/behavior/BehaviorCollectionInterface.php (1)
attach(22-22)src/product/invoice/RepresentationCollection.php (1)
attach(47-58)
src/product/behavior/TariffTypeBehaviorRegistry.php (4)
src/product/behavior/BehaviorTariffTypeCollection.php (2)
BehaviorTariffTypeCollection(11-33)__construct(16-24)src/product/behavior/BehaviorCollection.php (1)
__construct(20-22)src/product/TariffTypeDefinition.php (2)
__construct(26-30)tariffType(32-35)src/product/TariffTypeDefinitionInterface.php (1)
tariffType(15-15)
src/product/behavior/BehaviorCollectionInterface.php (7)
src/product/behavior/BehaviorCollection.php (1)
attach(29-38)src/product/TariffTypeDefinitionInterface.php (1)
end(36-36)src/product/behavior/BehaviorPriceTypeDefinitionCollection.php (1)
end(35-38)src/product/behavior/BehaviorTariffTypeCollection.php (1)
end(29-32)src/product/TariffTypeDefinition.php (1)
end(94-104)src/product/price/PriceTypeDefinitionInterface.php (1)
end(36-36)src/product/price/PriceTypeDefinition.php (1)
end(130-134)
src/product/TariffTypeDefinition.php (2)
src/product/TariffTypeDefinitionInterface.php (2)
ofProduct(25-25)setPricesSuggester(29-29)src/product/price/PriceTypeDefinition.php (1)
withBehaviors(173-178)
src/product/behavior/BehaviorPriceTypeDefinitionCollection.php (6)
src/product/behavior/BehaviorCollection.php (2)
BehaviorCollection(13-39)__construct(20-22)src/product/TariffTypeDefinition.php (3)
__construct(26-30)tariffType(32-35)end(94-104)src/product/price/PriceTypeDefinitionCollection.php (2)
__construct(22-29)end(49-52)src/product/price/PriceTypeDefinition.php (2)
__construct(59-69)end(130-134)src/product/behavior/BehaviorCollectionInterface.php (1)
end(24-24)src/product/price/PriceTypeDefinitionInterface.php (1)
end(36-36)
src/product/behavior/BehaviorTariffTypeCollection.php (5)
src/product/behavior/BehaviorCollection.php (2)
BehaviorCollection(13-39)__construct(20-22)src/product/behavior/BehaviorPriceTypeDefinitionCollection.php (1)
__construct(23-30)src/product/TariffTypeDefinition.php (2)
__construct(26-30)tariffType(32-35)src/product/behavior/TariffTypeBehaviorRegistry.php (1)
__construct(39-42)src/product/TariffTypeDefinitionInterface.php (1)
tariffType(15-15)
🪛 GitHub Check: PHP 8.3
src/product/behavior/BehaviorTariffTypeCollection.php
[failure] 11-11: InvalidTemplateParam
src/product/behavior/BehaviorTariffTypeCollection.php:11:7: InvalidTemplateParam: Cannot extend a strictly-enforced parent template param TPriceDefinition with a non-template type (see https://psalm.dev/183)
[failure] 11-11: MissingTemplateParam
src/product/behavior/BehaviorTariffTypeCollection.php:11:7: MissingTemplateParam: hiqdev\php\billing\product\behavior\BehaviorTariffTypeCollection has missing template params when extending hiqdev\php\billing\product\behavior\BehaviorCollection, expecting 1 (see https://psalm.dev/182)
🔇 Additional comments (38)
src/product/BillingRegistryInterface.php (1)
16-18: LGTM! Excellent type documentation improvement.The added PHPDoc annotation clearly specifies the return type, improving static analysis support and IDE integration.
src/product/price/PriceTypeDefinitionCollection.php (1)
10-11: Good improvement to generic variance annotations.Adding covariance to the template parameters enhances type safety by allowing subtypes to be used in return positions, which is appropriate for collection interfaces.
src/product/behavior/BehaviorCollection.php (2)
8-12: Excellent generic template annotations.The template parameter and interface implementation annotations, along with
@psalm-consistent-templates, provide proper generic typing for this abstract collection class.
29-29: Correct fluent interface return type change.Changing from
selftostaticenables proper late static binding, ensuring subclasses return their own type rather than the parent class type. This aligns with the interface definition and supports better fluent interface usage.src/product/behavior/HasBehaviorsInterface.php (1)
5-8: Good template annotations.The template parameter and consistency annotations align well with the broader typing improvements across the codebase.
src/product/behavior/BehaviorTariffTypeCollection.php (2)
16-24: Psalm annotations are well-structured but depend on fixing the parent template issue.The constructor annotations using
@psalm-param T $parentand the property annotation@var Tare correctly implemented for generic typing.
26-32: Generic return type annotation looks correct.The
@psalm-return Tannotation properly maintains the generic type consistency for the fluent interface pattern.src/product/behavior/BehaviorCollectionInterface.php (3)
13-13: Good addition of template consistency annotation.The
@psalm-consistent-templatesannotation ensures that template parameters are used consistently across all implementations of this interface.
22-22: Excellent improvement for fluent interface typing.Changing from
selftostaticreturn type enables proper late static binding, allowing subclasses to return their own type in fluent method chains.
24-24: Removing explicit return type allows for flexible generic implementations.This change enables implementing classes to use their own generic return type annotations while maintaining interface compatibility.
src/product/behavior/TariffTypeBehaviorRegistry.php (4)
26-28: Excellent use of covariant template parameter.The
@template-covariant T of TariffTypeDefinitionInterfaceis correctly designed sinceTappears only in output positions (return types and readonly properties), making covariance safe and appropriate.
31-34: Generic property annotation is correctly implemented.The
@var BehaviorTariffTypeCollection<T>annotation properly parameterizes the behavior collection with the template type.
36-42: Constructor parameter annotation maintains type consistency.The
@psalm-param T $tariffTypeDefinitionannotation ensures the generic type flows correctly through the constructor to the behavior collection.
44-50: Generic return type annotation completes the type safety.The
@return BehaviorTariffTypeCollection<T>annotation maintains the generic type relationship, ensuring callers get properly typed collection instances.src/product/price/PriceTypeDefinitionCollectionInterface.php (3)
5-5: Good addition of HasBehaviorsInterface import.The import is necessary for the new generic implementation declaration.
14-17: Well-structured generic template annotations.The covariant template parameter
TPriceTypeDefinitionwith proper constraint and the implementation ofHasBehaviorsInterface<TPriceTypeDefinition>correctly establish the generic relationship. The@psalm-consistent-templatesannotation ensures consistency across implementations.
25-28:Details
✅ Verification successful
Verify the PriceTypeDefinitionInterface generic usage.
The
@psalm-return PriceTypeDefinitionInterface<static>annotation suggests thatPriceTypeDefinitionInterfaceitself is generic. Please ensure this is correct.
🏁 Script executed:
#!/bin/bash # Check if PriceTypeDefinitionInterface has template parameters rg -A 10 -B 5 "@template" src/product/price/PriceTypeDefinitionInterface.php # Also check the interface declaration ast-grep --pattern 'interface PriceTypeDefinitionInterface { $$$ }'Length of output: 634
PriceTypeDefinitionInterface generic usage verified
PriceTypeDefinitionInterface (src/product/price/PriceTypeDefinitionInterface.php) is declared with
/** * @template TParentCollection * @psalm-consistent-templates */ interface PriceTypeDefinitionInterface { … }so the doc-block
/** @psalm-return PriceTypeDefinitionInterface<static> */ public function priceType(TypeInterface $type): PriceTypeDefinitionInterface;correctly binds the parent-collection type to
static. No changes required.src/product/TariffTypeDefinitionInterface.php (3)
11-11: Excellent addition of covariant template parameter.The
@template-covariant T of PriceTypeDefinitionCollectionInterfaceannotation properly establishes type variance for this interface, allowing safe type widening in inheritance hierarchies while maintaining type safety.
25-25: Good improvement to fluent interface typing.Changing return types from
selftostaticenables proper type inference in inheritance hierarchies and supports better fluent interface patterns. This allows subclasses to return their own type rather than the interface type.Also applies to: 29-29
32-36: Well-designed generic return type strategy.Removing explicit return type declarations while maintaining PHPDoc annotations (
@return TforwithPrices) provides flexibility for implementing classes while preserving static analysis benefits. This approach aligns well with the covariant template parameter.src/product/TariffTypeDefinition.php (2)
42-42: Consistent implementation of interface changes.The return type updates from
TariffTypeDefinitionInterfacetostaticproperly implement the interface changes and maintain fluent interface consistency. This ensures the concrete class returns the correct type for method chaining.Also applies to: 64-64
79-82: Precise generic annotation for behavior collection.The
@return BehaviorTariffTypeCollection<TariffTypeDefinition>annotation provides specific type information for static analysis while removing the explicit return type declaration maintains flexibility. This follows the same pattern established in the interface.src/product/behavior/BehaviorPriceTypeDefinitionCollection.php (4)
8-12: Well-structured generic template annotations.The template parameter
TPriceDefinitionwith proper extension and@psalm-consistent-templatesannotation establishes a solid foundation for type safety. This allows the collection to work with any price definition type while maintaining type constraints.
15-18: Appropriate Psalm type annotation for generic property.The
@psalm-var TPriceDefinitionannotation on the$parentproperty correctly establishes the relationship between the template parameter and the property type, enabling precise static analysis.
20-30: Good constructor refactoring for enhanced type control.Converting from property promotion to explicit property assignment with
@psalm-param TPriceDefinitionannotation provides better control over generic type annotations. This approach enables more precise static analysis while maintaining the same runtime behavior.
32-35: Consistent generic return type implementation.The
@return TPriceDefinitionannotation for theend()method aligns with the overall generic design pattern, allowing the method to return the specific templated type rather than a concrete type.src/product/invoice/RepresentationCollection.php (4)
12-14: Enhanced template flexibility with consistency annotations.Changing from
@template T of PriceTypeDefinitionto@template Tprovides greater flexibility for this collection class. The addition of@psalm-consistent-templatesensures template parameter consistency across the class hierarchy.
24-35: Improved constructor with explicit generic annotations.The refactoring from property promotion to explicit property declaration with
@psalm-var Tand@psalm-param Tannotations provides better control over generic type relationships and enhances static analysis capabilities.
47-47: Better inheritance support with static return type.Changing the
attachmethod return type fromselftostaticenables proper type inference in inheritance scenarios and supports fluent interface patterns more effectively.
63-63: Consistent generic return pattern.Removing the explicit return type declaration while maintaining the
@psalm-return Tannotation follows the established pattern throughout the codebase and provides flexibility for generic type resolution.src/product/price/PriceTypeDefinitionInterface.php (4)
17-20: LGTM! Correct use of Psalm generic templates.The template declaration and
@psalm-consistent-templatesannotation are properly implemented for enhanced type safety.
23-23: Excellent improvement for fluent interface support!Changing return types from
selftostaticenables proper late static binding, allowing fluent method chaining to work correctly in implementing classes and their subclasses.Also applies to: 25-25, 29-29, 43-43, 56-56, 58-58
33-36: Good use of generic return type.The
end()method correctly returns the genericTParentCollectiontype, maintaining type safety across the fluent interface chain.
71-71: Well-defined getter method.The nullable return type is appropriate for this getter, allowing implementations to indicate when no quantity formatter definition is set.
src/product/price/PriceTypeDefinition.php (4)
26-28: Correct implementation of generic interfaces.The class properly declares its template parameter and implements the generic interfaces with appropriate type constraints.
39-39: Well-structured property declarations and constructor.The nullable property initialization and explicit readonly property declaration with generic type are properly implemented.
Also applies to: 53-64
76-76: Consistent implementation of fluent interface pattern.All setter methods correctly return
staticto match the interface contract and support method chaining in inheritance hierarchies.Also applies to: 85-85, 103-103, 146-146, 194-194
216-222: Clean implementation of the interface method.The getter is properly implemented with appropriate visibility annotation.
| /** | ||
| * @template T | ||
| */ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: Fix template parameter inheritance from parent class.
The static analysis failures indicate that BehaviorCollection (the parent class) has a template parameter that isn't being properly extended. You need to either:
- Pass the template parameter
Tto the parent class, or - Ensure the parent class template requirements are satisfied.
Please run this script to examine the parent class template requirements:
🏁 Script executed:
#!/bin/bash
# Check the parent class BehaviorCollection template requirements
ast-grep --pattern 'abstract class BehaviorCollection {
$$$
}'
# Also check for template annotations in BehaviorCollection
rg -A 5 -B 5 "@template" src/product/behavior/BehaviorCollection.phpLength of output: 453
Fix Generic Type Annotation in BehaviorTariffTypeCollection
The parent class BehaviorCollection is declared with a template parameter named TPriceDefinition. Your BehaviorTariffTypeCollection must match that name and pass it through to the parent:
• File: src/product/behavior/BehaviorTariffTypeCollection.php (lines 8–10)
– Change the docblock from:
/**
* @template T
*/to:
/**
* @template TPriceDefinition
*/• Update the class declaration to pass the template through:
final class BehaviorTariffTypeCollection
extends BehaviorCollection<TPriceDefinition>
{
// …
}• Rename any occurrences of T in your constructor, end() method, or other generic annotations to TPriceDefinition.
These edits will align your child class’s generics with the parent’s @template TPriceDefinition and resolve the Psalm inheritance errors.
🤖 Prompt for AI Agents
In src/product/behavior/BehaviorTariffTypeCollection.php around lines 8 to 10,
the template parameter in the docblock is incorrectly named T instead of
matching the parent class BehaviorCollection's template parameter
TPriceDefinition. Update the docblock to use @template TPriceDefinition and
modify the class declaration to extend BehaviorCollection<TPriceDefinition>.
Also, rename any occurrences of T in the class's generic annotations, such as in
the constructor or methods, to TPriceDefinition to ensure proper template
inheritance and fix static analysis errors.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/product/TariffTypeDefinition.php(4 hunks)src/product/TariffTypeDefinitionInterface.php(2 hunks)src/product/behavior/BehaviorCollection.php(2 hunks)src/product/behavior/BehaviorCollectionInterface.php(2 hunks)src/product/behavior/BehaviorPriceTypeDefinitionCollection.php(1 hunks)src/product/behavior/BehaviorTariffTypeCollection.php(1 hunks)src/product/behavior/HasBehaviorsInterface.php(1 hunks)src/product/price/PriceTypeDefinition.php(11 hunks)src/product/price/PriceTypeDefinitionCollection.php(3 hunks)src/product/price/PriceTypeDefinitionCollectionInterface.php(1 hunks)src/product/price/PriceTypeDefinitionInterface.php(3 hunks)src/product/price/PriceTypeStorage.php(2 hunks)src/product/quantity/QuantityFormatterDefinition.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/product/quantity/QuantityFormatterDefinition.php
🚧 Files skipped from review as they are similar to previous changes (9)
- src/product/behavior/HasBehaviorsInterface.php
- src/product/behavior/BehaviorPriceTypeDefinitionCollection.php
- src/product/behavior/BehaviorCollection.php
- src/product/behavior/BehaviorTariffTypeCollection.php
- src/product/price/PriceTypeDefinitionCollectionInterface.php
- src/product/price/PriceTypeDefinitionInterface.php
- src/product/TariffTypeDefinition.php
- src/product/price/PriceTypeDefinitionCollection.php
- src/product/TariffTypeDefinitionInterface.php
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/product/behavior/BehaviorCollectionInterface.php (7)
src/product/behavior/BehaviorCollection.php (1)
attach(31-40)src/product/TariffTypeDefinitionInterface.php (1)
end(39-39)src/product/behavior/BehaviorPriceTypeDefinitionCollection.php (1)
end(36-39)src/product/behavior/BehaviorTariffTypeCollection.php (1)
end(32-35)src/product/TariffTypeDefinition.php (1)
end(108-118)src/product/price/PriceTypeDefinitionInterface.php (1)
end(40-40)src/product/price/PriceTypeDefinition.php (1)
end(135-139)
src/product/price/PriceTypeStorage.php (2)
src/product/price/PriceTypeDefinitionInterface.php (1)
type(49-49)src/product/price/PriceTypeDefinition.php (1)
type(163-166)
🪛 GitHub Check: PHP 8.3
src/product/price/PriceTypeDefinition.php
[failure] 144-144: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:144:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
[failure] 143-143: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:143:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
[failure] 182-182: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:182:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
[failure] 181-181: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:181:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
[failure] 180-180: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:180:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
🪛 GitHub Actions: Psalm Static Analysis
src/product/price/PriceTypeDefinition.php
[error] 143-143: Psalm: UnusedPsalmSuppress - This suppression is never used (see https://psalm.dev/207)
🔇 Additional comments (7)
src/product/behavior/BehaviorCollectionInterface.php (3)
12-14: Excellent generic typing improvements!The covariant template annotation with bounded types and
@psalm-consistent-templatessignificantly improves static analysis capabilities while maintaining interface flexibility.
23-23: Good fluent interface improvement.Changing the return type from
selftostaticenables proper type inference in inheritance hierarchies, allowing fluent method chaining to work correctly with subclasses.
26-28: Proper generic return type implementation.Using the generic
TParentContexttype instead of an explicit union return type provides better type safety and allows Psalm to infer the correct return type based on usage context.src/product/price/PriceTypeStorage.php (1)
16-22: Good defensive programming with null validation.Adding validation for null type names prevents potential issues downstream and provides a clear error message. This complements the broader typing improvements by ensuring data integrity at the storage level.
src/product/price/PriceTypeDefinition.php (3)
26-32: Comprehensive generic typing implementation.The extensive template annotations and implements clauses significantly improve static analysis capabilities. The consistent use of generics across the class hierarchy enhances type safety.
78-84: Proper fluent interface return type.Changing from
selftostaticenables correct type inference in inheritance scenarios, allowing fluent method chaining to work properly with subclasses.
228-233: Good encapsulation with internal getter.Adding the internal getter for
QuantityFormatterDefinitionprovides controlled access to the nullable property while maintaining encapsulation principles.
| * @psalm-suppress MoreSpecificReturnType | ||
| * @psalm-suppress LessSpecificReturnStatement | ||
| * @psalm-suppress LessSpecificImplementedReturnType | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused Psalm suppressions.
The static analysis indicates that several Psalm suppressions in the documentRepresentation() method are unused and should be removed to clean up the codebase.
/**
- * @return RepresentationCollection<PriceTypeDefinition>
- * @psalm-suppress MoreSpecificReturnType
- * @psalm-suppress LessSpecificReturnStatement
- * @psalm-suppress LessSpecificImplementedReturnType
+ * @return RepresentationCollection<PriceTypeDefinition>
*/Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: PHP 8.3
[failure] 144-144: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:144:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
[failure] 143-143: UnusedPsalmSuppress
src/product/price/PriceTypeDefinition.php:143:24: UnusedPsalmSuppress: This suppression is never used (see https://psalm.dev/207)
🪛 GitHub Actions: Psalm Static Analysis
[error] 143-143: Psalm: UnusedPsalmSuppress - This suppression is never used (see https://psalm.dev/207)
🤖 Prompt for AI Agents
In src/product/price/PriceTypeDefinition.php around lines 143 to 146, remove the
unused Psalm suppression annotations from the documentRepresentation() method.
Identify which @psalm-suppress comments are not necessary according to the
static analysis and delete them to clean up the code and improve
maintainability.
Summary by CodeRabbit