Skip to content

Fix phpstan/phpstan#13853: Smarter check about ununitialized readonly properties#5323

Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-iqi90gg
Open

Fix phpstan/phpstan#13853: Smarter check about ununitialized readonly properties#5323
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-iqi90gg

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a readonly property is assigned outside the constructor but is guarded by an isset() check (e.g., if (!isset($this->prop)) { $this->prop = ...; }), PHPStan no longer reports property.readOnlyAssignNotInConstructor. The isset() guard ensures the property can only be assigned once, preventing the runtime error that the rule is designed to catch.

Changes

  • src/Analyser/TypeSpecifier.php: In the falsy branch of isset($this->prop), track PropertyInitializationExpr with NeverType to indicate the property is known to be uninitialized.
  • src/Rules/Properties/ReadOnlyPropertyAssignRule.php: Skip readOnlyAssignNotInConstructor error when PropertyInitializationExpr for the property has NeverType (meaning we're in the false branch of an isset check).
  • src/Rules/IssetCheck.php: Exclude NeverType PropertyInitializationExpr from the "property is initialized" check so nested isset checks within !isset guards don't produce false positives.
  • src/Node/ClassPropertiesNode.php: Add hasPropertyInitialization() helper that treats NeverType PropertyInitializationExpr as "not initialized", fixing false "already assigned" errors in constructors using isset guards.

Root cause

The ReadOnlyPropertyAssignRule unconditionally reported any readonly property assignment outside the constructor as an error. It did not consider that an isset() guard makes the assignment safe by ensuring the property is only assigned when uninitialized. The fix leverages the existing PropertyInitializationExpr tracking mechanism, using NeverType as a sentinel value to distinguish "known uninitialized via isset" from "initialized" (which uses MixedType).

Test

Added tests/PHPStan/Rules/Properties/data/bug-13853.php with three test cases:

  1. BaseReportLocator: readonly property assigned after if (isset(...)) { return; } guard - no error expected
  2. AnotherExample: readonly property assigned inside if (!isset(...)) guard - no error expected
  3. NoIssetGuard: readonly property assigned without any isset guard - error still expected

Fixes phpstan/phpstan#13853

…sset guard is present

- Track PropertyInitializationExpr with NeverType in the falsy branch of isset($this->prop) in TypeSpecifier
- Skip readOnlyAssignNotInConstructor error in ReadOnlyPropertyAssignRule when PropertyInitializationExpr indicates property is known uninitialized
- Adjust IssetCheck and ClassPropertiesNode to treat NeverType PropertyInitializationExpr as "not initialized"
- New regression test in tests/PHPStan/Rules/Properties/data/bug-13853.php
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.

1 participant