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

Support for compound expressions #2515

Closed
mhitza opened this issue Dec 27, 2019 · 3 comments
Closed

Support for compound expressions #2515

mhitza opened this issue Dec 27, 2019 · 3 comments
Labels

Comments

@mhitza
Copy link

mhitza commented Dec 27, 2019

I have a compound expression as seen in the following example

ERROR: PossiblyNullReference - src/Workflow/Assessment.php:38:86 - Cannot call method getAttributeRoot on possibly null value
        if (is_null($project->getAssessment()) || is_null($project->getAssessment()->getAttributeRoot())) {

I would expect psalm to be able to infer that if these two checks pass (I throw an exception in the if block), those returns cannot be null afterwards.

While this isn't supported yet, is there a way for me to rewrite the code as to have psalm be able to infer everything is safe without just suppressing the PossiblyNullReference?

@weirdan
Copy link
Collaborator

weirdan commented Dec 27, 2019

It's a bit longer, but yes, you can rewrite the code using intermediate variables so there's no need to memoize the method call results:

<?php
  class Assessment {
     public function getAttributeRoot(): ?string {
       return rand(0, 1) ? 'root' : null;
     }
  }
  class Project {
     public function getAssessment(): ?Assessment {
       return rand(0, 1) ? new Assessment : null;
     }
  }

function f(Project $project): int {
  $assessment = $project->getAssessment();
  if (is_null($assessment)) {
    throw new RuntimeException('no assessment');
  }
  
  $attributeRoot = $assessment->getAttributeRoot();
  if (is_null($attributeRoot)) {
    throw new RuntimeException('no attribute root');
  }
  
  return strlen($attributeRoot);
}

https://psalm.dev/r/1efae3f536

@muglug muglug added the bug label Dec 27, 2019
@muglug
Copy link
Collaborator

muglug commented Dec 27, 2019

This should definitely work:

<?php
class Assessment {
    private ?string $root = null;
  
    /** @psalm-mutation-free */
    public function getRoot(): ?string {
        return $this->root;
    }
}

class Project {
    private ?Assessment $assessment = null;
  
    /** @psalm-mutation-free */
    public function getAssessment(): ?Assessment {
        return $this->assessment;
    }
}

function f(Project $project): int {
    if (($project->getAssessment() === null)
        || ($project->getAssessment()->getRoot() !== null)
    ) {
        throw new RuntimeException('no assessment');
    }
  
    return strlen($project->getAssessment()->getRoot());
}

https://psalm.dev/r/531c4ad15b

@muglug muglug closed this as completed in 982fe62 Dec 27, 2019
@mhitza
Copy link
Author

mhitza commented Dec 27, 2019

@muglug you're awesome, I can confirm it works with the latest master version and the @psalm-mutation-free annotation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants