Skip to content

Commit

Permalink
Make sure new calls are also filtered by definedIn (#203)
Browse files Browse the repository at this point in the history
Ran into some issues with the new `definedIn`, where it would report all `new` calls, to built in classes, as it had no definedIn, and since it wasn't passed along.
  • Loading branch information
spaze committed Jul 1, 2023
2 parents 906abf8 + 7da5e66 commit f80ca03
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 5 deletions.
4 changes: 3 additions & 1 deletion src/Calls/NewCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,12 @@ public function processNode(Node $node, Scope $scope): array
}

foreach ($names as $name) {
$classRef = $type->getClassReflection();
$definedIn = $classRef ? $classRef->getFileName() : null;
$name .= self::CONSTRUCT;
$errors = array_merge(
$errors,
$this->disallowedCallsRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, null, $this->disallowedCalls)
$this->disallowedCallsRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, $definedIn, $this->disallowedCalls)
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Calls/MethodCallsDefinedInTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ public function testRule(): void
// expect this error message:
'Calling Waldo\Quux\Blade::andSorcery() is forbidden, because reasons [Waldo\Quux\Blade::andSorcery() matches *()]',
// on this line:
9,
10,
],
[
'Calling Waldo\Quux\Blade::server() is forbidden, because reasons [Waldo\Quux\Blade::server() matches *()]',
10,
11,
],
]);
// Based on the configuration above, no errors in this file:
Expand Down
63 changes: 63 additions & 0 deletions tests/Calls/NewCallsDefinedInTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php
declare(strict_types = 1);

namespace Spaze\PHPStan\Rules\Disallowed\Calls;

use PHPStan\File\FileHelper;
use PHPStan\Rules\Rule;
use PHPStan\ShouldNotHappenException;
use PHPStan\Testing\RuleTestCase;
use Spaze\PHPStan\Rules\Disallowed\Allowed\Allowed;
use Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath;
use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
use Spaze\PHPStan\Rules\Disallowed\File\FilePath;
use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
use Spaze\PHPStan\Rules\Disallowed\Identifier\Identifier;
use Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer;
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallsRuleErrors;

class NewCallsDefinedInTest extends RuleTestCase
{

/**
* @throws ShouldNotHappenException
*/
protected function getRule(): Rule
{
$normalizer = new Normalizer();
$formatter = new Formatter($normalizer);
$filePath = new FilePath(new FileHelper(__DIR__), __DIR__ . '/..');
$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
return new NewCalls(
new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
new DisallowedCallFactory($formatter, $normalizer, $allowed),
[
[
'method' => '*',
'definedIn' => 'libs/Bl*',
'allowIn' => [
'src/disallowed-allow/*.php',
'src/*-allow/*.*',
],
],
]
);
}


public function testRule(): void
{
// Based on the configuration above, in this file:
$this->analyse([__DIR__ . '/../src/disallowed/methodCallsDefinedIn.php'], [
[
// expect this error message:
'Calling Waldo\Quux\Blade::__construct() is forbidden, because reasons [Waldo\Quux\Blade::__construct() matches *()]',
// on this line:
9,
],
]);
// Based on the configuration above, no errors in this file:
$this->analyse([__DIR__ . '/../src/disallowed-allow/methodCallsDefinedIn.php'], []);
}

}
5 changes: 3 additions & 2 deletions tests/libs/Option.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use EmptyIterator;
use IteratorAggregate;
use ArrayIterator;
use Traversable;

/**
* @template T
Expand Down Expand Up @@ -62,7 +63,7 @@ public static function create()
return self::$instance;
}

public function getIterator()
public function getIterator(): Traversable
{
return new EmptyIterator();
}
Expand Down Expand Up @@ -104,7 +105,7 @@ public static function create($value)
}


public function getIterator()
public function getIterator(): Traversable
{
return new ArrayIterator([$this->value]);
}
Expand Down
8 changes: 8 additions & 0 deletions tests/libs/TestException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php
declare(strict_types = 1);

namespace Exceptions;

class TestException extends \Exception
{
}
11 changes: 11 additions & 0 deletions tests/src/disallowed/methodCallsDefinedIn.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
declare(strict_types = 1);

use Exceptions\TestException;
use Waldo\Foo\Bar;
use Waldo\Quux\Blade;

Expand All @@ -16,3 +17,13 @@

// allowed because it's a built-in function
time();

// allowed because it's a built-in class
$exception = new Exception();
$exception->getMessage();
$exception->getPrevious();

// allowed because it's defined elsewhere
$testException = new TestException();
$testException->getMessage();
$testException->getPrevious();

0 comments on commit f80ca03

Please sign in to comment.