Skip to content

Conversation

@MarjovanLier
Copy link
Owner

@MarjovanLier MarjovanLier commented Feb 12, 2025

User description

Summary

This merge request updates the project to require PHP 8.3 (removing support for PHP 8.2) and introduces explicit type declarations for constants in both source and test files. These changes improve overall code clarity, enhance performance by leveraging PHP 8.3 optimisations, and streamline maintenance efforts.

Context and Background

Previously, the project allowed PHP 8.2 or higher. With the release and stabilisation of PHP 8.3, it is advantageous to align our configurations, documentation, and tooling to harness newer features and optimisations. Additionally, the codebase relied on docblocks to describe constant types, leading to potential ambiguity in code maintenance.

Problem Description

  1. PHP 8.2 references: The repository and CI workflows still referenced PHP 8.2, causing potential confusion for developers who have already adopted PHP 8.3.
  2. Implicit constant types: Several constants in the codebase and tests used docblocks or provided no explicit type declarations, reducing type safety and clarity.

Solution Description

  1. PHP 8.3 requirement: All PHP version references, including GitHub workflows, composer.json, phpstan, phan, and Rector configurations, are updated to PHP 8.3. References to PHP 8.2 are removed to maintain consistency.
  2. Explicit type declarations: Constants within both source and test files are prefixed with their explicit type (e.g. private const string, private const array). Redundant docblocks are removed, simplifying maintenance and ensuring consistent code expectations.

List of Changes

  • chore!: Require PHP 8.3, removing support for PHP 8.2
  • style: Add explicit type declarations (e.g. private const string) for constants throughout the codebase
  • style: Remove redundant docblocks and correct minor formatting issues

PR Type

Enhancement, Bug fix


Description

  • Update project to require PHP 8.3, removing PHP 8.2 support.

  • Add explicit type declarations for constants across source and test files.

  • Update configuration files (e.g., .phan/config.php, rector.php) to align with PHP 8.3.

  • Adjust CI workflows and documentation to reflect PHP 8.3 requirement.


Changes walkthrough 📝

Relevant files
Configuration changes
5 files
config.php
Update Phan configuration to target PHP 8.3                           
+2/-2     
rector.php
Update Rector configuration to PHP 8.3                                     
+3/-3     
codecov.yml
Update CI workflow to use PHP 8.3                                               
+2/-2     
php.yml
Remove PHP 8.2 from CI matrix                                                       
+1/-1     
phpstan.neon
Update PHPStan configuration to PHP 8.3                                   
+1/-1     
Enhancement
2 files
AccentNormalization.php
Add explicit type declarations for constants                         
+2/-6     
UnicodeMappings.php
Add explicit type declarations for constants                         
+1/-3     
Formatting
1 files
StringManipulation.php
Minor formatting adjustments in doc comments                         
+2/-2     
Tests
6 files
IsValidDateTest.php
Add explicit type declarations for test constants               
+3/-2     
NameFixTest.php
Adjust test data for name handling                                             
+1/-1     
SearchWordsTest.php
Add explicit type declarations for test constants               
+1/-1     
StrReplaceTest.php
Add explicit type declarations for test constants               
+4/-17   
TrimTest.php
Add explicit type declarations for test constants               
+1/-1     
Utf8AnsiTest.php
Add explicit type declarations for test constants               
+1/-4     
Documentation
1 files
README.md
Update system requirements to PHP 8.3                                       
+1/-1     
Dependencies
1 files
composer.json
Set minimum PHP requirement to 8.3                                             
+1/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Summary by CodeRabbit

    • Chores

      • Upgraded CI workflows and build configurations to target PHP 8.3/8.4, streamlining the testing process.
    • Documentation

      • Updated system requirements to mandate PHP 8.3 or later.
    • Tests

      • Enhanced type declarations in test suites for improved clarity and reliability.
    • Refactor

      • Improved internal code structure for enhanced type safety and maintainability.

    - Remove references to PHP 8.2 in workflows and configuration files
    - Update composer constraints to reflect the new requirement
    - Align development and production environments with PHP 8.3
    
    BREAKING CHANGE: Environments must now use PHP 8.3 or higher.
    - Introduce explicit type declarations for clarity and type safety
    - Remove redundant comments to streamline the code
    - Correct formatting for improved readability and consistency
    
    These changes align the codebase with established style guidelines,
    making it easier to maintain and collaborate on.
    @MarjovanLier MarjovanLier self-assigned this Feb 12, 2025
    @coderabbitai
    Copy link

    coderabbitai bot commented Feb 12, 2025

    📝 Walkthrough

    Walkthrough

    This pull request updates the PHP version used throughout the project from 8.2 to 8.3. The changes include modifications to GitHub Actions workflows (Codecov and CI), configuration files for static analysis and refactoring tools, and documentation updates to reflect the new PHP version requirement. Additionally, various source and test files have updated type declarations for constants to enforce stricter type safety. No alterations to the core business logic or control flow were made.

    Changes

    Files Change Summary
    .github/workflows/codecov.yml, .github/workflows/php.yml Updated CI configurations: switched PHP version from 8.2 to 8.3 (Codecov) and reduced the PHP matrix from ["8.2", "8.3", "8.4"] to ["8.3", "8.4"] in the build workflow.
    .phan/config.php, phpstan.neon, rector.php Upgraded PHP version settings: changed target PHP version from 8.2 to 8.3, including updates in static analysis (PHPStan) and refactoring (Rector) configurations.
    README.md, composer.json Revised PHP requirements: updated the minimum PHP version from 8.2 to 8.3 in system requirements and composer dependency constraints.
    src/AccentNormalization.php, src/UnicodeMappings.php, src/StringManipulation.php Adjusted constant declarations: added explicit array type hints and made minor documentation comment fixes.
    tests/Unit/IsValidDateTest.php, tests/Unit/SearchWordsTest.php, tests/Unit/StrReplaceTest.php, tests/Unit/TrimTest.php, tests/Unit/Utf8AnsiTest.php, tests/Unit/NameFixTest.php Enhanced type declarations: added explicit type hints to constants (string/array) in multiple test classes; a test case in NameFixTest was re-added without functional change.

    Possibly related PRs

    Suggested reviewers

    • qodo-merge-pro

    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.0.3)

    /bin/bash: line 1: /vendor/bin/phpstan: No such file or directory


    Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

    ❤️ Share
    🪧 Tips

    Chat

    There are 3 ways to chat with CodeRabbit:

    • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
      • I pushed a fix in commit <commit_id>, please review it.
      • Generate unit testing code for this file.
      • Open a follow-up GitHub issue for this discussion.
    • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
      • @coderabbitai generate unit testing code for this file.
      • @coderabbitai modularize this function.
    • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
      • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
      • @coderabbitai read src/utils.ts and generate unit testing code.
      • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
      • @coderabbitai help me debug CodeRabbit configuration file.

    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)

    • @coderabbitai pause to pause the reviews on a PR.
    • @coderabbitai resume to resume the paused reviews.
    • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
    • @coderabbitai full review to do a full review from scratch and review all the files again.
    • @coderabbitai summary to regenerate the summary of the PR.
    • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
    • @coderabbitai resolve resolve all the CodeRabbit review comments.
    • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
    • @coderabbitai help to get help.

    Other keywords and placeholders

    • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
    • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
    • Add @coderabbitai anywhere in the PR title to generate the title automatically.

    Documentation and Community

    • Visit our Documentation for detailed information on how to use CodeRabbit.
    • Join our Discord Community to get help, request features, and share feedback.
    • Follow us on X/Twitter for updates and announcements.

    @qodo-code-review
    Copy link
    Contributor

    Changelog updates: 🔄

    2025-02-12 *

    Changed

    • Updated minimum PHP requirement to 8.3
    • Added explicit type declarations for constants
    • Improved code clarity and type safety

    to commit the new content to the CHANGELOG.md file, please type:
    '/update_changelog --pr_update_changelog.push_changelog_changes=true'

    @qodo-code-review
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Improve test case organization

    The test case for "O'reilly" name handling was moved to a different position in
    the test array, which could affect test readability and grouping. Consider
    keeping related test cases together by moving it back near other similar name
    patterns.

    tests/Unit/NameFixTest.php [29-33]

     $names = [
    +    'o'reilly' => "O'reilly",
         'de la hoya' => 'de la Hoya',
         'de la tòrré' => 'de la Torre',
         'donald' => 'Donald',
         'johnson' => 'Johnson',
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    __

    Why: The suggestion addresses a minor organizational issue in test cases. While valid, reordering test cases has minimal impact on functionality and is primarily a readability concern.

    Low

    @qodo-code-review
    Copy link
    Contributor

    Auto-approved PR

    @codecov
    Copy link

    codecov bot commented Feb 12, 2025

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 98.80%. Comparing base (678d6d8) to head (87babd6).

    Additional details and impacted files
    @@            Coverage Diff            @@
    ##               main      #43   +/-   ##
    =========================================
      Coverage     98.80%   98.80%           
      Complexity       26       26           
    =========================================
      Files             1        1           
      Lines            84       84           
    =========================================
      Hits             83       83           
      Misses            1        1           

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

    Copy link

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 0

    🧹 Nitpick comments (7)
    tests/Unit/TrimTest.php (1)

    71-74: Consider removing redundant data provider annotation.

    Since you're using PHP 8+ attributes, the docblock @dataProvider annotation is redundant. The #[DataProvider] attribute is sufficient.

    -    /**
    -     * @dataProvider trimDataProvider
    -     */
         #[DataProvider('trimDataProvider')]

    Additionally, consider using string instead of mixed for the $expected parameter since all test cases return strings:

    -    public function testTrim(string $input, string $characters, mixed $expected): void
    +    public function testTrim(string $input, string $characters, string $expected): void
    tests/Unit/StrReplaceTest.php (1)

    17-17: Consider using more specific array type hints.

    While adding explicit types improves type safety, consider using PHP 8.3's new array shape syntax for more precise type declarations:

    -    private const array SEARCH = [
    +    private const array{0: string, 1: string, 2: string} SEARCH = [
    -    private const array REPLACE = [
    +    private const array{0: string, 1: string, 2: string} REPLACE = [

    Also applies to: 19-19, 25-25, 31-31

    src/UnicodeMappings.php (1)

    28-28: Enhance type safety with array shape type hint.

    Consider using PHP 8.3's array shape syntax to explicitly declare the key-value types:

    -    private const array UTF8_ANSI2 = [
    +    private const array<string, string> UTF8_ANSI2 = [
    tests/Unit/Utf8AnsiTest.php (1)

    17-17: Enhance type safety and maintain consistency.

    1. Use more specific array type hint for better type safety:
    -    private const array UTF8_TO_ANSI_MAP = [
    +    private const array<string, string> UTF8_TO_ANSI_MAP = [
    1. Consider extracting this mapping to a shared constant since it duplicates UTF8_ANSI2 from UnicodeMappings trait.
    src/AccentNormalization.php (1)

    29-29: Consider using more specific array type declarations.

    While the array type declaration is correct, you could leverage PHP 8.3's type system more effectively by using a more specific type hint.

    Apply this diff to improve type specificity:

    -    private const array REMOVE_ACCENTS_FROM = [
    +    private const array<int, string> REMOVE_ACCENTS_FROM = [
    
    -    private const array REMOVE_ACCENTS_TO = [
    +    private const array<int, string> REMOVE_ACCENTS_TO = [

    Also applies to: 273-273

    composer.json (1)

    45-45: Simplify PHP version constraint.

    The current constraint ">=8.3.0|>=8.4.0" can be simplified to just ">=8.3.0" since it already covers PHP 8.4 and future versions.

    -    "php": ">=8.3.0|>=8.4.0"
    +    "php": ">=8.3.0"
    src/StringManipulation.php (1)

    197-197: Fix parameter comment alignment.

    The parameter comments are not aligned correctly according to PER Coding Style 2.0 guidelines.

    -     *                            an empty string.
    +     *                           an empty string.
    -     *                        the expected format of the date.
    +     *                       the expected format of the date.

    Also applies to: 285-285

    🧰 Tools
    🪛 GitHub Check: Codacy Static Code Analysis

    [notice] 197-197: src/StringManipulation.php#L197
    Parameter comment not aligned correctly; expected 27 spaces but found 28

    📜 Review details

    Configuration used: .coderabbit.yaml
    Review profile: CHILL
    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between 678d6d8 and 87babd6.

    📒 Files selected for processing (16)
    • .github/workflows/codecov.yml (1 hunks)
    • .github/workflows/php.yml (1 hunks)
    • .phan/config.php (1 hunks)
    • README.md (1 hunks)
    • composer.json (1 hunks)
    • phpstan.neon (1 hunks)
    • rector.php (1 hunks)
    • src/AccentNormalization.php (2 hunks)
    • src/StringManipulation.php (2 hunks)
    • src/UnicodeMappings.php (1 hunks)
    • tests/Unit/IsValidDateTest.php (2 hunks)
    • tests/Unit/NameFixTest.php (1 hunks)
    • tests/Unit/SearchWordsTest.php (1 hunks)
    • tests/Unit/StrReplaceTest.php (1 hunks)
    • tests/Unit/TrimTest.php (1 hunks)
    • tests/Unit/Utf8AnsiTest.php (1 hunks)
    ✅ Files skipped from review due to trivial changes (3)
    • README.md
    • phpstan.neon
    • tests/Unit/NameFixTest.php
    🧰 Additional context used
    📓 Path-based instructions (1)
    `**/*.php`: Review PHP code for adherence to PER Coding Styl...

    **/*.php: Review PHP code for adherence to PER Coding Style 2.0 guidelines. Ensure proper namespace usage, code organisation, and separation of concerns. Verify that SOLID principles are followed and encourage FOOP techniques—such as employing immutable data, pure functions, and functional composition—to improve maintainability, testability, and performance.

    • src/UnicodeMappings.php
    • tests/Unit/TrimTest.php
    • tests/Unit/Utf8AnsiTest.php
    • tests/Unit/SearchWordsTest.php
    • rector.php
    • tests/Unit/IsValidDateTest.php
    • tests/Unit/StrReplaceTest.php
    • src/AccentNormalization.php
    • src/StringManipulation.php
    🪛 GitHub Check: Codacy Static Code Analysis
    src/StringManipulation.php

    [notice] 197-197: src/StringManipulation.php#L197
    Parameter comment not aligned correctly; expected 27 spaces but found 28


    [notice] 285-285: src/StringManipulation.php#L285
    Parameter comment not aligned correctly; expected 23 spaces but found 24

    ⏰ Context from checks skipped due to timeout of 90000ms (4)
    • GitHub Check: guardrails/scan
    • GitHub Check: Codacy Static Code Analysis
    • GitHub Check: build (8.4)
    • GitHub Check: build (8.3)
    🔇 Additional comments (9)
    tests/Unit/TrimTest.php (3)

    1-9: Well-structured file with proper declarations!

    The file follows PSR standards with strict typing enabled and proper namespace organization.


    11-16: Excellent class declaration with proper metadata!

    The class is correctly marked as final and includes proper PHPDoc annotations for internal usage and coverage tracking.


    18-18: Perfect type declaration for the constant!

    The explicit string type declaration aligns with PHP 8.3's enhanced type system and improves code clarity.

    rector.php (1)

    46-46: LGTM! PHP 8.3 configuration is properly updated.

    The Rector configuration has been correctly updated to PHP 8.3, including all necessary rule sets and level sets.

    Also applies to: 51-51, 56-56

    tests/Unit/SearchWordsTest.php (1)

    17-17: LGTM! Excellent use of PHP 8.3's typed class constant.

    The explicit string type declaration enhances type safety and code clarity.

    tests/Unit/IsValidDateTest.php (1)

    18-20: LGTM! Well-structured typed constants.

    The explicit string type declarations for date format constants improve type safety and self-documentation.

    .phan/config.php (1)

    57-58: LGTM! Appropriate Phan configuration update.

    The target PHP version update aligns correctly with the project's new PHP 8.3 requirement.

    .github/workflows/codecov.yml (1)

    12-15: LGTM! PHP 8.3 setup is correctly configured.

    The PHP version update in the Codecov workflow aligns with the PR objective to upgrade to PHP 8.3.

    .github/workflows/php.yml (1)

    18-18: LGTM! PHP versions matrix is correctly updated.

    The CI workflow now appropriately tests against PHP 8.3 and 8.4, aligning with the project's new minimum version requirement while ensuring forward compatibility.

    @MarjovanLier MarjovanLier merged commit d24e598 into main Feb 12, 2025
    8 of 9 checks passed
    @MarjovanLier MarjovanLier deleted the Package-Updates branch February 12, 2025 20:42
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants