Skip to content

Commit b671117

Browse files
AndrolGenhaldAndrolGenhald
committed
Improve @psalm-internal and prevent usage of IssueBuffer::add().
1 parent 9b4c8cb commit b671117

35 files changed

Lines changed: 347 additions & 138 deletions

‎src/Psalm/Internal/Analyzer/ClassAnalyzer.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ public function __construct(PhpParser\Node\Stmt $class, SourceAnalyzer $source,
135135
}
136136
}
137137

138+
/** @return non-empty-string */
138139
public static function getAnonymousClassName(PhpParser\Node\Stmt\Class_ $class, string $file_path): string
139140
{
140141
return preg_replace('/[^A-Za-z0-9]/', '_', $file_path)
@@ -251,7 +252,7 @@ public function analyze(
251252
}
252253

253254
foreach ($storage->docblock_issues as $docblock_issue) {
254-
IssueBuffer::add($docblock_issue);
255+
IssueBuffer::maybeAdd($docblock_issue);
255256
}
256257

257258
$classlike_storage_provider = $codebase->classlike_storage_provider;
@@ -1647,7 +1648,7 @@ private function analyzeClassMethod(
16471648
$config = Config::getInstance();
16481649

16491650
if ($stmt->stmts === null && !$stmt->isAbstract()) {
1650-
IssueBuffer::add(
1651+
IssueBuffer::maybeAdd(
16511652
new ParseError(
16521653
'Non-abstract class method must have statements',
16531654
new CodeLocation($this, $stmt)
@@ -1660,7 +1661,7 @@ private function analyzeClassMethod(
16601661
try {
16611662
$method_analyzer = new MethodAnalyzer($stmt, $source);
16621663
} catch (UnexpectedValueException $e) {
1663-
IssueBuffer::add(
1664+
IssueBuffer::maybeAdd(
16641665
new ParseError(
16651666
'Problem loading method: ' . $e->getMessage(),
16661667
new CodeLocation($this, $stmt)
@@ -2506,11 +2507,12 @@ private function checkParentClass(
25062507
);
25072508
}
25082509

2509-
if (!NamespaceAnalyzer::isWithin($fq_class_name, $parent_class_storage->internal)) {
2510+
if (!NamespaceAnalyzer::isWithinAny($fq_class_name, $parent_class_storage->internal)) {
25102511
IssueBuffer::maybeAdd(
25112512
new InternalClass(
2512-
$parent_fq_class_name . ' is internal to ' . $parent_class_storage->internal
2513-
. ' but called from ' . $fq_class_name,
2513+
$parent_fq_class_name . ' is internal to '
2514+
. InternalClass::listToPhrase($parent_class_storage->internal)
2515+
. ' but called from ' . $fq_class_name,
25142516
$code_location,
25152517
$parent_fq_class_name
25162518
),

‎src/Psalm/Internal/Analyzer/CommentAnalyzer.php‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Psalm\Exception\IncorrectDocblockException;
1010
use Psalm\Exception\TypeParseTreeException;
1111
use Psalm\FileSource;
12+
use Psalm\Internal\Scanner\DocblockParser;
1213
use Psalm\Internal\Scanner\ParsedDocblock;
1314
use Psalm\Internal\Scanner\VarDocblockComment;
1415
use Psalm\Internal\Type\TypeAlias;
@@ -21,7 +22,6 @@
2122
use function preg_match;
2223
use function preg_replace;
2324
use function preg_split;
24-
use function reset;
2525
use function rtrim;
2626
use function str_replace;
2727
use function strlen;
@@ -236,14 +236,7 @@ private static function decorateVarDocblockComment(
236236
}
237237
}
238238

239-
if (isset($parsed_docblock->tags['psalm-internal'])) {
240-
$psalm_internal = trim(reset($parsed_docblock->tags['psalm-internal']));
241-
242-
if (!$psalm_internal) {
243-
throw new DocblockParseException('psalm-internal annotation used without specifying namespace');
244-
}
245-
246-
$var_comment->psalm_internal = $psalm_internal;
239+
if (count($var_comment->psalm_internal = DocblockParser::handlePsalmInternal($parsed_docblock)) !== 0) {
247240
$var_comment->internal = true;
248241
}
249242

‎src/Psalm/Internal/Analyzer/FileAnalyzer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ public function analyze(
196196
$statements_analyzer = new StatementsAnalyzer($this, $this->node_data);
197197

198198
foreach ($file_storage->docblock_issues as $docblock_issue) {
199-
IssueBuffer::add($docblock_issue);
199+
IssueBuffer::maybeAdd($docblock_issue);
200200
}
201201

202202
// if there are any leftover statements, evaluate them,

‎src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ public function analyze(
209209
}
210210

211211
foreach ($storage->docblock_issues as $docblock_issue) {
212-
IssueBuffer::add($docblock_issue);
212+
IssueBuffer::maybeAdd($docblock_issue);
213213
}
214214

215215
$function_information = $this->getFunctionInformation(

‎src/Psalm/Internal/Analyzer/InterfaceAnalyzer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public function analyze(): void
133133
);
134134
}
135135
} elseif ($stmt instanceof PhpParser\Node\Stmt\Property) {
136-
IssueBuffer::add(
136+
IssueBuffer::maybeAdd(
137137
new ParseError(
138138
'Interfaces cannot have properties',
139139
new CodeLocation($this, $stmt)

‎src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php‎

Lines changed: 108 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
use ReflectionProperty;
1313
use UnexpectedValueException;
1414

15+
use function assert;
16+
use function count;
1517
use function implode;
1618
use function preg_replace;
1719
use function strpos;
1820
use function strtolower;
19-
use function trim;
21+
use function substr;
2022

2123
/**
2224
* @internal
@@ -152,33 +154,127 @@ public function getFileAnalyzer(): FileAnalyzer
152154
}
153155

154156
/**
155-
* Returns true if $className is the same as, or starts with $namespace, in a case-insensitive comparison.
157+
* Returns true if $calling_identifier is the same as, or is within with $identifier, in a
158+
* case-insensitive comparison. Identifiers can be namespaces, classlikes, functions, or methods.
156159
*
160+
* @psalm-pure
161+
*
162+
* @throws InvalidArgumentException if $identifier is not a valid identifier
163+
*/
164+
public static function isWithin(string $calling_identifier, string $identifier): bool
165+
{
166+
$normalized_calling_ident = self::normalizeIdentifier($calling_identifier);
167+
$normalized_ident = self::normalizeIdentifier($identifier);
168+
169+
if ($normalized_calling_ident === $normalized_ident) {
170+
return true;
171+
}
172+
173+
$normalized_calling_ident_parts = self::getIdentifierParts($normalized_calling_ident);
174+
$normalized_ident_parts = self::getIdentifierParts($normalized_ident);
175+
176+
if (count($normalized_calling_ident_parts) < count($normalized_ident_parts)) {
177+
return false;
178+
}
179+
180+
for ($i = 0; $i < count($normalized_ident_parts); ++$i) {
181+
if ($normalized_ident_parts[$i] !== $normalized_calling_ident_parts[$i]) {
182+
return false;
183+
}
184+
}
185+
186+
return true;
187+
}
188+
189+
/**
190+
* Returns true if $calling_identifier is the same as or is within any identifier
191+
* in $identifiers in a case-insensitive comparison, or if $identifiers is empty.
192+
* Identifiers can be namespaces, classlikes, functions, or methods.
157193
*
158194
* @psalm-pure
195+
*
196+
* @psalm-assert-if-false !empty $identifiers
197+
*
198+
* @param list<string> $identifiers
159199
*/
160-
public static function isWithin(string $calling_namespace, string $namespace): bool
200+
public static function isWithinAny(string $calling_identifier, array $identifiers): bool
161201
{
162-
if ($namespace === '') {
163-
return true; // required to prevent a warning from strpos with empty needle in PHP < 8
202+
if (count($identifiers) === 0) {
203+
return true;
164204
}
165205

166-
$calling_namespace = strtolower(trim($calling_namespace, '\\') . '\\');
167-
$namespace = strtolower(trim($namespace, '\\') . '\\');
206+
foreach ($identifiers as $identifier) {
207+
if (self::isWithin($calling_identifier, $identifier)) {
208+
return true;
209+
}
210+
}
168211

169-
return $calling_namespace === $namespace
170-
|| strpos($calling_namespace, $namespace) === 0;
212+
return false;
171213
}
172214

173215
/**
174-
* @param string $fullyQualifiedClassName, e.g. '\Psalm\Internal\Analyzer\NamespaceAnalyzer'
216+
* @param non-empty-string $fullyQualifiedClassName, e.g. '\Psalm\Internal\Analyzer\NamespaceAnalyzer'
175217
*
176-
* @return string , e.g. 'Psalm'
218+
* @return non-empty-string , e.g. 'Psalm'
177219
*
178220
* @psalm-pure
179221
*/
180222
public static function getNameSpaceRoot(string $fullyQualifiedClassName): string
181223
{
182-
return preg_replace('/^([^\\\]+).*/', '$1', $fullyQualifiedClassName);
224+
$root_namespace = preg_replace('/^([^\\\]+).*/', '$1', $fullyQualifiedClassName);
225+
if ($root_namespace === "") {
226+
throw new InvalidArgumentException("Invalid classname \"$fullyQualifiedClassName\"");
227+
}
228+
return $root_namespace;
229+
}
230+
231+
/**
232+
* @return ($lowercase is true ? lowercase-string : string)
233+
*
234+
* @psalm-pure
235+
*/
236+
public static function normalizeIdentifier(string $identifier, bool $lowercase = true): string
237+
{
238+
if ($identifier === "") {
239+
return "";
240+
}
241+
242+
$identifier = $identifier[0] === "\\" ? substr($identifier, 1) : $identifier;
243+
return $lowercase ? strtolower($identifier) : $identifier;
244+
}
245+
246+
/**
247+
* Splits an identifier into parts, eg `Foo\Bar::baz` becomes ["Foo", "\\", "Bar", "::", "baz"].
248+
*
249+
* @return list<non-empty-string>
250+
*
251+
* @psalm-pure
252+
*/
253+
public static function getIdentifierParts(string $identifier): array
254+
{
255+
$parts = [];
256+
while (($pos = strpos($identifier, "\\")) !== false) {
257+
if ($pos > 0) {
258+
$part = substr($identifier, 0, $pos);
259+
assert($part !== "");
260+
$parts[] = $part;
261+
}
262+
$parts[] = "\\";
263+
$identifier = substr($identifier, $pos + 1);
264+
}
265+
if (($pos = strpos($identifier, "::")) !== false) {
266+
if ($pos > 0) {
267+
$part = substr($identifier, 0, $pos);
268+
assert($part !== "");
269+
$parts[] = $part;
270+
}
271+
$parts[] = "::";
272+
$identifier = substr($identifier, $pos + 2);
273+
}
274+
if ($identifier !== "") {
275+
$parts[] = $identifier;
276+
}
277+
278+
return $parts;
183279
}
184280
}

‎src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public static function analyze(
7474

7575
foreach ($stmt->items as $item) {
7676
if ($item === null) {
77-
IssueBuffer::add(
77+
IssueBuffer::maybeAdd(
7878
new ParseError(
7979
'Array element cannot be empty',
8080
new CodeLocation($statements_analyzer, $stmt)

‎src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ protected static function processCustomAssertion(
959959
}
960960
} elseif ($assertion->var_id === '$this') {
961961
if (!$expr instanceof PhpParser\Node\Expr\MethodCall) {
962-
IssueBuffer::add(
962+
IssueBuffer::maybeAdd(
963963
new InvalidDocblock(
964964
'Assertion of $this can be done only on method of a class',
965965
new CodeLocation($source, $expr)

‎src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use Psalm\Issue\ImplicitToStringCast;
3838
use Psalm\Issue\ImpurePropertyAssignment;
3939
use Psalm\Issue\InaccessibleProperty;
40+
use Psalm\Issue\InternalClass;
4041
use Psalm\Issue\InternalProperty;
4142
use Psalm\Issue\InvalidPropertyAssignment;
4243
use Psalm\Issue\InvalidPropertyAssignmentValue;
@@ -420,7 +421,7 @@ public static function analyzeStatement(
420421
foreach ($stmt->props as $prop) {
421422
if ($prop->default) {
422423
if ($stmt->isReadonly()) {
423-
IssueBuffer::add(
424+
IssueBuffer::maybeAdd(
424425
new InvalidPropertyAssignment(
425426
'Readonly property ' . $context->self . '::$' . $prop->name->name
426427
. ' cannot have a default',
@@ -1258,11 +1259,11 @@ private static function analyzeAtomicAssignment(
12581259
);
12591260
}
12601261

1261-
if ($context->self && !NamespaceAnalyzer::isWithin($context->self, $property_storage->internal)) {
1262+
if ($context->self && !NamespaceAnalyzer::isWithinAny($context->self, $property_storage->internal)) {
12621263
IssueBuffer::maybeAdd(
12631264
new InternalProperty(
1264-
$property_id . ' is internal to ' . $property_storage->internal
1265-
. ' but called from ' . $context->self,
1265+
$property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal)
1266+
. ' but called from ' . $context->self,
12661267
new CodeLocation($statements_analyzer->getSource(), $stmt),
12671268
$property_id
12681269
),

‎src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public static function analyze(
271271
$codebase,
272272
$context,
273273
$method_id,
274-
$statements_analyzer->getNamespace(),
274+
$statements_analyzer->getFullyQualifiedFunctionMethodOrNamespaceName(),
275275
$name_code_location,
276276
$statements_analyzer->getSuppressedIssues()
277277
);

0 commit comments

Comments
 (0)