Merge pull request #8916 from jack-worman/Remove_useless_ifs

Remove useless ifs
vimeo · Dec 17, 2022 · a76a1d1 · a76a1d1
2 parents 62ced63 + 2f677f6
commit a76a1d1
Show file tree

Hide file tree

Showing 40 changed files with 422 additions and 560 deletions.
diff --git a/examples/plugins/FunctionCasingChecker.php b/examples/plugins/FunctionCasingChecker.php
@@ -1,10 +1,9 @@
 <?php
+
 namespace Psalm\Example\Plugin;
 
 use Exception;
 use PhpParser;
-use Psalm\Checker;
-use Psalm\Checker\StatementsChecker;
 use Psalm\CodeLocation;
 use Psalm\Issue\PluginIssue;
 use Psalm\IssueBuffer;
@@ -48,15 +47,13 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
             }
 
             if ($function_storage->cased_name !== (string)$expr->name) {
-                if (IssueBuffer::accepts(
+                IssueBuffer::maybeAdd(
                     new IncorrectFunctionCasing(
                         'Function is incorrectly cased, expecting ' . $function_storage->cased_name,
                         new CodeLocation($statements_source, $expr->name)
                     ),
                     $statements_source->getSuppressedIssues()
-                )) {
-                    // fall through
-                }
+                );
             }
         } catch (Exception $e) {
             // can throw if storage is missing
@@ -88,15 +85,13 @@ public static function afterFunctionCallAnalysis(AfterFunctionCallAnalysisEvent
             $function_name_parts = explode('\\', $function_storage->cased_name);
 
             if (end($function_name_parts) !== end($expr->name->parts)) {
-                if (IssueBuffer::accepts(
+                IssueBuffer::maybeAdd(
                     new IncorrectFunctionCasing(
                         'Function is incorrectly cased, expecting ' . $function_storage->cased_name,
                         new CodeLocation($statements_source, $expr->name)
                     ),
                     $statements_source->getSuppressedIssues()
-                )) {
-                    // fall through
-                }
+                );
             }
         } catch (Exception $e) {
             // can throw if storage is missing

diff --git a/examples/plugins/PreventFloatAssignmentChecker.php b/examples/plugins/PreventFloatAssignmentChecker.php
@@ -1,8 +1,8 @@
 <?php
+
 namespace Psalm\Example\Plugin;
 
 use PhpParser;
-use Psalm\Checker;
 use Psalm\CodeLocation;
 use Psalm\Issue\PluginIssue;
 use Psalm\IssueBuffer;
@@ -17,7 +17,7 @@ class PreventFloatAssignmentChecker implements AfterExpressionAnalysisInterface
     /**
      * Called after an expression has been checked
      *
-     * @return null|false
+     * @return null
      */
     public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $event): ?bool {
         $expr = $event->getExpr();
@@ -26,15 +26,13 @@ public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $eve
             && ($expr_type = $statements_source->getNodeTypeProvider()->getType($expr->expr))
             && $expr_type->hasFloat()
         ) {
-            if (IssueBuffer::accepts(
+            IssueBuffer::maybeAdd(
                 new NoFloatAssignment(
                     'Don’t assign to floats',
                     new CodeLocation($statements_source, $expr)
                 ),
                 $statements_source->getSuppressedIssues()
-            )) {
-                // fall through
-            }
+            );
         }
 
         return null;

diff --git a/examples/plugins/StringChecker.php b/examples/plugins/StringChecker.php
@@ -30,17 +30,14 @@ public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $eve
                 && preg_match($class_or_class_method, $expr->value)
             ) {
                 $absolute_class = preg_split('/[:]/', $expr->value)[0];
-
-                if (IssueBuffer::accepts(
+                IssueBuffer::maybeAdd(
                     new InvalidClass(
                         'Use ::class constants when representing class names',
                         new CodeLocation($statements_source, $expr),
                         $absolute_class
                     ),
                     $statements_source->getSuppressedIssues()
-                )) {
-                    // fall through
-                }
+                );
             }
         } elseif ($expr instanceof PhpParser\Node\Expr\BinaryOp\Concat
             && $expr->left instanceof PhpParser\Node\Expr\ClassConstFetch

diff --git a/examples/plugins/composer-based/echo-checker/EchoChecker.php b/examples/plugins/composer-based/echo-checker/EchoChecker.php
@@ -27,17 +27,14 @@ public static function afterStatementAnalysis(AfterStatementAnalysisEvent $event
                 $expr_type = $statements_source->getNodeTypeProvider()->getType($expr);
 
                 if (!$expr_type || $expr_type->hasMixed()) {
-                    if (IssueBuffer::accepts(
+                    IssueBuffer::maybeAdd(
                         new ArgumentTypeCoercion(
                             'Echo requires an unescaped string, ' . $expr_type . ' provided',
                             new CodeLocation($statements_source, $expr),
                             'echo'
                         ),
                         $statements_source->getSuppressedIssues()
-                    )) {
-                        // keep soldiering on
-                    }
-
+                    );
                     continue;
                 }
 
@@ -47,16 +44,14 @@ public static function afterStatementAnalysis(AfterStatementAnalysisEvent $event
                     if ($type instanceof TString
                         && !$type instanceof TLiteralString
                     ) {
-                        if (IssueBuffer::accepts(
+                        IssueBuffer::maybeAdd(
                             new ArgumentTypeCoercion(
                                 'Echo requires an unescaped string, ' . $expr_type . ' provided',
                                 new CodeLocation($statements_source, $expr),
                                 'echo'
                             ),
                             $statements_source->getSuppressedIssues()
-                        )) {
-                            // keep soldiering on
-                        }
+                        );
                     }
                 }
             }

diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php
@@ -447,13 +447,11 @@ public function analyze(
             } elseif ($stmt instanceof PhpParser\Node\Stmt\Property) {
                 foreach ($stmt->props as $prop) {
                     if ($storage->is_enum) {
-                        if (IssueBuffer::accepts(new NoEnumProperties(
+                        IssueBuffer::maybeAdd(new NoEnumProperties(
                             'Enums cannot have properties',
                             new CodeLocation($this, $prop),
                             $fq_class_name
-                        ))) {
-                            // fall through
-                        }
+                        ));
                         continue;
                     }
                     if ($prop->default) {
@@ -2421,48 +2419,44 @@ private function checkEnum(): void
         $seen_values = [];
         foreach ($storage->enum_cases as $case_storage) {
             if ($case_storage->value !== null && $storage->enum_type === null) {
-                if (IssueBuffer::accepts(
+                IssueBuffer::maybeAdd(
                     new InvalidEnumCaseValue(
                         'Case of a non-backed enum should not have a value',
                         $case_storage->stmt_location,
                         $storage->name
                     )
-                )) {
-                }
+                );
             } elseif ($case_storage->value === null && $storage->enum_type !== null) {
-                if (IssueBuffer::accepts(
+                IssueBuffer::maybeAdd(
                     new InvalidEnumCaseValue(
                         'Case of a backed enum should have a value',
                         $case_storage->stmt_location,
                         $storage->name
                     )
-                )) {
-                }
+                );
             } elseif ($case_storage->value !== null && $storage->enum_type !== null) {
                 if ((is_int($case_storage->value) && $storage->enum_type === 'string')
                     || (is_string($case_storage->value) && $storage->enum_type === 'int')
                 ) {
-                    if (IssueBuffer::accepts(
+                    IssueBuffer::maybeAdd(
                         new InvalidEnumCaseValue(
                             'Enum case value type should be ' . $storage->enum_type,
                             $case_storage->stmt_location,
                             $storage->name
                         )
-                    )) {
-                    }
+                    );
                 }
             }
 
             if ($case_storage->value !== null) {
                 if (in_array($case_storage->value, $seen_values, true)) {
-                    if (IssueBuffer::accepts(
+                    IssueBuffer::maybeAdd(
                         new DuplicateEnumCaseValue(
                             'Enum case values should be unique',
                             $case_storage->stmt_location,
                             $storage->name
                         )
-                    )) {
-                    }
+                    );
                 } else {
                     $seen_values[] = $case_storage->value;
                 }

diff --git a/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php
@@ -583,27 +583,27 @@ public static function checkPropertyVisibility(
                 return $emit_issues ? null : true;
 
             case self::VISIBILITY_PRIVATE:
-                if ($emit_issues && IssueBuffer::accepts(
-                    new InaccessibleProperty(
-                        'Cannot access private property ' . $property_id . ' from context ' . $context->self,
-                        $code_location
-                    ),
-                    $suppressed_issues
-                )) {
-                    // fall through
+                if ($emit_issues) {
+                    IssueBuffer::maybeAdd(
+                        new InaccessibleProperty(
+                            'Cannot access private property ' . $property_id . ' from context ' . $context->self,
+                            $code_location
+                        ),
+                        $suppressed_issues
+                    );
                 }
 
                 return null;
             case self::VISIBILITY_PROTECTED:
                 if (!$context->self) {
-                    if ($emit_issues && IssueBuffer::accepts(
-                        new InaccessibleProperty(
-                            'Cannot access protected property ' . $property_id,
-                            $code_location
-                        ),
-                        $suppressed_issues
-                    )) {
-                        // fall through
+                    if ($emit_issues) {
+                        IssueBuffer::maybeAdd(
+                            new InaccessibleProperty(
+                                'Cannot access protected property ' . $property_id,
+                                $code_location
+                            ),
+                            $suppressed_issues
+                        );
                     }
 
                     return null;
@@ -614,14 +614,14 @@ public static function checkPropertyVisibility(
                 }
 
                 if (!$codebase->classExtends($context->self, $appearing_property_class)) {
-                    if ($emit_issues && IssueBuffer::accepts(
-                        new InaccessibleProperty(
-                            'Cannot access protected property ' . $property_id . ' from context ' . $context->self,
-                            $code_location
-                        ),
-                        $suppressed_issues
-                    )) {
-                        // fall through
+                    if ($emit_issues) {
+                        IssueBuffer::maybeAdd(
+                            new InaccessibleProperty(
+                                'Cannot access protected property ' . $property_id . ' from context ' . $context->self,
+                                $code_location
+                            ),
+                            $suppressed_issues
+                        );
                     }
 
                     return null;

diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -361,13 +361,12 @@ public function analyze(
 
         if ($storage->unused_docblock_params) {
             foreach ($storage->unused_docblock_params as $param_name => $param_location) {
-                if (IssueBuffer::accepts(
+                IssueBuffer::maybeAdd(
                     new InvalidDocblockParamName(
                         'Incorrect param name $' . $param_name . ' in docblock for ' . $cased_method_id,
                         $param_location
                     )
-                )) {
-                }
+                );
             }
         }
 
@@ -388,7 +387,7 @@ public function analyze(
                 if ($overridden_storage->allow_named_arg_calls) {
                     IssueBuffer::maybeAdd(new MethodSignatureMismatch(
                         'Method ' . (string) $method_id . ' should accept named arguments '
-                        . ' as ' . (string) $overridden_method_id . ' does',
+                            . ' as ' . (string) $overridden_method_id . ' does',
                         $storage->location
                     ));
                 }

diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php
@@ -99,13 +99,13 @@ public static function checkStatic(
         CodeLocation $code_location,
         array $suppressed_issues,
         ?bool &$is_dynamic_this_method = false
-    ): bool {
+    ): void {
         $codebase_methods = $codebase->methods;
 
         if ($method_id->fq_class_name === 'Closure'
             && $method_id->method_name === 'fromcallable'
         ) {
-            return true;
+            return;
         }
 
         $original_method_id = $method_id;
@@ -114,7 +114,7 @@ public static function checkStatic(
 
         if (!$method_id) {
             if (InternalCallMapHandler::inCallMap((string) $original_method_id)) {
-                return true;
+                return;
             }
 
             throw new LogicException('Declaring method for ' . $original_method_id . ' should not be null');
@@ -134,7 +134,7 @@ public static function checkStatic(
                         ),
                         $suppressed_issues
                     )) {
-                        return false;
+                        return;
                     }
                 } else {
                     $is_dynamic_this_method = true;
@@ -149,12 +149,10 @@ public static function checkStatic(
                     ),
                     $suppressed_issues
                 )) {
-                    return false;
+                    return;
                 }
             }
         }
-
-        return true;
     }
 
     /**