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

Allow @var annotations over more statements #1916

Closed
muglug opened this issue Jul 7, 2019 · 12 comments
Closed

Allow @var annotations over more statements #1916

muglug opened this issue Jul 7, 2019 · 12 comments
Labels
easy problems Issues that can be fixed without background knowledge of Psalm enhancement

Comments

@muglug
Copy link
Collaborator

muglug commented Jul 7, 2019

Currently only supported above assertions, foreach and return statements.

@SignpostMarv
Copy link
Contributor

yield and yield from would be good :)

@Shira-3749
Copy link

if (preg_match('/dummy/', 'dummy', $matches, PREG_OFFSET_CAPTURE)) {
    // ...
}

No matter where you put /** @var array<int, array{string, int}> $matches */, $matches remains typed as array<array-key, string>.

Allowing @var or @psalm-assert anywhere would be useful in these cases.

@SignpostMarv
Copy link
Contributor

No matter where you put /** @var array<int, array{string, int}> $matches */, $matches remains typed as array<array-key, string>.

That's down to how it's defined in the callmap, I believe.

@Shira-3749
Copy link

That's down to how it's defined in the callmap, I believe.

It's defined simply as string[]. But depending on the passed $flags, $matches can become string[], array<string|null>, array<array{string, int}> or array<array{string|null, int}>. It doesn't look like this can be defined in the callmap (at least not reliably).

This works, but is less than ideal:

if (preg_match('/dummy/', 'dummy', $matches, PREG_OFFSET_CAPTURE)) {
    /** @var array<array{string, int}> $matches */
    [[$match, $offset]] = $matches;

    // ...
}

@SignpostMarv
Copy link
Contributor

I think the idea of parameter-mapped types definition has been discussed elsewhere- @muglug, is there a way around this that doesn't require implementing parameter-mapped types ?

@SignpostMarv
Copy link
Contributor

either static const type inheritance, or inline var on method/function params would be nice: https://psalm.dev/r/f9511a4340 https://3v4l.org/q93Ce

<?php
/**
* @template T as array<string, scalar|array|object|null>
*/
abstract class Foo {
  /**
  * @param array<int, key-of<T>>
  */
  const NULLABLES = [];
  
  /**
  * @param T $data
  */
  public function __construct(array $data) {
    foreach ($data as $k => $v) {
      $this->$k = $v;
    }
  }
  
  /**
  * @template K as key-of<T>
  *
  * @param K $k
  *
  * @return T[K]
  */
  public function __get(string $k) {
    /**
    * @var T[K]
    */
    return $this->$k;
  }
  
  /**
  * @template K as key-of<T>
  *
  * @param K $k
  * @param T[K] $v
  */
  public function __set(string $k, $v) : void {
    $this->$k = $v;
  }
  
  public function __unset(string $k) : void {
    if ( ! in_array(
      $k,
      /**
      * @var array<int, key-of<T>>
      */
      static::NULLABLES,
      true
    )) {
      throw new InvalidArgumentException(sprintf('%s::$%s is not nullable!', static::class, $k));
    }
    
    $this->$k = null;
  }
}

/**
* @template-extends Foo<array{baz:string|null}>
*
* @property string|null $baz
*/
class Bar extends Foo {
  const NULLABLES = ['baz'];
  
  /**
  * @var string|null
  */
  protected $baz;
}

/**
* @template-extends Foo<array{baz:string|null}>
*
* @property string $baz
*/
class Bat extends Foo {
  const NULLABLES = [];
  
  /**
  * @var string
  */
  protected $baz;
}

$a = new Bar(['baz' => 'with a string']);
$b = new Bar(['baz' => null]);
$c = new Bar(['baz' => 'this should not be here']);
$d = new Bat(['baz' => 'unsetting this will fail']);
unset($c->baz);

echo var_export($a->baz, true), "\n", var_export($b->baz, true), "\n", var_export($c->baz, true);

unset($d->baz);

@Shira-3749
Copy link

For the few cases where this is needed a possible workaround is to put a semicolon after the doc comment, then it's taken into account: https://psalm.dev/r/29441019f5

PHP seems to ignore empty statements so doing this should be ok (although some CS tools might complain).

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/29441019f5
<?php
  
function example(string $s): void
{
	if (preg_match('{foo-(\w+)}', $s, $m)) {
      /** @var array{string, string} $m */
      takesString($m[1]);
    }
}

function exampleWithSemicolon(string $s): void
{
	if (preg_match('{foo-(\w+)}', $s, $m)) {
      /** @var array{string, string} $m */;
      takesString($m[1]); // now this works
    }
}

function takesString(string $s): void
{
}
Psalm output (using commit b3f6b56):

INFO: PossiblyUndefinedIntArrayOffset - 7:19 - Possibly undefined array offset 'int(1)' is risky given expected type 'array-key'. Consider using isset beforehand.

@muglug muglug added the easy problems Issues that can be fixed without background knowledge of Psalm label Apr 26, 2020
@orklah
Copy link
Collaborator

orklah commented May 30, 2020

@muglug Do you have some hint to share on this? Where should I look to begin?

I suppose #1111 can be handled at the same time

I see some statements that could benefit from this:

@muglug
Copy link
Collaborator Author

muglug commented Jun 2, 2020

@orklah the StatementsAnalyzer has parsing for var docblocks already – I think you'd just want to assign the variables right there, except for foreach, return and assign statements

@ramsey
Copy link

ramsey commented Jul 13, 2020

In order to properly annotate the $matches array set by preg_match() and preg_match_all(), I've found myself doing something like this, which feels awkward, but I'm not sure of a better way. It would be nice to be able to use @var for $matches itself:

$text = '
foobar
foobaz
fooqux
foobop
';

preg_match_all("/(?'word'foob[\w]*)/", $text, $matches);

/**
 * @var array{
 *     word: string[]
 * } $params
 */
$params = $matches;

for ($i = 0; $i < count($params['word']); $i++) {
    echo $params['word'][$i] . "\n";
}

https://psalm.dev/r/d1eac9433a

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/d1eac9433a
<?php
$text = '
foobar
foobaz
fooqux
foobop
';

preg_match_all("/(?'word'foob[\w]*)/", $text, $matches);

/**
 * @var array{
 *     word: string[]
 * } $params
 */
$params = $matches;

for ($i = 0; $i < count($params['word']); $i++) {
    echo $params['word'][$i] . "\n";
}
Psalm output (using commit 931d35a):

No issues!

@muglug muglug closed this as completed in eddd7b8 Jul 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy problems Issues that can be fixed without background knowledge of Psalm enhancement
Projects
None yet
Development

No branches or pull requests

5 participants