Reformat #152
Reformat #152
Conversation
WalkthroughThis update primarily standardizes code style by reordering union types in type hints and adding blank lines after opening PHP tags across many files. Several functions now use multibyte-safe string trimming ( Changes
Sequence Diagram(s)Not applicable: All changes are related to code style, formatting, configuration, or internal implementation details without altering or introducing new control flows. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (71)
✅ Files skipped from review due to trivial changes (30)
🚧 Files skipped from review as they are similar to previous changes (41)
⏰ 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). (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v2 #152 +/- ##
=========================================
Coverage 46.31% 46.31%
Complexity 1250 1250
=========================================
Files 99 99
Lines 3565 3565
=========================================
Hits 1651 1651
Misses 1914 1914 ☔ 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: 1
♻️ Duplicate comments (4)
src/State/Attributes/AllowCreateAll.php (1)
1-2: Same PSR-12 concern as noted earlierSee previous comment regarding the unnecessary blank line after the opening tag.
src/Testing/EntityMock.php (1)
1-2: Same PSR-12 concern as noted earlierSee previous comment regarding the unnecessary blank line after the opening tag.
src/Search/EntityFilter.php (1)
1-2: Same PSR-12 concern as noted earlierSee previous comment regarding the unnecessary blank line after the opening tag.
src/State/ReceivedSet.php (1)
1-2: Same PSR-12 concern as noted earlierSee previous comment regarding the unnecessary blank line after the opening tag.
🧹 Nitpick comments (3)
src/Proxy/ImpureException.php (1)
1-2: Blank line after opening<?phpconflicts with PSR-12PSR-12 explicitly states “There MUST NOT be a blank line after the opening tag.”
If the project intentionally diverges from PSR-12, no action is needed; otherwise remove the extra newline or update your formatter rules.src/Glue/autoload.php (1)
29-38: Replacegotowith a straight-line flow for clarity.
gotoharms readability and can be avoided without changing behaviour.-// fast check for development -if (file_exists(__DIR__ . '/../../vendor/autoload.php')) { - require_once __DIR__ . '/../../vendor/autoload.php'; - goto verify_protocol; -} - -// fast check for standard installs -if (file_exists(__DIR__ . '/../../../../autoload.php')) { - require_once __DIR__ . '/../../../../autoload.php'; - goto verify_protocol; -} - -echo "ERROR: FAILED TO LOCATE AUTOLOADER\n"; -return; - -verify_protocol: +// Locate Composer autoloader (dev / standard installs) +$paths = [ + __DIR__ . '/../../vendor/autoload.php', + __DIR__ . '/../../../../autoload.php', +]; + +foreach ($paths as $path) { + if (file_exists($path)) { + require_once $path; + break; + } +} + +if (!class_exists(\Composer\Autoload\ClassLoader::class, false)) { + echo "ERROR: FAILED TO LOCATE AUTOLOADER\n"; + return; +}This keeps the control flow linear and removes the need for a label.
Also applies to: 51-55
.idea/php.xml (1)
153-154: IDE include-path additions – confirm repo policy.
Committing.ideametadata can be intentional, but it often causes churn across contributors. Double-check that these vendor paths must live in VCS; otherwise consider adding.idea/to.gitignore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (74)
.idea/php.xml(1 hunks)bootstrap.php(2 hunks)pint.json(2 hunks)src/ActivityInfo.php(1 hunks)src/DurableClientInterface.php(1 hunks)src/DurableLogger.php(2 hunks)src/Events/External.php(1 hunks)src/Events/GiveOwnership.php(1 hunks)src/Events/RaiseEvent.php(1 hunks)src/Events/ShareOwnership.php(1 hunks)src/Events/ShareWithRole.php(1 hunks)src/Events/ShareWithUser.php(1 hunks)src/Events/Shares/Mode.php(1 hunks)src/Events/Shares/NeedsSource.php(1 hunks)src/Events/Shares/NeedsTarget.php(1 hunks)src/Events/Shares/Operation.php(1 hunks)src/Events/TargetType.php(1 hunks)src/Events/WithPriority.php(1 hunks)src/Gateway/Graph/AstType.php(1 hunks)src/Gateway/Graph/AstVisitor.php(2 hunks)src/Gateway/Graph/GraphGenerator.php(2 hunks)src/Gateway/Graph/MetaParser.php(1 hunks)src/Gateway/Graph/Mode.php(1 hunks)src/Gateway/Graph/SchemaGenerator.php(1 hunks)src/Gateway/Graph/TypeManager.php(3 hunks)src/Glue/Provenance.php(1 hunks)src/Glue/autoload.php(3 hunks)src/OrchestrationContext.php(8 hunks)src/OrchestrationContextInterface.php(1 hunks)src/Proxy/Generator.php(1 hunks)src/Proxy/ImpureException.php(1 hunks)src/Proxy/Pure.php(1 hunks)src/Search/EntityFilter.php(1 hunks)src/SerializedArray.php(1 hunks)src/State/AbstractHistory.php(1 hunks)src/State/Attributes/AllowCreateAll.php(1 hunks)src/State/Attributes/AllowCreateForAuth.php(1 hunks)src/State/Attributes/AllowCreateForRole.php(1 hunks)src/State/Attributes/AllowCreateForUser.php(1 hunks)src/State/Attributes/EntryPoint.php(1 hunks)src/State/Attributes/Operation.php(1 hunks)src/State/Attributes/Tag.php(1 hunks)src/State/EntityHistory.php(1 hunks)src/State/EntrypointLocatorTrait.php(1 hunks)src/State/HistoricalStateTracker.php(4 hunks)src/State/Ids/StateId.php(2 hunks)src/State/OrchestrationHistory.php(1 hunks)src/State/ParameterFillerTrait.php(2 hunks)src/State/ReceivedSet.php(1 hunks)src/State/ResultSet.php(1 hunks)src/State/Session.php(1 hunks)src/Task.php(1 hunks)src/Testing/ActivityMock.php(1 hunks)src/Testing/EntityMock.php(1 hunks)src/Testing/Exceptions/ContinuedAsNew.php(1 hunks)tests/Common/LauncherEntity.php(1 hunks)tests/Common/SayHello.php(1 hunks)tests/Feature/ClientProxyTest.php(2 hunks)tests/Feature/OrchestratorProxyTest.php(1 hunks)tests/PerformanceTests/Bank/Account.php(1 hunks)tests/PerformanceTests/Bank/AccountInterface.php(1 hunks)tests/PerformanceTests/Bank/BankClient.php(1 hunks)tests/PerformanceTests/CounterEntity.php(1 hunks)tests/PerformanceTests/FanOutFanIn.php(1 hunks)tests/PerformanceTests/FanOutFanInClient.php(1 hunks)tests/PerformanceTests/HelloCities/Client.php(1 hunks)tests/PerformanceTests/HelloCities/HelloSequence.php(1 hunks)tests/PerformanceTests/Sequence.php(1 hunks)tests/PerformanceTests/SequenceClient.php(1 hunks)tests/PerformanceTests/bootstrap.php(1 hunks)tests/PerformanceTests/report.php(1 hunks)tests/StopWatch.php(1 hunks)tests/Unit/GraphQlGeneratorTest.php(3 hunks)tests/Unit/TypeTests.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
tests/Feature/OrchestratorProxyTest.php (1)
tests/Feature/ClientProxyTest.php (1)
pureExample(59-62)
src/Gateway/Graph/AstType.php (1)
src/Gateway/Graph/Union.php (1)
Union(5-16)
src/State/ParameterFillerTrait.php (1)
src/State/Serializer.php (2)
Serializer(31-82)deserialize(61-76)
src/State/Ids/StateId.php (4)
cli/ids/id.go (2)
EntityId(134-137)StateId(176-179)src/State/EntityId.php (1)
EntityId(33-51)src/State/OrchestrationInstance.php (1)
OrchestrationInstance(30-45)src/Events/With.php (1)
id(32-39)
src/Gateway/Graph/GraphGenerator.php (1)
src/Gateway/Graph/AstVisitor.php (1)
extractValue(311-322)
src/Gateway/Graph/AstVisitor.php (1)
src/Gateway/Graph/GraphGenerator.php (1)
extractValue(51-63)
src/OrchestrationContext.php (4)
src/OrchestrationContextInterface.php (3)
createTimer(110-110)isReplaying(160-160)entityOp(78-78)src/Testing/DummyOrchestrationContext.php (3)
createTimer(245-251)isReplaying(287-290)entityOp(137-179)src/State/EntityId.php (1)
EntityId(33-51)src/functions.php (1)
EntityId(13-16)
⏰ 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). (2)
- GitHub Check: build-docker
- GitHub Check: Unit Tests - PHP
🔇 Additional comments (77)
tests/StopWatch.php (1)
2-2: Formatting change looks goodAdding a blank line after the opening PHP tag aligns with common PSR-12 formatting conventions. No functional impact.
src/Gateway/Graph/Mode.php (1)
2-2: Aligns with PSR-12 blank-line requirement.Adding the blank line after the opening
<?phptag brings the file into PSR-12 compliance and improves readability.src/Gateway/Graph/SchemaGenerator.php (1)
1-2: Consistent header spacing achievedThe added blank line after
<?phpaligns with PSR-12 recommendations and keeps file headers uniform across the codebase. No functional impact—looks good.src/Gateway/Graph/MetaParser.php (1)
2-2: Blank line after<?phpcould trip PSR-12 lintersPSR-12 §2.2 states “There MUST NOT be a hard line break after the opening tag.”
The added empty line means there are now two consecutive breaks after<?php, which some linters (e.g. PHP_CodeSniffer’sPSR12.OpenTag) will flag.Confirm this aligns with the project’s formatter rules; otherwise consider reverting:
-<?php - +<?phptests/PerformanceTests/CounterEntity.php (1)
1-2: Header whitespace update is correct and PSR-12 compliant
No issues.tests/PerformanceTests/Bank/BankClient.php (1)
1-2: Blank line after<?phpimproves consistency
Change is cosmetic only; looks good.tests/Unit/TypeTests.php (1)
1-2: Formatting tweak accepted
Matches the project‐wide style adjustments in this PR.tests/PerformanceTests/FanOutFanIn.php (1)
1-2: Formatting change confirmed
Keeps file headers uniform; no further action needed.tests/PerformanceTests/FanOutFanInClient.php (1)
1-2: Consistent whitespace – looks good
Purely stylistic; no functional impact.src/State/Attributes/AllowCreateForRole.php (1)
2-2: Stylistic newline looks goodThe added blank line after the opening PHP tag aligns with PSR-12 readability guidelines.
src/State/Attributes/EntryPoint.php (1)
2-2: Stylistic newline looks goodConsistent with the project-wide formatting pass; no functional impact.
src/Events/Shares/Mode.php (1)
2-2: Stylistic newline looks goodKeeps header spacing uniform across source files.
src/State/ResultSet.php (1)
2-2: Stylistic newline looks goodMatches the updated code-style standard; no further feedback.
tests/PerformanceTests/HelloCities/HelloSequence.php (1)
2-2: Stylistic newline looks goodMaintains consistent opening spacing in test files as well.
tests/PerformanceTests/HelloCities/Client.php (1)
1-2: Consistent header spacing looks goodThe added blank line after the opening tag improves readability and brings the file in line with the rest of the codebase.
src/Testing/ActivityMock.php (1)
1-2: Formatting tweak approvedThe single blank line after
<?phpfollows the project-wide style update and introduces no functional changes.src/State/Attributes/Operation.php (1)
1-2: Header whitespace aligned with PSR-12No issues—this minor whitespace adjustment keeps the header consistent across files.
src/State/Session.php (1)
1-2: Style consistency maintainedThe extra blank line is harmless and makes the file header uniform with others.
src/Events/GiveOwnership.php (1)
1-2: Whitespace addition acceptedThe formatting change is correct and introduces no functional impact.
src/Events/ShareWithUser.php (1)
1-2: Blank line addition is fineThe single whitespace line after the opening tag improves readability and aligns with PSR-12. No functional impact.
tests/PerformanceTests/SequenceClient.php (1)
1-2: Consistent header formattingWhitespace after
<?phpmatches the new house style adopted across the codebase. Looks good.src/State/Attributes/AllowCreateForUser.php (1)
1-2: Header whitespace standardisedThe added blank line improves visual separation; no other changes.
src/Task.php (1)
1-2: Whitespace tweak acceptedInsertion of a blank line after the PHP opening tag is stylistic only and does not affect behaviour.
tests/PerformanceTests/Bank/AccountInterface.php (1)
1-2: Header whitespace alignment looks goodKeeps file headers consistent with the rest of the project.
src/Proxy/Pure.php (1)
1-2: PSR-12 newline addition looks goodThe single blank line after the opening tag aligns with common PHP style guides (e.g., PSR-12). No functional impact.
src/Testing/Exceptions/ContinuedAsNew.php (1)
1-2: Consistent formatting appliedSame stylistic newline as elsewhere in the PR; keeps headers uniform.
src/Events/Shares/NeedsTarget.php (1)
1-2: Formatting tweak acknowledgedNewline after
<?phpimproves readability; no concerns.src/State/Attributes/AllowCreateForAuth.php (1)
1-2: Header spacing conforms to project styleBlank line addition is harmless and consistent with other files.
tests/Common/SayHello.php (1)
1-2: Uniform header formattingThe added newline maintains a consistent file layout across the codebase.
src/State/EntrypointLocatorTrait.php (1)
1-2: Consistent header spacing looks goodAdding a blank line after the opening
<?phptag improves readability and aligns with the rest of the codebase’s formatting rules.src/Events/ShareWithRole.php (1)
1-2: Formatting tweak approvedThe extra newline after the PHP open tag is consistent with the new style standard introduced across the project.
src/Events/ShareOwnership.php (1)
1-2: OK – uniform spacing maintainedNo functional impact; the added blank line keeps header spacing uniform throughout the codebase.
tests/PerformanceTests/Bank/Account.php (1)
1-2: Header blank line acceptedMatches the updated formatting convention; no further action required.
tests/PerformanceTests/report.php (1)
1-2: Style update acknowledgedThe newline after
<?phpaligns this file with the standardized style guide.src/SerializedArray.php (1)
1-2: Opening-tag newline aligns with PSR-12Adding a blank line after
<?phpmatches PSR-12/PSR-1 recommendations and keeps this file consistent with the rest of the project.tests/PerformanceTests/bootstrap.php (1)
1-2: Stylistic improvement acknowledgedThe inserted newline after the opening tag brings the test bootstrap in line with the project-wide formatting change.
src/Events/TargetType.php (1)
1-2: Formatting tweak approvedConsistent blank line after the PHP opener; no functional impact.
tests/PerformanceTests/Sequence.php (1)
1-2: Uniform header spacingChange harmonises this test helper with the new style convention; looks good.
src/Events/Shares/Operation.php (1)
1-2: Consistent style – good to mergeThe extra newline satisfies the updated formatting standard without altering behaviour.
tests/Common/LauncherEntity.php (1)
2-2: Style-only change looks goodAdding a blank line after the opening PHP tag aligns with the new project‐wide formatting convention.
src/ActivityInfo.php (1)
2-2: Style-only change looks goodConsistent insertion of a blank line after
<?phpimproves readability and matches the rest of the reformatting effort.src/Events/WithPriority.php (1)
2-2: Style-only change looks goodNo functional impact; keeps the file in line with the updated code style.
src/Events/Shares/NeedsSource.php (1)
2-2: Style-only change looks goodUniform blank line after the opening tag maintains formatting consistency.
src/Glue/Provenance.php (1)
2-2: Style-only change looks goodFormatting is now consistent with other files; no further action needed.
src/Events/External.php (1)
2-2: Header spacing aligned with PSR-12 – looks good.
The blank line after<?phpbrings the file in line with community conventions and the new Pint rule set. No further action needed.bootstrap.php (1)
2-2: Minor style tweaks are fine; verify PHP min-version commitment.
Dropping the empty parentheses (new class) is syntactically valid from PHP 7 onward, but it will parse only if the project guarantees ≥7.x. The repo’sphp_language_level="8.4"setting covers this, so no follow-up required.Also applies to: 27-27
src/Glue/autoload.php (1)
2-2: Consistent header spacing – OK.
Purely cosmetic, keeps headers uniform.src/State/AbstractHistory.php (1)
42-42: Interface order change complies with new Pint rule – no functional impact.src/Gateway/Graph/AstType.php (1)
15-15: LGTM! Union type reordering for consistency.The change from
Union|stringtostring|Unionaligns with the codebase standardization effort. No functional impact since union types are order-independent in PHP.src/State/EntityHistory.php (1)
291-291: LGTM! Union type reordering for consistency.The parameter type change from
int|bool|Eventtobool|Event|intin the array filter callback maintains the same functionality while standardizing union type ordering across the codebase.src/State/OrchestrationHistory.php (1)
155-155: LGTM! Union type reordering for consistency.The parameter type change from
int|bool|Eventtobool|Event|intin the array filter callback maintains identical functionality while aligning with the codebase-wide union type ordering standardization.src/OrchestrationContextInterface.php (1)
78-78: LGTM! Union type reordering for consistency.The parameter type change from
string|EntityIdtoEntityId|stringin theentityOpmethod maintains the same functionality while standardizing union type ordering across the interface declarations.src/DurableClientInterface.php (1)
27-27: LGTM! Interface inheritance reordering for consistency.The change from
OrchestrationClientInterface, EntityClientInterfacetoEntityClientInterface, OrchestrationClientInterfacemaintains identical interface functionality while standardizing interface declaration order across the codebase.src/Proxy/Generator.php (1)
135-135: LGTM! Union type reordering aligns with code style rules.The reordering of union types with
nullplacement at the end is consistent with theordered_typesrule configuration inpint.json.src/Gateway/Graph/GraphGenerator.php (2)
51-51: LGTM! Return type reordering is consistent with codebase style.The return type reordering from
string|int|floattofloat|int|stringaligns with similar methods in related files likeAstVisitor.php.
82-82: LGTM! Parameter type reordering with null placement.The union type reordering places
nullat the end, which is consistent with theordered_typesrule configuration.src/State/ParameterFillerTrait.php (3)
2-2: LGTM! Standard formatting improvement.Adding a blank line after the opening PHP tag is a common formatting standard.
29-29: LGTM! Union type reordering for consistency.The reordering of
\ReflectionFunction|\ReflectionMethodmaintains the same functionality while following consistent type ordering patterns.
31-44: LGTM! Consistent code formatting improvements.The formatting changes add appropriate spacing after control keywords (
foreach,if) which improves code readability and follows standard PHP formatting conventions.src/Gateway/Graph/AstVisitor.php (2)
311-311: LGTM! Consistent return type ordering across similar methods.The return type reordering from
string|int|floattofloat|int|stringmatches the same change made inGraphGenerator.php, maintaining consistency across similar methods.
340-340: LGTM! Union type reordering with proper null placement.Moving
nullto the end of the union type follows theordered_typesrule withnull_adjustment: "always_last"configuration.pint.json (2)
3-5: LGTM! Improved array formatting.The multi-line formatting of the "exclude" array improves readability and follows consistent JSON formatting standards.
8-37: LGTM! Well-chosen formatting rules that support codebase consistency.The new rules added support the formatting improvements seen throughout the PR:
ordered_typeswithnull_adjustment: "always_last"explains the union type reorderings across multiple filesno_unused_importssupports import cleanupordered_interfacesenforces consistent interface ordering- Other rules (
attribute_empty_parentheses,backtick_to_shell_exec,simplified_if_return) are reasonable code quality improvementsThese configuration changes align perfectly with the formatting modifications applied across the codebase.
tests/Unit/GraphQlGeneratorTest.php (3)
77-77: LGTM: Test consistency maintained with source code changes.The switch from
trim()tomb_trim()in test assertions aligns with the corresponding changes in the source code, ensuring tests accurately verify the multibyte-safe string handling.
183-183: LGTM: Consistent multibyte-safe string handling.This change maintains the same pattern as the previous test assertion, ensuring consistent use of
mb_trim()throughout the test suite.
243-243: LGTM: Completing the consistent mb_trim() pattern.All three test assertions now consistently use
mb_trim(), maintaining uniformity across the test suite and alignment with source code changes.tests/Feature/ClientProxyTest.php (2)
42-42: LGTM: Union type ordering standardization.The reordering of
int|floattofloat|intis a cosmetic change that improves code style consistency across the codebase without affecting functionality.
59-59: LGTM: Implementation matches interface declaration.The union type reordering in the implementation correctly matches the interface declaration, maintaining consistency between the two.
src/State/Attributes/Tag.php (1)
14-14: LGTM: Improved multibyte character handling.The switch from
trim()tomb_trim()ensures proper handling of Unicode characters in tag names, making the code more robust for international use cases.tests/Feature/OrchestratorProxyTest.php (1)
45-45: LGTM: Consistent union type ordering across test files.The reordering from
int|floattofloat|intmaintains consistency with the same interface declaration intests/Feature/ClientProxyTest.php, ensuring uniform code style across the test suite.src/Events/RaiseEvent.php (1)
46-46: LGTM: Enhanced multibyte safety for event name processing.The switch from
ltrim()tomb_ltrim()ensures proper handling of Unicode characters when trimming leading underscores from event names, improving robustness for international character sets.src/Gateway/Graph/TypeManager.php (1)
120-121: Verify availability ofmb_trim()at runtime
mb_trim()is not part of ext-mbstring nor the PHP core. Unless a global helper (e.g.src/functions.php) defines it, these calls will raise a fatal error in production.
Double-check that a polyfill is always loaded beforeTypeManageris used (including in CLI tooling and tests), or replace withtrim()plus an explicit multibyte-safe implementation within this class.Also applies to: 147-148, 167-167
src/State/Ids/StateId.php (1)
63-66: Union-type re-ordering only – looks goodThe reordered union types keep the surface API unchanged and preserve contravariance / covariance rules. No action required.
Also applies to: 157-175
src/DurableLogger.php (1)
101-152: No behavioural change – union type reorder is harmlessParameter type unions were reordered for consistency. Implementation and externally-visible behaviour remain the same. 👍
src/OrchestrationContext.php (3)
207-213: Interface/implementation drift forcreateTimer()
OrchestrationContextInterface::createTimer()still accepts onlyDateTimeImmutable, but the implementation now allowsDateInterval|DateTimeImmutable.Code that is type-hinted against the interface cannot pass a
DateInterval, defeating the purpose of widening the parameter. Either:
- Broaden the interface signature to match, or
- Keep the implementation strict and add an overload/utility instead.
Choose one to avoid confusing consumers and static analysers.
491-552: Replay-aware logger overrides – only type-hint shuffleThe overridden logging methods merely reorder the
string|Stringableunion. Behaviour and replay logic remain intact. Nothing further.
567-608: Consistency with interface maintained
entityOp()now matches the interface order (EntityId|string), ensuring Psalm/php-stan don’t report a mismatch. 👍
Performance Metrics
Performance Metrics
|
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Summary by CodeRabbit
Style
Chores
Bug Fixes
Tests