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

Enhancement: Implement InstanceofStrategy #100

Merged
merged 1 commit into from Oct 28, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
@@ -1,6 +1,8 @@
# Changelog

## [Unreleased]
### Added
- Added an `InstanceofStrategy` which detects usages in `instanceof` expressions [#100](https://github.com/composer-unused/composer-unused/pull/100)
### Fixed
- Fixed an issue where `ext-ds` classes where not recognized as used [#88](https://github.com/composer-unused/composer-unused/pull/88)
- Fixed an issue where `extends` and `implements` of FQN was not markes as used [#90](https://github.com/composer-unused/composer-unused/pull/90)
Expand Down
2 changes: 2 additions & 0 deletions src/Parser/PHP/Factory/NodeVisitorFactory.php
Expand Up @@ -9,6 +9,7 @@
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ClassConstStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ExtendsParseStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ImplementsParseStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\InstanceofStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\NewParseStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\PhpExtensionStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\StaticParseStrategy;
Expand All @@ -31,6 +32,7 @@ public function __invoke(ContainerInterface $container): NodeVisitor
),
new ExtendsParseStrategy(),
new ImplementsParseStrategy(),
new InstanceofStrategy(),
], $container->get(ErrorHandlerInterface::class));
}
}
37 changes: 37 additions & 0 deletions src/Parser/PHP/Strategy/InstanceofStrategy.php
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Icanhazstring\Composer\Unused\Parser\PHP\Strategy;

use PhpParser\Node;

final class InstanceofStrategy implements ParseStrategyInterface
{
public function meetsCriteria(Node $node): bool
{
if (!$node instanceof Node\Expr\Instanceof_) {
return false;
}

if (!$node->class instanceof Node\Name) {
return false;
}

return $node->class->isFullyQualified() || $node->class->isQualified();
}

/**
* @param Node&Node\Expr\Instanceof_ $node
* @return array<string>
*/
public function extractNamespaces(Node $node): array
{
/** @var Node\Name $class */
$class = $node->class;

return [
$class->toString(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this method return an array of class names or an array of namespace names?

Copy link
Member

Choose a reason for hiding this comment

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

This should be the FQN of the class.
The method name is changed in #89 to extractSymbolNames which is more fitting.

Copy link

Choose a reason for hiding this comment

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

based on this question I am trying to improve phpdocs to make these point clear:

#111

];
}
}
11 changes: 10 additions & 1 deletion tests/Integration/Parser/PHP/NodeVisitorTest.php
Expand Up @@ -10,6 +10,7 @@
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ClassConstStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ExtendsParseStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ImplementsParseStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\InstanceofStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\NewParseStrategy;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\ParseStrategyInterface;
use Icanhazstring\Composer\Unused\Parser\PHP\Strategy\PhpExtensionStrategy;
Expand Down Expand Up @@ -177,7 +178,15 @@ public function itShouldParseUsagesDataProvider(): array
],
'inputFile' => ASSET_DIR . '/TestFiles/ClassImplements.php',
'strategy' => new ImplementsParseStrategy()
]
],
'Instanceof' => [
'expectedUseNamespaces' => [
'Foo\Bar\Qux',
'Foo\Bar\Quz',
],
'inputFile' => ASSET_DIR . '/TestFiles/Instanceof.php',
'strategy' => new InstanceofStrategy(),
],
];
}

Expand Down
33 changes: 33 additions & 0 deletions tests/assets/TestFiles/Instanceof.php
@@ -0,0 +1,33 @@
<?php

namespace Foo\Bar {
class Baz {}
class Qux {}
class Quz {}
class Corge {}
}

namespace {

// // https://github.com/composer-unused/composer-unused/pull/100#discussion_r506167242
//
// use Foo\Bar;
//
// if ($foo instanceof Bar\Baz) {
// // usage should be detected because class is a name, relative to imported namespace
// }

if ($foo instanceof Foo\Bar\Qux) {
// usage should be detected because class is a fully-qualified name used in the root namespace
}

if ($foo instanceof \Foo\Bar\Quz) {
// usage should be detected because class is a fully-qualified name relative to the root namespace
}

$name = 'Foo\Bar\Corge';

if ($foo instanceof $name) {
// usage should not be detected because class is a variable expression
}
}