Skip to content

Commit

Permalink
Revert "merged branch Tobion/uselessparamdefaults (PR #5400)"
Browse files Browse the repository at this point in the history
This reverts commit 0f61b2eb90f40e0665aed9ce82b94e6cfee7f07e, reversing
changes made to 5e7723fcbb1232f6324312975f21a7d37243b785.
  • Loading branch information
fabpot committed Sep 5, 2012
1 parent 475c055 commit f336fde
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 29 deletions.
18 changes: 4 additions & 14 deletions RouteCompiler.php
Expand Up @@ -23,8 +23,7 @@ class RouteCompiler implements RouteCompilerInterface
/**
* {@inheritDoc}
*
* @throws \LogicException If a variable is referenced more than once or if a required variable
* has a default value that doesn't meet its own requirement.
* @throws \LogicException If a variable is referenced more than once
*/
public function compile(Route $route)
{
Expand Down Expand Up @@ -68,23 +67,14 @@ public function compile(Route $route)
$tokens[] = array('text', substr($pattern, $pos));
}

// find the first optional token and validate the default values for non-optional variables
$optional = true;
// find the first optional token
$firstOptional = INF;
for ($i = count($tokens) - 1; $i >= 0; $i--) {
$token = $tokens[$i];
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
if ($optional) {
$firstOptional = $i;
} elseif (!preg_match('#^'.$token[2].'$#', $route->getDefault($token[3]))) {
throw new \LogicException(sprintf('The default value "%s" of the required variable "%s" in pattern "%s" does not match the requirement "%s". ' .
'This route definition makes no sense because this default can neither be used as default for generating URLs nor can it ever be returned by the matching process. ' .
'You should change the default to something that meets the requirement or remove it.',
$route->getDefault($token[3]), $token[3], $route->getPattern(), $token[2]
));
}
$firstOptional = $i;
} else {
$optional = false;
break;
}
}

Expand Down
6 changes: 3 additions & 3 deletions Tests/Generator/UrlGeneratorTest.php
Expand Up @@ -75,13 +75,13 @@ public function testRelativeUrlWithNullParameter()
}

/**
* @expectedException \LogicException
* @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
*/
public function testRelativeUrlWithNullParameterButNotOptional()
{
$routes = $this->getRoutes('test', new Route('/testing/{foo}/bar', array('foo' => null)));
// It should raise an exception when compiling this route because the given default value is absolutely
// irrelevant for both matching and generating URLs.
// This must raise an exception because the default requirement for "foo" is "[^/]+" which is not met with these params.
// Generating path "/testing//bar" would be wrong as matching this route would fail.
$this->getGenerator($routes)->generate('test', array(), false);
}

Expand Down
13 changes: 1 addition & 12 deletions Tests/RouteCompilerTest.php
Expand Up @@ -131,17 +131,6 @@ public function testRouteWithSameVariableTwice()
{
$route = new Route('/{name}/{name}');

$route->compile();
}

/**
* @expectedException \LogicException
*/
public function testRouteWithRequiredVariableAndBadDefault()
{
$route = new Route('/{foo}/', array('foo' => null));
// It should raise an exception when compiling this route because the given default value is absolutely
// irrelevant for both matching and generating URLs.
$route->compile();
$compiled = $route->compile();
}
}

0 comments on commit f336fde

Please sign in to comment.