Skip to content

Avoid doing work before its necessary in mutating scope.#2537

Merged
ondrejmirtes merged 1 commit intophpstan:1.10.xfrom
mad-briller:ms
Jul 19, 2023
Merged

Avoid doing work before its necessary in mutating scope.#2537
ondrejmirtes merged 1 commit intophpstan:1.10.xfrom
mad-briller:ms

Conversation

@mad-briller
Copy link
Copy Markdown
Contributor

@mad-briller mad-briller commented Jul 19, 2023

Few instances of asking for information before it's really necessary

@phpstan-bot
Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@mad-briller mad-briller changed the base branch from 1.11.x to 1.10.x July 19, 2023 15:55
@ondrejmirtes ondrejmirtes merged commit efce882 into phpstan:1.10.x Jul 19, 2023
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

@mad-briller
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes do you think it would be feasible to be able to detect this kind of thing with a custom phpstan rule?

i've written custom rules for single nodes but this would need to check more than one node so i don't know if it's possible, or if it is how i would approach it

as a simple minimum, the rule could be checking for something like "if a variable is assigned and not used before an exit point"

@ondrejmirtes
Copy link
Copy Markdown
Member

Please see this: phpstan/phpstan#8192

But read my last comment too - I was able to solve a bug without CFG even when I thought I needed CFG for that 😊 Maybe it will give you some ideas.

Also bear in mind that the called thing that we move lower needs to be pure/immutable otherwise moving the call might have unintended side effects.

@mad-briller
Copy link
Copy Markdown
Contributor Author

okay that gives me something to read and think about, thanks!

great point about purity, luckily phpstan already knows about the concept

@mad-briller
Copy link
Copy Markdown
Contributor Author

making changes to NodeScopeResolver isn't possible in an extension, does this mean you would be open to having this kind of rule in phpstan-src itself?

@ondrejmirtes
Copy link
Copy Markdown
Member

I don't understand the question. What kind of changes to the NodeScopeResolver?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants