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

Mutator/mbstring #664

Merged
merged 23 commits into from Apr 1, 2019
Merged

Mutator/mbstring #664

merged 23 commits into from Apr 1, 2019

Conversation

majkel89
Copy link
Contributor

@majkel89 majkel89 commented Mar 10, 2019

This PR:

Relates to issue #654

Converts function calls like mb_chr(74, 'utf-8'), mb_strlen('test', 'utf-8') to chr('test'), strlen('test').

Supported functions:

Pay attention to the function mapping when doing code review.

  • mb_chr -> chr
  • mb_ord -> ord
  • mb_parse_str -> parse_str
  • mb_send_mail -> mail
  • mb_strcut -> substr
  • mb_stripos -> stripos
  • mb_stristr -> stristr
  • mb_strlen -> strlen
  • mb_strpos -> strpos
  • mb_strrchr -> strrchr
  • mb_strripos -> strripos
  • mb_strrpos -> strrpos
  • mb_strstr -> strstr
  • mb_strtolower -> strtolower
  • mb_strtoupper -> strtoupper
  • mb_substr_count -> substr_count
  • mb_substr -> substr
  • mb_convert_case -> strtolower, strtoupper, ucwords depending on mode argument

New profile @extensions created to hold this mutator. It is on by default.

Configuration

    "mutators": {
        "MBString": {
            "settings": {
                "mb_strlen": false,
                "mb_ord": false,
            }
        }
    },

src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
@majkel89
Copy link
Contributor Author

majkel89 commented Mar 13, 2019

I will have to test it a little bit longer so that this mutator will not produce mutats that will be killed by differences in mb string functions and std string functions.

@majkel89
Copy link
Contributor Author

majkel89 commented Mar 18, 2019

I have tested the mapping and removed functions that cannot be easily mapped:

  • mb_split - uses regular expressions like ereg functions
  • mb_strrichr - have no equivalent in standard string functions

@majkel89
Copy link
Contributor Author

majkel89 commented Mar 21, 2019

final fixes added and doc PR created. I think this PR is polished enough to be merged

@maks-rafalko maks-rafalko requested review from BackEndTea and sanmai and removed request for BackEndTea Mar 22, 2019
maks-rafalko
maks-rafalko previously approved these changes Mar 22, 2019
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Looks very nice 👍

@majkel89
Copy link
Contributor Author

majkel89 commented Mar 26, 2019

could you merge this PR ?

Copy link
Member

@sanmai sanmai left a comment

The mutator could be simplified if we add a workaround against missing constants to the test.

src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
tests/Mutator/Extensions/MBStringTest.php Show resolved Hide resolved
src/Mutator/Extensions/MBString.php Outdated Show resolved Hide resolved
@sanmai
Copy link
Member

sanmai commented Mar 28, 2019

I'm sorry for going back and forth with this PR, but there isn't a test case for a mixed-case replacement as far as I can see, but please correct me if I overlook.

E.g. a test case displaying that this replacement happens:

- MB_Chr(74);
+ chr(74);

This is customary for other mutators, an example.

Other than that you're doing a beautiful work!

@sanmai sanmai self-requested a review Mar 28, 2019
Ocramius
Ocramius previously approved these changes Mar 29, 2019
Copy link
Member

@BackEndTea BackEndTea left a comment

Great PR,

Only thing i'm missing is that a testcase function call with a variable name is not mutated. (And mostly that it doesn't crash)

e.g.

$a = 'mb_chr'
$a(74)

Should not mutate, and not crash.

sanmai
sanmai approved these changes Mar 30, 2019
@maks-rafalko maks-rafalko added this to the 0.13.0 milestone Apr 1, 2019
@maks-rafalko
Copy link
Member

maks-rafalko commented Apr 1, 2019

Thank you @majkel89 👍

@maks-rafalko maks-rafalko merged commit e25b9d6 into infection:master Apr 1, 2019
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants