We would also need to find a way to represent other datatypes in xml like integer, float and boolean. Only supporting array without the other datatypes would be arbitrary.
Also possibly DateTime etc.

@Tobion Do you think that both array types and different scalar types should be done in the same pull request?

Yes because if we don't find a solution for all of them, I'd argue it's not worth implementing only one part.
Then we could just say, xml only supports flat structure with strings.

I don't fully agree. Even if you were not able to use integer, float or boolean values natively in XML, you would at least be able to retain the array structure. Scalar types can be casted in the controller. Achieving the same with arrays is not always that easy (at least with nested arrays).

But I also don't think that it is too hard adding support for the other data types. So I will look into this tonight.

@Tobion the XML loading in Symfony already converts values to boolean or number types when they look like that in the DI component. However, this is not applied in the routing loader (and applying it could be considered a BC break btw).

However, this way to represent arrays looks good (a bit painful if you want to represent lists as you have to map the keys explicitly though)

@Tobion the XML loading in Symfony already converts values to boolean or number types when they look like that in the DI component. However, this is not applied in the routing loader (and applying it could be considered a BC break btw).

@stof I thought about adding an optional type attribute to the default element. If present, the content will be casted accordingly. Otherwise it is treated as a string. This way we can maintain backward compatibility.

However, this way to represent arrays looks good (a bit painful if you want to represent lists as you have to map the keys explicitly though)

We can make the key attribute optional. This should work and shouldn't cause any BC issue. What do you think?

We can make the key attribute optional. This should work and shouldn't cause any BC issue. What do you think?

The problem is that the first level needs a string key name. Otherwise you would have numeric defaults which won't work (because they get removed since preg_match also adds them).

I thought about adding an optional type attribute to the default element. If present, the content will be casted accordingly.

This will not support XSD validation with types, can it? So you could add a string within a type=integer and XSD cannot raise an error.

The problem is that the first level needs a string key name. Otherwise you would have numeric defaults which won't work (because they get removed since preg_match also adds them).

Should be possible to validate that in the XSD.

This will not support XSD validation with types, can it? So you could add a string within a type=integer and XSD cannot raise an error.

No, unfortunately not. If we want that, we'll have to use different XML elements for each data type I guess.

Another possibility I see is to introduce a new attribute like infer-type which defaults to false. When this is set to true, the loader can try to infer the data type from the actual value instead of simply assuming a string value.

I think a solution that fully supports the mentioned data types both in the XML schema and in PHP and which is backward compatible requires some more work.

What I would suggest for the moment, is creating a new defaults (the name may need to be discussed) element on the same level as the current default elements. Inside it we would nest collection, string, double, integer and boolean elements. Keys can be defined the same way as before with the key attribute. To allow numerical indices, this attribute would be optional on all but the top level of the tree.

An example configuration may then look like this:

<defaults>
  <string key="foo">bar</string>
  <collection key="list">
    <boolean>true</boolean>
    <boolean>false</boolean>
    <collection>
      <string key="foobar">hash value</string>
    </collection>
  </collection>
  <integer key="baz">10</integer>
</defaults>

The process defaults would then be equal to this array:

$defaults = array(
    'foo' => 'bar',
    'list' => array(
        true,
        false,
        array('foobar' => 'hash value'),
    ),
    'baz' => 10,
);

The already existing default element would be kept with the current behaviour for backward compatibility reasons.

What do you think?

@xabbuh using a single <defaults> instead of multiple default elements would be totally inconsistent with the way other places look in Symfony XML files. for me, it is a -1.

Staying with multiple default elements would mean something like this (the attribute should then be forbidden on the top level default child elements):

<default key="foo">
  <string>bar</string>
</default>
<default key="list">
  <collection>
    <boolean>true</boolean>
    <boolean>false</boolean>
    <collection>
      <string key="foobar">hash value</string>
    </collection>
  </collection>
</default>
<default key="baz">
  <integer>10</integer>
</default>

For backward compatibility,

<default key="foo">
  <string>bar</string>
</default>

and

<default key="foo">bar</default>

would be equivalent.

The last proposal was also what I had in mind. But I would propose to distinguish between arrays (as data structure, not PHPs implementation) and maps/associative arrays. So you can validate in XSD that elements inside maps require a key and inside arrays must not have a key.
Otherwise you could mix elements with key and without inside collections which is possible in PHP but rather magical in general and not needed here. This would also prevent accidential user errors.

That's a good point since mixing list and maps also isn't possible with YAML. So, we can simply use a list and a map element.

If we agree on this, I will update the pull request to match this structure.

Yeah I agree with list and map. In php the list will still be an array that allows random access, so is not really a list. But that can be considered implementation detail and semantically people usually want to express a list a values in XML when not using an associative array/map.

I updated the code to support the several data types.

I just noticed that the new parseDefaultNode() method was capable to properly parse nodes with a namespace prefix. I fixed that and modified the already existing testLoadWithNamespacePrefix() to cover this too.

Can you please add a note about this in the routing changelog. Then I'm 👍 to merge this.

@fabpot considering all other loaders except xml already "supported" array defaults, we have two ways to deal with this

1. "remove" array default support in the other loaders, which will probably be a bc break for some
2. add support for array defaults in xml to be consistent and merge this PR

I'd vote for 2. because its not a bc break and it should not be a problem in php to support arrays. we deprecated apache dumper where it would probably be tedious to support arrays. so thats not a problem. also the xml loader already explicitly supported null, so also supporting other scalars like int makes sense. fabien, what do you prefer?

The support for non-string values in other formats was not a conscious decision; it just happens to work because there is no restrictions in place. So, I'm still against this change.

Well actually it was, at least for null: #7635

And according to RequestContext::setParameter a parameter can be mixed.

Re-reading this old PR, I still thing that defaults should only be strings or null. When parsing a path, you can only get back strings anyway, so allowing defaults to be something else seems wrong to me. I'm in favor of deprecating the possibility to use any other kind of value.

One point of routing is that you can change URIs without chaning the code to parse/generate them. Since array params can be used for query strings, at one point it would also make sense to allow arrays in the path. For example, URI templates standard supports that by expanding arrays to comma-separated values.
So removing non-string defaults would be a step in the wrong direction IMO.

Also with the introduction of scalar type hints, what if a controller parameter is typehinted to int for example? I guess this will be cast implicitly at the moment. But with strict types, it would cause an error. So in theory either the router itself could cast params (for example based on the requirement) or the code that calls the controller. So then route params would need to be non-strings as well.

ok, you're right. Let's finish this one then.

@Tobion strict types are irrelevant here, because the strict mode for arguments is determined by the calling code, not by the file declaring the controller, and the Kernel class (which is calling the controller) is not in strict mode

@stof I know. But who says Symfony 4 will not add scalar type hints in certain places and maybe strict types?

rebased here, if I don't miss anything all comments made so far have already been addressed

Thank you @xabbuh.

Maybe we should rename boolean and integer elements to bool and int elements as the short names are now more standard considering scalar typehints etc.