Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Php81] Add NullToStrictStringFuncCallArgRector #1655

Merged
merged 37 commits into from
Jan 13, 2022

Conversation

samsonasik
Copy link
Member

Add new rule to change null to strict string defined function call args

Fixes rectorphp/rector#6910

@samsonasik samsonasik marked this pull request as draft January 10, 2022 08:55
@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch 2 times, most recently from 4e76f87 to 2898bfb Compare January 10, 2022 09:07
@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from 655782e to 70bae0b Compare January 13, 2022 01:10
@samsonasik
Copy link
Member Author

update: implemented for non-named args.

@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from 50933f6 to ed049e3 Compare January 13, 2022 03:47
@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from 636c8c8 to be98cf0 Compare January 13, 2022 04:46
@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from c5a2e47 to 6f5cd0c Compare January 13, 2022 05:56
@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from dca159e to 53038c6 Compare January 13, 2022 06:02
@samsonasik
Copy link
Member Author

update: implemented for named args as well.

@samsonasik samsonasik marked this pull request as ready for review January 13, 2022 08:08
@samsonasik
Copy link
Member Author

@TomasVotruba it seems can be multi position in single function, eg on str_contains, it can pass both 1st and 2nd arg as null in php 8.0, but got deprecated in both in php 8.1, ref https://3v4l.org/pLBuJ

We possibly need to use type detection from parameter if it got string, need to pass as string in the loop:

$positions = [];
foreach ($parametersAcceptor->getParameters() as $position => $parameterReflection) {
            if ($parameterReflection->getType() === 'PHPStan\Type\StringType) {
                $positions[] = $position;
            }
        }

@samsonasik
Copy link
Member Author

implemented multi positions 🎉

@samsonasik
Copy link
Member Author

Apply multi positions seems cause error in CI https://github.com/rectorphp/rector-src/runs/4801793600?check_suite_focus=true

@samsonasik
Copy link
Member Author

CI notice resolved with check args in position exists as Expr 05eb2de

@samsonasik
Copy link
Member Author

It seems still error:

bin/rector process config utils scoper.php --ansi --no-progress-bar
  bin/rector process config utils scoper.php --ansi --no-progress-bar
  shell: /usr/bin/bash -e {0}
  env:
    COMPOSER_ROOT_VERSION: dev-main

                                                                                
 [ERROR] Could not process "scoper.php" file, due to:                           
         "System error: "Internal error."                                       
         Run Rector with "--debug" option and post the report here:             
         
https://github.com/rectorphp/rector/issues/new
". On line: 38           
                                                                               

@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from 8210d1a to 4a8b715 Compare January 13, 2022 11:04
@samsonasik
Copy link
Member Author

@TomasVotruba I rolled back multi positions automatically by string type check as it is not reliable.

I will update to support define array of arg name, something like:

    private const ARG_POSITION_NAME_NULL_TO_STRICT_STRING = [
        'preg_split' => ['subject'],
        'preg_match' => ['subject'],
        'preg_match_all' => ['subject'],
        'explode' => ['string'],
        'strlen' => ['string'],
       'str_contains' => ['haystack', 'needle'],
    ];

@samsonasik
Copy link
Member Author

implemented define multi positions per function defined 🎉

@samsonasik samsonasik force-pushed the add-null-to-strict-string-func-call-arg branch from f31345f to 70de008 Compare January 13, 2022 11:38
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba
Copy link
Member

Thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants