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

Check if argument passed to isset() is not a variable #10068

Merged
merged 1 commit into from Aug 1, 2023

Conversation

Nitamet
Copy link
Contributor

@Nitamet Nitamet commented Jul 31, 2023

Added the condition to IssetAnalyzer to emit the issue when anything other than a variable is passed to isset()
Fixes #9949

@weirdan weirdan marked this pull request as draft August 1, 2023 02:39
@weirdan
Copy link
Collaborator

weirdan commented Aug 1, 2023

@Nitamet thanks for your effort. Please target 5.x (current default) branch.

Apparently, this needs some more work, so I've marked this PR as a draft. Mark it as ready for review when you think you're done with it.

@Nitamet Nitamet changed the base branch from master to 5.x August 1, 2023 08:43
@Nitamet
Copy link
Contributor Author

Nitamet commented Aug 1, 2023

And I thought that running composer tests was enough 💀
Yeah, it seems that we should at least take checking array elements into account and maybe there are other cases.
I'll take a closer look at the Expr types and try to fix these problems.

@weirdan
Copy link
Collaborator

weirdan commented Aug 1, 2023

You need to rebase your changes on top of 5.x and force-push. Just changing the target branch added a bunch of master-only changes we're not going to push to 5.x (they are for Psalm 6).

@Nitamet
Copy link
Contributor Author

Nitamet commented Aug 1, 2023

I've added other valid statements and changed the branch, tests seem to be ok now, the failed one is not related to this PR.

There's a little issue related to null coalescing operator though
php.net states that foo ?? bar is just a shorthand for isset(foo) ? foo : bar, but in fact there are differences:

class Foo {
    public const BAR = 'bar';
}

// This works
Foo::BAR ?? "doesn't exist";
// And this crashes
isset(Foo::BAR) ? Foo::BAR : "doesn't exist";

$var = 0;
// This works
($a =& $var) ?? "hello";
// And this crashes
isset((a =& $var)) ? (a =& $var) : "hello";

So if we don't add ClassConstFetch and AssignRef as valid arguments, we will get errors on valid null coalescing operators like in the example above.
And vice versa, if we add them, we won't get errors for invalid arguments ClassConstFetch and AssignRef in isset().
I'm not sure if there's an easy way to distinguish between ?? and isset and if that's in the scope of this PR, so for now I've just added them both as valid to avoid getting false errors.

@Nitamet Nitamet marked this pull request as ready for review August 1, 2023 12:28
@weirdan
Copy link
Collaborator

weirdan commented Aug 1, 2023

so for now I've just added them both as valid to avoid getting false errors.

Sounds good to me.

@weirdan weirdan added the release:feature The PR will be included in 'Features' section of the release notes label Aug 1, 2023
@weirdan weirdan merged commit 378b31b into vimeo:5.x Aug 1, 2023
47 of 50 checks passed
@weirdan
Copy link
Collaborator

weirdan commented Aug 1, 2023

Thanks!

oguzhand95 pushed a commit to cerbos/cerbos-sdk-php that referenced this pull request Aug 21, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [google/protobuf](https://developers.google.com/protocol-buffers/)
([source](https://togithub.com/protocolbuffers/protobuf-php)) | require
| minor | `3.23.4` -> `3.24.1` |
| [grpc/grpc](https://grpc.io)
([source](https://togithub.com/grpc/grpc-php)) | require | minor |
`1.52.0` -> `1.57.0` |
| [phpstan/phpstan](https://togithub.com/phpstan/phpstan) | require-dev
| patch | `1.10.27` -> `1.10.29` |
| [phpunit/phpunit](https://phpunit.de/)
([source](https://togithub.com/sebastianbergmann/phpunit)) | require-dev
| patch | `10.3.1` -> `10.3.2` |
| [vimeo/psalm](https://togithub.com/vimeo/psalm) | require-dev | minor
| `5.14.1` -> `5.15.0` |

---

### Release Notes

<details>
<summary>protocolbuffers/protobuf-php (google/protobuf)</summary>

###
[`v3.24.1`](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.24.0...v3.24.1)

[Compare
Source](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.24.0...v3.24.1)

###
[`v3.24.0`](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.23.4...v3.24.0)

[Compare
Source](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.23.4...v3.24.0)

</details>

<details>
<summary>grpc/grpc-php (grpc/grpc)</summary>

###
[`v1.57.0`](https://togithub.com/grpc/grpc-php/compare/v1.52.0...v1.57.0)

[Compare
Source](https://togithub.com/grpc/grpc-php/compare/v1.52.0...v1.57.0)

</details>

<details>
<summary>phpstan/phpstan (phpstan/phpstan)</summary>

###
[`v1.10.29`](https://togithub.com/phpstan/phpstan/releases/tag/1.10.29)

[Compare
Source](https://togithub.com/phpstan/phpstan/compare/1.10.28...1.10.29)

# Improvements 🔧

-   Update nikic/php-parser to v4.17.1
- PHP 8.3 features are no longer parse errors. Full-fledged PHP 8.3
support is coming later this year.
-   Update BetterReflection to 6.12.0

# Bugfixes 🐛

- PHPStan Pro - when launching, `PHP_BINARY` needs to be escaped
(phpstan/phpstan-src@2c7cfd8)

# Function signature fixes 🤖

- Fix FTP-related function signatures
([#&#8203;2551](https://togithub.com/phpstan/phpstan-src/pull/2551)),
thanks [@&#8203;thg2k](https://togithub.com/thg2k)!

###
[`v1.10.28`](https://togithub.com/phpstan/phpstan/releases/tag/1.10.28)

[Compare
Source](https://togithub.com/phpstan/phpstan/compare/1.10.27...1.10.28)

# Improvements 🔧

- Update BetterReflection to 6.12.0
(phpstan/phpstan-src@7c49c94)

# Bugfixes 🐛

- Nullsafe operator on `null` results in `null`
(phpstan/phpstan-src@5c40c85),
[#&#8203;9721](https://togithub.com/phpstan/phpstan/issues/9721)
- Properties set in the native constructor are initialized in additional
constructors
(phpstan/phpstan-src@1b0c6a0),
[#&#8203;9619](https://togithub.com/phpstan/phpstan/issues/9619)
- Fix performance problem with nested BooleanOr and BooleanAnd
(phpstan/phpstan-src@9adae6c),
[#&#8203;9690](https://togithub.com/phpstan/phpstan/issues/9690),
[#&#8203;9676](https://togithub.com/phpstan/phpstan/issues/9676)
- CallableTypeHelper - copy variadic parameters if the accepting closure
has more parameters
(phpstan/phpstan-src@31ed326),
[#&#8203;9699](https://togithub.com/phpstan/phpstan/issues/9699)

# Internals 🔍

- Simplify access to ClassReflection in ClassPropertyNode
([#&#8203;2565](https://togithub.com/phpstan/phpstan-src/pull/2565)),
thanks [@&#8203;mad-briller](https://togithub.com/mad-briller)!

</details>

<details>
<summary>sebastianbergmann/phpunit (phpunit/phpunit)</summary>

###
[`v10.3.2`](https://togithub.com/sebastianbergmann/phpunit/compare/10.3.1...10.3.2)

[Compare
Source](https://togithub.com/sebastianbergmann/phpunit/compare/10.3.1...10.3.2)

</details>

<details>
<summary>vimeo/psalm (vimeo/psalm)</summary>

### [`v5.15.0`](https://togithub.com/vimeo/psalm/releases/tag/5.15.0)

[Compare
Source](https://togithub.com/vimeo/psalm/compare/5.14.1...5.15.0)

<!-- Release notes generated using configuration in .github/release.yml
at 5.x -->

#### What's Changed

##### Features

- Check if argument passed to isset() is not a variable by
[@&#8203;Nitamet](https://togithub.com/Nitamet) in
[vimeo/psalm#10068
- Fix [#&#8203;9997](https://togithub.com/vimeo/psalm/issues/9997)
dynamic properties on SimpleXmlElement by
[@&#8203;ygottschalk](https://togithub.com/ygottschalk) in
[vimeo/psalm#10049
- Nicer PHP version checking by
[@&#8203;Nitamet](https://togithub.com/Nitamet) in
[vimeo/psalm#10129

##### Fixes

- Fixed `DOMDocument::load*` signatures by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10073
- make (s)printf error reporting more correct/literal by
[@&#8203;kkmuffme](https://togithub.com/kkmuffme) in
[vimeo/psalm#10088
- Fix parameter having `object` type in PHPDoc and only `&` in the
method's definition, used with code having an object calling it's method
with itself as an argument for mentioned parameter by
[@&#8203;kubawerlos](https://togithub.com/kubawerlos) in
[vimeo/psalm#10104
- mysqli_field_seek returns true by
[@&#8203;kamil-tekiela](https://togithub.com/kamil-tekiela) in
[vimeo/psalm#10107
- Fix bcdiv nullable scale stub by
[@&#8203;TheDevick](https://togithub.com/TheDevick) in
[vimeo/psalm#10106
- Cleanup `test` command and `fixAll` action by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10108
- Correct type for `$enum->name` by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10109
- argv and argc were inconsistent in Context with VariableFetchAnalyzer
by [@&#8203;kkmuffme](https://togithub.com/kkmuffme) in
[vimeo/psalm#10093
- Fix `hash_pbkdf2` `$options` parameter by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10110
- Forbid faulty `nikic/php-parser` version by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10112
- `strip_tags()/$allowed_tags` can accept arrays since 7.4 by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10122
- Fix crash when assertion array is not a list by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10123
- Fix crash on array access to undefined class by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10134
- Correct exit code for invalid version format by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10135

#### New Contributors

- [@&#8203;Nitamet](https://togithub.com/Nitamet) made their first
contribution in
[vimeo/psalm#10068
- [@&#8203;TheDevick](https://togithub.com/TheDevick) made their first
contribution in
[vimeo/psalm#10106

**Full Changelog**:
vimeo/psalm@5.14.1...5.15.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 4am on Monday" (UTC),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/cerbos/cerbos-sdk-php).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40MC4zIiwidXBkYXRlZEluVmVyIjoiMzYuNDMuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
tcarrio pushed a commit to open-feature/php-sdk that referenced this pull request Sep 21, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [vimeo/psalm](https://togithub.com/vimeo/psalm) | require-dev | minor
| `~5.14.0` -> `~5.15.0` |

---

### Release Notes

<details>
<summary>vimeo/psalm (vimeo/psalm)</summary>

### [`v5.15.0`](https://togithub.com/vimeo/psalm/releases/tag/5.15.0)

[Compare
Source](https://togithub.com/vimeo/psalm/compare/5.14.1...5.15.0)

<!-- Release notes generated using configuration in .github/release.yml
at 5.x -->

#### What's Changed

##### Features

- Check if argument passed to isset() is not a variable by
[@&#8203;Nitamet](https://togithub.com/Nitamet) in
[vimeo/psalm#10068
- Fix [#&#8203;9997](https://togithub.com/vimeo/psalm/issues/9997)
dynamic properties on SimpleXmlElement by
[@&#8203;ygottschalk](https://togithub.com/ygottschalk) in
[vimeo/psalm#10049
- Nicer PHP version checking by
[@&#8203;Nitamet](https://togithub.com/Nitamet) in
[vimeo/psalm#10129

##### Fixes

- Fixed `DOMDocument::load*` signatures by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10073
- make (s)printf error reporting more correct/literal by
[@&#8203;kkmuffme](https://togithub.com/kkmuffme) in
[vimeo/psalm#10088
- Fix parameter having `object` type in PHPDoc and only `&` in the
method's definition, used with code having an object calling it's method
with itself as an argument for mentioned parameter by
[@&#8203;kubawerlos](https://togithub.com/kubawerlos) in
[vimeo/psalm#10104
- mysqli_field_seek returns true by
[@&#8203;kamil-tekiela](https://togithub.com/kamil-tekiela) in
[vimeo/psalm#10107
- Fix bcdiv nullable scale stub by
[@&#8203;TheDevick](https://togithub.com/TheDevick) in
[vimeo/psalm#10106
- Cleanup `test` command and `fixAll` action by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10108
- Correct type for `$enum->name` by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10109
- argv and argc were inconsistent in Context with VariableFetchAnalyzer
by [@&#8203;kkmuffme](https://togithub.com/kkmuffme) in
[vimeo/psalm#10093
- Fix `hash_pbkdf2` `$options` parameter by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10110
- Forbid faulty `nikic/php-parser` version by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10112
- `strip_tags()/$allowed_tags` can accept arrays since 7.4 by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10122
- Fix crash when assertion array is not a list by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10123
- Fix crash on array access to undefined class by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10134
- Correct exit code for invalid version format by
[@&#8203;weirdan](https://togithub.com/weirdan) in
[vimeo/psalm#10135

#### New Contributors

- [@&#8203;Nitamet](https://togithub.com/Nitamet) made their first
contribution in
[vimeo/psalm#10068
- [@&#8203;TheDevick](https://togithub.com/TheDevick) made their first
contribution in
[vimeo/psalm#10106

**Full Changelog**:
vimeo/psalm@5.14.1...5.15.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/open-feature/php-sdk).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40My4yIiwidXBkYXRlZEluVmVyIjoiMzYuNDMuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
tscni added a commit to tscni/psalm that referenced this pull request Nov 29, 2023
vimeo#10068 added isset restrictions that didn't consider null coalesces on match expressions.
This restores that support by converting it to a virtual variable for the isset analysis too.
tscni added a commit to tscni/psalm that referenced this pull request Nov 29, 2023
vimeo#10068 added isset restrictions that didn't consider null coalesces on match expressions.
This restores that support by converting the match expression to a virtual variable for the isset analysis, similar to other incompatible expressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature The PR will be included in 'Features' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Difference between isset($neverNull) and isset(neverNull())
2 participants