-
-
Notifications
You must be signed in to change notification settings - Fork 294
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
Fix #761, #762: support nullable identifier hint in proxies #763
Fix #761, #762: support nullable identifier hint in proxies #763
Conversation
…ate class with php7.1 check and fixed the regex for PATTERN_MATCH_ID_METHOD
return $this->identifierFieldReturnClassOneLetterNullable; | ||
} | ||
|
||
public function getIdentifierFieldReturnClassOneLetterNullableWithSpace(): ? A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know what is the correct syntax, but you set ? A
here and ?A
.
I suppose this is not wanted. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space is intentional, since both approaches are valid/accepted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh indeed.
But why do you have to test both syntaxes? The behavior is same, am I wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the code analyzer is regex-based (yeah, I know :S)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the code analyzer is regex-based
Damn me, indeed!
(yeah, I know :S)
Regex is good, eat it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok
Backported into |
Fixes #761
Fixes #762
Replaces #762 (includes its commits)