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

add function and consistent error for internal property #10867

Open
wants to merge 5 commits into
base: 5.x
Choose a base branch
from
Open
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
9 changes: 8 additions & 1 deletion src/Psalm/Internal/Analyzer/CommentAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

use function count;
use function is_string;
use function preg_grep;
use function preg_match;
use function preg_replace;
use function preg_split;
Expand Down Expand Up @@ -196,6 +197,8 @@ public static function arrayToDocblocks(
if (!$var_comments
&& (isset($parsed_docblock->tags['deprecated'])
|| isset($parsed_docblock->tags['internal'])
|| (isset($parsed_docblock->tags['access'])
&& preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))
|| isset($parsed_docblock->tags['readonly'])
|| isset($parsed_docblock->tags['psalm-readonly'])
|| isset($parsed_docblock->tags['psalm-readonly-allow-private-mutation'])
Expand All @@ -220,7 +223,11 @@ private static function decorateVarDocblockComment(
ParsedDocblock $parsed_docblock
): void {
$var_comment->deprecated = isset($parsed_docblock->tags['deprecated']);
$var_comment->internal = isset($parsed_docblock->tags['internal']);
if (isset($parsed_docblock->tags['internal'])
|| (isset($parsed_docblock->tags['access'])
&& preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))) {
$var_comment->internal = true;
}
$var_comment->readonly = isset($parsed_docblock->tags['readonly'])
|| isset($parsed_docblock->tags['psalm-readonly'])
|| isset($parsed_docblock->tags['psalm-readonly-allow-private-mutation']);
Expand Down
8 changes: 7 additions & 1 deletion src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,17 @@ public static function isWithin(string $calling_identifier, string $identifier):
*/
public static function isWithinAny(string $calling_identifier, array $identifiers): bool
{
if (count($identifiers) === 0) {
if ($identifiers === []) {
return true;
}

foreach ($identifiers as $identifier) {
if ($calling_identifier !== '' && $identifier === '') {
// if we have any non-empty, we ignore the empty one
// therefore continue instead of return
continue;
}

if (self::isWithin($calling_identifier, $identifier)) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Psalm\Internal\Analyzer\ClassAnalyzer;
use Psalm\Internal\Analyzer\ClassLikeAnalyzer;
use Psalm\Internal\Analyzer\FunctionLikeAnalyzer;
use Psalm\Internal\Analyzer\NamespaceAnalyzer;
use Psalm\Internal\Analyzer\Statements\Expression\Call\ClassTemplateParamCollector;
use Psalm\Internal\Analyzer\Statements\Expression\Call\MethodCallAnalyzer;
use Psalm\Internal\Analyzer\Statements\Expression\ExpressionIdentifier;
Expand All @@ -33,12 +32,9 @@
use Psalm\Internal\Type\TemplateResult;
use Psalm\Internal\Type\TemplateStandinTypeReplacer;
use Psalm\Internal\Type\TypeExpander;
use Psalm\Issue\DeprecatedProperty;
use Psalm\Issue\ImplicitToStringCast;
use Psalm\Issue\ImpurePropertyAssignment;
use Psalm\Issue\InaccessibleProperty;
use Psalm\Issue\InternalClass;
use Psalm\Issue\InternalProperty;
use Psalm\Issue\InvalidPropertyAssignment;
use Psalm\Issue\InvalidPropertyAssignmentValue;
use Psalm\Issue\LoopInvalidation;
Expand Down Expand Up @@ -1257,30 +1253,22 @@ private static function analyzeAtomicAssignment(
$declaring_class_storage = $codebase->classlike_storage_provider->get($declaring_property_class);

if (isset($declaring_class_storage->properties[$prop_name])) {
$property_storage = $declaring_class_storage->properties[$prop_name];
AtomicPropertyFetchAnalyzer::checkPropertyDeprecation(
$prop_name,
$declaring_property_class,
$stmt,
$statements_analyzer,
);

if ($property_storage->deprecated) {
IssueBuffer::maybeAdd(
new DeprecatedProperty(
$property_id . ' is marked deprecated',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
AtomicPropertyFetchAnalyzer::checkPropertyInternal(
$prop_name,
$declaring_property_class,
$stmt,
$statements_analyzer,
$context,
);

if ($context->self && !NamespaceAnalyzer::isWithinAny($context->self, $property_storage->internal)) {
IssueBuffer::maybeAdd(
new InternalProperty(
$property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal)
. ' but called from ' . $context->self,
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
$property_storage = $declaring_class_storage->properties[$prop_name];

self::trackPropertyImpurity(
$statements_analyzer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Psalm\Internal\Algebra\FormulaGenerator;
use Psalm\Internal\Analyzer\AlgebraAnalyzer;
use Psalm\Internal\Analyzer\FunctionLikeAnalyzer;
use Psalm\Internal\Analyzer\NamespaceAnalyzer;
use Psalm\Internal\Analyzer\Statements\Expression\CallAnalyzer;
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
use Psalm\Internal\Analyzer\StatementsAnalyzer;
Expand All @@ -23,6 +24,8 @@
use Psalm\Internal\Type\TypeCombiner;
use Psalm\Issue\DeprecatedFunction;
use Psalm\Issue\ImpureFunctionCall;
use Psalm\Issue\InternalClass;
use Psalm\Issue\InternalMethod;
use Psalm\Issue\InvalidFunctionCall;
use Psalm\Issue\MixedFunctionCall;
use Psalm\Issue\NullFunctionCall;
Expand Down Expand Up @@ -402,6 +405,30 @@ public static function analyze(
$statements_analyzer->getSuppressedIssues(),
);
}

if (!$context->collect_initializations
&& !$context->collect_mutations
&& $function_call_info->function_id !== null
) {
$caller_identifier = $statements_analyzer->getFullyQualifiedFunctionMethodOrNamespaceName();
if ($caller_identifier !== null
&& !NamespaceAnalyzer::isWithinAny(
$caller_identifier,
$function_call_info->function_storage->internal,
)) {
IssueBuffer::maybeAdd(
new InternalMethod(
'The function ' . $function_call_info->function_id
. ' is internal to '
. InternalClass::listToPhrase($function_call_info->function_storage->internal)
. ' but called from ' . ($caller_identifier ?: 'root namespace'),
$code_location,
$function_call_info->function_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
}
}

if ($function_name instanceof PhpParser\Node\Name && $function_call_info->function_id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,16 @@ private static function analyzeNamedConstructor(
);
}


if ($context->self
$caller_identifier = $context->self ?? $statements_analyzer->getNamespace();
if ($caller_identifier !== null
&& !$context->collect_initializations
&& !$context->collect_mutations
&& !NamespaceAnalyzer::isWithinAny($context->self, $storage->internal)
&& !NamespaceAnalyzer::isWithinAny($caller_identifier, $storage->internal)
) {
IssueBuffer::maybeAdd(
new InternalClass(
$fq_class_name . ' is internal to ' . InternalClass::listToPhrase($storage->internal)
. ' but called from ' . $context->self,
. ' but called from ' . ($caller_identifier ?: 'root namespace'),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$fq_class_name,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -840,11 +840,13 @@ private static function handleNamedCall(
);
}

if ($context->self && ! NamespaceAnalyzer::isWithinAny($context->self, $class_storage->internal)) {
$caller_identifier = $context->self ?? $statements_analyzer->getNamespace();
if ($caller_identifier !== null
&& !NamespaceAnalyzer::isWithinAny($caller_identifier, $class_storage->internal)) {
IssueBuffer::maybeAdd(
new InternalClass(
$fq_class_name . ' is internal to ' . InternalClass::listToPhrase($class_storage->internal)
. ' but called from ' . $context->self,
. ' but called from ' . ($caller_identifier ?: 'root namespace'),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$fq_class_name,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,16 +365,17 @@ public static function analyzeFetch(
}
}

if ($context->self
$caller_identifier = $context->self ?? $statements_analyzer->getNamespace();
if ($caller_identifier !== null
&& !$context->collect_initializations
&& !$context->collect_mutations
&& !NamespaceAnalyzer::isWithinAny($context->self, $const_class_storage->internal)
&& !NamespaceAnalyzer::isWithinAny($caller_identifier, $const_class_storage->internal)
) {
IssueBuffer::maybeAdd(
new InternalClass(
$fq_class_name . ' is internal to '
. InternalClass::listToPhrase($const_class_storage->internal)
. ' but called from ' . $context->self,
. ' but called from ' . ($caller_identifier ?: 'root namespace'),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$fq_class_name,
),
Expand Down Expand Up @@ -656,16 +657,17 @@ public static function analyzeFetch(
}
}

if ($context->self
$caller_identifier = $context->self ?? $statements_analyzer->getNamespace();
if ($caller_identifier !== null
&& !$context->collect_initializations
&& !$context->collect_mutations
&& !NamespaceAnalyzer::isWithinAny($context->self, $const_class_storage->internal)
&& !NamespaceAnalyzer::isWithinAny($caller_identifier, $const_class_storage->internal)
) {
IssueBuffer::maybeAdd(
new InternalClass(
$fq_class_name . ' is internal to '
. InternalClass::listToPhrase($const_class_storage->internal)
. ' but called from ' . $context->self,
. ' but called from ' . ($caller_identifier ?: 'root namespace'),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$fq_class_name,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,19 +456,9 @@ public static function analyze(
if (isset($declaring_class_storage->properties[$prop_name])) {
self::checkPropertyDeprecation($prop_name, $declaring_property_class, $stmt, $statements_analyzer);

$property_storage = $declaring_class_storage->properties[$prop_name];
self::checkPropertyInternal($prop_name, $declaring_property_class, $stmt, $statements_analyzer, $context);

if ($context->self && !NamespaceAnalyzer::isWithinAny($context->self, $property_storage->internal)) {
IssueBuffer::maybeAdd(
new InternalProperty(
$property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal)
. ' but called from ' . $context->self,
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
$property_storage = $declaring_class_storage->properties[$prop_name];

if ($context->inside_unset) {
InstancePropertyAssignmentAnalyzer::trackPropertyImpurity(
Expand Down Expand Up @@ -572,6 +562,42 @@ public static function checkPropertyDeprecation(
}
}

/**
* @param PropertyFetch|StaticPropertyFetch $stmt
*/
public static function checkPropertyInternal(
string $prop_name,
string $declaring_property_class,
PhpParser\Node\Expr $stmt,
StatementsAnalyzer $statements_analyzer,
Context $context
): void {
$property_id = $declaring_property_class . '::$' . $prop_name;
$codebase = $statements_analyzer->getCodebase();
$declaring_class_storage = $codebase->classlike_storage_provider->get(
$declaring_property_class,
);

if (isset($declaring_class_storage->properties[$prop_name])) {
$property_storage = $declaring_class_storage->properties[$prop_name];

$caller_identifier = $context->self ?? $statements_analyzer->getNamespace();

if ($caller_identifier !== null
&& !NamespaceAnalyzer::isWithinAny($caller_identifier, $property_storage->internal)) {
IssueBuffer::maybeAdd(
new InternalProperty(
$property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal)
. ' but called from ' . ($caller_identifier ?: 'root namespace'),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
}
}

private static function propertyFetchCanBeAnalyzed(
StatementsAnalyzer $statements_analyzer,
Codebase $codebase,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,14 @@ private static function handleScopedProperty(
$stmt,
$statements_analyzer,
);

AtomicPropertyFetchAnalyzer::checkPropertyInternal(
$stmt->name->name,
$declaring_property_class,
$stmt,
$statements_analyzer,
$context,
);
}

$codebase->properties->propertyExists(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,14 @@ public static function analyze(
$statements_analyzer,
);

AtomicPropertyFetchAnalyzer::checkPropertyInternal(
$prop_name,
$declaring_property_class,
$stmt,
$statements_analyzer,
$context,
);

$class_storage = $codebase->classlike_storage_provider->get($declaring_property_class);
$property = $class_storage->properties[$prop_name];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use function explode;
use function implode;
use function in_array;
use function preg_grep;
use function preg_last_error_msg;
use function preg_match;
use function preg_replace;
Expand Down Expand Up @@ -204,7 +205,9 @@ public static function parse(
$info->deprecated = true;
}

if (isset($parsed_docblock->tags['internal'])) {
if (isset($parsed_docblock->tags['internal'])
|| (isset($parsed_docblock->tags['access'])
&& preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))) {
$info->internal = true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,8 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
$storage->internal = $docblock_info->psalm_internal;
} elseif ($docblock_info->internal && $this->aliases->namespace) {
$storage->internal = [NamespaceAnalyzer::getNameSpaceRoot($this->aliases->namespace)];
} elseif ($docblock_info->internal) {
$storage->internal = [''];
}

if ($docblock_info->final && !$storage->final) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use function implode;
use function in_array;
use function pathinfo;
use function preg_grep;
use function preg_last_error_msg;
use function preg_match;
use function preg_replace;
Expand Down Expand Up @@ -436,7 +437,9 @@ public static function parse(
$info->deprecated = true;
}

if (isset($parsed_docblock->tags['internal'])) {
if (isset($parsed_docblock->tags['internal'])
|| (isset($parsed_docblock->tags['access'])
&& preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))) {
$info->internal = true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ public static function addDocblockInfo(
$storage->internal = $docblock_info->psalm_internal;
} elseif ($docblock_info->internal && $aliases->namespace) {
$storage->internal = [NamespaceAnalyzer::getNameSpaceRoot($aliases->namespace)];
} elseif ($docblock_info->internal && $classlike_storage && $classlike_storage->name) {
$storage->internal = [NamespaceAnalyzer::getNameSpaceRoot($classlike_storage->name)];
} elseif ($docblock_info->internal) {
$storage->internal = [''];
}

if (($storage->internal || ($classlike_storage && $classlike_storage->internal))
Expand Down