Skip to content

Commit

Permalink
Add support for @psalm-no-seal-properties and @psalm-no-seal-methods
Browse files Browse the repository at this point in the history
  • Loading branch information
robchett committed Apr 20, 2023
1 parent 5efddb4 commit bac4f3e
Show file tree
Hide file tree
Showing 13 changed files with 122 additions and 22 deletions.
26 changes: 25 additions & 1 deletion docs/annotating_code/supported_annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,10 @@ takesFoo(getFoo());

This provides the same, but for `false`. Psalm uses this internally for functions like `preg_replace`, which can return false if the given input has encoding errors, but where 99.9% of the time the function operates as expected.

### `@psalm-seal-properties`
### `@psalm-seal-properties`, `@psalm-no-seal-properties`

If you have a magic property getter/setter, you can use `@psalm-seal-properties` to instruct Psalm to disallow getting and setting any properties not contained in a list of `@property` (or `@property-read`/`@property-write`) annotations.
This is automatically enabled with the configuration option `sealAllProperties` and can be disabled for a class with `@psalm-no-seal-properties`

```php
<?php
Expand All @@ -181,6 +182,29 @@ $a = new A();
$a->bar = 5; // this call fails
```

### `@psalm-seal-methods`, `@psalm-no-seal-methods`

If you have a magic method caller, you can use `@psalm-seal-methods` to instruct Psalm to disallow calling any methods not contained in a list of `@method` annotations.
This is automatically enabled with the configuration option `sealAllMethods` and can be disabled for a class with `@psalm-no-seal-methods`

```php
<?php
/**
* @method foo(): string
* @psalm-seal-methods
*/
class A {
public function __call(string $name, array $args) {
if ($name === "foo") {
return "hello";
}
}
}

$a = new A();
$b = $a->bar(); // this call fails
```

### `@psalm-internal`

Used to mark a class, property or function as internal to a given namespace. Psalm treats this slightly differently to
Expand Down
1 change: 1 addition & 0 deletions src/Psalm/DocComment.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ final class DocComment
'assert', 'assert-if-true', 'assert-if-false', 'suppress',
'ignore-nullable-return', 'override-property-visibility',
'override-method-visibility', 'seal-properties', 'seal-methods',
'no-seal-properties', 'no-seal-methods',
'ignore-falsable-return', 'variadic', 'pure',
'ignore-variable-method', 'ignore-variable-property', 'internal',
'taint-sink', 'taint-source', 'assert-untainted', 'scope-this',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ private static function analyzeAtomicAssignment(
* If we have an explicit list of all allowed magic properties on the class, and we're
* not in that list, fall through
*/
if (!$var_id || !$class_storage->sealed_properties) {
if (!$var_id || !$class_storage->hasSealedProperties()) {
if (!$context->collect_initializations && !$context->collect_mutations) {
self::taintProperty(
$statements_analyzer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ private static function getMagicGetterOrSetterProperty(
case '__set':
// If `@psalm-seal-properties` is set, the property must be defined with
// a `@property` annotation
if (($class_storage->sealed_properties || $codebase->config->seal_all_properties)
if (($class_storage->hasSealedProperties())
&& !isset($class_storage->pseudo_property_set_types['$' . $prop_name])
) {
IssueBuffer::maybeAdd(
Expand Down Expand Up @@ -644,7 +644,7 @@ private static function getMagicGetterOrSetterProperty(
case '__get':
// If `@psalm-seal-properties` is set, the property must be defined with
// a `@property` annotation
if (($class_storage->sealed_properties || $codebase->config->seal_all_properties)
if (($class_storage->hasSealedProperties())
&& !isset($class_storage->pseudo_property_get_types['$' . $prop_name])
) {
IssueBuffer::maybeAdd(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public static function handleMagicMethod(
$context,
);

if ($class_storage->sealed_methods || $config->seal_all_methods) {
if ($class_storage->hasSealedMethods()) {
$result->non_existent_magic_method_ids[] = $method_id->__toString();

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ private static function propertyFetchCanBeAnalyzed(
* If we have an explicit list of all allowed magic properties on the class, and we're
* not in that list, fall through
*/
if (!($class_storage->sealed_properties || $codebase->config->seal_all_properties)
if (!($class_storage->hasSealedProperties())
&& !$override_property_visibility
) {
return false;
Expand Down
8 changes: 4 additions & 4 deletions src/Psalm/Internal/Codebase/Populator.php
Original file line number Diff line number Diff line change
Expand Up @@ -920,8 +920,8 @@ protected function inheritMethodsFromParent(
$fq_class_name = $storage->name;
$fq_class_name_lc = strtolower($fq_class_name);

if ($parent_storage->sealed_methods) {
$storage->sealed_methods = true;
if ($parent_storage->sealed_methods !== null) {
$storage->sealed_methods = $parent_storage->sealed_methods;
}

// register where they appear (can never be in a trait)
Expand Down Expand Up @@ -1032,8 +1032,8 @@ private function inheritPropertiesFromParent(
ClassLikeStorage $storage,
ClassLikeStorage $parent_storage
): void {
if ($parent_storage->sealed_properties) {
$storage->sealed_properties = true;
if ($parent_storage->sealed_properties !== null) {
$storage->sealed_properties = $parent_storage->sealed_properties;
}

// register where they appear (can never be in a trait)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,16 @@ public static function parse(
if (isset($parsed_docblock->tags['psalm-seal-properties'])) {
$info->sealed_properties = true;
}
if (isset($parsed_docblock->tags['psalm-no-seal-properties'])) {
$info->sealed_properties = false;
}

if (isset($parsed_docblock->tags['psalm-seal-methods'])) {
$info->sealed_methods = true;
}
if (isset($parsed_docblock->tags['psalm-no-seal-methods'])) {
$info->sealed_methods = false;
}

if (isset($parsed_docblock->tags['psalm-immutable'])
|| isset($parsed_docblock->tags['psalm-mutation-free'])
Expand Down Expand Up @@ -296,6 +302,9 @@ public static function parse(
}

if (isset($parsed_docblock->combined_tags['method'])) {
if ($info->sealed_methods === null) {
$info->sealed_methods = true;
}
foreach ($parsed_docblock->combined_tags['method'] as $offset => $method_entry) {
$method_entry = preg_replace('/[ \t]+/', ' ', trim($method_entry));

Expand Down Expand Up @@ -481,6 +490,12 @@ public static function parse(

$info->public_api = isset($parsed_docblock->tags['psalm-api']) || isset($parsed_docblock->tags['api']);

if (isset($parsed_docblock->tags['property'])) {
if ($info->sealed_properties === null) {
$info->sealed_properties = true;
}
}

self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'property');
self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'psalm-property');
self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'property-read');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,10 +577,6 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
);
}
}

if ($this->config->docblock_property_types_seal_properties) {
$storage->sealed_properties = true;
}
}

foreach ($docblock_info->methods as $method) {
Expand All @@ -607,8 +603,6 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
$lc_method_name,
);
}

$storage->sealed_methods = true;
}


Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ class ClassLikeDocblockComment
*/
public array $methods = [];

public bool $sealed_properties = false;
public ?bool $sealed_properties = null;

public bool $sealed_methods = false;
public ?bool $sealed_methods = null;

public bool $override_property_visibility = false;

Expand Down
25 changes: 21 additions & 4 deletions src/Psalm/Storage/ClassLikeStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Psalm\Aliases;
use Psalm\CodeLocation;
use Psalm\Config;
use Psalm\Internal\Analyzer\ClassLikeAnalyzer;
use Psalm\Internal\MethodIdentifier;
use Psalm\Internal\Type\TypeAlias\ClassTypeAlias;
Expand Down Expand Up @@ -66,14 +67,14 @@ final class ClassLikeStorage implements HasAttributesInterface
public $mixin_declaring_fqcln;

/**
* @var bool
* @var ?bool
*/
public $sealed_properties = false;
public $sealed_properties = null;

Check failure on line 72 in src/Psalm/Storage/ClassLikeStorage.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility

Property Psalm\Storage\ClassLikeStorage#$sealed_properties changed default value from false to NULL

/**
* @var bool
* @var ?bool
*/
public $sealed_methods = false;
public $sealed_methods = null;

Check failure on line 77 in src/Psalm/Storage/ClassLikeStorage.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility

Property Psalm\Storage\ClassLikeStorage#$sealed_methods changed default value from false to NULL

/**
* @var bool
Expand Down Expand Up @@ -500,4 +501,20 @@ public function getClassTemplateTypes(): array

return $type_params;
}

public function hasSealedProperties(): bool
{
return $this->sealed_properties !== null ?
$this->sealed_properties :
Config::getInstance()->seal_all_properties
;
}

public function hasSealedMethods(): bool
{
return $this->sealed_methods !== null ?
$this->sealed_methods :
Config::getInstance()->seal_all_methods
;
}
}
25 changes: 25 additions & 0 deletions tests/MagicMethodAnnotationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,31 @@ class B extends A {}
$this->analyzeFile('somefile.php', new Context());
}

public function testNoSealAllMethods(): void
{
Config::getInstance()->seal_all_methods = true;

$this->addFile(
'somefile.php',
'<?php
/** @psalm-no-seal-properties */
class A {
public function __call(string $method, array $args) {}
}
class B extends A {}
$b = new B();
$b->foo();
',
);

$error_message = 'UndefinedMagicMethod';
$this->expectException(CodeException::class);
$this->expectExceptionMessage($error_message);
$this->analyzeFile('somefile.php', new Context());
}

public function testSealAllMethodsWithFoo(): void
{
Config::getInstance()->seal_all_methods = true;
Expand Down
24 changes: 24 additions & 0 deletions tests/MagicPropertyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1213,4 +1213,28 @@ class B extends A {}
$this->expectExceptionMessage($error_message);
$this->analyzeFile('somefile.php', new Context());
}

public function testNoSealAllProperties(): void
{
Config::getInstance()->seal_all_properties = true;
Config::getInstance()->seal_all_methods = true;

$this->addFile(
'somefile.php',
'<?php
/** @psalm-no-seal-properties */
class A {
public function __get(string $name) {}
}
class B extends A {}
$b = new B();
/** @var string $result */
$result = $b->foo;
',
);

$this->analyzeFile('somefile.php', new Context());
}
}

0 comments on commit bac4f3e

Please sign in to comment.