Skip to content

Commit

Permalink
trackCallsOnMixed -> trackMixedAccess
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Nov 15, 2024
1 parent 1252245 commit ff2b46c
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 26 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class UserFacade
```neon
parameters:
shipmonkDeadCode:
trackCallsOnMixed: false
trackMixedAccess: false
```

- If you want to check how many of those cases are present in your codebase, you can run PHPStan analysis with `-vvv` and you will see some diagnostics:
Expand Down
16 changes: 10 additions & 6 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,20 @@ services:
tags:
- phpstan.collector
arguments:
trackCallsOnMixed: %shipmonkDeadCode.trackCallsOnMixed%
trackMixedAccess: %shipmonkDeadCode.trackMixedAccess%

-
class: ShipMonk\PHPStan\DeadCode\Collector\ClassDefinitionCollector
class: ShipMonk\PHPStan\DeadCode\Collector\ConstantFetchCollector
tags:
- phpstan.collector
arguments:
trackMixedAccess: %shipmonkDeadCode.trackMixedAccess%

-
class: ShipMonk\PHPStan\DeadCode\Collector\ConstantFetchCollector
class: ShipMonk\PHPStan\DeadCode\Collector\ClassDefinitionCollector
tags:
- phpstan.collector

-
class: ShipMonk\PHPStan\DeadCode\Collector\EntrypointCollector
tags:
Expand All @@ -78,12 +82,12 @@ services:
- phpstan.diagnoseExtension
arguments:
reportTransitivelyDeadMethodAsSeparateError: %shipmonkDeadCode.reportTransitivelyDeadMethodAsSeparateError%
trackCallsOnMixed: %shipmonkDeadCode.trackCallsOnMixed%
trackMixedAccess: %shipmonkDeadCode.trackMixedAccess%


parameters:
shipmonkDeadCode:
trackCallsOnMixed: true
trackMixedAccess: true
reportTransitivelyDeadMethodAsSeparateError: false
entrypoints:
vendor:
Expand All @@ -101,7 +105,7 @@ parameters:

parametersSchema:
shipmonkDeadCode: structure([
trackCallsOnMixed: bool()
trackMixedAccess: bool()
reportTransitivelyDeadMethodAsSeparateError: bool()
entrypoints: structure([
vendor: structure([
Expand Down
10 changes: 8 additions & 2 deletions src/Collector/ConstantFetchCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,15 @@ class ConstantFetchCollector implements Collector

private ReflectionProvider $reflectionProvider;

public function __construct(ReflectionProvider $reflectionProvider)
private bool $trackMixedAccess;

public function __construct(
ReflectionProvider $reflectionProvider,
bool $trackMixedAccess
)
{
$this->reflectionProvider = $reflectionProvider;
$this->trackMixedAccess = $trackMixedAccess;
}

public function getNodeType(): string
Expand Down Expand Up @@ -173,7 +179,7 @@ private function getDeclaringTypesWithConstant(
}
}

if ($result === []) { // TODO trackFetchesOnMixed
if ($this->trackMixedAccess && $result === []) {
$result[] = null; // call over unknown type
}

Expand Down
8 changes: 4 additions & 4 deletions src/Collector/MethodCallCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ class MethodCallCollector implements Collector
*/
private array $callsBuffer = [];

private bool $trackCallsOnMixed;
private bool $trackMixedAccess;

public function __construct(bool $trackCallsOnMixed)
public function __construct(bool $trackMixedAccess)
{
$this->trackCallsOnMixed = $trackCallsOnMixed;
$this->trackMixedAccess = $trackMixedAccess;
}

public function getNodeType(): string
Expand Down Expand Up @@ -261,7 +261,7 @@ private function getDeclaringTypesWithMethod(
}
}

if ($this->trackCallsOnMixed) {
if ($this->trackMixedAccess) {
$canBeObjectCall = !$typeNoNull->isObject()->no() && !$isStaticCall->yes();
$canBeClassStringCall = !$typeNoNull->isClassString()->no() && !$isStaticCall->no();

Expand Down
8 changes: 4 additions & 4 deletions src/Rule/DeadCodeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class DeadCodeRule implements Rule, DiagnoseExtension

private bool $reportTransitivelyDeadAsSeparateError;

private bool $trackCallsOnMixed;
private bool $trackMixedAccess;

/**
* memberKey => DeadMember
Expand All @@ -113,12 +113,12 @@ class DeadCodeRule implements Rule, DiagnoseExtension
public function __construct(
ClassHierarchy $classHierarchy,
bool $reportTransitivelyDeadMethodAsSeparateError,
bool $trackCallsOnMixed
bool $trackMixedAccess
)
{
$this->classHierarchy = $classHierarchy;
$this->reportTransitivelyDeadAsSeparateError = $reportTransitivelyDeadMethodAsSeparateError;
$this->trackCallsOnMixed = $trackCallsOnMixed;
$this->trackMixedAccess = $trackMixedAccess;
}

public function getNodeType(): string
Expand Down Expand Up @@ -622,7 +622,7 @@ private function isNeverReportedAsDead(BlackMember $blackMember): bool

public function print(Output $output): void
{
if ($this->mixedMemberUses === [] || !$output->isDebug() || !$this->trackCallsOnMixed) {
if ($this->mixedMemberUses === [] || !$output->isDebug() || !$this->trackMixedAccess) {
return;
}

Expand Down
12 changes: 7 additions & 5 deletions tests/Rule/DeadCodeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
class DeadCodeRuleTest extends RuleTestCase
{

private bool $trackMixedCalls = true;
private bool $trackMixedAccess = true;

private bool $emitErrorsInGroups = true;

Expand Down Expand Up @@ -70,8 +70,8 @@ protected function getCollectors(): array
return [
new EntrypointCollector($this->getEntrypointProviders()),
new ClassDefinitionCollector(),
new MethodCallCollector($this->trackMixedCalls),
new ConstantFetchCollector(self::createReflectionProvider()),
new MethodCallCollector($this->trackMixedAccess),
new ConstantFetchCollector(self::createReflectionProvider(), $this->trackMixedAccess),
];
}

Expand Down Expand Up @@ -109,12 +109,14 @@ private function doTestDead($files, ?int $lowestPhpVersion = null): void
public function testMixedCallsTracked(): void
{
$this->analyseFiles([__DIR__ . '/data/methods/mixed/tracked.php']);
$this->analyseFiles([__DIR__ . '/data/constants/mixed/tracked.php']);
}

public function testMixedCallsNotTracked(): void
{
$this->trackMixedCalls = false;
$this->trackMixedAccess = false;
$this->analyseFiles([__DIR__ . '/data/methods/mixed/untracked.php']);
$this->analyseFiles([__DIR__ . '/data/constants/mixed/untracked.php']);
}

public function testDiagnoseMixedCalls(): void
Expand Down Expand Up @@ -320,7 +322,7 @@ public static function provideFiles(): iterable
yield 'const-function' => [__DIR__ . '/data/constants/constant-function.php'];
yield 'const-dynamic' => [__DIR__ . '/data/constants/dynamic.php'];
yield 'const-expr' => [__DIR__ . '/data/constants/expr.php'];
yield 'const-mixed' => [__DIR__ . '/data/constants/mixed.php'];
yield 'const-mixed' => [__DIR__ . '/data/constants/mixed/tracked.php'];
yield 'const-override' => [__DIR__ . '/data/constants/override.php'];
yield 'const-static' => [__DIR__ . '/data/constants/static.php'];
yield 'const-traits-1' => [__DIR__ . '/data/constants/traits-1.php'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ function __construct($mixed, string $notClass, object $object, IFace $iface, int

function testMethodExists(Iface $iface)
{
if (method_exists($iface, 'someMethod')) {
echo $iface::SOME_CONST; // does not not mark Clazz
}

echo $iface::SOME_CONST; // does not not mark Clazz
echo $iface::CONST6; // not defined on Iface, but should mark used on its implementations but not on unrelated Clazz
}
}
Expand Down
64 changes: 64 additions & 0 deletions tests/Rule/data/constants/mixed/untracked.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php declare(strict_types = 1);

namespace DeadConstMixed2;


class Clazz {

const CONST1 = 1; // error: Unused DeadConstMixed2\Clazz::CONST1
const CONST2 = 1; // error: Unused DeadConstMixed2\Clazz::CONST2
const CONST3 = 1; // error: Unused DeadConstMixed2\Clazz::CONST3
const CONST4 = 1; // error: Unused DeadConstMixed2\Clazz::CONST4
const CONST5 = 1;
const CONST6 = 1; // error: Unused DeadConstMixed2\Clazz::CONST6

const SOME_CONST = 1; // error: Unused DeadConstMixed2\Clazz::SOME_CONST

}

interface IFace {

const CONST1 = 1; // error: Unused DeadConstMixed2\IFace::CONST1
const CONST2 = 1; // error: Unused DeadConstMixed2\IFace::CONST2
const CONST3 = 1; // error: Unused DeadConstMixed2\IFace::CONST3
const CONST4 = 1;
const CONST5 = 1; // error: Unused DeadConstMixed2\IFace::CONST5

}

class Implementor implements IFace {

const CONST1 = 1; // error: Unused DeadConstMixed2\Implementor::CONST1
const CONST2 = 1; // error: Unused DeadConstMixed2\Implementor::CONST2
const CONST3 = 1; // error: Unused DeadConstMixed2\Implementor::CONST3
const CONST4 = 1;
const CONST5 = 1; // error: Unused DeadConstMixed2\Implementor::CONST5
const CONST6 = 1;

}

class Tester
{
function __construct($mixed, string $notClass, object $object, IFace $iface, int|Clazz $maybeClass)
{
echo $mixed::CONST1; // may be valid

if (!$mixed instanceof Clazz) {
echo $mixed::CONST2; // ideally, should mark only IFace, not Clazz (not implemented)
}

echo $object::CONST3;
echo $iface::CONST4;
echo $maybeClass::CONST5; // mark only Clazz, not IFace

$this->testMethodExists();
}

function testMethodExists(Iface $iface)
{
echo $iface::SOME_CONST; // does not not mark Clazz
echo $iface::CONST6; // not defined on Iface, but should mark used on its implementations but not on unrelated Clazz
}
}

new Tester();

0 comments on commit ff2b46c

Please sign in to comment.