Skip to content

Add phpstan analysis#4458

Merged
fabpot merged 1 commit intotwigphp:3.xfrom
VincentLanglet:phpstan
Nov 20, 2024
Merged

Add phpstan analysis#4458
fabpot merged 1 commit intotwigphp:3.xfrom
VincentLanglet:phpstan

Conversation

@VincentLanglet
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet commented Nov 20, 2024 •
edited
Loading

As suggested by @ruudk here: #4456 (comment)
I think it could be useful to add phpstan analysis on twig.

This is an example of all the things reported when checking level 3 PHPStan (more will get more errors to fix).

VincentLanglet
VincentLanglet commented Nov 20, 2024
View reviewed changes
src/Node/Node.php
}

/**
* @param string|int $name
Copy link
Copy Markdown
Contributor Author

@VincentLanglet VincentLanglet Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Int is not accepted

Copy link
Copy Markdown
Contributor

@fabpot fabpot Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is accepted explicitly in Twig 4.0 (same for all similar methods).

@VincentLanglet
Copy link
Copy Markdown
Contributor Author

VincentLanglet commented Nov 20, 2024

Tests were already failing and fabbot error is unrelated
https://fabbot.io/report/twigphp/Twig/4458/93ca91b81d9be93d0a21441c03ef24ed7c274ab0

@VincentLanglet VincentLanglet marked this pull request as ready for review November 20, 2024 12:31
stof
stof reviewed Nov 20, 2024
View reviewed changes
stof
stof reviewed Nov 20, 2024
View reviewed changes
@fabpot
Copy link
Copy Markdown
Contributor

fabpot commented Nov 20, 2024

@VincentLanglet Can you also rebase to see (as stof fixed a bunch of test issues)

@VincentLanglet
Copy link
Copy Markdown
Contributor Author

VincentLanglet commented Nov 20, 2024 •
edited
Loading

@VincentLanglet Can you also rebase to see (as stof fixed a bunch of test issues)

I rebased and added back the int|string phpdoc @fabpot

In the next PR I'll try to bump level 4

@fabpot
Copy link
Copy Markdown
Contributor

fabpot commented Nov 20, 2024

Thank you @VincentLanglet.

@fabpot fabpot merged commit 679e413 into twigphp:3.x Nov 20, 2024
@VincentLanglet
Copy link
Copy Markdown
Contributor Author

VincentLanglet commented Nov 20, 2024

There is some fixes in this PR, would it be enough for a 3.15.1 version ?

@williamdes williamdes mentioned this pull request Nov 19, 2025
Merged
fabpot added a commit that referenced this pull request Nov 19, 2025
@fabpot
minor #4712 Update .gitattributes to exclude phpstan configs (william…
2b7cc0b 
…des)

This PR was merged into the 3.x branch.

Discussion
----------

Update .gitattributes to exclude phpstan configs

They where added in #4458

Versions >= 3.16.0 have the not needed files

Commits
-------

2e0c874 Update .gitattributes to exclude phpstan configs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot Awaiting requested review from fabpot

1 more reviewer

@stof stof stof left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@VincentLanglet @fabpot @stof