Skip to content

Commit

Permalink
Fix #5383 - prevent unsound use of new static for generics
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Apr 10, 2021
1 parent bb88cff commit 012dafa
Show file tree
Hide file tree
Showing 15 changed files with 619 additions and 91 deletions.
1 change: 1 addition & 0 deletions config.xsd
Expand Up @@ -435,6 +435,7 @@
<xs:element name="UnrecognizedStatement" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnresolvableInclude" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnsafeInstantiation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnsafeGenericInstantiation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClass" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedConstructor" type="MethodIssueHandlerType" minOccurs="0" />
Expand Down
8 changes: 8 additions & 0 deletions docs/annotating_code/supported_annotations.md
Expand Up @@ -51,6 +51,14 @@ function bat(): string {

There are a number of custom tags that determine how Psalm treats your code.

### `@psalm-consistent-constructor`

See [UnsafeInstantiation](../running_psalm/issues/UnsafeInstantiation.md)

### `@psalm-consistent-templates`

See [UnsafeGenericInstantiation](../running_psalm/issues/UnsafeGenericInstantiation.md)

### `@param-out`, `@psalm-param-out`

This is used to specify that a by-ref type is different from the one that entered. In the function below the first param can be null, but once the function has executed the by-ref value is not null.
Expand Down
157 changes: 157 additions & 0 deletions docs/running_psalm/issues/UnsafeGenericInstantiation.md
@@ -0,0 +1,157 @@
# UnsafeGenericInstantiation

Emitted when an attempt is made to instantiate a class using `new static` without a constructor that's final:

```php
<?php

/**
* @template T
* @psalm-consistent-constructor
*/
class Container {
/**
* @var T
*/
public $t;

/**
* @param T $t
*/
public function __construct($t) {
$this->t = $t;
}

/**
* @template U
* @param U $u
* @return static<U>
*/
public function getInstance($u) : static
{
return new static($u);
}
}
```


## What’s wrong here?

The problem comes when extending the class:

```php
<?php

/**
* @template T
* @psalm-consistent-constructor
*/
class Container {
/**
* @var T
*/
public $t;

/**
* @param T $t
*/
public function __construct($t) {
$this->t = $t;
}

/**
* @template U
* @param U $u
* @return static<U>
*/
public function getInstance($u) : static
{
return new static($u);
}
}

/**
* @extends Container<string>
*/
class StringContainer extends Container {}

$c = StringContainer::getInstance(new stdClass());
// creates StringContainer<stdClass>, clearly invalid
```

## How to fix

Either use `new self` instead of `new static`:

```php
<?php

/**
* @template T
* @psalm-consistent-constructor
*/
class Container {
/**
* @var T
*/
public $t;

/**
* @param T $t
*/
public function __construct($t) {
$this->t = $t;
}

/**
* @template U
* @param U $u
* @return self<U>
*/
public function getInstance($u) : self
{
return new self($u);
}
}
```

Or you can add a `@psalm-consistent-templates` annotation which ensures that any child class has the same generic params:

```php
<?php

/**
* @template T
* @psalm-consistent-constructor
* @psalm-consistent-templates
*/
class Container {
/**
* @var T
*/
public $t;

/**
* @param T $t
*/
public function __construct($t) {
$this->t = $t;
}

/**
* @template U
* @param U $u
* @return static<U>
*/
public function getInstance($u) : static
{
return new static($u);
}
}

/**
* @template T
* @psalm-extends Container<T>
*/
class LazyLoadingContainer extends Container {}
```
2 changes: 1 addition & 1 deletion docs/running_psalm/issues/UnsafeInstantiation.md
@@ -1,6 +1,6 @@
# UnsafeInstantiation

Emitted when an attempt is made to instantiate a class without a constructor that's final:
Emitted when an attempt is made to instantiate a class using `new static` or similar without a constructor that's final:

```php
<?php
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/DocComment.php
Expand Up @@ -35,7 +35,8 @@ class DocComment
'allow-private-mutation', 'readonly-allow-private-mutation',
'yield', 'trace', 'import-type', 'flow', 'taint-specialize', 'taint-escape',
'taint-unescape', 'self-out', 'consistent-constructor', 'stub-override',
'require-extends', 'require-implements', 'param-out', 'ignore-var'
'require-extends', 'require-implements', 'param-out', 'ignore-var',
'consistent-templates',
];

/**
Expand Down

0 comments on commit 012dafa

Please sign in to comment.