Merge pull request #208 from PHPCSStandards/universal/207-constructordestructorreturn

ConstructorDestructorReturn: respect a potentially set `php_version` config value

Author: jrfnl

Universal/Sniffs/CodeAnalysis/ConstructorDestructorReturnSniff.php

@@ -14,6 +14,7 @@
 use PHP_CodeSniffer\Sniffs\Sniff;
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\BackCompat\BCFile;
+use PHPCSUtils\BackCompat\Helper;
 use PHPCSUtils\Tokens\Collections;
 use PHPCSUtils\Utils\FunctionDeclarations;
 use PHPCSUtils\Utils\GetTokensAsString;
@@ -67,7 +68,12 @@ public function process(File $phpcsFile, $stackPtr)
         if ($functionNameLC === '__construct' || $functionNameLC === '__destruct') {
             $functionType = \sprintf('A "%s()" magic method', $functionNameLC);
         } else {
-            // This may be a PHP 4-style constructor.
+            // If the PHP version is explicitly set to PHP 8.0 or higher, ignore PHP 4-style constructors.
+            if ((int) Helper::getConfigData('php_version') >= 80000) {
+                return;
+            }
+
+            // This may be a PHP 4-style constructor which should be handled.
             $OOName = ObjectDeclarations::getName($phpcsFile, $scopePtr);
 
             if (empty($OOName) === true) {

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.inc renamed to Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.1.inc

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This test file is run with php_version set to a value of 80000 or higher.
+ *
+ * The PHP4-style constructor should no longer be recognized as a constructor.
+ * No errors should be thrown for it, nor any auto-fixes made.
+ */
+class ReturnsAValue {
+    public function __construct(): self {
+        return $this;
+    }
+
+    public function __destruct():string {
+        return 'destructed';
+    }
+
+    function returnsavalue(): string
+    {
+        return 'php4style';
+    }
+}

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.inc.fixed renamed to Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.1.inc.fixed

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This test file is run with php_version set to a value of 80000 or higher.
+ *
+ * The PHP4-style constructor should no longer be recognized as a constructor.
+ * No errors should be thrown for it, nor any auto-fixes made.
+ */
+class ReturnsAValue {
+    public function __construct() {
+        return $this;
+    }
+
+    public function __destruct() {
+        return 'destructed';
+    }
+
+    function returnsavalue(): string
+    {
+        return 'php4style';
+    }
+}

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.2.inc

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This test file is run with php_version set to a value of 70999 or lower.
+ *
+ * The PHP4-style constructor should be recognized as a constructor and handled as usual.
+ */
+
+class ReturnsAValue {
+    public function __construct(): self {
+        return $this;
+    }
+
+    public function __destruct():string {
+        return 'destructed';
+    }
+
+    function returnsavalue(): string
+    {
+        return 'php4style';
+    }
+}

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.2.inc.fixed

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This test file is run with php_version set to a value of 70999 or lower.
+ *
+ * The PHP4-style constructor should be recognized as a constructor and handled as usual.
+ */
+
+class ReturnsAValue {
+    public function __construct() {
+        return $this;
+    }
+
+    public function __destruct() {
+        return 'destructed';
+    }
+
+    function returnsavalue()
+    {
+        return 'php4style';
+    }
+}

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.3.inc

@@ -0,0 +1,14 @@
+<?php
+
+/*
+ * This test file is only here to reset the value of the php_version configuration option.
+ *
+ * The PHP4-style constructor should be recognized as a constructor and handled as usual.
+ */
+
+class ReturnsAValue {
+    function returnsavalue(): string
+    {
+        return 'php4style';
+    }
+}

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.3.inc.fixed

@@ -0,0 +1,14 @@
+<?php
+
+/*
+ * This test file is only here to reset the value of the php_version configuration option.
+ *
+ * The PHP4-style constructor should be recognized as a constructor and handled as usual.
+ */
+
+class ReturnsAValue {
+    function returnsavalue()
+    {
+        return 'php4style';
+    }
+}

Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.4.inc

@@ -11,6 +11,7 @@
 namespace PHPCSExtra\Universal\Tests\CodeAnalysis;
 
 use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+use PHPCSUtils\BackCompat\Helper;
 
 /**
  * Unit test class for the ConstructorDestructorReturn sniff.
@@ -22,37 +23,114 @@
 final class ConstructorDestructorReturnUnitTest extends AbstractSniffUnitTest
 {
 
+    /**
+     * Set CLI values before the file is tested.
+     *
+     * @param string                  $testFile The name of the file being tested.
+     * @param \PHP_CodeSniffer\Config $config   The config data for the test run.
+     *
+     * @return void
+     */
+    public function setCliValues($testFile, $config)
+    {
+        switch ($testFile) {
+            case 'ConstructorDestructorReturnUnitTest.2.inc':
+                Helper::setConfigData('php_version', '80025', true, $config); // PHP 8.0.25.
+                break;
+
+            case 'ConstructorDestructorReturnUnitTest.3.inc':
+                Helper::setConfigData('php_version', '70313', true, $config); // PHP 7.3.13.
+                break;
+
+            default:
+                Helper::setConfigData('php_version', null, true, $config); // No PHP version set.
+                break;
+        }
+    }
+
     /**
      * Returns the lines where errors should occur.
      *
+     * @param string $testFile The name of the file being tested.
+     *
      * @return array <int line number> => <int number of errors>
      */
-    public function getErrorList()
+    public function getErrorList($testFile = '')
     {
-        return [
-            85  => 1,
-            89  => 1,
-            101 => 1,
-            116 => 1,
-            118 => 1,
-            122 => 1,
-            124 => 1,
-        ];
+        switch ($testFile) {
+            case 'ConstructorDestructorReturnUnitTest.1.inc':
+                return [
+                    85  => 1,
+                    89  => 1,
+                    101 => 1,
+                    116 => 1,
+                    118 => 1,
+                    122 => 1,
+                    124 => 1,
+                ];
+
+            case 'ConstructorDestructorReturnUnitTest.2.inc':
+                return [
+                    10 => 1,
+                    14 => 1,
+                ];
+
+            case 'ConstructorDestructorReturnUnitTest.3.inc':
+                return [
+                    10 => 1,
+                    14 => 1,
+                    18 => 1,
+                ];
+
+            case 'ConstructorDestructorReturnUnitTest.4.inc':
+                return [
+                    10 => 1,
+                ];
+
+            default:
+                return [];
+        }
     }
 
     /**
      * Returns the lines where warnings should occur.
      *
+     * @param string $testFile The name of the file being tested.
+     *
      * @return array <int line number> => <int number of warnings>
      */
-    public function getWarningList()
+    public function getWarningList($testFile = '')
     {
-        return [
-            86  => 1,
-            90  => 1,
-            95  => 1,
-            103 => 1,
-            107 => 1,
-        ];
+        switch ($testFile) {
+            case 'ConstructorDestructorReturnUnitTest.1.inc':
+                return [
+                    86  => 1,
+                    90  => 1,
+                    95  => 1,
+                    103 => 1,
+                    107 => 1,
+                ];
+
+            case 'ConstructorDestructorReturnUnitTest.2.inc':
+                return [
+                    11 => 1,
+                    15 => 1,
+                ];
+
+            case 'ConstructorDestructorReturnUnitTest.3.inc':
+                return [
+                    11 => 1,
+                    15 => 1,
+                    20 => 1,
+                ];
+
+            case 'ConstructorDestructorReturnUnitTest.4.inc':
+                return [
+                    12 => 1,
+                ];
+
+            default:
+                return [];
+        }
     }
 }