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

fix: static typing and final classes #621

Merged
merged 3 commits into from May 2, 2024
Merged

Conversation

usu
Copy link
Contributor

@usu usu commented May 1, 2024

Fixes #603

Mark internal classes final

I sticked to the rules outlined by Ocramius in https://ocramius.github.io/blog/when-to-declare-classes-final/

Static typing

Fixes most typing related errors from `phpstan analyze -l 6 src'

@usu
Copy link
Contributor Author

usu commented May 1, 2024

I also included removal of legacy code in Configuration.php as it was a mix of legacy code and static typing.

With extension phpstan/phpstan-symfony, this now passes Level 6 with one remaining error only (which is obvious, as the CloudFront package is only included as a suggest, not as dependency).

 ------ ---------------------------------------------------------------------
  Line   Configuration.php
 ------ ---------------------------------------------------------------------
  593    Class JeanBeru\HttpCacheCloudFront\Proxy\CloudFront not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ ---------------------------------------------------------------------

@usu usu mentioned this pull request May 1, 2024
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks a lot for this one! just one small question.

we should add a ci step for phpstan similar to FOSHttpCache https://github.com/FriendsOfSymfony/FOSHttpCache/blob/3.x/.github/workflows/static.yml

(where we could require the optional dependency, or alternatively tell phpstan about its possible existence)

@usu
Copy link
Contributor Author

usu commented May 1, 2024

we should add a ci step for phpstan similar to FOSHttpCache https://github.com/FriendsOfSymfony/FOSHttpCache/blob/3.x/.github/workflows/static.yml

(where we could require the optional dependency, or alternatively tell phpstan about its possible existence)

Yeah, fully agree. I can make a seperate PR for this.

@dbu dbu merged commit 31ae54c into FriendsOfSymfony:3.x May 2, 2024
10 checks passed
@dbu
Copy link
Contributor

dbu commented May 2, 2024

thanks a lot!

@dbu dbu mentioned this pull request May 2, 2024
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