search

laminas / laminas-form

Validate and display simple and complex forms, casting forms to business objects and vice versa
https://docs.laminas.dev/laminas-form/
BSD 3-Clause "New" or "Revised" License
80 stars 52 forks source link

Improve type inference for element attributes #245

Closed gsteel closed 11 months ago

gsteel commented 11 months ago
Q A
Documentation yes
QA yes

Description

Marks all attribute arrays and iterables as <string, mixed> which is slightly better than <array-key, mixed>

This is pretty minor stuff, but it really helps in consumer projects running psalm.

I considered that we could have gone for <string, scalar|null> but I'm fairly sure there are cases where arrays are accepted, or maybe even iterables at which point, <string, mixed> felt like a better middle ground considering ElementInterface::getAttribute(string $name): mixed

gsteel commented 11 months ago

Thanks @Slamdunk - I've adjusted the tests to use setValueOptions where appropriate and also properly deprecated providing "value options" in the options key to setAttributes - Also added deprecated tests to cover the deprecation.

Sorry, the patch is a bit bigger now, but the change to scalar|null of course caused a lot of valid psalm issues that needed fixing

Slamdunk commented 11 months ago

What a refreshing patch, thank you @gsteel

gsteel commented 11 months ago

Thanks to you too @Slamdunk and @Ocramius for the reviews :)

pavattt commented 11 months ago

@Slamdunk Do you have any rules on what changes can go into a minor version and what cannot? Signature change in minor version is not something I'd expect.

froschdesign commented 11 months ago

@pavattt

Signature change in minor version is not something I'd expect.

Only allowed in major versions. Do you find a problem?

pavattt commented 11 months ago

@froschdesign I've upgraded laminas/laminas-form from 3.13.1 to 3.17, and now I have a ton of issues in static analysis I have to fix.

Ocramius commented 11 months ago

@Slamdunk Do you have any rules on what changes can go into a minor version and what cannot? Signature change in minor version is not something I'd expect.

It's a bit of a gray area here, because SA shifts regularly (even with patch versions of psalm and phpstan).

We keep BC on anything that would otherwise require changes in the code for:

For changes such as more precise data structures and types, it is very improbable that you'll get BC on that, as the types can easily shift with tiny changes, even in other packages that declare better types.

My general suggestion when doing a composer.lock change that introduces new type errors is to have a baseline file (supported by both PHPStan and Psalm), updating that, and deciding which errors that were introduced are worth fixing immediately.