Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Form] Nullable FormInterface::getPropertyPath() #24740

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

Conversation

vudaltsov
Copy link
Contributor

Q A
Branch? 4.0
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #24560
License MIT
Doc PR

Symfony\Component\Form\Form::getPropertyPath() returns null when the form has an empty name. It allows for unprefixed children.

<?php

namespace App\Controller;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Form\Extension\Core\Type\TextType;

class IndexController extends AbstractController
{
    /**
     * @Route(name="index")
     * @Template()
     */
    public function indexAction()
    {
        $form = $this->get('form.factory')
            ->createNamedBuilder('')
            ->add('text', TextType::class)
            ->getForm();

        return [
            'form' => $form->createView(),
        ];
    }
}
<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8">
        <title>Welcome!</title>
    </head>
    <body>
        <form name="" method="post">
            <label for="text">Text</label>
            <input type="text" id="text" name="text">
        </form>
    </body>
</html>

But the return type of the Symfony\Component\Form\FormInterface::getPropertyPath() is not nullable.

We cannot change the behaviour, obviously. At least it's useful in API controllers.

So I decided to change the doc block of the interface.

@vudaltsov vudaltsov changed the title Made FormInterface::getPropertyPath() nullable [Form] Nullable FormInterface::getPropertyPath() Oct 29, 2017
@nicolas-grekas
Copy link
Member

For 2.7? Merging that for 4.0 only is suspicious.

@vudaltsov
Copy link
Contributor Author

Is this a bc-break? Technically yes. Because I've changed the signature of the interface. But on the other hand null was possible because static typing was not used...

@chalasr chalasr removed the BC Break label Oct 31, 2017
@chalasr chalasr added this to the 2.7 milestone Oct 31, 2017
@chalasr chalasr added the Form label Oct 31, 2017
@chalasr
Copy link
Member

chalasr commented Oct 31, 2017

Fine for me on 2.7.

@vudaltsov vudaltsov force-pushed the form-interface-property-path-nullable branch from d3d0daa to d56632a Compare November 1, 2017 09:38
@vudaltsov vudaltsov changed the base branch from master to 2.7 November 1, 2017 09:38
@vudaltsov
Copy link
Contributor Author

@chalasr, rebased onto 2.7

Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

Technically, that's a BC break. But the main implementation already had this behaviour.

@fabpot
Copy link
Member

fabpot commented Nov 5, 2017

Thank you @vudaltsov.

@fabpot fabpot merged commit d56632a into symfony:2.7 Nov 5, 2017
fabpot added a commit that referenced this pull request Nov 5, 2017
…sov)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Nullable FormInterface::getPropertyPath()

| Q             | A
| ------------- | ---
| Branch?       | 4.0
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24560
| License       | MIT
| Doc PR        |

`Symfony\Component\Form\Form::getPropertyPath()` returns `null` when the form has an empty name. It allows for unprefixed children.

```php
<?php

namespace App\Controller;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Form\Extension\Core\Type\TextType;

class IndexController extends AbstractController
{
    /**
     * @route(name="index")
     * @template()
     */
    public function indexAction()
    {
        $form = $this->get('form.factory')
            ->createNamedBuilder('')
            ->add('text', TextType::class)
            ->getForm();

        return [
            'form' => $form->createView(),
        ];
    }
}
```

```html
<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8">
        <title>Welcome!</title>
    </head>
    <body>
        <form name="" method="post">
            <label for="text">Text</label>
            <input type="text" id="text" name="text">
        </form>
    </body>
</html>
```

But the return type of the `Symfony\Component\Form\FormInterface::getPropertyPath()` is not nullable.

We cannot change the behaviour, obviously. At least it's useful in API controllers.

So I decided to change the doc block of the interface.

Commits
-------

d56632a FormInterface::getPropertyPath(): PropertyPathInterface|null
@vudaltsov vudaltsov deleted the form-interface-property-path-nullable branch November 5, 2017 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.