Deprecate Once #150
Deprecate Once #150
Conversation
WalkthroughMultiple small formatting and docblock edits across serializer classes and mappers, reinstatement and renaming of reflection variables plus restored JSON mapping methods in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant JsonSerializer
participant Reflect as Reflection (props/params)
participant MapAttr as MapName/MapInput/MapOutput
participant Mapper
participant Encoder as JsonEncoder
Caller->>JsonSerializer: serialize(object)
JsonSerializer->>Reflect: inspect properties / parameters
Reflect->>MapAttr: check attributes (hasMapNameAttributes)
alt attribute present
MapAttr->>Mapper: instantiate mapper (new $mapper)
Mapper->>JsonSerializer: mapped names
end
JsonSerializer->>Encoder: format/encode with encoders()
Encoder-->>Caller: JSON output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.31)Invalid configuration: Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Objects/Once.php (1)
13-16: Deprecation notice is clear, but consider adding version and timeline information.The @deprecated tag properly communicates the replacement class (StaticCache), and the message is actionable. However, the deprecation notice could be enhanced with version and removal timeline information to set clear expectations for users.
Consider enriching the deprecation metadata following PHP conventions:
-/** - * @codeCoverageIgnore Ignored since this is a wrapper around an Illuminate class - * @deprecated use StaticCache instead. - */ +/** + * @codeCoverageIgnore Ignored since this is a wrapper around an Illuminate class + * @deprecated since v2.0.0 Use StaticCache instead. Will be removed in v3.0.0. + */Additionally, consider adding a runtime deprecation warning to alert users at call time, which would provide visibility even in contexts where PHPDoc isn't readily available:
public static function handle(callable $callable): mixed { self::version(); + @trigger_error('Once is deprecated. Use StaticCache instead.', E_USER_DEPRECATED); return once($callable); } public static function flush(): void { self::version(); + @trigger_error('Once is deprecated. Use StaticCache instead.', E_USER_DEPRECATED); BaseOnce::instance()->flush(); }Note: If you choose to add runtime warnings, adjust version numbers based on your actual release versioning scheme.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Objects/Once.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Objects/Once.php (2)
src/Exceptions/MissingMinimumDependencyException.php (1)
MissingMinimumDependencyException(8-21)src/Support/Internal/Composer/Composer.php (1)
Composer(13-36)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Objects/Serializers/JsonSerializer.php (1)
64-73: Inconsistent empty-array checks.Lines 64-66 use
!== []to check for class-level attributes, while lines 71-73 use!empty()for property-level attributes. Both patterns work, but consistency improves readability.Consider standardizing on
!== []throughout:- foreach ($reflection->getProperties() as $reflectionProperty) { - if (!empty($reflectionProperty->getAttributes(MapName::class)) || - !empty($reflectionProperty->getAttributes(MapInputName::class)) || - !empty($reflectionProperty->getAttributes(MapOutputName::class))) { + foreach ($reflection->getProperties() as $reflectionProperty) { + if ($reflectionProperty->getAttributes(MapName::class) !== [] || + $reflectionProperty->getAttributes(MapInputName::class) !== [] || + $reflectionProperty->getAttributes(MapOutputName::class) !== []) { return true; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.git-blame-ignore-revs(1 hunks)src/Objects/Once.php(1 hunks)src/Objects/Serializers/Attributes/MapInputName.php(1 hunks)src/Objects/Serializers/Attributes/MapName.php(2 hunks)src/Objects/Serializers/Attributes/MapOutputName.php(1 hunks)src/Objects/Serializers/JsonSerializer.php(7 hunks)src/Objects/Serializers/Mappers/CamelCaseMapper.php(1 hunks)src/Objects/Serializers/Mappers/IPropertyMapper.php(1 hunks)src/Objects/Serializers/Mappers/PascalCaseMapper.php(1 hunks)src/Objects/Serializers/Mappers/ProvidedNameMapper.php(1 hunks)src/Objects/Serializers/Mappers/SnakeCaseMapper.php(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- src/Objects/Serializers/Mappers/IPropertyMapper.php
- .git-blame-ignore-revs
- src/Objects/Serializers/Attributes/MapName.php
- src/Objects/Serializers/Mappers/CamelCaseMapper.php
- src/Objects/Serializers/Mappers/SnakeCaseMapper.php
- src/Objects/Serializers/Attributes/MapInputName.php
- src/Objects/Serializers/Mappers/PascalCaseMapper.php
- src/Objects/Serializers/Attributes/MapOutputName.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Objects/Once.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/Objects/Serializers/JsonSerializer.php (2)
src/Objects/Serializers/AbstractSerializer.php (4)
AbstractSerializer(11-69)normalizers(41-48)format(66-66)encoders(68-68)src/Objects/Serializers/Mappers/IPropertyMapper.php (1)
map(12-12)
🔇 Additional comments (8)
src/Objects/Serializers/Mappers/ProvidedNameMapper.php (1)
2-2: LGTM!The blank line addition aligns with the project-wide formatting cleanup.
src/Objects/Serializers/JsonSerializer.php (7)
2-2: LGTM!Formatting change consistent with the project-wide cleanup.
16-16: LGTM!The
ReflectionParameterimport is properly used in the updated parameter resolution logic, and adding type hints to constants improves type safety.Also applies to: 25-26
47-55: LGTM!The reinstated
format()andencoders()methods properly implement the parent class's abstract methods with appropriate JSON-specific configuration.
86-92: LGTM!The consistent use of
$reflectionPropertynaming improves code clarity.
117-118: LGTM!The consistent
$reflectionParameternaming and explicit type hint improve code clarity and type safety.Also applies to: 124-124
176-181: LGTM!The consistent use of
!== []for attribute checks improves code clarity and aligns with the checks at lines 64-66.
155-169: ****The concern about mapper instantiation is unfounded. All mappers used as string class names (SnakeCaseMapper, CamelCaseMapper, PascalCaseMapper) have no required constructor arguments and instantiate correctly with the
new $mappersyntax. ProvidedNameMapper, which requires constructor arguments, is always instantiated directly as an object in attributes (e.g.,#[MapName(new ProvidedNameMapper('custom_field_name'))]) and never via the string instantiation path, so the change does not affect it. Per PHP documentation, parentheses are optional for constructors with no arguments.Likely an incorrect or invalid review comment.
Summary by CodeRabbit