Skip to content
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

[make:*] add types for PHPStan #1542

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Maker/MakeWebhook.php
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ private function getRequestMatcherArguments(string $requestMatcherClass): string
IsJsonRequestMatcher::class => '',
MethodRequestMatcher::class => '\'POST\'',
PortRequestMatcher::class => '443',
SchemeRequestMatcher::class => 'https',
SchemeRequestMatcher::class => '\'https\'',
default => '[]',
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/Resources/skeleton/crud/controller/Controller.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function edit(Request $request, <?= $entity_class_name ?> $<?= $entity_va
<?= $generator->generateRouteForControllerMethod(sprintf('/{%s}', $entity_identifier), sprintf('%s_delete', $route_name), ['POST']) ?>
public function delete(Request $request, <?= $entity_class_name ?> $<?= $entity_var_singular ?>, EntityManagerInterface $entityManager): Response
{
if ($this->isCsrfTokenValid('delete'.$<?= $entity_var_singular ?>->get<?= ucfirst($entity_identifier) ?>(), $request->getPayload()->get('_token'))) {
if ($this->isCsrfTokenValid('delete'.$<?= $entity_var_singular ?>->get<?= ucfirst($entity_identifier) ?>(), $request->getPayload()->getString('_token'))) {
$entityManager->remove($<?= $entity_var_singular ?>);
$entityManager->flush();
}
Expand Down
2 changes: 1 addition & 1 deletion src/Resources/skeleton/message/MessageHandler.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#[AsMessageHandler]
final class <?= $class_name ?>
{
public function __invoke(<?= $message_class_name ?> $message)
public function __invoke(<?= $message_class_name ?> $message): void
{
// do something with your message
}
Expand Down
23 changes: 13 additions & 10 deletions src/Resources/skeleton/registration/RegistrationController.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
class <?= $class_name; ?> extends AbstractController
{
<?php if ($will_verify_email): ?>
public function __construct(private <?= $generator->getPropertyType($email_verifier_class_details) ?>$emailVerifier)
{
public function __construct(
private readonly <?= $generator->getPropertyType($email_verifier_class_details) ?>$emailVerifier
) {
}

<?php endif; ?>
Expand All @@ -20,13 +21,11 @@ public function register(Request $request, UserPasswordHasherInterface $userPass
$form->handleRequest($request);

if ($form->isSubmitted() && $form->isValid()) {
/** @var string $plainPassword */
$plainPassword = $form->get('plainPassword')->getData();

// encode the plain password
$user->set<?= ucfirst($password_field) ?>(
$userPasswordHasher->hashPassword(
$user,
$form->get('plainPassword')->getData()
)
);
$user->set<?= ucfirst($password_field) ?>($userPasswordHasher->hashPassword($user, $plainPassword));

$entityManager->persist($user);
$entityManager->flush();
Expand All @@ -36,7 +35,7 @@ public function register(Request $request, UserPasswordHasherInterface $userPass
$this->emailVerifier->sendEmailConfirmation('app_verify_email', $user,
(new TemplatedEmail())
->from(new Address('<?= $from_email ?>', '<?= $from_email_name ?>'))
->to($user-><?= $email_getter ?>())
->to((string) $user-><?= $email_getter ?>())
->subject('Please Confirm your Email')
->htmlTemplate('registration/confirmation_email.html.twig')
);
Expand Down Expand Up @@ -84,7 +83,11 @@ public function verifyUserEmail(Request $request<?php if ($translator_available)

// validate email confirmation link, sets User::isVerified=true and persists
try {
$this->emailVerifier->handleEmailConfirmation($request, <?= $verify_email_anonymously ? '$user' : '$this->getUser()' ?>);
<?php if (!$verify_email_anonymously): ?>
/** @var <?= $user_class_name ?> $user */
$user = $this->getUser();
<?php endif; ?>
$this->emailVerifier->handleEmailConfirmation($request, $user);
} catch (VerifyEmailExceptionInterface $exception) {
$this->addFlash('verify_email_error', <?php if ($translator_available): ?>$translator->trans($exception->getReason(), [], 'VerifyEmailBundle')<?php else: ?>$exception->getReason()<?php endif ?>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ public function request(Request $request, MailerInterface $mailer<?php if ($tran
$form->handleRequest($request);

if ($form->isSubmitted() && $form->isValid()) {
return $this->processSendingPasswordResetEmail(
$form->get('<?= $email_field ?>')->getData(),
$mailer<?php if ($translator_available): ?>,
$translator<?php endif ?><?= "\n" ?>
);
/** @var string $email */
$email = $form->get('<?= $email_field ?>')->getData();

return $this->processSendingPasswordResetEmail($email, $mailer<?php if ($translator_available): ?>, $translator<?php endif ?><?= "\n" ?>);
}

return $this->render('reset_password/request.html.twig', [
Expand Down Expand Up @@ -94,13 +93,11 @@ public function reset(Request $request, UserPasswordHasherInterface $passwordHas
// A password reset token should be used only once, remove it.
$this->resetPasswordHelper->removeResetRequest($token);

// Encode(hash) the plain password, and set it.
$encodedPassword = $passwordHasher->hashPassword(
$user,
$form->get('plainPassword')->getData()
);
/** @var string $plainPassword */
$plainPassword = $form->get('plainPassword')->getData();

$user-><?= $password_setter ?>($encodedPassword);
// Encode(hash) the plain password, and set it.
$user-><?= $password_setter ?>($passwordHasher->hashPassword($user, $plainPassword));
$this->entityManager->flush();

// The session is cleaned up after the password has been changed.
Expand Down Expand Up @@ -143,7 +140,7 @@ private function processSendingPasswordResetEmail(string $emailFormData, MailerI

$email = (new TemplatedEmail())
->from(new Address('<?= $from_email ?>', '<?= $from_email_name ?>'))
->to($user-><?= $email_getter ?>())
->to((string) $user-><?= $email_getter ?>())
->subject('Your password reset request')
->htmlTemplate('reset_password/email.html.twig')
->context([
Expand Down
2 changes: 1 addition & 1 deletion src/Resources/skeleton/validator/Constraint.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ class <?= $class_name ?> extends Constraint
* Any public properties become valid options for the annotation.
* Then, use these in your validator class.
*/
public $message = 'The value "{{ value }}" is not valid.';
public string $message = 'The value "{{ value }}" is not valid.';
}
2 changes: 1 addition & 1 deletion src/Resources/skeleton/validator/Validator.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

class <?= $class_name ?> extends ConstraintValidator
{
public function validate($value, Constraint $constraint)
public function validate(mixed $value, Constraint $constraint): void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is possible (e.g. maker has enough knowledge about $value) but can we do string|array<int, mixed>|int|etc..... here instead of mixed? Thinking about phpstan level 9? where it complains if something is mixed or if an array does not define its shape.

I'm not suggesting we do all potential types as a union, but if we know $value could be say, either a string or a int - we should use string|int as the type instead of mixed.

else mixed is perfectly fine here 😉

{
/** @var <?= $constraint_class_name ?> $constraint */

Expand Down
8 changes: 4 additions & 4 deletions src/Resources/skeleton/verifyEmail/EmailVerifier.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<?= $use_statements; ?>

class <?= $class_name; ?><?= "\n" ?>
readonly class <?= $class_name; ?><?= "\n" ?>
{
public function __construct(
private VerifyEmailHelperInterface $verifyEmailHelper,
Expand All @@ -19,10 +19,10 @@ public function sendEmailConfirmation(string $verifyEmailRouteName, <?= $user_cl
$verifyEmailRouteName,
(string) $user-><?= $id_getter ?>(),
<?php if ($verify_email_anonymously): ?>
$user-><?= $email_getter ?>(),
(string) $user-><?= $email_getter ?>(),
['id' => $user-><?= $id_getter ?>()]
<?php else: ?>
$user-><?= $email_getter ?>()
(string) $user-><?= $email_getter ?>()
<?php endif; ?>
);

Expand All @@ -41,7 +41,7 @@ public function sendEmailConfirmation(string $verifyEmailRouteName, <?= $user_cl
*/
public function handleEmailConfirmation(Request $request, <?= $user_class_name ?> $user): void
{
$this->verifyEmailHelper->validateEmailConfirmationFromRequest($request, (string) $user-><?= $id_getter ?>(), $user-><?= $email_getter?>());
$this->verifyEmailHelper->validateEmailConfirmationFromRequest($request, (string) $user-><?= $id_getter ?>(), (string) $user-><?= $email_getter?>());

$user->setVerified(true);

Expand Down
8 changes: 4 additions & 4 deletions src/Resources/skeleton/webhook/RequestParser.tpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ protected function doParse(Request $request, #[\SensitiveParameter] string $secr
}

// Parse the request payload and return a RemoteEvent object.
$payload = $request->getPayload()->all();
$payload = $request->getPayload();

return new RemoteEvent(
$payload['name'],
$payload['id'],
$payload,
$payload->getString('name'),
$payload->getString('id'),
$payload->all(),
);
}
}
6 changes: 3 additions & 3 deletions src/Security/UserClassBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ private function addGetPassword(ClassSourceManipulator $manipulator, UserClassCo
// add an empty method only
$builder = $manipulator->createMethodBuilder(
'getPassword',
'string',
'?string',
true,
[
'This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.',
Comment on lines 181 to 185
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need to add this method anymore? I'm just looking at the comment that we add - maker only supports symfony 6.4+

If we do not need the method - lets leave this alone and remove the method in a separate PR.

Expand Down Expand Up @@ -216,7 +216,7 @@ private function addGetPassword(ClassSourceManipulator $manipulator, UserClassCo

$manipulator->addGetter(
'password',
'string',
'?string',
true
);

Expand All @@ -231,7 +231,7 @@ private function addGetPassword(ClassSourceManipulator $manipulator, UserClassCo
$manipulator->addAccessorMethod(
'password',
'getPassword',
'string',
'?string',
false,
[
'@see PasswordAuthenticatedUserInterface',
Expand Down
Loading