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

Psalm 5 #293

Merged
merged 10 commits into from Dec 17, 2022
Merged

Psalm 5 #293

merged 10 commits into from Dec 17, 2022

Conversation

seferov
Copy link
Member

@seferov seferov commented Dec 12, 2022

No description provided.

@danog
Copy link

danog commented Dec 12, 2022

Btw you can cherry-pick 7a1c3fd to pull in my optimization fixes if you want to finish this PR, I won't have much time today to work on the PR :)

@danog
Copy link

danog commented Dec 17, 2022

Have some severe doubts about how you removed those taint tests, the problem here is that Psalm is not detecting the issue (false negative), not that it shouldn't detect it (false positive).

@seferov
Copy link
Member Author

seferov commented Dec 17, 2022

Btw you can cherry-pick 7a1c3fd to pull in my optimization fixes if you want to finish this PR, I won't have much time today to work on the PR :)

thank you very much! It seems the commit is reverting Immutable/Mutable Union types which IMO is a downgrade.

@seferov
Copy link
Member Author

seferov commented Dec 17, 2022

Have some severe doubts about how you removed those taint tests, the problem here is that Psalm is not detecting the issue (false negative), not that it shouldn't detect it (false positive).

Taint tests are not removed except one (not removed but commented out temporarily) which does not work correctly due to Psalm bug

@danog
Copy link

danog commented Dec 17, 2022

Yeah, there's a psalm bug in taint detection that should be fixed before merging this MR imo

@seferov seferov changed the base branch from 4.x to 5.x December 17, 2022 17:18
@seferov
Copy link
Member Author

seferov commented Dec 17, 2022

fixes #285

@seferov seferov merged commit e58db2b into 5.x Dec 17, 2022
@seferov seferov deleted the psalm5 branch December 17, 2022 17:19
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.

None yet

2 participants