add more samples/tests

GoogleCloudPlatform · Aug 28, 2024 · 3042912 · 3042912
1 parent 9f2822c
commit 3042912
Show file tree

Hide file tree

Showing 8 changed files with 170 additions and 41 deletions.
diff --git a/src/Fixers/ClientUpgradeFixer/ClientUpgradeFixer.php b/src/Fixers/ClientUpgradeFixer/ClientUpgradeFixer.php
@@ -78,9 +78,10 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
             $clientShortNames[$clientClass] = $shortName;
         }
         $clientVars = array_merge(
+            ClientVar::getClientVarsFromConfiguration($this->configuration),
             ClientVar::getClientVarsFromNewKeyword($tokens, $clientShortNames),
             ClientVar::getClientVarsFromVarTypehint($tokens, $clientShortNames),
-            ClientVar::getClientVarsFromConfiguration($this->configuration),
+            ClientVar::getClientVarsFromTypehint($tokens, $clientShortNames),
         );
 
         // Find the RPC methods being called on the clients

diff --git a/src/Fixers/ClientUpgradeFixer/ClientVar.php b/src/Fixers/ClientUpgradeFixer/ClientVar.php
@@ -14,9 +14,12 @@ class ClientVar
 
     public function __construct(
         $varName,
-        $className
+        $className,
+        $parent = null
     ) {
-        if (str_contains($varName, '->')) {
+        if ($parent) {
+            $this->parent = $parent;
+        } elseif (str_contains($varName, '->')) {
             list($this->parent, $varName) = explode('->', $varName);
         } elseif (str_contains($varName, '::')) {
             list($this->parent, $varName) = explode('::', $varName);
@@ -184,6 +187,37 @@ public static function getClientVarsFromVarTypehint(Tokens $tokens, array $clien
         return $clientVars;
     }
 
+    public static function getClientVarsFromTypehint(Tokens $tokens, array $clientShortNames): array
+    {
+        $clientVars = [];
+        foreach ($tokens as $index => $token) {
+            // get variables which are set directly
+            if (!in_array($token->getContent(), $clientShortNames)) {
+                continue;
+            }
+
+            $varToken = $tokens[$tokens->getNextMeaningfulToken($index)];
+            if (!$varToken->isGivenKind(T_VARIABLE)) {
+                continue;
+            }
+
+            $prevToken = $tokens[$tokens->getPrevMeaningfulToken($index)];
+            $varName = $varToken->getContent();
+            $parent = null;
+            if (in_array($prevToken->getContent(), ['protected', 'private', 'public'])) {
+                $parent = '$this';
+                $varName = substr($varName, 1);
+            } elseif ($prevToken->getContent() === 'static') {
+                $parent = 'self';
+            }
+            if ($clientClass = array_search($token->getContent(), $clientShortNames)) {
+                $clientVars[] = new ClientVar($varName, $clientClass, $parent);
+            }
+        }
+
+        return $clientVars;
+    }
+
     public static function getClientVarsFromConfiguration(array $configuration): array
     {
         $clientVars = [];

diff --git a/src/Fixers/ClientUpgradeFixer/examples/var_typehint.new.php b/src/Fixers/ClientUpgradeFixer/examples/var_typehint.new.php
@@ -59,8 +59,9 @@ class VariablesInsideClass extends TestCase
     public function callDlp()
     {
         // These should update
-        $listInfoTypesRequest2 = new ListInfoTypesRequest();
-        $infoTypes = $this->dlp->listInfoTypes($listInfoTypesRequest2);
-        $secrets = $this->secretmanager->listSecrets('this/is/a/parent');
+        $infoTypes = $this->dlp->listInfoTypes();
+        $listSecretsRequest2 = (new ListSecretsRequest())
+            ->setParent('this/is/a/parent');
+        $secrets = $this->secretmanager->listSecrets($listSecretsRequest2);
     }
 }
diff --git a/src/Fixers/ClientUpgradeFixer/examples/vars_defined_elsewhere.legacy.php b/src/Fixers/ClientUpgradeFixer/examples/vars_defined_elsewhere.legacy.php
@@ -11,7 +11,7 @@
 // this should update (from detection)
 $infoTypes = $dlpServiceClient->listInfoTypes();
 // this should also update (from config)
-$secrets = $secretmanager->listSecrets('this/is/a/parent');
+$secrets = $secretmanagerFromConfig->listSecrets('this/is/a/parent');
 
 // these shouldn't update
 $operations = $longrunning->listOperations();
@@ -23,22 +23,24 @@ class MyClass extends SomethingWhichDefinedAClient
 
     public function callTheDlpClient()
     {
-        $this->dlp->listInfoTypes();
-        self::$dlp->listInfoTypes();
+        // These are updated from values in the "clientVars" confguration
+        $this->dlpFromConfig->listInfoTypes();
+        self::$dlpFromConfig->listInfoTypes();
     }
 
     public function callTheDlpClientWithADifferentParent()
     {
         // these should not be updated
-        $this->parent->dlp->listInfoTypes();
-        $this->parent::$dlp->listInfoTypes();
+        $this->parent->dlpFromConfig->listInfoTypes();
+        $this->parent::$dlpFromConfig->listInfoTypes();
     }
 
     public function callSecretManagerWithWildcardParent()
     {
-        $this->secretManagerClient->listSecrets();
-        $this::$secretManagerClient->listSecrets();
-        $this->parent->secretManagerClient->listSecrets();
-        $this->parent::$secretManagerClient->listSecrets();
+        // These are updated from values in the "clientVars" confguration
+        $this->secretManagerClientFromConfig->listSecrets();
+        $this::$secretManagerClientFromConfig->listSecrets();
+        $this->parent->secretManagerClientFromConfig->listSecrets();
+        $this->parent::$secretManagerClientFromConfig->listSecrets();
     }
 }
diff --git a/src/Fixers/ClientUpgradeFixer/examples/vars_defined_elsewhere.new.php b/src/Fixers/ClientUpgradeFixer/examples/vars_defined_elsewhere.new.php
@@ -16,7 +16,7 @@
 // this should also update (from config)
 $listSecretsRequest = (new ListSecretsRequest())
     ->setParent('this/is/a/parent');
-$secrets = $secretmanager->listSecrets($listSecretsRequest);
+$secrets = $secretmanagerFromConfig->listSecrets($listSecretsRequest);
 
 // these shouldn't update
 $operations = $longrunning->listOperations();
@@ -28,28 +28,30 @@ class MyClass extends SomethingWhichDefinedAClient
 
     public function callTheDlpClient()
     {
+        // These are updated from values in the "clientVars" confguration
         $listInfoTypesRequest2 = new ListInfoTypesRequest();
-        $this->dlp->listInfoTypes($listInfoTypesRequest2);
+        $this->dlpFromConfig->listInfoTypes($listInfoTypesRequest2);
         $listInfoTypesRequest3 = new ListInfoTypesRequest();
-        self::$dlp->listInfoTypes($listInfoTypesRequest3);
+        self::$dlpFromConfig->listInfoTypes($listInfoTypesRequest3);
     }
 
     public function callTheDlpClientWithADifferentParent()
     {
         // these should not be updated
-        $this->parent->dlp->listInfoTypes();
-        $this->parent::$dlp->listInfoTypes();
+        $this->parent->dlpFromConfig->listInfoTypes();
+        $this->parent::$dlpFromConfig->listInfoTypes();
     }
 
     public function callSecretManagerWithWildcardParent()
     {
+        // These are updated from values in the "clientVars" confguration
         $listSecretsRequest2 = new ListSecretsRequest();
-        $this->secretManagerClient->listSecrets($listSecretsRequest2);
+        $this->secretManagerClientFromConfig->listSecrets($listSecretsRequest2);
         $listSecretsRequest3 = new ListSecretsRequest();
-        $this::$secretManagerClient->listSecrets($listSecretsRequest3);
+        $this::$secretManagerClientFromConfig->listSecrets($listSecretsRequest3);
         $listSecretsRequest4 = new ListSecretsRequest();
-        $this->parent->secretManagerClient->listSecrets($listSecretsRequest4);
+        $this->parent->secretManagerClientFromConfig->listSecrets($listSecretsRequest4);
         $listSecretsRequest5 = new ListSecretsRequest();
-        $this->parent::$secretManagerClient->listSecrets($listSecretsRequest5);
+        $this->parent::$secretManagerClientFromConfig->listSecrets($listSecretsRequest5);
     }
 }
diff --git a/src/Fixers/ClientUpgradeFixer/examples/vars_in_constructor.legacy.php b/src/Fixers/ClientUpgradeFixer/examples/vars_in_constructor.legacy.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace Google\Cloud\Samples\Dlp;
+
+// new client surface exists
+use Google\Cloud\Dlp\V2\DlpServiceClient;
+use Google\Cloud\SecretManager\V1\SecretManagerServiceClient;
+
+class ClientWrapper extends TestCase
+{
+    private static DlpServiceClient $staticDlp;
+
+    public function __construct(
+        private DlpServiceClient $dlp,
+        private SecretManagerServiceClient $secretmanager
+    ) {
+    }
+
+    public function callDlp()
+    {
+        $infoTypes = $this->dlp->listInfoTypes();
+    }
+
+    public function callSecretManager()
+    {
+        $secrets = $this->secretmanager->listSecrets('this/is/a/parent');
+    }
+
+    public function callStatic()
+    {
+        // These shouldn't update
+        $secrets = self::$dlp->listInfoTypes();
+        $secrets = self::$secretmanager->listSecrets('this/is/a/parent');
+
+        // This should
+        $secrets = self::$staticDlp->listInfoTypes();
+    }
+}
diff --git a/src/Fixers/ClientUpgradeFixer/examples/vars_in_constructor.new.php b/src/Fixers/ClientUpgradeFixer/examples/vars_in_constructor.new.php
@@ -0,0 +1,44 @@
+<?php
+
+namespace Google\Cloud\Samples\Dlp;
+
+// new client surface exists
+use Google\Cloud\Dlp\V2\Client\DlpServiceClient;
+use Google\Cloud\Dlp\V2\ListInfoTypesRequest;
+use Google\Cloud\SecretManager\V1\Client\SecretManagerServiceClient;
+use Google\Cloud\SecretManager\V1\ListSecretsRequest;
+
+class ClientWrapper extends TestCase
+{
+    private static DlpServiceClient $staticDlp;
+
+    public function __construct(
+        private DlpServiceClient $dlp,
+        private SecretManagerServiceClient $secretmanager
+    ) {
+    }
+
+    public function callDlp()
+    {
+        $listInfoTypesRequest = new ListInfoTypesRequest();
+        $infoTypes = $this->dlp->listInfoTypes($listInfoTypesRequest);
+    }
+
+    public function callSecretManager()
+    {
+        $listSecretsRequest = (new ListSecretsRequest())
+            ->setParent('this/is/a/parent');
+        $secrets = $this->secretmanager->listSecrets($listSecretsRequest);
+    }
+
+    public function callStatic()
+    {
+        // These shouldn't update
+        $secrets = self::$dlp->listInfoTypes();
+        $secrets = self::$secretmanager->listSecrets('this/is/a/parent');
+
+        // This should
+        $listInfoTypesRequest2 = new ListInfoTypesRequest();
+        $secrets = self::$staticDlp->listInfoTypes($listInfoTypesRequest2);
+    }
+}
diff --git a/test/Fixers/ClientUpgradeFixerTest.php b/test/Fixers/ClientUpgradeFixerTest.php
@@ -13,26 +13,16 @@ class ClientUpgradeFixerTest extends TestCase
 
     private const SAMPLES_DIR = __DIR__ . '/../../src/Fixers/ClientUpgradeFixer/examples/';
 
-    public function setUp(): void
-    {
-        $this->fixer = new ClientUpgradeFixer();
-        $this->fixer->configure([
-            'clientVars' => [
-                '$secretmanager' => 'Google\\Cloud\\SecretManager\\V1\\SecretManagerServiceClient',
-                '$dlpClient' => 'Google\\Cloud\\Dlp\\V2\\DlpServiceClient',
-                '$this->dlp' => 'Google\\Cloud\\Dlp\\V2\\DlpServiceClient',
-                'self::$dlp' => 'Google\\Cloud\\Dlp\\V2\\DlpServiceClient',
-                'secretManagerClient' => 'Google\\Cloud\\SecretManager\\V1\\SecretManagerServiceClient',
-                '$secretManagerClient' => 'Google\\Cloud\\SecretManager\\V1\\SecretManagerServiceClient',
-            ]
-        ]);
-    }
-
     /**
      * @dataProvider provideLegacySamples
      */
-    public function testLegacySamples($filename)
+    public function testLegacySamples(string $filename, array $config = [])
     {
+        $this->fixer = new ClientUpgradeFixer();
+        if ($config) {
+            $this->fixer->configure($config);
+        }
+
         $legacyFilepath = self::SAMPLES_DIR . $filename;
         $newFilepath = str_replace('legacy.', 'new.', $legacyFilepath);
         $tokens = Tokens::fromCode(file_get_contents($legacyFilepath));
@@ -53,12 +43,29 @@ public function testLegacySamples($filename)
 
     public static function provideLegacySamples()
     {
-        return array_map(
+        $samples = array_map(
             fn ($file) => [basename($file)],
             array_filter(
                 glob(self::SAMPLES_DIR . '*'),
                 fn ($file) => '.legacy.php' === substr(basename($file), -11)
             )
         );
+        $samples = array_combine(
+            array_map(fn ($file) => substr($file[0], 0, -11), $samples),
+            $samples
+        );
+
+        // add custom config for vars_defined_elsewhere samples
+        $samples['vars_defined_elsewhere'][] = [
+            'clientVars' => [
+                '$secretmanagerFromConfig' => 'Google\\Cloud\\SecretManager\\V1\\SecretManagerServiceClient',
+                '$this->dlpFromConfig' => 'Google\\Cloud\\Dlp\\V2\\DlpServiceClient',
+                'self::$dlpFromConfig' => 'Google\\Cloud\\Dlp\\V2\\DlpServiceClient',
+                'secretManagerClientFromConfig' => 'Google\\Cloud\\SecretManager\\V1\\SecretManagerServiceClient',
+                '$secretManagerClientFromConfig' => 'Google\\Cloud\\SecretManager\\V1\\SecretManagerServiceClient',
+            ]
+        ];
+
+        return $samples;
     }
 }