Skip to content

Conversation

@calebdw
Copy link
Contributor

@calebdw calebdw commented Jan 8, 2026

Hello!

Motivation

See https://x.com/VotrubaT/status/2008248184540418398

Currently, the DeclareStrictTypesRector just indiscriminately adds declare(strict_types=1) to all files. However:

  1. this is very unsafe and has great potential to break existing files---therefore this is basically useless and is not part of any set by default
  2. there's already a php-cs-fixer rule for this, so again it's rather useless in its current form

This PR creates a new rule (SafeDeclareStrictTypesRector) that is (should be) completely safe and only adds strict types to the file when it won't break code. I've also added it to the codeQuality ruleset (but let me know if you'd rather it go elsewhere).

Implementation

Basically, strict_types only affects calling code and prevents scalar type coercion in the following cases:

  • arguments passed to CallLikes and Attributes
  • values returned from FunctionLikes
  • values assigned to properties

This PR adds a new service class (StrictTypeSafetyChecker) that iterates through all the above and checks the type compatibility. If the file is completely safe then the refactor is performed, otherwise the file is skipped. I conservatively skip files when we can't be sure (e.g., dynamic calls, unpacks, etc.).

All of the actual type checking is deferred to phpstan's Type::accepts($type, strictTypes: true) api which really makes things clean on our end.

CC: @TomasVotruba

Thanks!

@calebdw calebdw force-pushed the calebdw/push-oxpklwvkytpo branch 5 times, most recently from dac996f to e51df27 Compare January 8, 2026 16:07
@TomasVotruba
Copy link
Member

Hi, thanks for such a fast kick off. Wow!

Before I review, can you use a new Rector rule, e.g. Safe declare...Rector? People are used to current scope of the rule and ignore it as not safe.

@calebdw
Copy link
Contributor Author

calebdw commented Jan 8, 2026

ignore it as not safe.

they should not be ignoring it because it's not part of any default set to ignore---if they wanted to use it then they would have needed to manually include it in their configs

However, I can certainly create a new rule if you still think it's needed.

@TomasVotruba
Copy link
Member

Indeed, but devs are lazy and it has a bad karma already. I would not recommend it to anyone :)

Could you rename it?

@calebdw calebdw force-pushed the calebdw/push-oxpklwvkytpo branch from e51df27 to fa3c381 Compare January 8, 2026 18:19
@calebdw calebdw changed the title refactor: make DeclareStrictTypesRector safe and add to ruleset feat: create new SafeDeclareStrictTypesRector Jan 8, 2026
@calebdw
Copy link
Contributor Author

calebdw commented Jan 8, 2026

@TomasVotruba, done!

@calebdw calebdw force-pushed the calebdw/push-oxpklwvkytpo branch from fa3c381 to 743d9a0 Compare January 8, 2026 18:39
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.

2 participants