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

PHPStan Latte $level++ #141

Closed
spaze opened this issue Jun 5, 2023 · 6 comments · Fixed by #181
Closed

PHPStan Latte $level++ #141

spaze opened this issue Jun 5, 2023 · 6 comments · Fixed by #181
Assignees

Comments

@spaze
Copy link
Owner

spaze commented Jun 5, 2023

The app is running PHPStan on level 8 for quite some time already (5cc25c4) and now even on level 9 #147, so let's see how far I can bring the Latte extensions.

PHPStan Latte Levels and PRs:

Lnil:

L0:

L1:

L2:

L3:

L4:

L5:

L6:

L7:

L8:

L9:

+ Bleeding Edge:

Some 🐛🐛🐛 discovered later by for example newer versions of the PHPStan Latte extension

@spaze spaze self-assigned this Jun 5, 2023
spaze added a commit that referenced this issue Jun 6, 2023
This is what should have been done in
5cca31a as part of `stdClass` removal
but I guess I was lazy back then.

Ref #141
@spaze spaze changed the title PHPStan Latte Level 3 PHPStan Latte $level++ Jun 6, 2023
spaze added a commit that referenced this issue Jun 6, 2023
This is what should have been done in
5cca31a as part of `stdClass` removal
but I guess I was lazy back then.

Ref #141
spaze added a commit that referenced this issue Jul 14, 2023
spaze added a commit that referenced this issue Jul 14, 2023
spaze added a commit that referenced this issue Jul 14, 2023
* Change action `$param` type to int where possible
* Work around efabrica-team/phpstan-latte#396

Ref #141
spaze added a commit that referenced this issue Jul 14, 2023
Oh look this was easy.

Ref #141
spaze added a commit that referenced this issue Jul 14, 2023
Oh look this was easy 😅

Ref #141
spaze added a commit that referenced this issue Jul 15, 2023
[9 errors remaining](https://github.com/spaze/michalspacek.cz/actions/runs/5562124085/jobs/10160242058)
and they all seem to be related one way or another to form containers,
so maybe the following will help?
- efabrica-team/phpstan-latte#380

Ref #141
spaze added a commit that referenced this issue Jul 17, 2023
Ignoring just errors related to form containers which are not yet (fully) supported.

I hope these would be solved one day when efabrica-team/phpstan-latte#380 is closed but currently, even when testing that branch, aren't.

Ref #141
spaze added a commit that referenced this issue Jul 18, 2023
spaze added a commit that referenced this issue Jul 18, 2023
- Interview source name & href are always filled, no need to have them nullable
- Make sure nullable values have a value when doing something with them

Ref #141
spaze added a commit that referenced this issue Jul 21, 2023
spaze added a commit that referenced this issue Jul 21, 2023
@spaze
Copy link
Owner Author

spaze commented Jul 22, 2023

image

@spaze spaze closed this as completed Jul 22, 2023
@spaze
Copy link
Owner Author

spaze commented Jul 22, 2023

Hey @lulco @MartinMystikJonas look I made it to and past the final boss 🦾

Thanks for creating the extension, looking forward to the next releases. If you want, and think it would be useful, feel free to use this code base for some kind of tests, like regression or so. The license allows it but I wanted to be explicit.

The extension helped me to find some buggy code, like for example that I had a form component and a form container sharing the same name. It worked but probably just by coincidence.

@lulco
Copy link
Contributor

lulco commented Jul 22, 2023

Thanks for using our extension! I wish there were more developers using Nette doing it too. Maybe you can share your experiences O:-)

Ad your error - it is corect to use same name for container and control in it. Extension reports it because of its actual implementation which is hacky. We are trying to improve it step by step.

@spaze
Copy link
Owner Author

spaze commented Jul 22, 2023

Interesting, having a form container and a field with the same name felt wrong (because of accessing it with $form[foo]) even though it worked, probably because the field is inside the container sharing the same name, not on the same level.

Anyway, this was an easy change, unlike some other refactoring (which should have been done much much earlier but I was lazy and didn't have the right tools available).

@spaze spaze reopened this Jul 22, 2023
spaze added a commit that referenced this issue Jul 22, 2023
spaze added a commit that referenced this issue Jul 22, 2023
@lulco
Copy link
Contributor

lulco commented Jul 22, 2023

Interesting, having a form container and a field with the same name felt wrong (because of accessing it with $form[foo])

This is kind of transformation we have done to make forms work with extension. The original compiled template is different.

Anyway, this was an easy change, unlike some other refactoring (which should have been done much much earlier but I was lazy and didn't have the right tools available).

@spaze
Copy link
Owner Author

spaze commented Jul 22, 2023

This is kind of transformation we have done to make forms work with extension. The original compiled template is different.

Good to know this can also be somehow changed!

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 a pull request may close this issue.

2 participants