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

feat: Concat does not generate mutant when both operands are the same #1602

Merged

Conversation

michalbundyra
Copy link
Contributor

@michalbundyra michalbundyra commented Oct 29, 2021

Closes #1601

This PR:

@michalbundyra michalbundyra marked this pull request as ready for review October 29, 2021 10:35
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

Looks good.

IMO psalm's issues can be manually ignored in this case.

Also, probably it will be safer to do

- if ($printer->prettyPrint([$node]) !== $printer->prettyPrint([$newNode])) {
+ if ($printer->prettyPrint([clone $node]) !== $printer->prettyPrint([$newNode])) {

since $node potentially can be modified?

@maks-rafalko maks-rafalko enabled auto-merge (squash) October 30, 2021 13:34
auto-merge was automatically disabled October 30, 2021 15:48

Head branch was pushed to by a user without write access

@maks-rafalko
Copy link
Member

could you please ignore these Psalm errors? and we are good to merge

@michalbundyra
Copy link
Contributor Author

@maks-rafalko done

@michalbundyra
Copy link
Contributor Author

Not sure how it should be done then...

with

/** @psalm-suppress ImpureMethodCall */

issues were suppressed, but CS failed...

with:

/* @psalm-suppress ImpureMethodCall */

cs passes, but psalm issues are not suppressed....

@maks-rafalko
Copy link
Member

maks-rafalko commented Oct 31, 2021

then let's do it in psalm.xml:

<?xml version="1.0"?>
<psalm
    errorLevel="8"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    errorBaseline="psalm-baseline.xml">
    <projectFiles>
        <directory name="src/Mutator" />
        <ignoreFiles>
            <directory name="vendor" />
           <directory name="src/PhpParser" />
        </ignoreFiles>
    </projectFiles>

+    <issueHandlers>
+        <ImpureMethodCall>
+            <errorLevel type="suppress">
+                <file name="src/Mutator/Operator/Concat.php"></file>
+            </errorLevel>
+        </ImpureMethodCall>
+    </issueHandlers>
</psalm>

@maks-rafalko maks-rafalko merged commit 1892158 into infection:master Oct 31, 2021
@maks-rafalko
Copy link
Member

maks-rafalko commented Oct 31, 2021

Thank you @michalbundyra

cool first contribution

@michalbundyra michalbundyra deleted the feat/1601-concat-flip-the-same branch October 31, 2021 11:05
@maks-rafalko maks-rafalko added this to the next milestone Nov 10, 2021
@maks-rafalko maks-rafalko modified the milestones: next, 0.26.0 Jan 11, 2022
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.

Empty diff - Concat swap
2 participants