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

Kill startLine & endLine mutants #1407

Merged
merged 2 commits into from
Mar 18, 2024
Merged

Kill startLine & endLine mutants #1407

merged 2 commits into from
Mar 18, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Mar 13, 2024

lets see how nikic/PHP-Parser#985 affects mutations

refs #1382

@staabm
Copy link
Contributor Author

staabm commented Mar 13, 2024

@kukulich it looks like it doesn't improve mutations. I guess phpdoc-types are not useful for that (and only native types are taken into account?)

@Ocramius
Copy link
Member

Phpdoc types are considered: need to likely checked in more depth 🤔

Thanks for trying though!

@kukulich
Copy link
Collaborator

@staabm

I think this can work:

$startLine = $node->getStartLine()
if ($startLine !== -1) {
    $this->startLine = $startLine;
}

No assert -> no mutation and no mutant :) Psalm/Phpstan should be happy too thanks to your change in PhpParser.

The current solution probably does not work because if ($node->hasAttribute('startLine')) { does not tell Psalm that $node->getStartLine() is only positive-int. That's why the assert is not reported by Psalm as useless.

@staabm
Copy link
Contributor Author

staabm commented Mar 13, 2024

Thank you. I will give your idea a try tomorrow

(Btw: the php-parser PR was merged)

@staabm
Copy link
Contributor Author

staabm commented Mar 14, 2024

the psalm error is a bug, reported in vimeo/psalm#10826

@kukulich
Copy link
Collaborator

It looks it's very complicated to kill one mutant 😄

@staabm
Copy link
Contributor Author

staabm commented Mar 14, 2024

It looks it's very complicated to kill one mutant 😄

hehe - agree, its a lot of change. but I feel the code is easier to read after the PR.

@kukulich
Copy link
Collaborator

I feel the code is easier to read after the PR.

I agree :)

@staabm staabm changed the title try nikic-parser dev branch Kill startLine & endLine mutants Mar 14, 2024
@staabm staabm marked this pull request as ready for review March 14, 2024 17:46
@staabm
Copy link
Contributor Author

staabm commented Mar 14, 2024

@Ocramius mutants will be killed on the next php-parser release. I think its good to go.

<code><![CDATA[traverse]]></code>
</ImpureMethodCall>
<PropertyTypeCoercion>
Copy link
Contributor Author

@staabm staabm Mar 14, 2024

Choose a reason for hiding this comment

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

these baselined errors are related to #1407 (comment)

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

- 4164 mutations were generated:
+ 4158 mutations were generated:
-    3333 mutants were killed
+    3329 mutants were killed
       0 mutants were configured to be ignored
      14 mutants were not covered by tests
-      10 covered mutants were not detected
+       8 covered mutants were not detected
       6 errors were encountered
       0 syntax errors were encountered
      14 time outs were encountered
     787 mutants required more time than configured

Also here, seems like a very good improvement with code reduction!

@Ocramius Ocramius self-assigned this Mar 18, 2024
@Ocramius Ocramius added enhancement dependencies Pull requests that update a dependency file labels Mar 18, 2024
@Ocramius Ocramius added this to the 6.28.0 milestone Mar 18, 2024
@Ocramius Ocramius merged commit 148b2d1 into Roave:6.28.x Mar 18, 2024
23 checks passed
@staabm staabm deleted the parser-pr branch March 18, 2024 17:42
@Ocramius
Copy link
Member

Thanks @staabm!

@Ocramius
Copy link
Member

Btw: notice the reduction in mutations: that really says a lot whenever we refactor :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants