Skip to content

[SYMFONY62] OhMySMTP, ArgumentValueResolverInterface, Security etc. #278 #279

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

Merged
merged 7 commits into from
Nov 22, 2022

Conversation

JohJohan
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Nov 21, 2022

Fixes #278

@ghost
Copy link

ghost commented Nov 21, 2022

What is the best way to remove the function call to ->createView()? I can make a rule in class method and check if it has a render function call that contains form?

@stefantalen
Copy link
Contributor

stefantalen commented Nov 21, 2022

  1. Check arguments of AbstractController::render()
  2. Check if argument is an instance of FormInstance
  3. Remove ->createView() from argument

However, doing it this way might not update implementations like:

$form = $contactForm->createView();
return $this->render('template.html.twig', [
    'contact_form' => $form,
];

But I think it'll be safer to limit this feature to update calls within the render() method only.

@ghost
Copy link

ghost commented Nov 21, 2022

@stefantalen i agree, and the implementation like you mention also still works

@TomasVotruba
Copy link
Member

Let me know when this is ready.

The createView() removal will indeed need a custom rule. The conditions list seem valid.

@samsonasik
Copy link
Member

Please run:

vendor/bin/rector && composer fix-cs 

to make CI green.

@samsonasik
Copy link
Member

You can verify phpstan with run:

composer phpstan

@ghost
Copy link

ghost commented Nov 22, 2022

@samsonasik its ready now :)

@ghost
Copy link

ghost commented Nov 22, 2022

@TomasVotruba its ready

@ghost
Copy link

ghost commented Nov 22, 2022

Note i added a bit more rules

// @see https://github.com/symfony/symfony/pull/47711
new MethodCallRename('Symfony\Component\Mime\Email', 'attachPart', 'addPart'),
// @see https://github.com/symfony/symfony/pull/47363
new MethodCallRename(
Copy link

@ghost ghost Nov 22, 2022

Choose a reason for hiding this comment

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

Is this allowed because return type is different supports: bool resolve: array

Copy link
Member

Choose a reason for hiding this comment

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

Per comment on #283 (comment)

this part may need to be reverted

@TomasVotruba
Copy link
Member

Thank you 👏

samsonasik pushed a commit to JohJohan/rector-symfony that referenced this pull request Nov 23, 2022
@ghost ghost mentioned this pull request Dec 6, 2022
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.

4 participants