danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00
Code Issues Projects Releases Wiki Activity

Fix multiple issues with @internal and @psalm-internal (#3841)

Browse Source 
* Add passing tests for property fetch on an @internal class

I'm trying to work out why the equivilent InvalidCodeParse test is
failing for PsalmInternal

* Treat all properties of a psalm-internal class as psalm-internal

* Remove all $internal properties from storage - use psalm_internal instead

@internal can be represented as internal to the namespace root, avoiding
the need to check for both properties in storage later.

* Raise InternalClass issue when an internal class is used with e.g. instanceOf

* fix docs and tests

* Add return type declartion to code example in doc

* Don't allow class psalm-internal to overide a tighter method psalm-internal

* Break up long line

* Code style - move && from EOL to SOL

* Restore misplaced &&

* Fix code style

* Fix namespace fetching so it works

Co-authored-by: Matthew Brown <github@muglug.com>
This commit is contained in:
Barney Laurance 2020-07-23 00:27:35 +01:00 committed by GitHub
parent 9430a9b204
commit 3bc91b9944
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 210 additions and 147 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/running_psalm/issues/InternalClass.md
View File

@ -15,7 +15,7 @@ namespace A {
namespace B {
class Bat {
public function batBat() {
public function batBat(): void {
$a = new \A\Foo();
}
}

2
docs/running_psalm/issues/InternalMethod.md
View File

@ -17,7 +17,7 @@ namespace A {
}
namespace B {
class Bat {
public function batBat() {
public function batBat(): void {
\A\Foo::barBar();
}
}

21
src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View File

@ -302,27 +302,6 @@ class ClassAnalyzer extends ClassLikeAnalyzer
}
}
if ($parent_class_storage->internal) {
$code_location = new CodeLocation(
$this,
$class->extends,
$class_context ? $class_context->include_location : null,
true
);
if (! NamespaceAnalyzer::nameSpaceRootsMatch($fq_class_name, $parent_fq_class_name)) {
if (IssueBuffer::accepts(
new InternalClass(
$parent_fq_class_name . ' is marked internal',
$code_location,
$parent_fq_class_name
),
$storage->suppressed_issues + $this->getSuppressedIssues()
)) {
// fall through
}
}
}
if ($parent_class_storage->psalm_internal &&
! NamespaceAnalyzer::isWithin($fq_class_name, $parent_class_storage->psalm_internal)
) {

23
src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php
View File

@ -8,8 +8,10 @@ use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
use Psalm\Issue\InaccessibleProperty;
use Psalm\Issue\InternalClass;
use Psalm\Issue\InvalidClass;
use Psalm\Issue\MissingDependency;
use Psalm\Issue\PsalmInternalError;
use Psalm\Issue\ReservedWord;
use Psalm\Issue\UndefinedClass;
use Psalm\Issue\UndefinedDocblockClass;
@ -399,6 +401,27 @@ abstract class ClassLikeAnalyzer extends SourceAnalyzer implements StatementsSou
}
}
if (!$inferred) {
if ($class_storage->psalm_internal) {
$sourceNamespace = $statements_source->getNamespace();
if (!$sourceNamespace
|| ! NamespaceAnalyzer::isWithin($sourceNamespace, $class_storage->psalm_internal)
) {
if (IssueBuffer::accepts(
new InternalClass(
$class_storage->name . ' is internal to ' . $class_storage->psalm_internal,
$code_location,
$class_storage->name
),
$suppressed_issues
)
) {
// fall through
}
}
}
}
return true;
}

7
src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php
View File

@ -184,16 +184,11 @@ class NamespaceAnalyzer extends SourceAnalyzer implements StatementsSource
return $className === $namespace || strpos($className, $namespace) === 0;
}
public static function nameSpaceRootsMatch(string $fqcnA, string $fqcnB): bool
{
return strtolower(self::getNameSpaceRoot($fqcnA)) === strtolower(self::getNameSpaceRoot($fqcnB));
}
/**
* @param string $fullyQualifiedClassName, e.g. '\Psalm\Internal\Analyzer\NamespaceAnalyzer'
* @return string , e.g. 'Psalm'
*/
private static function getNameSpaceRoot(string $fullyQualifiedClassName): string
public static function getNameSpaceRoot(string $fullyQualifiedClassName): string
{
return preg_replace('/^([^\\\]+).*/', '$1', $fullyQualifiedClassName);
}

15
src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php
View File

@ -690,21 +690,6 @@ class InstancePropertyAssignmentAnalyzer
}
}
if ($property_storage->internal && $context->self) {
if (! NamespaceAnalyzer::nameSpaceRootsMatch($context->self, $declaring_property_class)) {
if (IssueBuffer::accepts(
new InternalProperty(
$property_id . ' is marked internal',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
}
}
// prevents writing to readonly properties
if ($property_storage->readonly) {
$appearing_property_class = $codebase->properties->getAppearingClassForProperty(

22
src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallProhibitionAnalyzer.php
View File

@ -67,28 +67,6 @@ class MethodCallProhibitionAnalyzer
}
}
}
if ($storage->internal
&& $context->self
&& !$context->collect_initializations
&& !$context->collect_mutations
) {
$declaring_class = $method_id->fq_class_name;
if (! NamespaceAnalyzer::nameSpaceRootsMatch($context->self, $declaring_class)) {
if (IssueBuffer::accepts(
new InternalMethod(
'The method ' . $codebase_methods->getCasedMethodId($method_id) .
' has been marked as internal',
$code_location,
(string) $method_id
),
$suppressed_issues
)) {
// fall through
}
}
}
return null;
}
}

15
src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php
View File

@ -425,21 +425,6 @@ class NewAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\CallAna
}
}
if ($storage->internal && $context->self) {
if (! NamespaceAnalyzer::nameSpaceRootsMatch($context->self, $fq_class_name)) {
if (IssueBuffer::accepts(
new InternalClass(
$fq_class_name . ' is marked internal',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$fq_class_name
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
}
}
$method_id = new \Psalm\Internal\MethodIdentifier($fq_class_name, '__construct');
if ($codebase->methods->methodExists(

19
src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php
View File

@ -779,25 +779,6 @@ class StaticCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\
}
}
if ($class_storage->internal
&& $context->self
&& !$context->collect_initializations
&& !$context->collect_mutations
) {
if (! NamespaceAnalyzer::nameSpaceRootsMatch($context->self, $fq_class_name)) {
if (IssueBuffer::accepts(
new InternalClass(
$fq_class_name . ' is marked internal',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$fq_class_name
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
}
}
if (Method\MethodVisibilityAnalyzer::analyze(
$method_id,
$context,

16
src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php
View File

@ -28,6 +28,7 @@ use Psalm\Issue\UndefinedPropertyFetch;
use Psalm\Issue\UndefinedThisPropertyFetch;
use Psalm\Issue\UninitializedProperty;
use Psalm\IssueBuffer;
use Psalm\Storage\PropertyStorage;
use Psalm\Type;
use Psalm\Storage\ClassLikeStorage;
use Psalm\Type\Atomic\TGenericObject;
@ -834,21 +835,6 @@ class InstancePropertyFetchAnalyzer
}
}
}
if ($property_storage->internal && $context->self) {
if (! NamespaceAnalyzer::nameSpaceRootsMatch($context->self, $declaring_property_class)) {
if (IssueBuffer::accepts(
new InternalProperty(
$property_id . ' is marked internal',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
}
}
}
$class_property_type = $codebase->properties->getPropertyType(

16
src/Psalm/Internal/Codebase/Populator.php
View File

@ -20,6 +20,7 @@ use Psalm\Type;
use function reset;
use function strpos;
use function strtolower;
use function strlen;
/**
* @internal
@ -249,16 +250,25 @@ class Populator
}
}
if ($storage->internal
if ($storage->psalm_internal
&& !$storage->is_interface
&& !$storage->is_trait
) {
foreach ($storage->methods as $method) {
$method->internal = true;
if (null === $method->psalm_internal ||
strlen($storage->psalm_internal) > strlen($method->psalm_internal)
) {
$method->psalm_internal = $storage->psalm_internal;
}
}
foreach ($storage->properties as $property) {
$property->internal = true;
if (null === $property->psalm_internal ||
strlen($storage->psalm_internal) > strlen($property->psalm_internal)
) {
$property->psalm_internal = $storage->psalm_internal;
}
}
}

43
src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php
View File

@ -1,6 +1,7 @@
<?php
namespace Psalm\Internal\PhpVisitor;
use Psalm\Internal\Analyzer\NamespaceAnalyzer;
use function array_filter;
use function array_merge;
use function array_pop;
@ -1308,8 +1309,16 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
}
$storage->deprecated = $docblock_info->deprecated;
$storage->internal = $docblock_info->internal;
$storage->psalm_internal = $docblock_info->psalm_internal;
if ($docblock_info->internal
&& !$docblock_info->psalm_internal
&& $this->aliases->namespace
) {
$storage->psalm_internal = explode('\\', $this->aliases->namespace)[0];
} else {
$storage->psalm_internal = $docblock_info->psalm_internal;
}
$storage->final = $storage->final || $docblock_info->final;
if ($docblock_info->mixin) {
@ -2122,8 +2131,13 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
$doc_comment = $stmt->getDocComment();
if ($class_storage && ! $class_storage->is_trait) {
$storage->internal = $class_storage->internal;
if ($class_storage
&& !$class_storage->is_trait
&& $class_storage->psalm_internal
&& (!$storage->psalm_internal
|| strlen($class_storage->psalm_internal) > strlen($storage->psalm_internal)
)
) {
$storage->psalm_internal = $class_storage->psalm_internal;
}
@ -2180,14 +2194,15 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
$storage->deprecated = true;
}
if ($docblock_info->internal) {
$storage->internal = true;
}
if (null === $class_storage ||
null === $class_storage->psalm_internal ||
(null !== $docblock_info->psalm_internal &&
strlen($docblock_info->psalm_internal) > strlen($class_storage->psalm_internal)
if ($docblock_info->internal
&& !$docblock_info->psalm_internal
&& $this->aliases->namespace
) {
$storage->psalm_internal = explode('\\', $this->aliases->namespace)[0];
} elseif (!$class_storage
|| !$class_storage->psalm_internal
|| ($docblock_info->psalm_internal
&& strlen($docblock_info->psalm_internal) > strlen($class_storage->psalm_internal)
)
) {
$storage->psalm_internal = $docblock_info->psalm_internal;
@ -3403,8 +3418,10 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
$property_storage->stmt_location = new CodeLocation($this->file_scanner, $stmt);
$property_storage->has_default = $property->default ? true : false;
$property_storage->deprecated = $var_comment ? $var_comment->deprecated : false;
$property_storage->internal = $var_comment ? $var_comment->internal : false;
$property_storage->psalm_internal = $var_comment ? $var_comment->psalm_internal : null;
if (! $property_storage->psalm_internal && $var_comment && $var_comment->internal) {
$property_storage->psalm_internal = NamespaceAnalyzer::getNameSpaceRoot($fq_classlike_name);
}
$property_storage->readonly = $var_comment ? $var_comment->readonly : false;
$property_storage->allow_private_mutation = $var_comment ? $var_comment->allow_private_mutation : false;

5
src/Psalm/Storage/ClassLikeStorage.php
View File

@ -87,11 +87,6 @@ class ClassLikeStorage
*/
public $deprecated = false;
/**
* @var bool
*/
public $internal = false;
/**
* @var null|string
*/

5
src/Psalm/Storage/FunctionLikeStorage.php
View File

@ -66,11 +66,6 @@ abstract class FunctionLikeStorage
*/
public $deprecated;
/**
* @var ?bool
*/
public $internal;
/**
* @var null|string
*/

5
src/Psalm/Storage/PropertyStorage.php
View File

@ -64,11 +64,6 @@ class PropertyStorage
*/
public $deprecated = false;
/**
* @var bool
*/
public $internal = false;
/**
* @var bool
*/

2
tests/Config/PluginTest.php
View File

@ -185,6 +185,8 @@ class PluginTest extends \Psalm\Tests\TestCase
$this->addFile(
$file_path,
'<?php
namespace Psalm;
class A {
const C = [
"foo" => \Psalm\Internal\Analyzer\ProjectAnalyzer::class . "::foo",

49
tests/InternalAnnotationTest.php
View File

@ -76,6 +76,29 @@ class InternalAnnotationTest extends TestCase
}
}',
],
'internalClassWithPropertyFetch' => [
'<?php
namespace A\B {
/**
* @internal
*/
class Foo {
public int $barBar = 0;
}
function getFoo(): Foo {
return new Foo();
}
}
namespace A\C {
class Bat {
public function batBat(\A\B\Foo $instance): void {
\A\B\getFoo()->barBar;
}
}
}',
],
'internalClassExtendingNamespaceWithStaticCall' => [
'<?php
namespace A {
@ -240,7 +263,7 @@ class InternalAnnotationTest extends TestCase
namespace B {
class Bat {
public function batBat() {
public function batBat(): void {
\A\Foo::barBar();
}
}
@ -293,6 +316,30 @@ class InternalAnnotationTest extends TestCase
}',
'error_message' => 'InternalMethod',
],
'internalClassWithPropertyFetch' => [
'<?php
namespace A\B {
/**
* @internal
*/
class Foo {
public int $barBar = 0;
}
function getFoo(): Foo {
return new Foo();
}
}
namespace C {
class Bat {
public function batBat(): void {
\A\B\getFoo()->barBar;
}
}
}',
'error_message' => 'A\B\Foo::$barBar is marked internal',
],
'internalClassWithNew' => [
'<?php
namespace A {

90
tests/PsalmInternalAnnotationTest.php
View File

@ -117,6 +117,30 @@ class PsalmInternalAnnotationTest extends TestCase
}
}',
],
'internalClassWithPropertyFetch' => [
'<?php
namespace A\B {
/**
* @internal
* @psalm-internal A\B
*/
class Foo {
public int $barBar = 0;
}
function getFoo(): Foo {
return new Foo();
}
}
namespace A\B\C {
class Bat {
public function batBat(\A\B\Foo $instance): void {
\A\B\getFoo()->barBar;
}
}
}',
],
'internalClassExtendingNamespaceWithStaticCall' => [
'<?php
namespace A {
@ -162,6 +186,26 @@ class PsalmInternalAnnotationTest extends TestCase
}
}',
],
'internalClassWithInstanceOf' => [
'<?php
namespace A\B {
interface Bar {};
/**
* @internal
* @psalm-internal A\B
*/
class Foo { }
}
namespace A\B\C {
class Bat {
public function batBat(\A\B\Bar $bar) : void {
$bar instanceOf \A\B\Foo;
}
}
}',
],
'internalClassWithExtends' => [
'<?php
namespace A\B {
@ -319,6 +363,31 @@ class PsalmInternalAnnotationTest extends TestCase
}',
'error_message' => 'InternalClass',
],
'internalClassWithPropertyFetch' => [
'<?php
namespace A\B {
/**
* @internal
* @psalm-internal A\B
*/
class Foo {
public int $barBar = 0;
}
function getFoo(): Foo {
return new Foo();
}
}
namespace A\C {
class Bat {
public function batBat(): void {
\A\B\getFoo()->barBar;
}
}
}',
'error_message' => 'A\B\Foo::$barBar is marked internal to A\B',
],
'internalClassWithInstanceCall' => [
'<?php
namespace A\B {
@ -364,6 +433,27 @@ class PsalmInternalAnnotationTest extends TestCase
}',
'error_message' => 'InternalClass',
],
'internalClassWithInstanceOf' => [
'<?php
namespace A\B {
interface Bar {};
/**
* @internal
* @psalm-internal A\B
*/
class Foo { }
}
namespace A\C {
class Bat {
public function batBat(\A\B\Bar $bar) : void {
$bar instanceOf \A\B\Foo;
}
}
}',
'error_message' => 'A\B\Foo is internal to A\B',
],
'internalClassWithExtends' => [
'<?php
namespace A\B {