Skip to content

Fix #7692, Add fixture for TypedPropertyFromAssignsRector#3244

Closed
puniserv wants to merge 1 commit intorectorphp:mainfrom
puniserv:fix/7692-types-property-from-assigns-rector
Closed

Fix #7692, Add fixture for TypedPropertyFromAssignsRector#3244
puniserv wants to merge 1 commit intorectorphp:mainfrom
puniserv:fix/7692-types-property-from-assigns-rector

Conversation

@puniserv
Copy link
Copy Markdown

No description provided.

@samsonasik
Copy link
Copy Markdown
Member

It seems you need to move the fixture under

https://github.com/rectorphp/rector-src/tree/main/rules-tests/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector/FixtureComplexTypes

to demonstrate invalid usage on php8+ feature

@samsonasik
Copy link
Copy Markdown
Member

Various notes:

  • The php74 env for nullable change is correct, just to keep the docblock DateTime&MockObject
  • Under php8+, the change is incorrect for combine nullable with intersection:
+    private ?\DateTime&\PHPUnit\Framework\MockObject\MockObject $property = null;

Ref https://getrector.org/demo/9639d6c3-b5a5-47ef-8565-f3037cde9e64

nullable can't be combined with intersection directly, see:

https://3v4l.org/QQi2E#v8.2.0 (nullable ?)
https://3v4l.org/QP7Rh#v8.2.0 (union nullable)

so it should be :

+    private null|(\DateTime&\PHPUnit\Framework\MockObject\MockObject) $property = null;

@samsonasik
Copy link
Copy Markdown
Member

Based on above notes, it seems the solution is skip on php 7.4 and 8.0 for that, and apply changes on php 8.1+ with intersection types enabled:

-    /**
-     * @var DateTime&MockObject
-     */
+    private null|(\DateTime&\PHPUnit\Framework\MockObject\MockObject) $property = null;

like @VincentLanglet comment at rectorphp/rector#7692 (comment)

@samsonasik
Copy link
Copy Markdown
Member

Actually, without the setUp() called, nullable still ok on php 7.4, but just not to remove the docblock

@samsonasik
Copy link
Copy Markdown
Member

@puniserv I cherry-picked your commit at PR #3372

samsonasik added a commit that referenced this pull request Feb 12, 2023
@puniserv puniserv deleted the fix/7692-types-property-from-assigns-rector branch February 13, 2023 00:19
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