mirror of
https://github.com/danog/psalm.git
synced 2024-11-27 04:45:20 +01:00
Merge pull request #7788 from AndrolGenhald/attribute-analysis-improvements
More attribute fixes.
This commit is contained in:
commit
32f10c392d
@ -7,6 +7,7 @@ use PhpParser\Node\Arg;
|
||||
use PhpParser\Node\Attribute;
|
||||
use PhpParser\Node\AttributeGroup;
|
||||
use PhpParser\Node\Expr\New_;
|
||||
use PhpParser\Node\Name\FullyQualified;
|
||||
use PhpParser\Node\Stmt\Expression;
|
||||
use Psalm\CodeLocation;
|
||||
use Psalm\Context;
|
||||
@ -17,18 +18,17 @@ use Psalm\Internal\Scanner\UnresolvedConstantComponent;
|
||||
use Psalm\Issue\InvalidAttribute;
|
||||
use Psalm\Issue\UndefinedClass;
|
||||
use Psalm\IssueBuffer;
|
||||
use Psalm\Storage\AttributeStorage;
|
||||
use Psalm\Storage\ClassLikeStorage;
|
||||
use Psalm\Storage\HasAttributesInterface;
|
||||
use Psalm\Type\Atomic\TLiteralString;
|
||||
use Psalm\Type\Union;
|
||||
use RuntimeException;
|
||||
|
||||
use function array_shift;
|
||||
use function array_values;
|
||||
use function assert;
|
||||
use function count;
|
||||
use function reset;
|
||||
use function strtolower;
|
||||
|
||||
class AttributesAnalyzer
|
||||
{
|
||||
@ -42,9 +42,19 @@ class AttributesAnalyzer
|
||||
40 => 'promoted property',
|
||||
];
|
||||
|
||||
// Copied from Attribute class since that class might not exist at runtime
|
||||
public const TARGET_CLASS = 1;
|
||||
public const TARGET_FUNCTION = 2;
|
||||
public const TARGET_METHOD = 4;
|
||||
public const TARGET_PROPERTY = 8;
|
||||
public const TARGET_CLASS_CONSTANT = 16;
|
||||
public const TARGET_PARAMETER = 32;
|
||||
public const TARGET_ALL = 63;
|
||||
public const IS_REPEATABLE = 64;
|
||||
|
||||
/**
|
||||
* @param array<array-key, AttributeGroup> $attribute_groups
|
||||
* @param 1|2|4|8|16|32|40 $target
|
||||
* @param key-of<self::TARGET_DESCRIPTIONS> $target
|
||||
* @param array<array-key, string> $suppressed_issues
|
||||
*/
|
||||
public static function analyze(
|
||||
@ -57,21 +67,25 @@ class AttributesAnalyzer
|
||||
): void {
|
||||
$codebase = $source->getCodebase();
|
||||
$appearing_non_repeatable_attributes = [];
|
||||
$attribute_iterator = self::iterateAttributeNodes($attribute_groups);
|
||||
foreach ($storage->getAttributeStorages() as $attribute_storage) {
|
||||
if (!$attribute_iterator->valid()) {
|
||||
throw new RuntimeException("Expected attribute count to match attribute storage count");
|
||||
foreach (self::iterateAttributeNodes($attribute_groups) as $attribute) {
|
||||
if ($attribute->name instanceof FullyQualified) {
|
||||
$fq_attribute_name = (string) $attribute->name;
|
||||
} else {
|
||||
$fq_attribute_name = ClassLikeAnalyzer::getFQCLNFromNameObject($attribute->name, $source->getAliases());
|
||||
}
|
||||
$attribute = $attribute_iterator->current();
|
||||
|
||||
$attribute_class_storage = $codebase->classlikes->classExists($attribute_storage->fq_class_name)
|
||||
? $codebase->classlike_storage_provider->get($attribute_storage->fq_class_name)
|
||||
$attribute_name = (string) $attribute->name;
|
||||
$attribute_name_location = new CodeLocation($source, $attribute->name);
|
||||
|
||||
$attribute_class_storage = $codebase->classlikes->classExists($fq_attribute_name)
|
||||
? $codebase->classlike_storage_provider->get($fq_attribute_name)
|
||||
: null;
|
||||
|
||||
$attribute_class_flags = self::getAttributeClassFlags(
|
||||
$source,
|
||||
$attribute_storage->fq_class_name,
|
||||
$attribute_storage->name_location,
|
||||
$attribute_name,
|
||||
$fq_attribute_name,
|
||||
$attribute_name_location,
|
||||
$attribute_class_storage,
|
||||
$suppressed_issues
|
||||
);
|
||||
@ -79,42 +93,36 @@ class AttributesAnalyzer
|
||||
self::analyzeAttributeConstruction(
|
||||
$source,
|
||||
$context,
|
||||
$attribute_storage,
|
||||
$fq_attribute_name,
|
||||
$attribute,
|
||||
$suppressed_issues,
|
||||
$storage instanceof ClassLikeStorage ? $storage : null
|
||||
);
|
||||
|
||||
if (($attribute_class_flags & 64) === 0) {
|
||||
if (($attribute_class_flags & self::IS_REPEATABLE) === 0) {
|
||||
// Not IS_REPEATABLE
|
||||
if (isset($appearing_non_repeatable_attributes[$attribute_storage->fq_class_name])) {
|
||||
if (isset($appearing_non_repeatable_attributes[$fq_attribute_name])) {
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
"Attribute {$attribute_storage->fq_class_name} is not repeatable",
|
||||
$attribute_storage->location
|
||||
"Attribute {$attribute_name} is not repeatable",
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
}
|
||||
$appearing_non_repeatable_attributes[$attribute_storage->fq_class_name] = true;
|
||||
$appearing_non_repeatable_attributes[$fq_attribute_name] = true;
|
||||
}
|
||||
|
||||
if (($attribute_class_flags & $target) === 0) {
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
"Attribute {$attribute_storage->fq_class_name} cannot be used on a "
|
||||
"Attribute {$attribute_name} cannot be used on a "
|
||||
. self::TARGET_DESCRIPTIONS[$target],
|
||||
$attribute_storage->name_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
}
|
||||
|
||||
$attribute_iterator->next();
|
||||
}
|
||||
|
||||
if ($attribute_iterator->valid()) {
|
||||
throw new RuntimeException("Expected attribute count to match attribute storage count");
|
||||
}
|
||||
}
|
||||
|
||||
@ -124,15 +132,17 @@ class AttributesAnalyzer
|
||||
private static function analyzeAttributeConstruction(
|
||||
SourceAnalyzer $source,
|
||||
Context $context,
|
||||
AttributeStorage $attribute_storage,
|
||||
string $fq_attribute_name,
|
||||
Attribute $attribute,
|
||||
array $suppressed_issues,
|
||||
?ClassLikeStorage $classlike_storage = null
|
||||
): void {
|
||||
$attribute_name_location = new CodeLocation($source, $attribute->name);
|
||||
|
||||
if (ClassLikeAnalyzer::checkFullyQualifiedClassLikeName(
|
||||
$source,
|
||||
$attribute_storage->fq_class_name,
|
||||
$attribute_storage->location,
|
||||
$fq_attribute_name,
|
||||
$attribute_name_location,
|
||||
null,
|
||||
null,
|
||||
$suppressed_issues,
|
||||
@ -148,12 +158,12 @@ class AttributesAnalyzer
|
||||
return;
|
||||
}
|
||||
|
||||
if ($attribute_storage->fq_class_name === 'Attribute' && $classlike_storage) {
|
||||
if (strtolower($fq_attribute_name) === 'attribute' && $classlike_storage) {
|
||||
if ($classlike_storage->is_trait) {
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
'Traits cannot act as attribute classes',
|
||||
$attribute_storage->name_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
@ -161,7 +171,7 @@ class AttributesAnalyzer
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
'Interfaces cannot act as attribute classes',
|
||||
$attribute_storage->name_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
@ -169,7 +179,7 @@ class AttributesAnalyzer
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
'Abstract classes cannot act as attribute classes',
|
||||
$attribute_storage->name_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
@ -179,7 +189,7 @@ class AttributesAnalyzer
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
'Classes with protected/private constructors cannot act as attribute classes',
|
||||
$attribute_storage->name_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
@ -187,7 +197,7 @@ class AttributesAnalyzer
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
'Enums cannot act as attribute classes',
|
||||
$attribute_storage->name_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
@ -200,16 +210,21 @@ class AttributesAnalyzer
|
||||
);
|
||||
$statements_analyzer->addSuppressedIssues(array_values($suppressed_issues));
|
||||
|
||||
$had_returned = $context->has_returned;
|
||||
$context->has_returned = false;
|
||||
|
||||
IssueBuffer::startRecording();
|
||||
$statements_analyzer->analyze(
|
||||
[new Expression(new New_($attribute->name, $attribute->args, $attribute->getAttributes()))],
|
||||
// Use a new Context for the Attribute attribute so that it can't access `self`
|
||||
$attribute_storage->fq_class_name === "Attribute" ? new Context() : $context
|
||||
strtolower($fq_attribute_name) === "attribute" ? new Context() : $context
|
||||
);
|
||||
$context->has_returned = $had_returned;
|
||||
|
||||
$issues = IssueBuffer::clearRecordingLevel();
|
||||
IssueBuffer::stopRecording();
|
||||
foreach ($issues as $issue) {
|
||||
if ($issue instanceof UndefinedClass && $issue->fq_classlike_name === $attribute_storage->fq_class_name) {
|
||||
if ($issue instanceof UndefinedClass && $issue->fq_classlike_name === $fq_attribute_name) {
|
||||
// Remove UndefinedClass for the attribute, since we already added UndefinedAttribute
|
||||
continue;
|
||||
}
|
||||
@ -223,24 +238,25 @@ class AttributesAnalyzer
|
||||
private static function getAttributeClassFlags(
|
||||
SourceAnalyzer $source,
|
||||
string $attribute_name,
|
||||
CodeLocation $attribute_location,
|
||||
string $fq_attribute_name,
|
||||
CodeLocation $attribute_name_location,
|
||||
?ClassLikeStorage $attribute_class_storage,
|
||||
array $suppressed_issues
|
||||
): int {
|
||||
if ($attribute_name === "Attribute") {
|
||||
if (strtolower($fq_attribute_name) === "attribute") {
|
||||
// We override this here because we still want to analyze attributes
|
||||
// for PHP 7.4 when the Attribute class doesn't yet exist.
|
||||
return 1;
|
||||
return self::TARGET_CLASS;
|
||||
}
|
||||
|
||||
if ($attribute_class_storage === null) {
|
||||
return 63; // Defaults to TARGET_ALL
|
||||
return self::TARGET_ALL; // Defaults to TARGET_ALL
|
||||
}
|
||||
|
||||
foreach ($attribute_class_storage->attributes as $attribute_attribute) {
|
||||
if ($attribute_attribute->fq_class_name === 'Attribute') {
|
||||
if (!$attribute_attribute->args) {
|
||||
return 63; // Defaults to TARGET_ALL
|
||||
return self::TARGET_ALL; // Defaults to TARGET_ALL
|
||||
}
|
||||
|
||||
$first_arg = reset($attribute_attribute->args);
|
||||
@ -258,7 +274,7 @@ class AttributesAnalyzer
|
||||
}
|
||||
|
||||
if (!$first_arg_type->isSingleIntLiteral()) {
|
||||
return 63; // Fall back to default if it's invalid
|
||||
return self::TARGET_ALL; // Fall back to default if it's invalid
|
||||
}
|
||||
|
||||
return $first_arg_type->getSingleIntLiteral()->value;
|
||||
@ -268,12 +284,12 @@ class AttributesAnalyzer
|
||||
IssueBuffer::maybeAdd(
|
||||
new InvalidAttribute(
|
||||
"The class {$attribute_name} doesn't have the Attribute attribute",
|
||||
$attribute_location
|
||||
$attribute_name_location
|
||||
),
|
||||
$suppressed_issues
|
||||
);
|
||||
|
||||
return 63; // Fall back to default if it's invalid
|
||||
return self::TARGET_ALL; // Fall back to default if it's invalid
|
||||
}
|
||||
|
||||
/**
|
||||
@ -309,22 +325,22 @@ class AttributesAnalyzer
|
||||
|
||||
switch ($method_id) {
|
||||
case "ReflectionClass::getattributes":
|
||||
$target = 1;
|
||||
$target = self::TARGET_CLASS;
|
||||
break;
|
||||
case "ReflectionFunction::getattributes":
|
||||
$target = 2;
|
||||
$target = self::TARGET_FUNCTION;
|
||||
break;
|
||||
case "ReflectionMethod::getattributes":
|
||||
$target = 4;
|
||||
$target = self::TARGET_METHOD;
|
||||
break;
|
||||
case "ReflectionProperty::getattributes":
|
||||
$target = 8;
|
||||
$target = self::TARGET_PROPERTY;
|
||||
break;
|
||||
case "ReflectionClassConstant::getattributes":
|
||||
$target = 16;
|
||||
$target = self::TARGET_CLASS_CONSTANT;
|
||||
break;
|
||||
case "ReflectionParameter::getattributes":
|
||||
$target = 32;
|
||||
$target = self::TARGET_PARAMETER;
|
||||
break;
|
||||
default:
|
||||
return;
|
||||
@ -358,6 +374,7 @@ class AttributesAnalyzer
|
||||
$class_attribute_target = self::getAttributeClassFlags(
|
||||
$statements_analyzer,
|
||||
$class_string->value,
|
||||
$class_string->value,
|
||||
$arg_location,
|
||||
$class_storage,
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
|
@ -402,7 +402,7 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
$class_context,
|
||||
$storage,
|
||||
$class->attrGroups,
|
||||
1,
|
||||
AttributesAnalyzer::TARGET_CLASS,
|
||||
$storage->suppressed_issues + $this->getSuppressedIssues()
|
||||
);
|
||||
|
||||
@ -1526,7 +1526,7 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
$context,
|
||||
$property_storage,
|
||||
$stmt->attrGroups,
|
||||
8,
|
||||
AttributesAnalyzer::TARGET_PROPERTY,
|
||||
$property_storage->suppressed_issues + $this->getSuppressedIssues()
|
||||
);
|
||||
|
||||
|
@ -824,7 +824,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
|
||||
$context,
|
||||
$storage,
|
||||
$this->function->attrGroups,
|
||||
$storage instanceof MethodStorage ? 4 : 2,
|
||||
$storage instanceof MethodStorage ? AttributesAnalyzer::TARGET_METHOD : AttributesAnalyzer::TARGET_FUNCTION,
|
||||
$storage->suppressed_issues + $this->getSuppressedIssues()
|
||||
);
|
||||
|
||||
@ -1272,7 +1272,8 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
|
||||
$context,
|
||||
$function_param,
|
||||
$param_stmts[$offset]->attrGroups,
|
||||
$function_param->promoted_property ? 40 : 32,
|
||||
AttributesAnalyzer::TARGET_PARAMETER
|
||||
| ($function_param->promoted_property ? AttributesAnalyzer::TARGET_PROPERTY : 0),
|
||||
$storage->suppressed_issues + $this->getSuppressedIssues()
|
||||
);
|
||||
}
|
||||
|
@ -102,7 +102,7 @@ class InterfaceAnalyzer extends ClassLikeAnalyzer
|
||||
$interface_context,
|
||||
$class_storage,
|
||||
$this->class->attrGroups,
|
||||
1,
|
||||
AttributesAnalyzer::TARGET_CLASS,
|
||||
$class_storage->suppressed_issues + $this->getSuppressedIssues()
|
||||
);
|
||||
|
||||
|
@ -69,7 +69,7 @@ class TraitAnalyzer extends ClassLikeAnalyzer
|
||||
$context,
|
||||
$storage,
|
||||
$stmt->attrGroups,
|
||||
1,
|
||||
AttributesAnalyzer::TARGET_CLASS,
|
||||
$storage->suppressed_issues + $statements_analyzer->getSuppressedIssues()
|
||||
);
|
||||
}
|
||||
|
@ -18,11 +18,15 @@ class AttributeStorage
|
||||
|
||||
/**
|
||||
* @var CodeLocation
|
||||
*
|
||||
* @psalm-suppress PossiblyUnusedProperty part of public API
|
||||
*/
|
||||
public $location;
|
||||
|
||||
/**
|
||||
* @var CodeLocation
|
||||
*
|
||||
* @psalm-suppress PossiblyUnusedProperty part of public API
|
||||
*/
|
||||
public $name_location;
|
||||
|
||||
|
@ -10,6 +10,8 @@ interface HasAttributesInterface
|
||||
* Returns a list of AttributeStorages with the same order they appear in the AttributeGroups they come from.
|
||||
*
|
||||
* @return list<AttributeStorage>
|
||||
*
|
||||
* @psalm-suppress PossiblyUnusedMethod part of public API
|
||||
*/
|
||||
public function getAttributeStorages(): array;
|
||||
}
|
||||
|
@ -2,6 +2,7 @@
|
||||
|
||||
namespace Psalm\Tests;
|
||||
|
||||
use Psalm\Context;
|
||||
use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait;
|
||||
use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait;
|
||||
|
||||
@ -12,6 +13,29 @@ class AttributeTest extends TestCase
|
||||
use InvalidCodeAnalysisTestTrait;
|
||||
use ValidCodeAnalysisTestTrait;
|
||||
|
||||
public function testStubsWithDifferentAttributes(): void
|
||||
{
|
||||
$this->addStubFile(
|
||||
'stubOne.phpstub',
|
||||
'<?php
|
||||
#[Attribute]
|
||||
class Attr {}
|
||||
|
||||
#[Attr]
|
||||
class Foo {}
|
||||
'
|
||||
);
|
||||
|
||||
$this->addFile(
|
||||
'somefile.php',
|
||||
'<?php
|
||||
class Foo {}
|
||||
'
|
||||
);
|
||||
|
||||
$this->analyzeFile('somefile.php', new Context());
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
|
||||
*/
|
||||
@ -700,6 +724,33 @@ class AttributeTest extends TestCase
|
||||
',
|
||||
'error_message' => 'InvalidAttribute - src' . DIRECTORY_SEPARATOR . 'somefile.php:5:28 - Attribute Foo is not repeatable',
|
||||
],
|
||||
'invalidAttributeConstructionWithReturningFunction' => [
|
||||
'<?php
|
||||
enum Enumm
|
||||
{
|
||||
case SOME_CASE;
|
||||
}
|
||||
|
||||
#[Attribute]
|
||||
final class Attr
|
||||
{
|
||||
public function __construct(public Enumm $e) {}
|
||||
}
|
||||
|
||||
final class SomeClass
|
||||
{
|
||||
#[Attr(Enumm::WRONG_CASE)]
|
||||
public function anotherMethod(): string
|
||||
{
|
||||
return "";
|
||||
}
|
||||
}
|
||||
',
|
||||
'error_message' => 'UndefinedConstant',
|
||||
[],
|
||||
false,
|
||||
'8.1',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
@ -103,6 +103,16 @@ class TestCase extends BaseTestCase
|
||||
$this->project_analyzer->getCodebase()->scanner->addFileToShallowScan($file_path);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $file_path
|
||||
* @param string $contents
|
||||
*/
|
||||
public function addStubFile($file_path, $contents): void
|
||||
{
|
||||
$this->file_provider->registerFile($file_path, $contents);
|
||||
$this->project_analyzer->getConfig()->addStubFile($file_path);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $file_path
|
||||
*
|
||||
|
Loading…
Reference in New Issue
Block a user