Fix #133, #134: Don't add not substituted arguments and empty values to query string

CHANGELOG.md

@@ -2,7 +2,8 @@
 
 ## 3.0.1 under development
 
-- no changes in this release.
+- Bug #133: Don't add not substituted arguments to a query string (@rustamwin)
+- Bug #134: Don't add query string if it's empty (@rustamwin)
 
 ## 3.0.0 February 17, 2023
 

src/UrlGenerator.php

@@ -101,11 +101,11 @@
                 return rtrim($host, '/') . $url;
             }
 
             if ((empty($scheme) || $lastRequestScheme === null) && $host !== '' && $this->isRelative($host)) {
                 $host = '//' . $host;
             }
 
             return $this->ensureScheme(rtrim($host, '/') . $url, $scheme ?? $lastRequestScheme);
         }
 
         return $uri === null ? $url : $this->generateAbsoluteFromLastMatchedRequest($url, $uri, $scheme);
@@ -157,7 +157,7 @@
     {
         $port = '';
         $uriPort = $uri->getPort();
         if ($uriPort !== 80 && $uriPort !== null) {
             $port = ':' . $uriPort;
         }
 
@@ -207,7 +207,7 @@
      */
     private function isRelative(string $url): bool
     {
         return strncmp($url, '//', 2) && !str_contains($url, '://');
     }
 
     public function getUriPrefix(): string
@@ -254,7 +254,7 @@
             }
         }
 
-        // All required arguments are availgit logable.
+        // All required arguments are available.
         return [];
     }
 
@@ -292,15 +292,16 @@
                     ? rawurlencode($arguments[$part[0]])
                     : urlencode($arguments[$part[0]]);
             }
             unset($notSubstitutedArguments[$part[0]]);
         }
 
         $path = str_replace('//', '/', $path);
 
-        return $path . (
-            $notSubstitutedArguments !== [] || $queryParameters !== [] ?
-                '?' . http_build_query(array_merge($notSubstitutedArguments, $queryParameters))
-                : ''
-        );
+        $queryString = '';
+        if (!empty($queryParameters)) {
+            $queryString = http_build_query($queryParameters);
+        }
+
+        return $path . (!empty($queryString) ? '?' . $queryString : '');
     }
 }

tests/UrlGeneratorTest.php

@@ -196,7 +196,7 @@ public function testQueryParametersAddedAsQueryString(): void
         $this->assertEquals('/test/post?id=12&sort=asc', $url);
     }
 
-    public function testExtraArgumentsAddedAsQueryString(): void
+    public function testQueryParametersAddedAsQueryStringWithEmptyValues(): void
     {
         $routes = [
             Route::get('/test/{name}')
@@ -205,8 +205,8 @@ public function testExtraArgumentsAddedAsQueryString(): void
 
         $url = $this
             ->createUrlGenerator($routes)
-            ->generate('test', ['name' => 'post', 'id' => 12, 'sort' => 'asc']);
-        $this->assertEquals('/test/post?id=12&sort=asc', $url);
+            ->generate('test', ['name' => 'post'], ['id' => null]);
+        $this->assertEquals('/test/post', $url);
     }
 
     public function testQueryParametersOverrideExtraArguments(): void
@@ -222,7 +222,7 @@ public function testQueryParametersOverrideExtraArguments(): void
         $this->assertEquals('/test/post?id=12&sort=asc', $url);
     }
 
-    public function testQueryParametersMergedWithExtraArguments(): void
+    public function testNotSubstitutedArgumentsRemove(): void
     {
         $routes = [
             Route::get('/test/{name}')
@@ -232,7 +232,7 @@ public function testQueryParametersMergedWithExtraArguments(): void
         $url = $this
             ->createUrlGenerator($routes)
             ->generate('test', ['name' => 'post', 'id' => 11], ['sort' => 'asc']);
-        $this->assertEquals('/test/post?id=11&sort=asc', $url);
+        $this->assertEquals('/test/post?sort=asc', $url);
     }
 
     public function testDefaultNotUsedForOptionalArgument(): void