Skip to content

TASK: Raise PhpStan to level 8 - Part 1 #3515

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.

Sign up for GitHub

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

Jump to bottom
Merged
mhsdesign merged 30 commits into from
Dec 12, 2025
Merged

TASK: Raise PhpStan to level 8 - Part 1 #3515

mhsdesign merged 30 commits into from
Dec 12, 2025
+552 −365

Conversation

@mficzel
Copy link
Member

@mficzel mficzel commented Oct 24, 2025 •
edited
Loading

Raise PhpStan to level 8 for:

  • Neos.Flow.Log
  • Neos.FluidAdaptor
  • Neos.Http.Factories
  • Neos.Kickstarter
  • Neos.Utility.Arrays
  • Neos.Utility.Files
  • Neos.Utility.MediaTypes
  • Neos.Utility.ObjectHandling
  • Neos.Utility.OpcodeCache
  • Neos.Utility.Pdo
  • Neos.Utility.Schema
  • Neos.Utility.Unicode

Upgrade instructions

No changes needed.

Review instructions

This the first of multiple PRs to cherry pick the changes from #3474 from @nezaniel and @mhsdesign step by step and fix issues and breakiness.

This is intended to be unbreaky but be the foundation for future refactorings that can at least partly be automated once we have really solid type informations.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@github-actions github-actions bot added the 9.0 label Oct 24, 2025
@mficzel mficzel force-pushed the phpstanLevel8-v3 branch 10 times, most recently from 0ab77ed to 4064484 Compare October 24, 2025 19:50
@mhsdesign
Copy link
Member

mhsdesign commented Oct 26, 2025

wow thanks a lot! If you need help here let me know;) Also lets ignore the style CI failures as we want to move to php cs either way and they might have different expectations again;)

@mficzel mficzel changed the base branch from 9.0 to 9.1 October 27, 2025 13:23
@mficzel mficzel changed the base branch from 9.1 to 9.0 October 27, 2025 13:23
@mficzel mficzel changed the title DRAFT: Raise PhpStan to level 8 - V3 DRAFT: Raise PhpStan to level 8 - Part 1 Oct 27, 2025
@mficzel mficzel force-pushed the phpstanLevel8-v3 branch 2 times, most recently from eabd7e0 to 0d20ec9 Compare October 27, 2025 14:05
@mficzel mficzel marked this pull request as ready for review October 27, 2025 14:05
@mficzel
Copy link
Member Author

mficzel commented Oct 27, 2025

@mhsdesign is suggest to merge this in parts as otherwise the reviews will get really overwhelming.

@mficzel mficzel changed the base branch from 9.0 to 9.1 October 27, 2025 14:07
@github-actions github-actions bot added 9.1 and removed 9.0 labels Oct 27, 2025
@mficzel mficzel requested review from mhsdesign and nezaniel and removed request for mhsdesign October 27, 2025 14:08
@mficzel mficzel changed the title DRAFT: Raise PhpStan to level 8 - Part 1 TASK: Raise PhpStan to level 8 - Part 1 Oct 27, 2025
@github-actions github-actions bot added the Task label Oct 27, 2025
@mficzel mficzel mentioned this pull request Oct 27, 2025
Draft
6 tasks
@mhsdesign
Copy link
Member

mhsdesign commented Oct 27, 2025

oke i assume my commits will be part of another change then? idk why i did those but i probably did them to get flow back running

image

@mficzel
Copy link
Member Author

mficzel commented Oct 27, 2025

@mhsdesign yes your commits were mostly regarding flow. I cherry pick commits for each section one by one and afterwards remove breakiness and make tests green + verify neos still works.

@mhsdesign
Copy link
Member

mhsdesign commented Oct 28, 2025

Okay so this is the final set that should be tested and go into Flow 9.1 then ill review and test that ;) Thanks for splitting this mammoth!

@mficzel
Copy link
Member Author

mficzel commented Oct 28, 2025

I will try to prepare follow ups that may or may not be ready for 9.1 but imho this is about the maximal size for a pr that is chewable. So imho this can be reviewed and merged

@Sebobo Sebobo moved this to In review in Neos 9.1 Release Board Dec 11, 2025
@mhsdesign
TASK: Add phpstan template/generics to getAllSubClassNamesForClass()
1fec38c
@mhsdesign
PATCH: Dont annotate DependencyProxy in @var as this is an antipa…
84f811a 
…ttern in flow

if this would be injected flow would not handle this
@mhsdesign
PATCH: Revert to correct types in PaginateController
d7568f8 
the change in c6e3247 was half thought and phpstan only complained due to temporary results written.
@mhsdesign
PATCH: Remove obsolete @var type hint (isMainRequest already tell…
ea4df23 
…s phpstan whats up)
@mhsdesign
PATCH: Document what _legacy_ phpstan means (lvl3) and fix one whites…
ea82bf0 
…pace *g
@mhsdesign
BUGFIX: Throw exception when `Files::getRecursiveDirectoryGenerator()…
5769192 
…` is used with `$returnRealPath` but realpaths cannot be calculated

This is the case when using a virtual file system for testing in example (`vfs://`) or really any stream wrapper.

Silently swallowing all the `realpath` misses disguises bugs and hiders developer experience when using a vfs.
mhsdesign
mhsdesign approved these changes Dec 12, 2025
View reviewed changes
Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

Thanks for reviving this change and battling with it to make it run :D

I did a full review locally and pushed some small adjustment changes - just mostly syntactical stuff.

I see we now use a lot of times the pattern ?: '' to convert a false to a string. That is often okay as this the false case should never happen for constant preg expressions but in some cases ignoring this type juggling disguises bugs. So if there is real reason to believe it could happen im definitely for having a sane fallback or telling whats wrong.

Thats exactly why i added a new exception to \Neos\Utility\Files::getRecursiveDirectoryGenerator see 5769192:

BUGFIX: Throw exception when Files::getRecursiveDirectoryGenerator() is used with $returnRealPath but realpaths cannot be calculated

This is the case when using a virtual file system for testing in example (vfs://) or really any stream wrapper.

Silently swallowing all the realpath misses disguises bugs and hiders developer experience when using a vfs.

@mhsdesign
Merge remote-tracking branch 'origin/9.1' into phpstanLevel8-v3
61a8b72
@mhsdesign
TASK: Throw if rotateLogFile cannot create or open lock file
2d2fe5c 
`fopen` is multi thread safe. $lockResource is never `false` unless something is severely wrong with the system like flow has no write access to Data/
@mhsdesign mhsdesign merged commit e6fbba4 into 9.1 Dec 12, 2025
10 checks passed
@mhsdesign mhsdesign deleted the phpstanLevel8-v3 branch December 12, 2025 16:15
@github-project-automation github-project-automation bot moved this from In review to Done in Neos 9.1 Release Board Dec 12, 2025
mhsdesign added a commit to neos/neos-development-collection that referenced this pull request Dec 14, 2025
@mhsdesign
PATH: Fix unit tests for Code Migrations which dont work fully on vfs
0ff7c55 
> Cannot get realpath for "vfs://fusion/Target.Package/Resources/SomeFile.fusion". $returnRealPath cannot be used with stream wrappers.

see neos/flow-development-collection#3515 (review)
neos-bot pushed a commit to neos/neos that referenced this pull request Dec 15, 2025
@mhsdesign
PATH: Fix unit tests for Code Migrations which dont work fully on vfs
293288a 
> Cannot get realpath for "vfs://fusion/Target.Package/Resources/SomeFile.fusion". $returnRealPath cannot be used with stream wrappers.

see neos/flow-development-collection#3515 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhsdesign mhsdesign mhsdesign approved these changes

@nezaniel nezaniel Awaiting requested review from nezaniel

Assignees

No one assigned

Labels

9.1 Task

Projects

No open projects
Neos 9.1 Release Board
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mficzel @mhsdesign