Enhance type safety with Psalm domain types and fix static analysis errors #299
Conversation
- Create Types.php with 40+ Psalm type definitions for Ray.Di framework - Apply type definitions to core files: Container, Bind, Dependency, Arguments, Name - Achieve 99.41% type inference coverage with zero static analysis errors - Add proper type annotations for dependency injection patterns - Include CLAUDE.md documentation for future development 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Rename template variables to be unique and descriptive: - T → TInterceptor for MethodInterceptor types - T → TProvider for Provider types - T → TClass for object class types - T → TOptional for optional binding types - Maintain 99.41% type inference coverage with zero errors 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove unused type definitions: - BindingErrorReason (not used in codebase) - SerializableDependencyData (not used in codebase) - InjectionPointSerialData (not used in codebase) - ModuleInstallChain (not used in codebase) - ModuleOverrideChain (not used in codebase) - Improve PHPDoc formatting with clean group headers - Maintain 99.41% type inference coverage with zero errors - Keep only practically used type definitions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace array<SetterMethod> with SetterMethodsList in SetterMethods and VisitorInterface
- Replace array<Argument> with ArgumentList in VisitorInterface
- Replace array<int, Pointcut> with PointcutList in Dependency and ModuleString
- Replace array<class-string<MethodInterceptor>> with InterceptorClassList in AbstractModule
- Replace array<non-empty-string, list<MethodInterceptor>> with MethodInterceptorBindings in AspectBind
- Replace array<object> with QualifierList in InjectionPointInterface
- Replace array<string, string> with ParameterNameMapping in Name and Bind
- Replace array<string, mixed> with NamedArguments in AssistedInjectInterceptor
- Replace non-empty-array<AbstractModule> with ModuleList in Injector and ContainerFactory
- Replace array{0: string, 1: string, 2: bool} with InjectionPointDefinition in InjectionPoints
Enhance Types.php with domain-specific type aliases while maintaining separation
between technical types and domain types. Improve static analysis coverage to 99.23%.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Fix array offset access in Bind.php with null coalescing operator - Update Container.php with proper return type annotations and suppress mixed return - Refine ParameterNameMapping type to allow empty string keys for Name::ANY - Add missing type imports and improve parameter handling in Name.php - Update Arguments.php return type annotation to list<mixed> - Clean up AssistedInjectInterceptor type annotations - Remove redundant psalm-assert annotation from DependencyInterface - Configure PHPStan raw error format for better CI integration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's GuideThis PR centralizes Psalm domain type definitions in a new Types.php file, updates docblocks across the Ray.Di codebase to leverage these aliases for stronger type safety, and resolves various Psalm/PHPStan errors by refining annotations, removing obsolete assertions, and tuning CI configuration. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis set of changes introduces comprehensive Psalm type alias definitions in a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DemoClass
participant RayDiContainer
User->>DemoClass: Instantiate with PHP 8 promoted/annotated constructor
DemoClass->>RayDiContainer: Requests dependency via attribute (e.g., #[Inject])
RayDiContainer-->>DemoClass: Injects dependency per attribute/type
DemoClass->>User: Ready for use with injected dependencies
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @koriym - I've reviewed your changes - here's some feedback:
- Consider refining the few remaining @psalm-suppress annotations by tightening your code-level types so suppressions are only used where absolutely necessary.
- The newly added Types.php file centralizes many Psalm type aliases—consider grouping or segmenting related type groups (e.g. AOP, injection, multi-binding) into smaller modules to improve readability and maintainability.
- There are still two PHPStan warnings—address them now or add contextual suppressions to fully close the gap in your static analysis coverage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider refining the few remaining @psalm-suppress annotations by tightening your code-level types so suppressions are only used where absolutely necessary.
- The newly added Types.php file centralizes many Psalm type aliases—consider grouping or segmenting related type groups (e.g. AOP, injection, multi-binding) into smaller modules to improve readability and maintainability.
- There are still two PHPStan warnings—address them now or add contextual suppressions to fully close the gap in your static analysis coverage.
## Individual Comments
### Comment 1
<location> `src/di/Types.php:48` </location>
<code_context>
+ * @psalm-type MethodInterceptorBindings array<non-empty-string, list<MethodInterceptor>>
+ * @psalm-type InterceptorClassList array<class-string<MethodInterceptor>>
+ * @psalm-type VisitorResult = object|array<array-key, mixed>|null
+ * @psalm-type SetterMethodList = array<SetterMethod>
+ * @psalm-type ArgumentList = array<Argument>
+ *
+ * Reflection and Metadata Types
</code_context>
<issue_to_address>
Duplicate or inconsistent type aliases for SetterMethodList/SetterMethodsList and ArgumentList/ArgumentsList.
Please standardize the naming of these type aliases to ensure consistency and prevent confusion.
Suggested implementation:
```
* @psalm-type SetterMethodsList = array<SetterMethod>
* @psalm-type ArgumentsList = array<Argument>
```
- Update all usages of `SetterMethodList` to `SetterMethodsList` and `ArgumentList` to `ArgumentsList` throughout the codebase for consistency.
- If there are other aliases like `SetterMethodsList` or `ArgumentsList` already defined elsewhere, remove the duplicates to avoid confusion.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (99.49%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## 2.x #299 +/- ##
=============================================
- Coverage 100.00% 99.49% -0.51%
- Complexity 359 365 +6
=============================================
Files 49 50 +1
Lines 963 996 +33
=============================================
+ Hits 963 991 +28
- Misses 0 5 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
src/di/Grapher.php (1)
61-67: Align the parameter PHPDoc with the imported alias.Replace
list<mixed>with the importedMethodArgumentsto gain the benefit of the centralized type definition and eliminate the unused-import warning.- * @param string $class class name - * @param list<mixed> $params construct parameters (MethodArguments) + * @param string $class class name + * @param MethodArguments $params constructor argumentssrc/di/VisitorInterface.php (1)
79-83: Propagate the alias fix to parameter annotationAfter correcting the import line, update the parameter tag to stay in sync:
- * @param ArgumentList $arguments + * @param ArgumentsList $argumentssrc/di/Name.php (1)
115-123: Possiblenullarray access when only an empty binding name is provided
$this->namesis accessed without being initialised when$this->name === ''(i.e.Name::ANY).
Constructingnew Name(Name::ANY)sets$this->nameto an empty string but never populates$this->names, so calling the object later in__invoke()will raise “Trying to access array offset on null”.A minimal defensive fix:
- return $this->names[$parameterName] ?? $this->names[self::ANY] ?? self::ANY; + return $this->names[$parameterName] // per-parameter binding + ?? $this->names[self::ANY] ?? self::ANY; // fallback bindingsor, guard the property first:
return $this->names[$parameterName] ?? ($this->names[self::ANY] ?? self::ANY);
🧹 Nitpick comments (15)
src/di/SpyCompiler.php (1)
32-40: Consider using a standardized@psalm-suppress ... justificationline instead of a prose NOTEPsalm recognises an optional justification string placed on the same
@psalm-suppressline (e.g.@psalm-suppress MoreSpecificReturnType Dummy class used only for logging).
Encoding the rationale that way keeps the comment machine-parsable and consistent with the rest of the docblock style, while avoiding additional prose lines.CLAUDE.md (1)
1-4: Use a descriptive title instead of the filenameStarting the document with the literal filename is uncommon in Markdown docs. A descriptive heading improves readability when the file is rendered on GitHub/GitLab.
-# CLAUDE.md +# Guide for Claude Code 🤖src/di/Types.php (1)
48-65: Duplicate / confusing aliases for setter methods
SetterMethodList(singular) andSetterMethodsList(plural) are both defined but map to the same shape.
Consider keeping only one to avoid ambiguity.src/di/AssistedInjectInterceptor.php (2)
23-32: Redundant twin docblocks – collapse into oneYou have a file-scope import block (lines 23-26) and then replicate the same imports in the class-level docblock (line 31). One is enough; keep the one closest to the class.
72-90: Leverage the newNamedArgumentsalias in return annotation
getNamedArguments()currently returnsarray<string, mixed>even thoughNamedArgumentsis imported.- * @return array<string, mixed> + * @return NamedArgumentsThis gives Psalm stronger guarantees and eliminates another mixed array.
src/di/MultiBinder.php (1)
13-16: Imported aliases are unused
BindableInterfaceandLazyBindingListare not referenced anywhere in this class.
Remove the import block or use the aliases to document$container/$multiBindings.src/di/MultiBinding/MultiBindingModule.php (1)
13-15: UnusedBindableInterfaceimportSame situation as in
MultiBinder.php– either use the alias or drop the import block.tests/di/BindTest.php (1)
54-56: Use a consistent PHPStan suppression style.Earlier in this file you suppress PHPStan with the inline form
// @phpstan-ignore-line, whereas here you switch to the docblock form/** @phpstan-ignore-next-line */. Mixing the two makes the test harder to scan for suppressions.- /** @phpstan-ignore-next-line */ + // @phpstan-ignore-line new Bind(new Container(), 'invalid-interface');src/di/Injector.php (1)
34-35: Consider giving$modulea real union typeNow that PHP 8+ supports union types you could surface the same information in the method signature:
-public function __construct($module = null, string $tmpDir = '') +public function __construct( + AbstractModule|array|null $module = null, + string $tmpDir = '' +)This would let Psalm/PhpStan skip the PHPDoc fallback.
src/di/NewInstance.php (2)
54-56: Minor: repeated Psalm suppression could be avoided
$reflection->newInstanceArgs()is always safe here because$arguments->inject()returns an array. Consider adding a narrow return type onArguments::inject()instead of suppressingMixedMethodCallevery use-site.
70-71: Parameter still typed asarraySince you now document
$paramsasMethodArguments, you could update the method signature for extra static safety:- public function newInstanceArgs(Container $container, array $params): object + public function newInstanceArgs(Container $container, array $params /* MethodArguments */): object(Or declare the alias directly once Psalm supports it in signatures.)
src/di/Dependency.php (1)
88-90: Same note as inNewInstance– you may eventually exposeMethodArgumentsin the signature when possible to drop the genericarray.src/di/ContainerFactory.php (1)
35-36: Same ordering issue ingetModule()– swap the@paramannotations to match the actual signature.src/di/Name.php (1)
160-167: Typo:$trimedKeyshould be$trimmedKeyMisspelling in a frequently-used helper makes grep-based searches harder and is an easy source of copy-paste bugs.
- $trimedKey = trim((string) $key); - $names[$trimedKey] = trim($value); + $trimmedKey = trim((string) $key); + $names[$trimmedKey] = trim($value);src/di/Container.php (1)
33-35: Property annotation conflicts with the default value
@var DependencyContainerrequires non-empty string keys, yet the property is initialised to an empty array.
Psalm treats[]asarray<empty, empty>which is incompatible and will raiseInvalidPropertyAssignmentValue.Either:
- /** @var DependencyContainer */ - private $container = []; + /** @var DependencyContainer */ + private $container = []; // @phpstan-ignore-line (empty initial state is allowed)or loosen the alias to allow the empty state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
CLAUDE.md(1 hunks)phpstan.neon(1 hunks)src/di/AbstractModule.php(4 hunks)src/di/Argument.php(4 hunks)src/di/Arguments.php(2 hunks)src/di/AspectBind.php(2 hunks)src/di/AssistedInjectInterceptor.php(1 hunks)src/di/Bind.php(6 hunks)src/di/Container.php(7 hunks)src/di/ContainerFactory.php(2 hunks)src/di/Dependency.php(3 hunks)src/di/DependencyInterface.php(2 hunks)src/di/Grapher.php(2 hunks)src/di/InjectionPointInterface.php(2 hunks)src/di/InjectionPoints.php(2 hunks)src/di/Injector.php(3 hunks)src/di/ModuleString.php(1 hunks)src/di/MultiBinder.php(1 hunks)src/di/MultiBinding/MultiBindingModule.php(1 hunks)src/di/MultiBinding/MultiBindings.php(1 hunks)src/di/Name.php(6 hunks)src/di/NewInstance.php(3 hunks)src/di/SetterMethods.php(1 hunks)src/di/SpyCompiler.php(1 hunks)src/di/Types.php(1 hunks)src/di/VisitorInterface.php(3 hunks)tests/di/BindTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: the use of "ray/aop": "^2.x-dev" in composer.json is intentional as it depends on features from the ...
Learnt from: koriym
PR: ray-di/Ray.Di#297
File: composer.json:17-17
Timestamp: 2025-02-22T08:34:22.655Z
Learning: The use of "ray/aop": "^2.x-dev" in composer.json is intentional as it depends on features from the upcoming version that hasn't been released yet.
Applied to files:
src/di/ModuleString.phpsrc/di/Dependency.php
🧬 Code Graph Analysis (1)
src/di/MultiBinding/MultiBindingModule.php (1)
src/di/Types.php (1)
Types(71-73)
🔇 Additional comments (20)
CLAUDE.md (1)
25-46: All Composer scripts referenced in “Development Commands” are presentThe listed commands (
test,coverage,pcov,cs,cs-fix,sa,clean,build,tests) in CLAUDE.md match the entries undercomposer.json > scripts. No updates needed.src/di/InjectionPointInterface.php (1)
11-13: LGTM – type alias import is correctly applied.The new
QualifierListalias clarifies the return type without impacting runtime behaviour.Also applies to: 37-39
src/di/ModuleString.php (1)
16-24: LGTM – pointcut collection now strongly typed.
ThePointcutListalias improves clarity while keeping runtime unchanged.src/di/AspectBind.php (1)
12-30: LGTM – return type correctly narrowed with MethodInterceptorBindings.src/di/SetterMethods.php (1)
9-19: Psalm alias import & annotations look correctThe new
@psalm-import-type SetterMethodsListimport and the updated property / constructor annotations are consistent with the alias naming convention used across the PR and will improve static-analysis fidelity.
No further action required.src/di/InjectionPoints.php (2)
10-20: Type-alias adoption is sound
InjectionPointDefinitionandInjectionPointsListare imported and applied consistently to both the property and the helper method parameter. These doc-block refinements align with the new centralTypesdefinitions and need no changes.
53-58: Method signature remains compatibleThe helper keeps the runtime signature
array $point, while the doc-block narrows the expected structure via the alias. This is the accepted pattern when PHP can’t express tuple-like shapes. Good to merge.src/di/Arguments.php (2)
10-17: Accurate alias usage for ArgumentsImporting
ArgumentsListandMethodArgumentsplus annotating the property enhances Psalm coverage without affecting runtime behaviour.
30-34: Return type narrowed tolist<mixed>The new doc-block correctly reflects the sequential array guarantee delivered by the implementation. No further change needed.
src/di/DependencyInterface.php (1)
7-34: Consistent container alias, but check namingThe interface now imports
DependencyContainer,ScopeType, andInjectableValue; theregisterdoc-block referencesDependencyContainerand marks it@param-out, which is correct for a by-ref array.
Looks good.src/di/MultiBinding/MultiBindings.php (1)
8-14: Type-alias import is spot-onImporting
Typesand re-using theLazyBindingListalias tightens Psalm coverage and removes duplication across the codebase. No other concerns here.src/di/Injector.php (1)
21-24: Good use of central Psalm aliasesThe
BindableInterfaceandModuleListimports remove a lot of inline doc clutter. Looks correct given the newTypesclass.src/di/NewInstance.php (1)
14-16: Alias import keeps docs conciseImporting
MethodArgumentshere is consistent with the rest of the refactor. No issues.src/di/Dependency.php (2)
18-22: Psalm aliases correctly appliedThe switch to
MethodArguments/PointcutListkeeps the docs in sync with the rest of the PR.
123-124: Pointcut list alias looks goodThe new docblock matches the actual usage in
weaveAspects().src/di/ContainerFactory.php (1)
11-13: Consistent alias importThe
ModuleListimport is correct and lines up with the rest of the refactor.src/di/Argument.php (1)
112-118:__unserialize()DocBlock shape does not match the real payloadThe annotation advertises six elements (
0 … 5) but__serialize()actually returns only five.
This makes Psalm / PHPStan ignore valuable structural checks.- * @param array{0: DependencyIndex, 1: bool, 2: string, 3: string, 4: string, 5: array{0: string, 1: string, 2:string}} $unserialized + * @param array{ + * 0: DependencyIndex, + * 1: bool, + * 2: mixed, # same type as $default + * 3: string, + * 4: array{0:string,1:string,2:string} + * } $unserializedAdjusting the DocBlock (or the actual payload) keeps static analysis reliable.
src/di/AbstractModule.php (1)
80-88:InterceptorClassListmay be too restrictive for the real usage
bindInterceptor()accepts both concrete interceptor classes and qualifier interfaces (see Lines 88-96).
IfInterceptorClassListis defined aslist<class-string<MethodInterceptor>>, qualifier interface names will violate the alias and Psalm will complain.Either broaden the alias (e.g.
class-string<MethodInterceptor>|class-string) or add a second, looser param for interfaces.src/di/Bind.php (1)
231-236: Nice guard against undefined indexUsing the null-coalescing operator prevents
Undefined array keynotices when a constructor-parameter name is omitted in the mapping. 👍phpstan.neon (1)
1-3: Verify thatrawis the desirederrorFormatfor CI consumers
errorFormat: rawgives plain-text, human-oriented output.
If any downstream CI step (e.g. GitHub Actions annotations, SARIF converters, merge-gate scripts) expects a structured format such asjson,jsonlines,table, orgithub, those integrations will silently break.Please double-check the tooling that parses PHPStan output and confirm it supports the raw format.
You can run the pipeline locally and inspect the produced artifact or annotate the job to be sure the change does not regress reporting. If structured output is still required, consider switching to
"github"(good for PR annotations) or"table"while keepingrawfor local runs via an override flag.
- Remove old demo/ directory using DocBlock annotations (PHP7 era) - Rename demo-php8/ to demo/ as the primary demo directory - All demos now use PHP8 Attributes instead of DocBlock annotations - Fixes demo execution issues with named binding (@nAmed annotation) - Improves user experience with working examples 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
demo/tmp/Sorter-.php (1)
5-7: Consider error handling and namespace qualificationWhile this appears to be generated code, consider these improvements for robustness:
- The
Sorterclass lacks namespace qualification - ensure it's available in the global namespace- No error handling for potential instantiation failures
If this is generated code, ensure the generator includes proper error handling:
-$instance = new \Sorter(); +try { + $instance = new \Sorter(); +} catch (\Throwable $e) { + throw new \RuntimeException('Failed to create Sorter instance: ' . $e->getMessage(), 0, $e); +}demo/finder/DbFinder.php (1)
14-16: Address unused parameter in demo methodThe
$dbparameter is unused as flagged by static analysis. Since this is a demo file, consider either:
- Adding a comment explaining this is a demonstration method
- Adding minimal implementation to show usage
public function setDb(DbInterface $db) { + // Demo setter method - $db would be stored in real implementation }demo/02-provider-binding.php (1)
42-45: Excellent use of PHP 8 constructor property promotion.The refactoring to use constructor property promotion is a great modernization that makes the code more concise and readable while maintaining type safety. This pattern is consistent with similar updates across the demo files.
Minor formatting suggestion: Consider moving the closing brace to line 45 for better consistency:
public function __construct( public FinderInterface $finder -){ +) { }demo/tmp/MovieFinder_4221959595.php (1)
5-6: Remove unused import.The
Assistedimport is not used in this generated proxy class.-use Ray\Di\Di\Assisted; use Ray\Di\Di\Inject;demo/tmp/MovieFinder_1102178874.php (1)
5-6: Remove unused import.Same as the other proxy class, the
Assistedimport is not used.-use Ray\Di\Di\Assisted; use Ray\Di\Di\Inject;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
demo/tmp/_compile.logis excluded by!**/*.log
📒 Files selected for processing (107)
.github/workflows/demo-php8.yml(3 hunks).github/workflows/update-copyright-years-in-license-file.yml(1 hunks)demo/01a-linked-binding.php(1 hunks)demo/01b-linked-binding-setter-injection.php(1 hunks)demo/02-provider-binding.php(1 hunks)demo/02a-named-by-qualifier.php(1 hunks)demo/02b-named-by-named.php(1 hunks)demo/03-injection-point.php(2 hunks)demo/05b-constructor-binding-setter-injection.php(1 hunks)demo/finder/Db.php(1 hunks)demo/finder/DbFinder.php(1 hunks)demo/finder/MovieFinder.php(1 hunks)demo/finder/MovieLister.php(1 hunks)demo/run.php(1 hunks)demo/tmp/-Ray_Di_Annotation_ScriptDir.php(1 hunks)demo/tmp/-dsn.php(1 hunks)demo/tmp/-password.php(1 hunks)demo/tmp/-username.php(1 hunks)demo/tmp/03/03419867.php(1 hunks)demo/tmp/04/0424916e.php(1 hunks)demo/tmp/05/05cacb22.php(1 hunks)demo/tmp/06/063abd7e.php(1 hunks)demo/tmp/09/09a2a810.php(1 hunks)demo/tmp/0a/0aac2e56.php(1 hunks)demo/tmp/0f/0f4883c8.php(1 hunks)demo/tmp/11/118d374f.php(1 hunks)demo/tmp/16/16db7ca3.php(1 hunks)demo/tmp/18/181572f3.php(1 hunks)demo/tmp/1a/1a87dd68.php(1 hunks)demo/tmp/1c/1c2a6b4f.php(1 hunks)demo/tmp/1d/1d519d44.php(1 hunks)demo/tmp/23/233715a9.php(1 hunks)demo/tmp/25/25bfde8a.php(1 hunks)demo/tmp/2d/2d60ca4c.php(1 hunks)demo/tmp/2f/2f2190de.php(1 hunks)demo/tmp/30/302b05b8.php(1 hunks)demo/tmp/32/3278bb46.php(1 hunks)demo/tmp/36/360f8996.php(1 hunks)demo/tmp/37/37ac895e.php(1 hunks)demo/tmp/37/37b68821.php(1 hunks)demo/tmp/38/382f29d1.php(1 hunks)demo/tmp/3c/3c01e384.php(1 hunks)demo/tmp/3c/3cbc861b.php(1 hunks)demo/tmp/3d/3d02f4dc.php(1 hunks)demo/tmp/3e/3eab3b2e.php(1 hunks)demo/tmp/42/4293e451.php(1 hunks)demo/tmp/44/44a694c9.php(1 hunks)demo/tmp/45/452443ba.php(1 hunks)demo/tmp/46/46f82e41.php(1 hunks)demo/tmp/48/48cd4d82.php(1 hunks)demo/tmp/4e/4e6bce49.php(1 hunks)demo/tmp/57/57826e15.php(1 hunks)demo/tmp/58/5870f1b7.php(1 hunks)demo/tmp/59/591f77f6.php(1 hunks)demo/tmp/5c/5cb7bcd1.php(1 hunks)demo/tmp/5d/5d62ed99.php(1 hunks)demo/tmp/61/61b0b607.php(1 hunks)demo/tmp/68/685871c9.php(1 hunks)demo/tmp/6f/6f006109.php(1 hunks)demo/tmp/6f/6f5236fc.php(1 hunks)demo/tmp/74/74d78426.php(1 hunks)demo/tmp/7d/7d4c13f6.php(1 hunks)demo/tmp/7e/7e51911c.php(1 hunks)demo/tmp/7e/7ebe39af.php(1 hunks)demo/tmp/84/847273f5.php(1 hunks)demo/tmp/87/87d912fb.php(1 hunks)demo/tmp/8a/8aa50a17.php(1 hunks)demo/tmp/8b/8bfbbd3c.php(1 hunks)demo/tmp/8f/8fd42a7d.php(1 hunks)demo/tmp/93/93710521.php(1 hunks)demo/tmp/95/9517c9b8.php(1 hunks)demo/tmp/96/9653b90b.php(1 hunks)demo/tmp/96/96c72435.php(1 hunks)demo/tmp/98/98bb2f1d.php(1 hunks)demo/tmp/9e/9ef70bb3.php(1 hunks)demo/tmp/9f/9fd59cba.php(1 hunks)demo/tmp/DbInterface-.php(1 hunks)demo/tmp/FinderInterface-.php(1 hunks)demo/tmp/MovieFinder_1102178874.php(1 hunks)demo/tmp/MovieFinder_4221959595.php(1 hunks)demo/tmp/MovieListerInterface-.php(1 hunks)demo/tmp/Ray_Di_InjectorInterface-.php(1 hunks)demo/tmp/Sorter-.php(1 hunks)demo/tmp/_aop.txt(1 hunks)demo/tmp/a0/a0a2d23a.php(1 hunks)demo/tmp/a5/a566d957.php(1 hunks)demo/tmp/a6/a65d4eab.php(1 hunks)demo/tmp/a9/a9a47135.php(1 hunks)demo/tmp/ab/aba86a5b.php(1 hunks)demo/tmp/b1/b1701da3.php(1 hunks)demo/tmp/b3/b36f6c1c.php(1 hunks)demo/tmp/b9/b9b64f51.php(1 hunks)demo/tmp/ba/ba30bacc.php(1 hunks)demo/tmp/be/be749378.php(1 hunks)demo/tmp/c0/c03c4f15.php(1 hunks)demo/tmp/c5/c51071a2.php(1 hunks)demo/tmp/c5/c5d461fa.php(1 hunks)demo/tmp/c6/c642a16c.php(1 hunks)demo/tmp/c8/c898fdad.php(1 hunks)demo/tmp/cd/cdb4c31a.php(1 hunks)demo/tmp/cf/cf1cb878.php(1 hunks)demo/tmp/d1/d1463758.php(1 hunks)demo/tmp/d1/d1b6e4e5.php(1 hunks)demo/tmp/d1/d1e364f0.php(1 hunks)demo/tmp/d2/d238a6bc.php(1 hunks)demo/tmp/d4/d49ada52.php(1 hunks)demo/tmp/d9/d99048f1.php(1 hunks)
⛔ Files not processed due to max files limit (9)
- demo/tmp/db/db5e0a2a.php
- demo/tmp/e1/e1830981.php
- demo/tmp/e9/e90020b6.php
- demo/tmp/eb/eb7b5616.php
- demo/tmp/ec/ec467936.php
- demo/tmp/ed/ed227561.php
- demo/tmp/ed/ed812cd3.php
- demo/tmp/ef/ef61eae4.php
- demo/tmp/f8/f8758f25.php
✅ Files skipped from review due to trivial changes (94)
- demo/tmp/7e/7ebe39af.php
- demo/finder/Db.php
- demo/tmp/3c/3cbc861b.php
- demo/tmp/c5/c51071a2.php
- demo/tmp/_aop.txt
- demo/tmp/1a/1a87dd68.php
- demo/tmp/18/181572f3.php
- demo/tmp/06/063abd7e.php
- demo/tmp/05/05cacb22.php
- demo/tmp/3d/3d02f4dc.php
- demo/tmp/03/03419867.php
- demo/tmp/1d/1d519d44.php
- demo/tmp/7e/7e51911c.php
- demo/tmp/16/16db7ca3.php
- demo/tmp/7d/7d4c13f6.php
- demo/tmp/36/360f8996.php
- demo/tmp/0f/0f4883c8.php
- demo/tmp/5d/5d62ed99.php
- .github/workflows/update-copyright-years-in-license-file.yml
- demo/tmp/-username.php
- demo/tmp/-password.php
- demo/tmp/04/0424916e.php
- demo/tmp/58/5870f1b7.php
- demo/run.php
- demo/tmp/09/09a2a810.php
- demo/tmp/59/591f77f6.php
- demo/tmp/ab/aba86a5b.php
- demo/tmp/c8/c898fdad.php
- demo/tmp/4e/4e6bce49.php
- demo/tmp/8b/8bfbbd3c.php
- demo/tmp/74/74d78426.php
- demo/tmp/8a/8aa50a17.php
- demo/tmp/11/118d374f.php
- demo/tmp/57/57826e15.php
- demo/tmp/Ray_Di_InjectorInterface-.php
- demo/tmp/b3/b36f6c1c.php
- demo/tmp/b9/b9b64f51.php
- demo/tmp/5c/5cb7bcd1.php
- demo/tmp/a5/a566d957.php
- demo/tmp/c0/c03c4f15.php
- demo/tmp/45/452443ba.php
- demo/tmp/3e/3eab3b2e.php
- demo/tmp/87/87d912fb.php
- demo/tmp/3c/3c01e384.php
- demo/tmp/9e/9ef70bb3.php
- demo/tmp/a0/a0a2d23a.php
- demo/tmp/25/25bfde8a.php
- demo/tmp/1c/1c2a6b4f.php
- demo/tmp/96/9653b90b.php
- demo/tmp/be/be749378.php
- demo/tmp/d9/d99048f1.php
- demo/tmp/8f/8fd42a7d.php
- demo/tmp/-Ray_Di_Annotation_ScriptDir.php
- demo/tmp/2f/2f2190de.php
- demo/tmp/b1/b1701da3.php
- demo/05b-constructor-binding-setter-injection.php
- demo/tmp/84/847273f5.php
- demo/tmp/9f/9fd59cba.php
- demo/tmp/c6/c642a16c.php
- demo/tmp/48/48cd4d82.php
- demo/tmp/68/685871c9.php
- demo/tmp/37/37ac895e.php
- demo/tmp/46/46f82e41.php
- demo/tmp/32/3278bb46.php
- demo/tmp/c5/c5d461fa.php
- demo/tmp/95/9517c9b8.php
- demo/tmp/ba/ba30bacc.php
- demo/tmp/30/302b05b8.php
- demo/finder/MovieLister.php
- demo/tmp/cf/cf1cb878.php
- demo/tmp/38/382f29d1.php
- demo/tmp/cd/cdb4c31a.php
- demo/tmp/d1/d1463758.php
- demo/tmp/37/37b68821.php
- demo/tmp/6f/6f5236fc.php
- demo/tmp/93/93710521.php
- demo/tmp/98/98bb2f1d.php
- demo/tmp/44/44a694c9.php
- demo/tmp/a9/a9a47135.php
- demo/tmp/d1/d1b6e4e5.php
- demo/tmp/61/61b0b607.php
- demo/tmp/d4/d49ada52.php
- demo/tmp/0a/0aac2e56.php
- demo/tmp/6f/6f006109.php
- demo/tmp/23/233715a9.php
- demo/tmp/2d/2d60ca4c.php
- demo/tmp/d2/d238a6bc.php
- demo/tmp/-dsn.php
- demo/tmp/96/96c72435.php
- demo/tmp/a6/a65d4eab.php
- demo/tmp/42/4293e451.php
- demo/tmp/d1/d1e364f0.php
- demo/02b-named-by-named.php
- demo/03-injection-point.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: php 8.4 has been released and is supported in github actions....
Learnt from: koriym
PR: ray-di/Ray.Di#293
File: .github/workflows/continuous-integration.yml:13-13
Timestamp: 2024-11-29T13:50:16.707Z
Learning: PHP 8.4 has been released and is supported in GitHub Actions.
Applied to files:
.github/workflows/demo-php8.yml
🧬 Code Graph Analysis (6)
demo/01a-linked-binding.php (3)
demo/02-provider-binding.php (2)
__construct(19-22)__construct(42-45)demo/03-injection-point.php (3)
__construct(24-27)__construct(39-42)__construct(47-50)demo/finder/MovieLister.php (1)
__construct(9-11)
demo/02-provider-binding.php (3)
demo/02a-named-by-qualifier.php (1)
__construct(27-30)demo/01a-linked-binding.php (1)
__construct(24-26)demo/03-injection-point.php (3)
__construct(24-27)__construct(39-42)__construct(47-50)
demo/01b-linked-binding-setter-injection.php (2)
demo/finder/DbFinder.php (2)
Inject(13-16)Inject(18-21)demo/finder/MovieLister.php (1)
Inject(13-16)
demo/finder/MovieFinder.php (3)
demo/01b-linked-binding-setter-injection.php (1)
Inject(27-31)demo/finder/DbFinder.php (2)
Inject(13-16)Inject(18-21)demo/finder/MovieLister.php (1)
Inject(13-16)
demo/02a-named-by-qualifier.php (5)
demo/02-provider-binding.php (2)
__construct(19-22)__construct(42-45)demo/01a-linked-binding.php (1)
__construct(24-26)demo/03-injection-point.php (3)
__construct(24-27)__construct(39-42)__construct(47-50)demo/02b-named-by-named.php (1)
__construct(29-31)demo/finder/MovieLister.php (1)
__construct(9-11)
demo/finder/DbFinder.php (2)
demo/01b-linked-binding-setter-injection.php (1)
Inject(27-31)demo/finder/MovieLister.php (1)
Inject(13-16)
🪛 PHPMD (2.15.0)
demo/finder/DbFinder.php
14-14: Avoid unused parameters such as '$db'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (15)
.github/workflows/demo-php8.yml (3)
14-14: LGTM: Updated to actions/checkout@v4Good modernization to use the latest stable version of the checkout action.
25-25: LGTM: Replaced deprecated set-output commandCorrectly updated from the deprecated
::set-outputcommand to the modern$GITHUB_OUTPUTfile approach.
28-28: LGTM: Updated to actions/cache@v4Good modernization to use the latest stable version of the cache action.
demo/01a-linked-binding.php (1)
24-26: LGTM: Excellent use of PHP 8 constructor property promotionThe refactoring from traditional property declaration and assignment to constructor property promotion is well-executed. This modernization reduces boilerplate code while maintaining the same functionality and is consistent with similar updates across other demo files.
demo/finder/DbFinder.php (1)
13-13: LGTM: Successful migration to PHP 8 attributesGood modernization from PHPDoc
@Injectto native PHP 8#[Inject]attribute. This change is consistent with the broader codebase modernization and improves type safety integration with static analysis tools.demo/tmp/FinderInterface-.php (1)
1-10: Generated DI container code follows expected patterns.This appears to be generated dependency injection container code that correctly implements the expected pattern: constructor injection, setter injection, singleton flag configuration, and instance return. The structure is appropriate for compiled DI containers.
demo/tmp/MovieListerInterface-.php (1)
1-9: Generated DI container code is structurally correct.This generated dependency injection container code follows the same correct pattern as other generated files: constructor injection, setter injection with multiple dependencies, and proper instance configuration.
demo/01b-linked-binding-setter-injection.php (2)
25-25: Excellent type safety improvement.Adding the
FinderInterfacetype declaration to the property enhances type safety and provides better IDE support. This aligns well with the overall modernization effort across the demo files.
27-27: Good modernization to PHP 8 attributes.The migration from
@Injectdocblock annotation to#[Inject]attribute follows modern PHP 8 practices and provides better language-level support for dependency injection metadata. This change is consistent with similar updates throughout the demo codebase.demo/finder/MovieFinder.php (2)
6-6: Necessary import for PHP 8 attribute usage.Adding the import for
Ray\Di\Di\Injectis required to support the new attribute syntax used in the method parameter.
10-10: Modern parameter-level dependency injection attribute.The migration from
@Assisted({"finder"})docblock to#[Inject]attribute directly on the parameter follows PHP 8 best practices for dependency injection metadata. This approach provides better language-level support and is consistent with the modernization efforts across the demo codebase.demo/tmp/MovieFinder_4221959595.php (1)
8-14: Well-structured AOP proxy class.The proxy class correctly implements the weaving interface and delegates method calls to the intercept mechanism. The PHP 8 attribute syntax and union types are properly used.
demo/02a-named-by-qualifier.php (3)
25-25: Good addition of strict typing.Adding explicit
FinderInterfacetyping to the property enhances type safety and aligns with the PR's goal of improving type safety.
27-27: Excellent modernization to PHP 8 attributes.The migration from docblock annotations to native PHP 8 attributes (
#[Legacy]) is well-executed and consistent with the modernization effort across other demo files likedemo/02b-named-by-named.php.
33-36: Properly structured PHP 8 attribute definition.The
Legacyqualifier is correctly defined as a PHP 8 attribute with appropriate target and qualifier traits. This modernization enhances static analysis capabilities.
- Remove all generated cache and temporary files from demo/tmp/ - Keep only .gitkeep file to preserve directory structure - Remove demo/error.log file - Ensure clean repository state without build artifacts 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove duplicate SetterMethodList and ArgumentList type aliases from Types.php, keeping the standardized SetterMethodsList and ArgumentsList variants. Update VisitorInterface.php to use consistent ArgumentsList type. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/demo.yml (1)
28-32: Also pinactions/cacheto a SHA
Same security rationale as withactions/checkout.- uses: actions/cache@v4 + uses: actions/cache@v4.0.2 # replace with the current commit SHA
🧹 Nitpick comments (1)
.github/workflows/demo.yml (1)
25-26: Quote$GITHUB_OUTPUTto avoid issues with path expansion
If the runner ever setsGITHUB_OUTPUTto a path containing spaces, the current command will fail. Quoting the variable is a one-liner fix.- run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/demo.yml(3 hunks)src/di/Types.php(1 hunks)src/di/VisitorInterface.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/di/VisitorInterface.php
- src/di/Types.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: koriym
PR: ray-di/Ray.Di#297
File: composer.json:17-17
Timestamp: 2025-02-22T08:34:22.655Z
Learning: The use of "ray/aop": "^2.x-dev" in composer.json is intentional as it depends on features from the upcoming version that hasn't been released yet.
📚 Learning: php 8.4 has been released and is supported in github actions....
Learnt from: koriym
PR: ray-di/Ray.Di#293
File: .github/workflows/continuous-integration.yml:13-13
Timestamp: 2024-11-29T13:50:16.707Z
Learning: PHP 8.4 has been released and is supported in GitHub Actions.
Applied to files:
.github/workflows/demo.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sa / Psalm
🔇 Additional comments (1)
.github/workflows/demo.yml (1)
38-38: Verify updated demo path exists in the repository
The script path changed fromdemo-php8/run.phptodemo/run.php. Please confirm that the file has indeed been moved/renamed in this PR to avoid a runtime “file not found” failure.
koriym
force-pushed
the
feature/psalm-domain-types
branch
from
10363e7 to
d016ec3
Compare
|
1 participant
Summary
Changes
Test plan
🤖 Generated with Claude Code
Summary by Sourcery
Enhance type safety across the Ray.Di dependency injection framework by defining reusable Psalm type aliases, updating docblocks to import those types, and fixing static analysis issues reported by Psalm and PHPStan.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
Summary by CodeRabbit
Documentation
New Features
Refactor
Style
Chores
Tests
Bug Fixes