diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 987aab124ad..e30b49b6d9d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + tags['variablesfrom'][0]]]> @@ -457,8 +457,6 @@ - - @@ -935,7 +933,6 @@ - diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php index 2b7b8b607ae..d5d7e06dce2 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php @@ -2,6 +2,7 @@ namespace Psalm\Internal\Analyzer\Statements\Expression; +use DateTimeInterface; use PhpParser; use Psalm\CodeLocation; use Psalm\Context; @@ -17,6 +18,7 @@ use Psalm\Internal\Codebase\VariableUseGraph; use Psalm\Internal\DataFlow\DataFlowNode; use Psalm\Internal\MethodIdentifier; +use Psalm\Internal\Type\Comparator\UnionTypeComparator; use Psalm\Issue\DocblockTypeContradiction; use Psalm\Issue\ImpureMethodCall; use Psalm\Issue\InvalidOperand; @@ -33,6 +35,7 @@ use UnexpectedValueException; use function in_array; +use function sprintf; use function strlen; /** @@ -47,6 +50,8 @@ public static function analyze( int $nesting = 0, bool $from_stmt = false ): bool { + $codebase = $statements_analyzer->getCodebase(); + if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Concat && $nesting > 100) { $statements_analyzer->node_data->setType($stmt, Type::getString()); @@ -163,7 +168,6 @@ public static function analyze( $new_parent_node->id => $new_parent_node, ]); - $codebase = $statements_analyzer->getCodebase(); $event = new AddRemoveTaintsEvent($stmt, $context, $statements_analyzer, $codebase); $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event); @@ -255,6 +259,41 @@ public static function analyze( ); } + if (($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\NotEqual + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Greater + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\GreaterOrEqual + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Smaller + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\SmallerOrEqual) + && $statements_analyzer->getCodebase()->config->strict_binary_operands + && $stmt_left_type + && $stmt_right_type + && ($stmt_left_type->hasObjectType() || $stmt_right_type->hasObjectType()) + && (!UnionTypeComparator::isContainedBy($codebase, $stmt_left_type, $stmt_right_type) + || !UnionTypeComparator::isContainedBy($codebase, $stmt_right_type, $stmt_left_type)) + // It is okay if both sides implement \DateTimeInterface + && !( + UnionTypeComparator::isContainedBy( + $codebase, + $stmt_left_type, + new Union([new TNamedObject(DateTimeInterface::class)]), + ) + && UnionTypeComparator::isContainedBy( + $codebase, + $stmt_right_type, + new Union([new TNamedObject(DateTimeInterface::class)]), + ) + ) + ) { + IssueBuffer::maybeAdd( + new InvalidOperand( + sprintf('Cannot compare %s to %s.', $stmt_left_type->getId(), $stmt_right_type->getId()), + new CodeLocation($statements_analyzer, $stmt), + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + if (($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal || $stmt instanceof PhpParser\Node\Expr\BinaryOp\NotEqual || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index e2dafc18724..ec680cc231e 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -5,6 +5,7 @@ use Psalm\Config; use Psalm\Context; use Psalm\Exception\CodeException; +use Psalm\Issue\InvalidOperand; use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; @@ -316,6 +317,19 @@ function takesI(I $i): void public function providerValidCodeParse(): iterable { return [ + 'strict_binary_operands: objects of different shape' => [ + 'code' =><<<'PHP' + new \DateTime(); + new \DateTimeImmutable() > new \DateTimeImmutable(); + // special case for DateTimeInterface + new \DateTime() > new \DateTimeImmutable(); + PHP, + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => null, + 'config_options' => ['strict_binary_operands' => true], + ], 'regularAddition' => [ 'code' => ' [ + 'code' =><<<'PHP' + new \DateTimeImmutable(); + PHP, + 'error_message' => InvalidOperand::getIssueType(), + 'error_levels' => [], + 'php_version' => null, + 'config_options' => ['strict_binary_operands' => true], + ], + 'strict_binary_operands: different object shapes' => [ + 'code' =><<<'PHP' + 0, 'b' => 1]) > ((object) ['a' => 0, 'c' => 1]); + PHP, + 'error_message' => InvalidOperand::getIssueType(), + 'error_levels' => [], + 'php_version' => null, + 'config_options' => ['strict_binary_operands' => true], + ], 'badAddition' => [ 'code' => ', - * php_version?: string + * php_version?: string, + * config_options?: psalmConfigOptions, * } * @psalm-type NamedArgumentsDataProviderArrayNotation = array{ * code: string, * error_message: string, * error_levels?: list, - * php_version?: string + * php_version?: string|null, + * config_options?: psalmConfigOptions, * } */ trait InvalidCodeAnalysisTestTrait @@ -43,12 +49,14 @@ abstract public function providerInvalidCodeParse(): iterable; * @dataProvider providerInvalidCodeParse * @small * @param list $error_levels + * @param psalmConfigOptions $config_options */ public function testInvalidCode( string $code, string $error_message, array $error_levels = [], - ?string $php_version = null + ?string $php_version = null, + array $config_options = [] ): void { $test_name = $this->getTestName(); if (strpos($test_name, 'PHP80-') !== false) { @@ -76,18 +84,22 @@ public function testInvalidCode( $code = str_replace("\n", "\r\n", $code); } + $config = Config::getInstance(); foreach ($error_levels as $error_level) { $issue_name = $error_level; $error_level = Config::REPORT_SUPPRESS; - Config::getInstance()->setCustomErrorLevel($issue_name, $error_level); + $config->setCustomErrorLevel($issue_name, $error_level); + } + if (array_key_exists('strict_binary_operands', $config_options)) { + $config->strict_binary_operands = $config_options['strict_binary_operands']; } $this->project_analyzer->setPhpVersion($php_version, 'tests'); $file_path = self::$src_dir_path . 'somefile.php'; - // $error_message = preg_replace('/ src[\/\\\\]somefile\.php/', ' src/somefile.php', $error_message); + // $error_message = (string) preg_replace('/ src[\/\\\\]somefile\.php/', ' src/somefile.php', $error_message); $this->expectException(CodeException::class); diff --git a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php index cfe4dba87b0..a345958010d 100644 --- a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php +++ b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php @@ -5,6 +5,7 @@ use Psalm\Config; use Psalm\Context; +use function array_key_exists; use function str_replace; use function strpos; use function strtoupper; @@ -37,17 +38,22 @@ * This is trait allows testing for the second case, when the value of * "throw_exception" will be "false". * + * @psalm-type psalmConfigOptions = array{ + * strict_binary_operands?: bool, + * } * @psalm-type DeprecatedDataProviderArrayNotation = array{ * code: string, * error_message: string, * ignored_issues?: list, - * php_version?: string + * php_version?: string|null, + * config_options?: psalmConfigOptions, * } * @psalm-type NamedArgumentsDataProviderArrayNotation = array{ * code: string, * error_message: string, * error_levels?: list, - * php_version?: string + * php_version?: string|null, + * config_options?: psalmConfigOptions, * } */ trait InvalidCodeAnalysisWithIssuesTestTrait @@ -64,13 +70,16 @@ abstract public function providerInvalidCodeParse(): iterable; * @dataProvider providerInvalidCodeParse * @small * @param list $error_levels + * @param psalmConfigOptions $config_options */ public function testInvalidCodeWithIssues( string $code, string $error_message, array $error_levels = [], - string $php_version = '7.4' + ?string $php_version = null, + array $config_options = [] ): void { + $php_version ??= '7.4'; $test_name = $this->getTestName(); if (strpos($test_name, 'PHP80-') !== false) { if (version_compare(PHP_VERSION, '8.0.0', '<')) { @@ -89,11 +98,15 @@ public function testInvalidCodeWithIssues( $code = str_replace("\n", "\r\n", $code); } + $config = Config::getInstance(); foreach ($error_levels as $error_level) { $issue_name = $error_level; $error_level = Config::REPORT_SUPPRESS; - Config::getInstance()->setCustomErrorLevel($issue_name, $error_level); + $config->setCustomErrorLevel($issue_name, $error_level); + } + if (array_key_exists('strict_binary_operands', $config_options)) { + $config->strict_binary_operands = $config_options['strict_binary_operands']; } $this->project_analyzer->setPhpVersion($php_version, 'tests'); diff --git a/tests/Traits/ValidCodeAnalysisTestTrait.php b/tests/Traits/ValidCodeAnalysisTestTrait.php index 5be98d38931..84a16ae4f84 100644 --- a/tests/Traits/ValidCodeAnalysisTestTrait.php +++ b/tests/Traits/ValidCodeAnalysisTestTrait.php @@ -5,6 +5,7 @@ use Psalm\Config; use Psalm\Context; +use function array_key_exists; use function str_replace; use function strlen; use function strpos; @@ -14,6 +15,11 @@ use const PHP_OS; use const PHP_VERSION_ID; +/** + * @psalm-type psalmConfigOptions = array{ + * strict_binary_operands?: bool, + * } + */ trait ValidCodeAnalysisTestTrait { /** @@ -23,7 +29,8 @@ trait ValidCodeAnalysisTestTrait * code: string, * assertions?: array, * ignored_issues?: list, - * php_version?: string, + * php_version?: string|null, + * config_options?: psalmConfigOptions, * } * > */ @@ -33,13 +40,15 @@ abstract public function providerValidCodeParse(): iterable; * @dataProvider providerValidCodeParse * @param array $assertions * @param list $ignored_issues + * @param psalmConfigOptions $config_options * @small */ public function testValidCode( string $code, array $assertions = [], array $ignored_issues = [], - ?string $php_version = null + ?string $php_version = null, + array $config_options = [] ): void { $test_name = $this->getTestName(); if (strpos($test_name, 'PHP80-') !== false) { @@ -71,8 +80,12 @@ public function testValidCode( $this->fail('Test case must have a setCustomErrorLevel($issue_name, Config::REPORT_SUPPRESS); + $config->setCustomErrorLevel($issue_name, Config::REPORT_SUPPRESS); + } + if (array_key_exists('strict_binary_operands', $config_options)) { + $config->strict_binary_operands = $config_options['strict_binary_operands']; } if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {