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 #1221

Merged
merged 14 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/NodeCompiler/CompileNodeToValue.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ private function constantExists(string $constantName, CompilerContext $context):
private function getConstantValue(Node\Expr\ConstFetch $node, string|null $constantName, CompilerContext $context): mixed
{
// It's not resolved when constant value is expression
// @infection-ignore-all Assignment, AssignCoalesce: There's no difference, ??= is just optimization
$constantName ??= $this->resolveConstantName($node, $context);

if (defined($constantName)) {
Expand All @@ -227,6 +228,7 @@ private function resolveClassConstantName(Node\Expr\ClassConstFetch $node, Compi
private function getClassConstantValue(Node\Expr\ClassConstFetch $node, string|null $classConstantName, CompilerContext $context): mixed
{
// It's not resolved when constant value is expression
// @infection-ignore-all Assignment, AssignCoalesce: There's no difference, ??= is just optimization
$classConstantName ??= $this->resolveClassConstantName($node, $context);

[$className, $constantName] = explode('::', $classConstantName);
Expand Down
2 changes: 2 additions & 0 deletions src/NodeCompiler/CompilerContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public function getFileName(): string|null
return $this->contextReflection->getFileName();
}

// @infection-ignore-all Coalesce: There's no difference
return $this->getClass()?->getFileName() ?? $this->getFunction()?->getFileName();
}

Expand All @@ -43,6 +44,7 @@ public function getNamespace(): string
return $this->contextReflection->getNamespaceName();
}

// @infection-ignore-all Coalesce: There's no difference
return $this->getClass()?->getNamespaceName() ?? $this->getFunction()?->getNamespaceName() ?? '';
}

Expand Down
1 change: 1 addition & 0 deletions src/Reflection/ReflectionAttribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public function getTarget(): int
$this->owner instanceof ReflectionProperty => Attribute::TARGET_PROPERTY,
$this->owner instanceof ReflectionClassConstant => Attribute::TARGET_CLASS_CONSTANT,
$this->owner instanceof ReflectionEnumCase => Attribute::TARGET_CLASS_CONSTANT,
// @infection-ignore-all InstanceOf_: There's no other option
$this->owner instanceof ReflectionParameter => Attribute::TARGET_PARAMETER,
};
}
Expand Down
1 change: 1 addition & 0 deletions src/Reflection/ReflectionClass.php
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,7 @@ private function addStringableInterface(array $interfaces): array
// Stringable interface does not exist on target PHP version
}

// @infection-ignore-all Break_: There's no difference between break and continue - break is just optimization
break;
}
}
Expand Down
27 changes: 14 additions & 13 deletions src/Reflection/ReflectionClassConstant.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ReflectionClassConstant
/** @var non-empty-string */
private string $name;

private int $modifiers = 0;
private int $modifiers;

private Node\Expr $value;

Expand Down Expand Up @@ -59,14 +59,13 @@ private function __construct(
$name = $node->consts[$positionInNode]->name->name;
assert($name !== '');

$this->name = $name;
$this->value = $node->consts[$positionInNode]->value;
$this->name = $name;
$this->modifiers = $this->computeModifiers($node);
$this->value = $node->consts[$positionInNode]->value;

$this->docComment = GetLastDocComment::forNode($node);
$this->attributes = ReflectionAttributeHelper::createAttributes($reflector, $this, $node->attrGroups);

$this->computeModifiers($node);

$startLine = $node->getStartLine();
assert($startLine > 0);
$endLine = $node->getEndLine();
Expand Down Expand Up @@ -147,7 +146,8 @@ public function isPublic(): bool
*/
public function isPrivate(): bool
{
return ($this->modifiers & CoreReflectionClassConstant::IS_PRIVATE) === CoreReflectionClassConstant::IS_PRIVATE;
// Private constant cannot be final
return $this->modifiers === CoreReflectionClassConstant::IS_PRIVATE;
}

/**
Expand Down Expand Up @@ -259,18 +259,19 @@ public function getAttributesByInstance(string $className): array
return ReflectionAttributeHelper::filterAttributesByInstance($this->getAttributes(), $className);
}

private function computeModifiers(ClassConst $node): void
private function computeModifiers(ClassConst $node): int
{
if ($node->isFinal()) {
$this->modifiers = self::IS_FINAL;
}
$modifiers = $node->isFinal() ? self::IS_FINAL : 0;

if ($node->isPrivate()) {
$this->modifiers += CoreReflectionClassConstant::IS_PRIVATE;
// No += because private constant cannot be final
kukulich marked this conversation as resolved.
Show resolved Hide resolved
$modifiers = CoreReflectionClassConstant::IS_PRIVATE;
} elseif ($node->isProtected()) {
$this->modifiers += CoreReflectionClassConstant::IS_PROTECTED;
$modifiers += CoreReflectionClassConstant::IS_PROTECTED;
} else {
$this->modifiers += CoreReflectionClassConstant::IS_PUBLIC;
$modifiers += CoreReflectionClassConstant::IS_PUBLIC;
}

return $modifiers;
}
}
15 changes: 3 additions & 12 deletions src/Reflection/ReflectionFunctionAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use PhpParser\Node;
use PhpParser\Node\Expr\Yield_ as YieldNode;
use PhpParser\Node\Expr\YieldFrom as YieldFromNode;
use PhpParser\Node\Param as ParamNode;
use PhpParser\Node\Stmt\Throw_ as NodeThrow;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitor\FindingVisitor;
Expand Down Expand Up @@ -101,26 +100,18 @@ public function getParameters(): array
$paramNode,
$this,
$paramIndex,
$this->isParameterOptional($nodeParams, $paramNode, $paramIndex),
$this->isParameterOptional($nodeParams, $paramIndex),
);
}

return $parameters;
}

/** @param list<Node\Param> $parameterNodes */
private function isParameterOptional(array $parameterNodes, ParamNode $parameterNode, int $parameterIndex): bool
private function isParameterOptional(array $parameterNodes, int $parameterIndex): bool
Copy link
Member

Choose a reason for hiding this comment

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

Somehow, roave/backward-compatibility-check picked up this change:

[BC] CHANGED: The parameter $parameterNode of Roave\BetterReflection\Reflection\ReflectionFunctionAbstract#isParameterOptional() changed from PhpParser\Node\Param to a non-contravariant int
[BC] CHANGED: The parameter $parameterNode of Roave\BetterReflection\Reflection\ReflectionFunctionAbstract#isParameterOptional() changed from PhpParser\Node\Param to int
[BC] CHANGED: Parameter 1 of Roave\BetterReflection\Reflection\ReflectionFunctionAbstract#isParameterOptional() changed name from parameterNode to parameterIndex

Ignoring for now, just an interesting bug though: possibly due to abstract class not having many filters on method visibility analysis.

{
if ($parameterNode->variadic) {
return true;
}

if ($parameterNode->default === null) {
return false;
}

foreach ($parameterNodes as $otherParameterIndex => $otherParameterNode) {
if ($otherParameterIndex <= $parameterIndex) {
if ($otherParameterIndex < $parameterIndex) {
continue;
}

Expand Down
6 changes: 3 additions & 3 deletions src/Reflection/ReflectionIntersectionType.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use Roave\BetterReflection\Reflector\Reflector;

use function array_map;
use function array_values;
use function assert;
use function implode;

Expand All @@ -25,12 +24,12 @@ public function __construct(
IntersectionType $type,
) {
/** @var non-empty-list<ReflectionNamedType> $types */
Copy link
Member

Choose a reason for hiding this comment

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

In theory not needed? Should be inferred 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ERROR: MixedPropertyTypeCoercion - src/Reflection/ReflectionIntersectionType.php:33:24 - $this->types expects 'non-empty-list<Roave\BetterReflection\Reflection\ReflectionNamedType>',  parent type `array<array-key, Roave\BetterReflection\Reflection\ReflectionNamedType>` provided (see https://psalm.dev/196)
        $this->types = $types;

Copy link
Member

Choose a reason for hiding this comment

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

We just did an array_map() with a closure with well defined return types, and it failed inference? :O

$types = array_values(array_map(static function (Node\Identifier|Node\Name $type) use ($reflector, $owner): ReflectionNamedType {
$types = array_map(static function (Node\Identifier|Node\Name $type) use ($reflector, $owner): ReflectionNamedType {
$type = ReflectionType::createFromNode($reflector, $owner, $type);
assert($type instanceof ReflectionNamedType);

return $type;
}, $type->types));
}, $type->types);

$this->types = $types;
}
Expand All @@ -48,6 +47,7 @@ public function allowsNull(): bool

public function __toString(): string
{
// @infection-ignore-all UnwrapArrayMap: It works without array_map() as well but this is less magical
return implode('&', array_map(static fn (ReflectionNamedType $type): string => $type->__toString(), $this->types));
}
}
9 changes: 5 additions & 4 deletions src/Reflection/ReflectionMethod.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,17 @@ public function getPrototype(): self
$currentClass = $currentClass->getParentClass();

if ($currentClass === null || ! $currentClass->hasMethod($this->getName())) {
// @infection-ignore-all Break_: There's no difference between break and continue - break is just optimization
break;
}

$prototype = $currentClass->getMethod($this->getName())->findPrototype();

if ($prototype !== null) {
if ($this->isConstructor() && ! $prototype->isAbstract()) {
break;
}
if ($prototype === null) {
break;
}

if (! $this->isConstructor() || $prototype->isAbstract()) {
return $prototype;
}
}
Expand Down
25 changes: 13 additions & 12 deletions src/Reflection/ReflectionProperty.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class ReflectionProperty
/** @var non-empty-string */
private string $name;

private int $modifiers = 0;
private int $modifiers;

private ReflectionNamedType|ReflectionUnionType|ReflectionIntersectionType|null $type;

Expand Down Expand Up @@ -86,13 +86,12 @@ private function __construct(
assert($name !== '');

$this->name = $name;
$this->modifiers = $this->computeModifiers($node);
$this->type = $this->createType($node);
$this->default = $node->props[$positionInNode]->default;
$this->docComment = GetLastDocComment::forNode($node);
$this->attributes = ReflectionAttributeHelper::createAttributes($reflector, $this, $node->attrGroups);

$this->computeModifiers($node);

$startLine = null;
if ($node->hasAttribute('startLine')) {
$startLine = $node->getStartLine();
Expand Down Expand Up @@ -566,22 +565,24 @@ private function assertObject(mixed $object): object
return $object;
}

private function computeModifiers(PropertyNode $node): void
private function computeModifiers(PropertyNode $node): int
{
if ($node->isStatic()) {
$this->modifiers = CoreReflectionProperty::IS_STATIC;
}

if ($node->isReadonly()) {
$this->modifiers += self::IS_READONLY;
$modifiers = CoreReflectionProperty::IS_STATIC;
} elseif ($node->isReadonly()) {
$modifiers = self::IS_READONLY;
} else {
$modifiers = 0;
}

if ($node->isPrivate()) {
$this->modifiers += CoreReflectionProperty::IS_PRIVATE;
$modifiers += CoreReflectionProperty::IS_PRIVATE;
} elseif ($node->isProtected()) {
$this->modifiers += CoreReflectionProperty::IS_PROTECTED;
$modifiers += CoreReflectionProperty::IS_PROTECTED;
} else {
$this->modifiers += CoreReflectionProperty::IS_PUBLIC;
$modifiers += CoreReflectionProperty::IS_PUBLIC;
}

return $modifiers;
}
}
5 changes: 2 additions & 3 deletions src/Reflection/ReflectionUnionType.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Roave\BetterReflection\Reflector\Reflector;

use function array_map;
use function array_values;
use function assert;
use function implode;
use function sprintf;
Expand All @@ -28,12 +27,12 @@ public function __construct(
UnionType $type,
) {
/** @var non-empty-list<ReflectionNamedType|ReflectionIntersectionType> $types */
Copy link
Member

Choose a reason for hiding this comment

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

Should also not be needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ERROR: MixedPropertyTypeCoercion - src/Reflection/ReflectionUnionType.php:36:24 - $this->types expects 'non-empty-list<Roave\BetterReflection\Reflection\ReflectionIntersectionType|Roave\BetterReflection\Reflection\ReflectionNamedType>',  parent type `array<array-key, Roave\BetterReflection\Reflection\ReflectionIntersectionType|Roave\BetterReflection\Reflection\ReflectionNamedType>` provided (see https://psalm.dev/196)
        $this->types = $types;

$types = array_values(array_map(static function (Identifier|Name|IntersectionType $type) use ($reflector, $owner): ReflectionNamedType|ReflectionIntersectionType {
$types = array_map(static function (Identifier|Name|IntersectionType $type) use ($reflector, $owner): ReflectionNamedType|ReflectionIntersectionType {
$type = ReflectionType::createFromNode($reflector, $owner, $type);
assert($type instanceof ReflectionNamedType || $type instanceof ReflectionIntersectionType);

return $type;
}, $type->types));
}, $type->types);

$this->types = $types;
}
Expand Down
2 changes: 1 addition & 1 deletion src/SourceLocator/Ast/Exception/ParseToAstFailure.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static function fromLocatedSource(LocatedSource $locatedSource, Throwable
$fileName = $locatedSource->getFileName();

if ($fileName !== null) {
$additionalInformation .= sprintf(' in file %s', $fileName);
$additionalInformation = sprintf(' in file %s', $fileName);
}

if ($previous instanceof Error) {
Expand Down
1 change: 1 addition & 0 deletions src/SourceLocator/Type/AutoloadSourceLocator.php
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ private function locateConstantByName(string $constantName): array|null
// Note: looking at files in reverse order, since newer files are more likely to have
// defined a constant that is being looked up. Earlier files are possibly related
// to libraries/frameworks that we rely upon.
// @infection-ignore-all UnwrapArrayReverse: Ignore because the result is some with or without array_reverse()
foreach (array_reverse(get_included_files()) as $includedFileName) {
try {
FileChecker::assertReadableFile($includedFileName);
Expand Down
4 changes: 4 additions & 0 deletions test/unit/Fixture/ExampleClass.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ class ExampleClass

public static $publicStaticProperty;

protected static $protectedStaticProperty;

private static $privateStaticProperty;

public function __construct(/** Some doccomment */ private ?int $promotedProperty = 123, $noPromotedProperty = null)
{
}
Expand Down
10 changes: 6 additions & 4 deletions test/unit/Fixture/ExampleClassExport.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Class [ <user> class Roave\BetterReflectionTest\Fixture\ExampleClass ] {
@@ %s/test/unit/Fixture/ExampleClass.php 10-54
@@ %s/test/unit/Fixture/ExampleClass.php 10-58

- Constants [7] {
Constant [ public integer MY_CONST_1 ] { 123 }
Expand All @@ -11,8 +11,10 @@ Class [ <user> class Roave\BetterReflectionTest\Fixture\ExampleClass ] {
Constant [ final protected integer MY_CONST_7 ] { 789 }
}

- Static properties [1] {
- Static properties [3] {
Property [ public static $publicStaticProperty ]
Property [ protected static $protectedStaticProperty ]
Property [ private static $privateStaticProperty ]
}

- Static methods [0] {
Expand All @@ -28,7 +30,7 @@ Class [ <user> class Roave\BetterReflectionTest\Fixture\ExampleClass ] {

- Methods [2] {
Method [ <user, ctor> public method __construct ] {
@@ %s/test/unit/Fixture/ExampleClass.php 47 - 49
@@ %s/test/unit/Fixture/ExampleClass.php 51 - 53

- Parameters [2] {
Parameter #0 [ <optional> ?int $promotedProperty = 123 ]
Expand All @@ -37,7 +39,7 @@ Class [ <user> class Roave\BetterReflectionTest\Fixture\ExampleClass ] {
}

Method [ <user> public method someMethod ] {
@@ %s/test/unit/Fixture/ExampleClass.php 51 - 53
@@ %s/test/unit/Fixture/ExampleClass.php 55 - 57
}
}
}
Loading