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

Improved mutation score #983

Merged
merged 12 commits into from
Jan 20, 2022
Merged

Improved mutation score #983

merged 12 commits into from
Jan 20, 2022

Conversation

kukulich
Copy link
Collaborator

No description provided.

@kukulich kukulich marked this pull request as ready for review January 19, 2022 22:55
src/Reflection/ReflectionClass.php Show resolved Hide resolved
src/Reflection/ReflectionProperty.php Outdated Show resolved Hide resolved
src/Util/ClassExistenceChecker.php Show resolved Hide resolved
src/Util/FileHelper.php Show resolved Hide resolved
test/unit/Fixture/InheritedClassProperties.php Outdated Show resolved Hide resolved
test/unit/Fixture/Methods.php Outdated Show resolved Hide resolved
test/unit/SourceLocator/Type/ClosureSourceLocatorTest.php Outdated Show resolved Hide resolved
test/unit/Util/ClassExistenceCheckerTest.php Outdated Show resolved Hide resolved
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.

Awesome, LGTM! 🚢

Thanks @kukulich!

@Ocramius Ocramius self-assigned this Jan 20, 2022
@Ocramius Ocramius added this to the 5.1.0 milestone Jan 20, 2022
@Ocramius Ocramius merged commit 02e9013 into Roave:5.1.x Jan 20, 2022
@kukulich
Copy link
Collaborator Author

Some of the remaining mutants can probably be ignored, eg. mutants with ?? or ??= operators.

@kukulich kukulich deleted the mutants branch January 20, 2022 14:31
@Ocramius
Copy link
Member

TBH, it always depends: I believe most escaped mutated nullability checks are hidden bugs/unused caches, so we'll need to evaluate case-by-case

@kukulich
Copy link
Collaborator Author

Yes

  • Some are optimization - it works with the change, it's just a little slower
  • Some cannot be broken - eg. some of the ??
  • Some can be broken only with more changes - one mutant is not enough

Ocramius added a commit to Ocramius/psalm that referenced this pull request Feb 1, 2022
Fixes vimeo#7478

As discussed in the upstream issue, `trait_exists()` always returns `bool`: while
it can return `null` when the arguments passed to it do not match (either no arguments, or
3 or more arguments), we do not support that scenario, as that already doesn't respect the
type signature of this function.

We cut to the point: always make it `bool`, which is the scenario that works under healthy
operational conditions.

Ref: Roave/BetterReflection#983 (comment)
Ref: https://psalm.dev/r/c41a43805d
Ref: vimeo#7478 (comment)
Ref: vimeo#7478 (comment)
Ref: https://3v4l.org/XpHmh
Ocramius added a commit to Ocramius/psalm that referenced this pull request Feb 1, 2022
Fixes vimeo#7478

As discussed in the upstream issue, `trait_exists()` always returns `bool`: while
it can return `null` when the arguments passed to it do not match (either no arguments, or
3 or more arguments), we do not support that scenario, as that already doesn't respect the
type signature of this function.

We cut to the point: always make it `bool`, which is the scenario that works under healthy
operational conditions.

Ref: Roave/BetterReflection#983 (comment)
Ref: https://psalm.dev/r/c41a43805d
Ref: vimeo#7478 (comment)
Ref: vimeo#7478 (comment)
Ref: https://3v4l.org/XpHmh
Ocramius added a commit to Ocramius/psalm that referenced this pull request Feb 1, 2022
Fixes vimeo#7478

As discussed in the upstream issue, `trait_exists()` always returns `bool`: while
it can return `null` when the arguments passed to it do not match (either no arguments, or
3 or more arguments), we do not support that scenario, as that already doesn't respect the
type signature of this function.

We cut to the point: always make it `bool`, which is the scenario that works under healthy
operational conditions.

Ref: Roave/BetterReflection#983 (comment)
Ref: https://psalm.dev/r/c41a43805d
Ref: vimeo#7478 (comment)
Ref: vimeo#7478 (comment)
Ref: https://3v4l.org/XpHmh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants