Skip to content

Conversation

@AlexSkrypnyk
Copy link
Member

@AlexSkrypnyk AlexSkrypnyk commented Dec 3, 2025

Closes #2151

Summary by CodeRabbit

  • Chores

    • Added code quality validation rules for standardized type hint formatting.
  • Style

    • Standardized formatting of type declarations across the codebase to align with updated code style guidelines.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

This PR enforces a new PHPCS coding standard requiring no spaces around union type operators (|) in PHP. The changes add the SlevomatCodingStandard.TypeHints.DNFTypeHintFormat rule to three PHPCS configuration files and update method signatures and type hints across source and test files to comply with the standard.

Changes

Cohort / File(s) Summary
PHPCS Configuration
phpcs.xml, .vortex/installer/phpcs.xml, .vortex/tests/phpcs.xml
Added new rule SlevomatCodingStandard.TypeHints.DNFTypeHintFormat with property withSpacesAroundOperators set to "no" to enforce no spaces around union type operators across the codebase.
Installer Source Files
.vortex/installer/src/Command/InstallCommand.php, .vortex/installer/src/Runner/AbstractRunner.php, .vortex/installer/src/Runner/CommandRunner.php, .vortex/installer/src/Runner/RunnerInterface.php, .vortex/installer/src/Task/TaskOutput.php
Removed spaces around union type operators in method signatures and type hints (e.g., string | arraystring|array, string | iterablestring|iterable) to comply with the new PHPCS standard.
Test Files
.vortex/installer/tests/Functional/Command/InstallCommandTest.php, .vortex/installer/tests/Functional/FunctionalTestCase.php, .vortex/installer/tests/Unit/Runner/AbstractRunnerTest.php
Updated test method signatures and type annotations to remove spaces around union type operators for consistency with the new code style standard.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~8 minutes

  • Changes are purely formatting and configuration with no behavioral modifications
  • Consistent, repetitive pattern applied mechanically across all files
  • PHPCS rule additions are straightforward and low-risk
  • No complex logic, control flow, or edge cases to evaluate

Areas for attention:

  • Verify the PHPCS rule is syntactically correct and properly placed in all three configuration files
  • Confirm all union type instances across the affected files have been consistently updated

Possibly related PRs

Poem

A rabbit hops through union types with glee, 🐰
No spaces around the pipes—clean as can be!
From config to code, the standard runs true,
Slevomat says the spacing won't do.
Formatted code, so tidy and tight,
The style guide's enforced: perfection in sight! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective of the PR: enforcing no spaces around union types using the Slevomat standard.
Linked Issues check ✅ Passed The PR successfully implements all requirements from #2151: adds the DNFTypeHintFormat rule with withSpacesAroundOperators='no' to all phpcs.xml files and removes spaces around union types throughout the codebase.
Out of Scope Changes check ✅ Passed All changes are directly related to enforcing no spaces around union types; no out-of-scope modifications are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/2151-no-union-type-spaces

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 327934d and 4b00937.

⛔ Files ignored due to path filters (10)
  • .vortex/installer/tests/Fixtures/handler_process/_baseline/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/hosting_acquia/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_claro/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_olivero/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/theme_stark/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests_circleci/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_behat/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/phpcs.xml is excluded by !.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (11)
  • .vortex/installer/phpcs.xml (1 hunks)
  • .vortex/installer/src/Command/InstallCommand.php (2 hunks)
  • .vortex/installer/src/Runner/AbstractRunner.php (1 hunks)
  • .vortex/installer/src/Runner/CommandRunner.php (2 hunks)
  • .vortex/installer/src/Runner/RunnerInterface.php (1 hunks)
  • .vortex/installer/src/Task/TaskOutput.php (2 hunks)
  • .vortex/installer/tests/Functional/Command/InstallCommandTest.php (1 hunks)
  • .vortex/installer/tests/Functional/FunctionalTestCase.php (2 hunks)
  • .vortex/installer/tests/Unit/Runner/AbstractRunnerTest.php (1 hunks)
  • .vortex/tests/phpcs.xml (1 hunks)
  • phpcs.xml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.

Applied to files:

  • .vortex/installer/tests/Unit/Runner/AbstractRunnerTest.php
  • .vortex/installer/tests/Functional/Command/InstallCommandTest.php
  • .vortex/installer/tests/Functional/FunctionalTestCase.php
🧬 Code graph analysis (5)
.vortex/installer/src/Runner/AbstractRunner.php (1)
.vortex/installer/src/Runner/RunnerInterface.php (1)
  • getOutput (67-67)
.vortex/installer/src/Runner/RunnerInterface.php (1)
.vortex/installer/src/Runner/AbstractRunner.php (1)
  • getOutput (188-200)
.vortex/installer/src/Command/InstallCommand.php (1)
.vortex/installer/src/Task/Task.php (1)
  • action (17-68)
.vortex/installer/src/Runner/CommandRunner.php (1)
.vortex/installer/src/Task/TaskOutput.php (2)
  • write (31-36)
  • writeln (46-51)
.vortex/installer/src/Task/TaskOutput.php (1)
.vortex/installer/src/Runner/CommandRunner.php (2)
  • write (109-118)
  • writeln (128-135)
⏰ 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). (10)
  • GitHub Check: build (1)
  • GitHub Check: build (0)
  • GitHub Check: vortex-test-workflow (4)
  • GitHub Check: vortex-test-workflow (3)
  • GitHub Check: vortex-test-workflow (1)
  • GitHub Check: vortex-test-workflow (0)
  • GitHub Check: vortex-test-workflow (2)
  • GitHub Check: vortex-test-common
  • GitHub Check: vortex-test-installer (8.4)
  • GitHub Check: vortex-test-installer (8.3)
🔇 Additional comments (14)
.vortex/tests/phpcs.xml (1)

9-13: LGTM! PHPCS rule correctly configured.

The SlevomatCodingStandard.TypeHints.DNFTypeHintFormat rule has been properly added with withSpacesAroundOperators set to "no", which will enforce the desired formatting standard for union types.

.vortex/installer/phpcs.xml (1)

12-16: LGTM! Rule configuration matches test config.

The PHPCS rule is consistently configured across configuration files, ensuring uniform enforcement of the no-spaces-around-union-types standard in the installer code.

phpcs.xml (1)

16-20: LGTM! Root configuration updated.

The rule is now enforced project-wide, completing the consistent application of the union type formatting standard across the template, installer, and tests.

.vortex/installer/src/Command/InstallCommand.php (1)

178-178: LGTM! Union type formatting updated.

The union types have been correctly updated to remove spaces around the | operator, aligning with the new PHPCS standard. No behavioral changes.

Also applies to: 398-398

.vortex/installer/src/Task/TaskOutput.php (1)

31-31: LGTM! Method signatures updated consistently.

Both public method signatures have been updated to remove spaces around union type operators, maintaining consistency with the OutputInterface contract and the new formatting standard.

Also applies to: 46-46

.vortex/installer/tests/Functional/Command/InstallCommandTest.php (1)

59-59: LGTM! Test mock type hint updated.

The mock callback return type formatting has been updated to match the new standard. No changes to test logic or assertions.

.vortex/installer/src/Runner/AbstractRunner.php (1)

188-188: LGTM! Return type formatting aligned with interface.

The return type has been correctly updated to remove spaces around the union operator, maintaining consistency with the RunnerInterface and the new formatting standard.

.vortex/installer/tests/Unit/Runner/AbstractRunnerTest.php (1)

441-441: LGTM! Test method signature updated.

The test method parameter type has been formatted consistently with the production code changes, ensuring the tests validate the correct type formatting.

.vortex/installer/src/Runner/RunnerInterface.php (1)

67-67: Formatting change approved: union type spacing normalized.

The return type string|array correctly removes spaces around the union operator, aligning with the PR objective and the new Slevomat coding standard. The type hint remains valid and consistent with the documented return type in the docblock.

.vortex/installer/src/Runner/CommandRunner.php (2)

109-109: write() method: union type spacing normalized.

The parameter type string|iterable correctly removes spaces around the union operator. The change is consistent with the docblock documentation and aligns with the Slevomat standard.


128-128: writeln() method: union type spacing normalized.

The parameter type string|iterable correctly removes spaces around the union operator. The change is consistent with the docblock documentation and aligns with the Slevomat standard.

.vortex/installer/tests/Functional/FunctionalTestCase.php (3)

111-111: assertSutContains() method: union type spacing normalized.

The parameter type string|array correctly removes spaces around the union operator, maintaining consistency with the PR's Slevomat standard enforcement.


128-128: assertSutNotContains() method: union type spacing normalized.

The parameter type string|array correctly removes spaces around the union operator, maintaining consistency with the PR's Slevomat standard enforcement.


1-145: Verify PHPCS configuration files were updated per PR objectives.

The provided files show consistent application of the union type spacing formatting. However, the PR objectives specify adding the SlevomatCodingStandard.TypeHints.DNFTypeHintFormat rule to three PHPCS configuration files. These configuration files are not included in this review.

Please confirm that:

  1. The PHPCS rule was added to all three mentioned configuration files
  2. The withSpacesAroundOperators property is set to "no" in each file
  3. Any related PHPCS rules that enforce spacing conventions are not conflicting

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.68%. Comparing base (327934d) to head (4b00937).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2155      +/-   ##
===========================================
- Coverage    76.35%   75.68%   -0.67%     
===========================================
  Files          109      102       -7     
  Lines         5675     5516     -159     
  Branches        44        0      -44     
===========================================
- Hits          4333     4175     -158     
+ Misses        1342     1341       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexSkrypnyk AlexSkrypnyk merged commit 8b4a5be into develop Dec 3, 2025
26 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/2151-no-union-type-spaces branch December 3, 2025 05:29
@github-project-automation github-project-automation bot moved this from BACKLOG to Release queue in Vortex Dec 3, 2025
@AlexSkrypnyk AlexSkrypnyk added this to the 25.11.0 milestone Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Released in 1.34.0

Development

Successfully merging this pull request may close these issues.

Enforce "No Spaces Around Union Types" Using Slevomat Standard

2 participants