fix: prevent reserved variable names in tests (#785)

Author: bshaffer

src/Generation/TestNameValueProducer.php

@@ -94,20 +94,21 @@ public function __construct($prod, $f, $forceValues)
             });
     }
 
-    public function perFieldRequest(MethodDetails $method): array
+    public function perFieldRequest(MethodDetails $method, array $reservedNames = []): array
     {
         // Handle test request value generation.
         $perField = static::filterFirstOneOf($method->allFields)
-            ->map(fn ($f) => new class($this, $method, $f) {
-                public function __construct($prod, $method, $f)
+            ->map(fn ($f) => new class($this, $method, $f, $reservedNames) {
+                public function __construct($prod, $method, $f, $reservedNames)
                 {
                     // This code is arranged in this way, as a name must not be generated
                     // if this field ends up not being used.
                     // Name generation is stateful.
                     // TODO: Consider refactoring this.
                     $this->field = $f;
                     $this->name = ($f->useResourceTestValue ? 'formatted_' : '') . $prod->name($f->name);
-                    $this->var = AST::var(Helpers::toCamelCase($this->name));
+                    $varPrefix = in_array($this->name, $reservedNames) ? 'request_' : '';
+                    $this->var = AST::var(Helpers::toCamelCase($varPrefix . $this->name));
                     $astAcc = Vector::new([]);
                     $prod->fieldInit($method, $f, $this->var, $this->name, null, $astAcc);
                     $this->initCode = $astAcc;

src/Generation/UnitTestsV2Generator.php

@@ -309,7 +309,7 @@ private function testExceptionalCaseNormal(MethodDetails $method): PhpMethod
         $client = AST::var(self::CLIENT_VARIABLE);
         $status = AST::var('status');
         $expectedExceptionMessage  = AST::var('expectedExceptionMessage ');
-        [$requestPerField, $requestCallArgs] = $prod->perFieldRequest($method);
+        [$requestPerField, $requestCallArgs] = $prod->perFieldRequest($method, ['status']);
         $ex = AST::var('ex');
         list($initializedFields, $requestAssignment) = $this->initializeRequest($requestPerField, $method->requestType);
         return AST::method($method->testExceptionMethodName)

tests/Unit/ProtoTests/Basic/basic.proto

@@ -48,6 +48,7 @@ message RequestWithArgs {
   repeated PartOfRequestA part_of_request_a = 4 [(google.api.field_behavior) = REQUIRED];
   repeated PartOfRequestB part_of_request_b = 5;
   PartOfRequestC part_of_request_c = 6;
+  int32 status = 7 [(google.api.field_behavior) = REQUIRED]; // reserved variable name
 }
 
 message Response {

tests/Unit/ProtoTests/Basic/out/samples/BasicClient/method_with_args.php

@@ -33,8 +33,9 @@
  * Test including method args.
  *
  * @param string $aString A required field...
+ * @param int    $status  reserved variable name
  */
-function method_with_args_sample(string $aString): void
+function method_with_args_sample(string $aString, int $status): void
 {
     // Create a client.
     $basicClient = new BasicClient();
@@ -43,7 +44,8 @@ function method_with_args_sample(string $aString): void
     $partOfRequestA = [new PartOfRequestA()];
     $request = (new RequestWithArgs())
         ->setAString($aString)
-        ->setPartOfRequestA($partOfRequestA);
+        ->setPartOfRequestA($partOfRequestA)
+        ->setStatus($status);
 
     // Call the API and handle any network failures.
     try {
@@ -67,7 +69,8 @@ function method_with_args_sample(string $aString): void
 function callSample(): void
 {
     $aString = '[A_STRING]';
+    $status = 0;
 
-    method_with_args_sample($aString);
+    method_with_args_sample($aString, $status);
 }
 // [END basic_generated_Basic_MethodWithArgs_sync]

tests/Unit/ProtoTests/Basic/out/tests/Unit/Client/BasicClientTest.php

@@ -129,9 +129,11 @@ public function methodWithArgsTest()
         // Mock request
         $aString = 'aString-929604177';
         $partOfRequestA = [];
+        $status = 892481550;
         $request = (new RequestWithArgs())
             ->setAString($aString)
-            ->setPartOfRequestA($partOfRequestA);
+            ->setPartOfRequestA($partOfRequestA)
+            ->setStatus($status);
         $response = $gapicClient->methodWithArgs($request);
         $this->assertEquals($expectedResponse, $response);
         $actualRequests = $transport->popReceivedCalls();
@@ -143,6 +145,8 @@ public function methodWithArgsTest()
         $this->assertProtobufEquals($aString, $actualValue);
         $actualValue = $actualRequestObject->getPartOfRequestA();
         $this->assertProtobufEquals($partOfRequestA, $actualValue);
+        $actualValue = $actualRequestObject->getStatus();
+        $this->assertProtobufEquals($status, $actualValue);
         $this->assertTrue($transport->isExhausted());
     }
 
@@ -167,9 +171,11 @@ public function methodWithArgsExceptionTest()
         // Mock request
         $aString = 'aString-929604177';
         $partOfRequestA = [];
+        $requestStatus = 892481550;
         $request = (new RequestWithArgs())
             ->setAString($aString)
-            ->setPartOfRequestA($partOfRequestA);
+            ->setPartOfRequestA($partOfRequestA)
+            ->setStatus($requestStatus);
         try {
             $gapicClient->methodWithArgs($request);
             // If the $gapicClient method call did not throw, fail the test