From 4ded1080e3655149353f2273abfcbb310b8d0160 Mon Sep 17 00:00:00 2001 From: orklah Date: Mon, 30 Nov 2020 03:40:13 +0100 Subject: [PATCH] Check from_docblock property to emit the right issue (#4736) --- .../Type/SimpleNegatedAssertionReconciler.php | 88 ++++++++++++------- tests/AssertAnnotationTest.php | 2 +- tests/FunctionCallTest.php | 2 +- 3 files changed, 57 insertions(+), 35 deletions(-) diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php index b1b766dc8..b620183e9 100644 --- a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php @@ -2,6 +2,7 @@ namespace Psalm\Internal\Type; +use Psalm\Issue\RedundantConditionGivenDocblockType; use function get_class; use Psalm\CodeLocation; use Psalm\Issue\ParadoxicalCondition; @@ -637,20 +638,27 @@ class SimpleNegatedAssertionReconciler extends Reconciler $existing_var_type->addType(new Type\Atomic\TNonEmptyMixed); } } elseif ($existing_var_type->isMixed() && !$is_equality) { - if ($code_location - && $key - && IssueBuffer::accepts( - new RedundantCondition( + if ($code_location && $key) { + if ($existing_var_type->from_docblock) { + $issue = new RedundantConditionGivenDocblockType( 'Found a redundant condition when evaluating ' . $key - . ' of type ' . $existing_var_type->getId() - . ' and trying to reconcile it with a non-' . $assertion . ' assertion', + . ' of type ' . $existing_var_type->getId() + . ' and trying to reconcile it with a non-' . $assertion . ' assertion', $code_location, $existing_var_type->getId() . ' ' . $assertion - ), - $suppressed_issues - ) - ) { - // fall through + ); + } else { + $issue = new RedundantCondition( + 'Found a redundant condition when evaluating ' . $key + . ' of type ' . $existing_var_type->getId() + . ' and trying to reconcile it with a non-' . $assertion . ' assertion', + $code_location, + $existing_var_type->getId() . ' ' . $assertion + ); + } + if (IssueBuffer::accepts($issue, $suppressed_issues)) { + // fall through + } } } @@ -668,20 +676,27 @@ class SimpleNegatedAssertionReconciler extends Reconciler $existing_var_type->addType(new Type\Atomic\TNonEmptyScalar); } } elseif ($existing_var_type->isSingle() && !$is_equality) { - if ($code_location - && $key - && IssueBuffer::accepts( - new RedundantCondition( + if ($code_location && $key) { + if ($existing_var_type->from_docblock) { + $issue = new RedundantConditionGivenDocblockType( 'Found a redundant condition when evaluating ' . $key - . ' of type ' . $existing_var_type->getId() - . ' and trying to reconcile it with a non-' . $assertion . ' assertion', + . ' of type ' . $existing_var_type->getId() + . ' and trying to reconcile it with a non-' . $assertion . ' assertion', $code_location, $existing_var_type->getId() . ' ' . $assertion - ), - $suppressed_issues - ) - ) { - // fall through + ); + } else { + $issue = new RedundantCondition( + 'Found a redundant condition when evaluating ' . $key + . ' of type ' . $existing_var_type->getId() + . ' and trying to reconcile it with a non-' . $assertion . ' assertion', + $code_location, + $existing_var_type->getId() . ' ' . $assertion + ); + } + if (IssueBuffer::accepts($issue, $suppressed_issues)) { + // fall through + } } } @@ -705,20 +720,27 @@ class SimpleNegatedAssertionReconciler extends Reconciler $existing_var_type->addType(new Type\Atomic\TNonEmptyString); } } elseif ($existing_var_type->isSingle() && !$is_equality) { - if ($code_location - && $key - && IssueBuffer::accepts( - new RedundantCondition( + if ($code_location && $key) { + if ($existing_var_type->from_docblock) { + $issue = new RedundantConditionGivenDocblockType( 'Found a redundant condition when evaluating ' . $key - . ' of type ' . $existing_var_type->getId() - . ' and trying to reconcile it with a non-' . $assertion . ' assertion', + . ' of type ' . $existing_var_type->getId() + . ' and trying to reconcile it with a non-' . $assertion . ' assertion', $code_location, $existing_var_type->getId() . ' ' . $assertion - ), - $suppressed_issues - ) - ) { - // fall through + ); + } else { + $issue = new RedundantCondition( + 'Found a redundant condition when evaluating ' . $key + . ' of type ' . $existing_var_type->getId() + . ' and trying to reconcile it with a non-' . $assertion . ' assertion', + $code_location, + $existing_var_type->getId() . ' ' . $assertion + ); + } + if (IssueBuffer::accepts($issue, $suppressed_issues)) { + // fall through + } } } diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index 009be9403..9e0b1feed 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -1480,7 +1480,7 @@ class AssertAnnotationTest extends TestCase if ($bar) {} }', - 'error_message' => 'RedundantCondition - src' . DIRECTORY_SEPARATOR . 'somefile.php:19:29', + 'error_message' => 'RedundantConditionGivenDocblockType - src' . DIRECTORY_SEPARATOR . 'somefile.php:19:29', ], 'assertOneOfStrings' => [ ' 'RedundantCondition', + 'error_message' => 'RedundantConditionGivenDocblockType', ], 'strposNoSetFirstParam' => [ '