Merge pull request #205 from PHPCSStandards/develop

Release PHPCSExtra 1.0.1
PHPCSStandards · Jan 5, 2023 · 0f55c12 · 0f55c12
2 parents 5ab4e9e + a1ff522
commit 0f55c12
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 2 deletions.
diff --git a/.github/release-checklist.md b/.github/release-checklist.md
@@ -16,7 +16,7 @@ PR for tracking changes for the x.x.x release. Target release date: **DOW MONTH
 - [ ] Merge this PR
 - [ ] Make sure all CI builds are green.
 - [ ] Tag and create a release (careful, GH defaults to `develop`!) & copy & paste the changelog to it.
-    :pencil2: Don't forget to copy the link collection from the bottom of the changelog!
+    :pencil2: Check if anything from the link collection at the bottom of the changelog needs to be copied in!
 - [ ] Make sure all CI builds are green.
 - [ ] Close the milestone
 - [ ] Open a new milestone for the next release

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,18 @@ This projects adheres to [Keep a CHANGELOG](http://keepachangelog.com/) and uses
 _Nothing yet._
 
 
+## [1.0.1] - 2023-01-05
+
+### Fixed
+
+#### Universal
+
+* `Universal.CodeAnalysis.ConstructorDestructorReturn`: fixed false positive for return statements in nested functions/closures declared within constructor/destructor methods. Thanks [@anomiex] for reporting! [#201], [#202]
+
+[#201]: https://github.com/PHPCSStandards/PHPCSExtra/issues/201
+[#202]: https://github.com/PHPCSStandards/PHPCSExtra/pull/202
+
+
 ## [1.0.0] - 2023-01-04
 
 :warning: Important: this package now requires [PHPCSUtils 1.0.0]. Please make sure you use `--with-[all-]dependencies` when running `composer update`. :exclamation:
@@ -375,10 +387,12 @@ This initial alpha release contains the following sniffs:
 [Composer PHPCS plugin]: https://github.com/PHPCSStandards/composer-installer
 
 [Unreleased]: https://github.com/PHPCSStandards/PHPCSExtra/compare/stable...HEAD
+[1.0.1]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.0...1.0.1
 [1.0.0]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.0-rc1...1.0.0
 [1.0.0-RC1]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.0-alpha3...1.0.0-rc1
 [1.0.0-alpha3]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.0-alpha2...1.0.0-alpha3
 [1.0.0-alpha2]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.0-alpha1...1.0.0-alpha2
 
+[@anomiex]:   https://github.com/anomiex
 [@derickr]:   https://github.com/derickr
 [@GaryJones]: https://github.com/GaryJones
diff --git a/Universal/Sniffs/CodeAnalysis/ConstructorDestructorReturnSniff.php b/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\Tokens\Collections;
 use PHPCSUtils\Utils\FunctionDeclarations;
 use PHPCSUtils\Utils\GetTokensAsString;
 use PHPCSUtils\Utils\NamingConventions;
@@ -128,12 +129,24 @@ public function process(File $phpcsFile, $stackPtr)
         $current = $tokens[$stackPtr]['scope_opener'];
         $end     = $tokens[$stackPtr]['scope_closer'];
 
+        // Not searching for arrow functions as those have an implicit return, so no
+        $search            = Collections::functionDeclarationTokens();
+        $search[\T_RETURN] = \T_RETURN;
+
         do {
-            $current = $phpcsFile->findNext(\T_RETURN, ($current + 1), $end);
+            $current = $phpcsFile->findNext($search, ($current + 1), $end);
             if ($current === false) {
                 break;
             }
 
+            if (isset(Collections::functionDeclarationTokens()[$tokens[$current]['code']])
+                && isset($tokens[$current]['scope_closer'])
+            ) {
+                // Skip over nested function/closure declarations.
+                $current = $tokens[$current]['scope_closer'];
+                continue;
+            }
+
             $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($current + 1), $end, true);
             if ($next === false
                 || $tokens[$next]['code'] === \T_SEMICOLON

diff --git a/Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.inc b/Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.inc
@@ -123,3 +123,34 @@ interface InterfaceMethodReturnTypes {
 
     public function __destruct(): void;
 }
+
+// Issue #201 - prevent false positives for nested functions/closures.
+class NestedFunctions {
+    public function __construct() {
+        function global_function() {
+            return true;
+        }
+    }
+
+    public function __construct() {
+        function global_function($foo) {
+            return $foo;
+        }
+    }
+}
+
+class NestedClosures {
+    public function __construct() {
+        do_something(
+            function () {
+                return true;
+            },
+        );
+    }
+
+    public function __construct() {
+        $this->callback = function () {
+            return 10;
+        };
+    }
+}
diff --git a/Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.inc.fixed b/Universal/Tests/CodeAnalysis/ConstructorDestructorReturnUnitTest.inc.fixed
@@ -122,3 +122,34 @@ interface InterfaceMethodReturnTypes {
 
     public function __destruct();
 }
+
+// Issue #201 - prevent false positives for nested functions/closures.
+class NestedFunctions {
+    public function __construct() {
+        function global_function() {
+            return true;
+        }
+    }
+
+    public function __construct() {
+        function global_function($foo) {
+            return $foo;
+        }
+    }
+}
+
+class NestedClosures {
+    public function __construct() {
+        do_something(
+            function () {
+                return true;
+            },
+        );
+    }
+
+    public function __construct() {
+        $this->callback = function () {
+            return 10;
+        };
+    }
+}