Make exception PHPDoc consistent and self-documenting#14
Closed
koriym wants to merge 196 commits into
Closed
Conversation
Serializable provider provider
Removed unnecessary type suppression comments and refined type annotations to enhance code clarity across multiple classes. Updated several development dependencies in the composer.lock to their latest versions, ensuring compatibility and consistency. This update primarily affects internal documentation and tooling without altering functionality.
Add a check in the Injector class to verify that the temporary directory is writable before proceeding with the class directory initialization. Introduced a new exception, DirectoryNotWritable, to handle cases where the directory is not writable, enhancing error handling and code robustness.
Handle missing dependencies in setter methods with @Inject(optional=true)
The continuous integration workflow now supports PHP 8.4 as the current stable version. PHP 8.3 has been moved to the old_stable versions list to ensure compatibility continues with earlier versions.
Upgraded PHPStan and PHPStan-PHPUnit to version 2.0 to ensure compatibility with the latest features and improvements. Additionally, Symfony components have been updated to version 7.2.0 to benefit from bug fixes and performance enhancements. This update maintains alignment with the supported PHP versions and related dependencies.
This commit eliminates redundant assert statements to streamline the code and improve readability. Additionally, it uses `@phpstan-ignore-line` annotations to suppress specific warnings, enhancing compatibility with static analysis tools.
Removed unnecessary assertions such as `assert` and `property_exists` to make the tests cleaner and more efficient. Adjusted array handling by casting `bindings` to an array for better compatibility and added necessary code comments to suppress static analysis warnings where applicable. These changes improve the maintainability and readability of the codebase without affecting the functionality.
The PHPUnit version constraint in composer.json has been updated to "^8.5.40 || ^9.5" from "^8.5.24 || ^9.5" to ensure compatibility with recent bug fixes and improvements. This change will help maintain the stability and reliability of the development and testing environment.
This change updates the "ray/aop" package to version 2.16 in the composer.json file. The update aims to incorporate enhancements, bug fixes, or security improvements that come with the new version, ensuring better performance or compliance with the latest standards.
Update CI to support PHP 8.4 and include PHP 8.3 in old_stable
Add @psalm-pure and @psalm-immutable annotations to various classes to improve static analysis and enforce immutability. These changes ensure better code quality and maintainability by providing more explicit type information for tools like Psalm.
Cleaned up code by removing the default value for `Named::$value` and `Inject::$optional`, ensuring they must be explicitly set upon instantiation. The `psalm-pure` annotation was also removed from the `getStringName` function, reflecting the function's possible side effects or method calls. These changes improve code clarity and enforce proper initialization practices.
Corrected the spelling of "LazyInteterface" to "LazyInterface" in all relevant files to ensure consistent naming and prevent potential interface reference errors. This change impacts class implementations and annotations that depended on the misspelled interface name.
Enhance code quality with Psalm annotations
Update license copyright year(s)
The "baseline" script generates static analysis baselines for PHPStan and Psalm. This simplifies managing analysis results and helps suppress existing issues for future incremental improvements.
Introduced baseline files for PHPStan and Psalm to suppress known issues temporarily, improving focus on new errors. Added references to these baselines in the respective configuration files to aid static analysis setup.
Moved Aspect-Oriented Programming (AOP) binding logic to a new `enableAop` method for better modularity and clarity. Replaced direct assertions with a safer method call (`__setBindings`) to streamline the binding process and improve maintainability.
Switched the Ray/AOP dependency from a stable version to the ^2.x-dev branch. This allows for using the latest development changes while maintaining compatibility with the project.
Added stricter type assertion in `AnnotatedClassMethods` with `assert` to ensure constructor type safety. Removed unnecessary suppression comments and improved type annotations for better code reliability and maintainability.
Upgraded multiple packages to ensure compatibility and receive the latest bug fixes and improvements. Adjusted metadata such as branch aliases and autoload paths where necessary. This helps maintain system stability and prepares for future development.
Disabled the `--show-info` output in the `sa` script for psalm to reduce unnecessary verbosity. This ensures the analysis output remains focused on essential issues.
Added a normalization function to standardize line endings to '\n' in string assertions for consistent behavior across environments. Updated the test to apply this normalization to both expected and actual strings.
Upgraded `vimeo/psalm` to `^6.8` and adjusted `phpmetrics` to support version `^2.7 || v3.0.0rc8`. Removed `psalm/plugin-phpunit` as it is no longer required.
- Escape resolved href/data-src values (quotes included) and only link http(s) repositories, so composer.lock-derived URLs can't break the anchor markup or become a javascript: href. - Contain the source-map build in try/catch: a malformed composer.lock (bad JSON via JSON_THROW_ON_ERROR, or unexpected shapes) omits the map instead of crashing the render. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… DI core BindingsHtml is a pure markdown-to-HTML renderer: it imports no Ray\Di class and nothing in the core references it, so it doesn't belong in the Ray\Di namespace next to the DI primitives. The recording classes it sat beside — BindingLog, BindingEvent, BindingsMarkdown — stay in Ray\Di, because Container and ContainerFactory depend on them (moving those would cycle: ADP). - src-bindings/BindingsHtml.php, namespace Ray\Bindings, autoloaded from the same ray/di package (vendor/bin/bindings-html stays built-in for every user) - tests-bindings/BindingsHtmlTest.php, self-contained on a markdown fixture (no Injector/fakes), mirroring the source layout - cs / phpstan / psalm / phpunit configured to cover the new directories Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A binding woven with aspects rendered as one long line that repeated the same
interceptor once per method. The Bindings section now shows each binding as a
ruled row with an indented sub-line that groups the intercepted methods under
each interceptor (by short class name, linked to source):
Interface- => …Null
aop DbQueryInterceptor: listByCustomer, item, put, delete
The markdown is unchanged: this is a viewer-only re-decomposition, keeping the
plain-text record flat for grep/diff while the HTML presents the hierarchy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A toNull() binding resolves to a generated {Interface}{hash}Null class whose
hash is meaningless noise. The viewer now renders such targets as (null object)
— matching the provenance label, dropping the long generated class name (which
also fixes the awkward line wrap on AOP null-object bindings). The exact class
stays in the markdown.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A class bound to itself (bind(X) with no ->to()) renders as X- => (dependency) X. The provenance already labels this (untargeted) via BindingEvent; the viewer now does the same in the Bindings section, matching. The markdown is unchanged — ModuleString still emits the full form (a data-layer change could align it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ModuleString now labels a class bound to itself (X- => (dependency) X) as (untargeted), mirroring BindingEvent so the Bindings section and the provenance agree — no benefit to keeping the redundant self-reference. Covered by a new BindingsMarkdownTest case; ModuleTest::testToString's golden string is updated to match (FakeDoubleInterceptor is self-bound). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add Ray\Di\BindingsHtml: a shared HTML viewer for bindings.md
- static-analysis: 8.4 -> 8.5 - mutation-testing: 8.4 -> 8.5 - coding-standards: 8.3 -> 8.4 - demo: 8.3 -> 8.4 - prefer-lowest: stays at 8.2 (minimum supported) - continuous-integration: unchanged (already tests 8.2-8.5)
Bump CI tooling to PHP 8.5/8.4
Optimize bindings markdown generation
Fix assisted injection re-entry
Each exception leads with its class name stating what went wrong and uses the phpdoc only for what the name cannot carry: a "Message format:" line when the message is just variables, plus the path by which execution reaches the throw and any design reason or recovery behavior. Class names that already say it all (Unbound, CircularDependency, RenameTargetAlreadyBound, UncallableSetterMethod, DirectoryNotWritable, InvalidToConstructorNameParameter) drop the redundant "Thrown when ..." summary line and open directly with the message format. No runtime behavior changes; phpdoc only. The convention is also recorded under Framework Patterns in CLAUDE.md.
|
Important Review skippedToo many files! This PR contains 251 files, which is 101 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (251)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 2 minor |
| Security | 5 critical 38 high |
| CodeStyle | 1 minor |
| Complexity | 2 medium |
🟢 Metrics 158 complexity · 0 duplication
Metric Results Complexity 158 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 2.x #14 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 351 434 +83
============================================
Files 47 51 +4
Lines 911 1130 +219
============================================
+ Hits 911 1130 +219 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rationale
Exception class names should provide the primary summary of a failure. PHPDoc should add only details the name cannot express, such as message structure, execution context, recovery behavior, or compatibility constraints.
This is a documentation-only change with no runtime behavior changes.
Validation
composer test— 269 tests, 459 assertions