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.

Sign up for GitHub

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

Jump to bottom

Fix detecting magic static methods #10704

Merged
merged 22 commits into from Feb 17, 2024
Merged

Fix detecting magic static methods #10704

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6e361aa
Failed and regression tests for magic static methods
issidorov Feb 7, 2024
29b4c38
Add argument with_pseudo for method finding
issidorov Feb 6, 2024
083b8e2
Add issue message for magic methods
issidorov Feb 6, 2024
2567a99
Extract checking method "__callStatic" from block "if"
issidorov Feb 6, 2024
df2067d
Resolve testReferencedMethods
issidorov Jan 31, 2024
0ba346c
Delete code of replacing variable method_id
issidorov Jan 31, 2024
5f89fa1
Resolved all tests
issidorov Jan 31, 2024
4fce070
Failed and regression tests with usage config
issidorov Feb 7, 2024
14316f5
Resolve testAnnotationWithoutCallConfigWithExtendsWithStatic
issidorov Jan 31, 2024
6009631
Fix invalid test
issidorov Feb 10, 2024
6f17469
Failed tests for StaticInvocation and NonStaticSelfCall
issidorov Feb 10, 2024
4c645e1
Resolve tests for StaticInvocation and NonStaticSelfCall
issidorov Feb 7, 2024
d588b3d
Support for testing with the creation of a list of issues
issidorov Feb 13, 2024
9b6ef8b
Failed and regression tests with creation of a list of issues
issidorov Feb 13, 2024
e940de5
Resolve tests with creation of a list of issues
issidorov Feb 1, 2024
db92991
Fix message in TestCase::assertHasIssueType
issidorov Feb 14, 2024
cfd0fd1
Use snake_case in TestCase::assertHasIssueType
issidorov Feb 14, 2024
08a479a
Rename to TestCase::assertHasIssue
issidorov Feb 14, 2024
b7a2080
Fix description in InvalidCodeAnalysisWithIssuesTestTrait
issidorov Feb 14, 2024
3724062
Fix description InvalidCodeAnalysisWithIssuesTestTrait
issidorov Feb 14, 2024
5a66742
Failed and regression tests with suppression "UndefinedMethod"
issidorov Feb 14, 2024
8a70bc2
Resolve tests with suppression "UndefinedMethod"
issidorov Feb 14, 2024
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
35 changes: 27 additions & 8 deletions src/Psalm/Internal/Analyzer/MethodAnalyzer.php
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@
use Psalm\Issue\InvalidStaticInvocation;
use Psalm\Issue\MethodSignatureMustOmitReturnType;
use Psalm\Issue\NonStaticSelfCall;
use Psalm\Issue\UndefinedMagicMethod;
use Psalm\Issue\UndefinedMethod;
use Psalm\IssueBuffer;
use Psalm\StatementsSource;
Expand Down Expand Up @@ -110,8 +111,9 @@ public static function checkStatic(
}

$original_method_id = $method_id;
$with_pseudo = true;

$method_id = $codebase_methods->getDeclaringMethodId($method_id);
$method_id = $codebase_methods->getDeclaringMethodId($method_id, $with_pseudo);

if (!$method_id) {
if (InternalCallMapHandler::inCallMap((string) $original_method_id)) {
Expand All @@ -121,7 +123,7 @@ public static function checkStatic(
throw new LogicException('Declaring method for ' . $original_method_id . ' should not be null');
}

$storage = $codebase_methods->getStorage($method_id);
$storage = $codebase_methods->getStorage($method_id, $with_pseudo);

if (!$storage->is_static) {
if ($self_call) {
Expand Down Expand Up @@ -165,7 +167,8 @@ public static function checkMethodExists(
MethodIdentifier $method_id,
CodeLocation $code_location,
array $suppressed_issues,
?string $calling_method_id = null
?string $calling_method_id = null,
bool $with_pseudo = false
): ?bool {
if ($codebase->methods->methodExists(
$method_id,
Expand All @@ -176,15 +179,31 @@ public static function checkMethodExists(
: null,
null,
$code_location->file_path,
true,
false,
$with_pseudo,
)) {
return true;
}

if (IssueBuffer::accepts(
new UndefinedMethod('Method ' . $method_id . ' does not exist', $code_location, (string) $method_id),
$suppressed_issues,
)) {
return false;
if ($with_pseudo) {
if (IssueBuffer::accepts(
new UndefinedMagicMethod(
'Magic method ' . $method_id . ' does not exist',
$code_location,
(string) $method_id,
),
$suppressed_issues,
)) {
return false;
}
} else {
if (IssueBuffer::accepts(
new UndefinedMethod('Method ' . $method_id . ' does not exist', $code_location, (string) $method_id),
$suppressed_issues,
)) {
return false;
}
}

return null;
Expand Down
6 changes: 4 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodVisibilityAnalyzer.php
View file
Open in desktop
Expand Up @@ -40,6 +40,8 @@ public static function analyze(
$fq_classlike_name = $method_id->fq_class_name;
$method_name = $method_id->method_name;

$with_pseudo = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not used until line N70, so move it there, perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been thinking about it too.
This is a declaration of a local variable.
And it seemed to me more beautiful to do this at the beginning of the method, next to the declaration of other variables.

Does it need to be fixed? Please confirm.


if ($codebase_methods->visibility_provider->has($fq_classlike_name)) {
$method_visible = $codebase_methods->visibility_provider->isMethodVisible(
$source,
Expand All @@ -65,7 +67,7 @@ public static function analyze(
}
}

$declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id);
$declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id, $with_pseudo);

if (!$declaring_method_id) {
if ($method_name === '__construct'
Expand Down Expand Up @@ -109,7 +111,7 @@ public static function analyze(
return null;
}

$storage = $codebase->methods->getStorage($declaring_method_id);
$storage = $codebase->methods->getStorage($declaring_method_id, $with_pseudo);
$visibility = $storage->visibility;

if ($appearing_method_name
Expand Down
159 changes: 61 additions & 98 deletions ...lm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php
View file
Open in desktop
Expand Up @@ -32,12 +32,8 @@
use Psalm\Issue\UndefinedClass;
use Psalm\Issue\UndefinedMethod;
use Psalm\IssueBuffer;
use Psalm\Node\Expr\VirtualArray;
use Psalm\Node\Expr\VirtualArrayItem;
use Psalm\Node\Expr\VirtualMethodCall;
use Psalm\Node\Expr\VirtualVariable;
use Psalm\Node\Scalar\VirtualString;
use Psalm\Node\VirtualArg;
use Psalm\Storage\ClassLikeStorage;
use Psalm\Storage\MethodStorage;
use Psalm\Type;
Expand All @@ -57,7 +53,6 @@
use Psalm\Type\Union;

use function array_filter;
use function array_map;
use function array_values;
use function assert;
use function count;
Expand Down Expand Up @@ -562,35 +557,59 @@ private static function handleNamedCall(
return true;
}

$callstatic_id = new MethodIdentifier(
$fq_class_name,
'__callstatic',
);

$callstatic_method_exists = $codebase->methods->methodExists($callstatic_id);

$with_pseudo = $callstatic_method_exists
|| $codebase->config->use_phpdoc_method_without_magic_or_parent;

if ($codebase->methods->getDeclaringMethodId($method_id, $with_pseudo)) {
if ((!$stmt->class instanceof PhpParser\Node\Name
|| $stmt->class->getFirst() !== 'parent'
|| $statements_analyzer->isStatic())
&& (
!$context->self
|| $statements_analyzer->isStatic()
|| !$codebase->classExtends($context->self, $fq_class_name)
)
) {
MethodAnalyzer::checkStatic(
$method_id,
($stmt->class instanceof PhpParser\Node\Name
&& strtolower($stmt->class->getFirst()) === 'self')
|| $context->self === $fq_class_name,
!$statements_analyzer->isStatic(),
$codebase,
new CodeLocation($statements_analyzer, $stmt),
$statements_analyzer->getSuppressedIssues(),
$is_dynamic_this_method,
);

if ($is_dynamic_this_method) {
return self::forwardCallToInstanceMethod(
$statements_analyzer,
$stmt,
$stmt_name,
$context,
);
}
}
}

if (!$naive_method_exists
|| !MethodAnalyzer::isMethodVisible(
$method_id,
$context,
$statements_analyzer->getSource(),
)
|| $fake_method_exists
|| ($found_method_and_class_storage
&& ($config->use_phpdoc_method_without_magic_or_parent || $class_storage->parent_class))
|| $found_method_and_class_storage
) {
$callstatic_id = new MethodIdentifier(
$fq_class_name,
'__callstatic',
);

if ($codebase->methods->methodExists(
$callstatic_id,
$context->calling_method_id,
$codebase->collect_locations
? new CodeLocation($statements_analyzer, $stmt_name)
: null,
!$context->collect_initializations
&& !$context->collect_mutations
? $statements_analyzer
: null,
$statements_analyzer->getFilePath(),
true,
$context->insideUse(),
)) {
if ($callstatic_method_exists) {
$callstatic_declaring_id = $codebase->methods->getDeclaringMethodId($callstatic_id);
assert($callstatic_declaring_id !== null);
$callstatic_pure = false;
Expand Down Expand Up @@ -691,39 +710,7 @@ private static function handleNamedCall(
return false;
}
}

$array_values = array_map(
static fn(PhpParser\Node\Arg $arg): PhpParser\Node\Expr\ArrayItem => new VirtualArrayItem(
$arg->value,
null,
false,
$arg->getAttributes(),
),
$args,
);

$args = [
new VirtualArg(
new VirtualString((string) $method_id, $stmt_name->getAttributes()),
false,
false,
$stmt_name->getAttributes(),
),
new VirtualArg(
new VirtualArray($array_values, $stmt->getAttributes()),
false,
false,
$stmt->getAttributes(),
),
];

$method_id = new MethodIdentifier(
$fq_class_name,
'__callstatic',
);
} elseif ($found_method_and_class_storage
&& ($config->use_phpdoc_method_without_magic_or_parent || $class_storage->parent_class)
) {
} elseif ($found_method_and_class_storage) {
[$pseudo_method_storage, $defining_class_storage] = $found_method_and_class_storage;

if (self::checkPseudoMethod(
Expand All @@ -740,7 +727,9 @@ private static function handleNamedCall(
return false;
}

if ($pseudo_method_storage->return_type) {
if ($pseudo_method_storage->return_type
&& ($naive_method_exists || $with_pseudo)
) {
return true;
}
} elseif ($stmt->class instanceof PhpParser\Node\Name && $stmt->class->getFirst() === 'parent'
Expand Down Expand Up @@ -802,13 +791,18 @@ private static function handleNamedCall(
}
}

$does_method_exist = MethodAnalyzer::checkMethodExists(
$codebase,
$method_id,
new CodeLocation($statements_analyzer, $stmt),
$statements_analyzer->getSuppressedIssues(),
$context->calling_method_id,
);
if (!$callstatic_method_exists || $class_storage->hasSealedMethods($config)) {
$does_method_exist = MethodAnalyzer::checkMethodExists(
$codebase,
$method_id,
new CodeLocation($statements_analyzer, $stmt),
$statements_analyzer->getSuppressedIssues(),
$context->calling_method_id,
$with_pseudo,
);
} else {
$does_method_exist = null;
}

if (!$does_method_exist) {
if (ArgumentsAnalyzer::analyze(
Expand Down Expand Up @@ -870,37 +864,6 @@ private static function handleNamedCall(
return false;
}

if ((!$stmt->class instanceof PhpParser\Node\Name
|| $stmt->class->getFirst() !== 'parent'
|| $statements_analyzer->isStatic())
&& (
!$context->self
|| $statements_analyzer->isStatic()
|| !$codebase->classExtends($context->self, $fq_class_name)
)
) {
MethodAnalyzer::checkStatic(
$method_id,
($stmt->class instanceof PhpParser\Node\Name
&& strtolower($stmt->class->getFirst()) === 'self')
|| $context->self === $fq_class_name,
!$statements_analyzer->isStatic(),
$codebase,
new CodeLocation($statements_analyzer, $stmt),
$statements_analyzer->getSuppressedIssues(),
$is_dynamic_this_method,
);

if ($is_dynamic_this_method) {
return self::forwardCallToInstanceMethod(
$statements_analyzer,
$stmt,
$stmt_name,
$context,
);
}
}

$has_existing_method = true;

ExistingAtomicStaticCallAnalyzer::analyze(
Expand Down