-
-
Notifications
You must be signed in to change notification settings - Fork 374
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
Add known type casting to AnnotationToAttributeRector #6217
Conversation
1258558
to
7c4469d
Compare
/** | ||
* @var array<string, string[]> | ||
*/ | ||
private const INTEGER_KEYS_BY_CLASS = [ | ||
AnnotationClassName::LENGTH => ['min', 'max'], | ||
]; | ||
|
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.
@etshy I made a static list to cast known keys. What other keys do you think we should include here?
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.
I only had the case on min/max for Length constraints so I'm not sure right now.
Basically any integer available ? I guess they all could be written as string in annotation but type hinted as int as Attributes.
I'll take a look of the Constraints list after work hours and could make a list from that
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.
I guess they all could be written as string in annotation but type hinted as int as Attributes.
Good point. I'll see if we can do reflection on existing attribute constructor and infer the int
type from those.
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.
I had a few minute before going back to work so here a non exhaustive list if you still need it
CIDR
netmaskMin
netmaskMax
PasswordStrength
minScore
NoSuspiciousCharacters
checks (there are constants in docs but it's still an int)
restrictionLevel (same)
Range
max
min
File
filenameMaxLength
Image
maxHeight
maxPixels
maxRatio (float)
maxWidth
minHeight
minPixels
minRatio (float)
minWidth
Count
divisibleBy
max
min
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.
Awesome, thanks for checking. I'll use this list to test this rule in the wild, once deployed.
9b8805c
to
4cda518
Compare
a688600
to
f0c57bc
Compare
@@ -39,6 +39,10 @@ public function removeNullTypeFromUnionType(UnionType $unionType): UnionType | |||
$unionedTypesWithoutNullType[] = $type; | |||
} | |||
|
|||
if ($unionedTypesWithoutNullType !== []) { |
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.
on else, union with empty types is impossible, I think this can be handled by count($unionedTypesWithoutNullType) === 1
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.
Closes rectorphp/rector#8713