From 84945a7d1b3df50c452b4e990b6383a8d57ed849 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Fri, 24 Jul 2020 10:08:57 -0400 Subject: [PATCH] Fix #3877 - prevent impossible subtr comparisons --- .../Expression/BinaryOpAnalyzer.php | 82 +++++++++++++++++++ src/psalm-language-server.php | 14 ---- src/psalm-refactor.php | 11 --- src/psalter.php | 11 --- tests/BinaryOperationTest.php | 11 +++ 5 files changed, 93 insertions(+), 36 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php index 0f10014de..86ca13a0d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php @@ -157,6 +157,88 @@ class BinaryOpAnalyzer $stmt_left_type = $statements_analyzer->node_data->getType($stmt->left); $stmt_right_type = $statements_analyzer->node_data->getType($stmt->right); + if (($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\NotEqual + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical) + && $stmt->left instanceof PhpParser\Node\Expr\FuncCall + && $stmt->left->name instanceof PhpParser\Node\Name + && $stmt->left->name->parts === ['substr'] + && isset($stmt->left->args[1]) + && $stmt_right_type + && $stmt_right_type->hasLiteralString() + ) { + $from_type = $statements_analyzer->node_data->getType($stmt->left->args[1]->value); + + $length_type = isset($stmt->left->args[2]) + ? ($statements_analyzer->node_data->getType($stmt->left->args[2]->value) ?: Type::getMixed()) + : null; + + $string_length = null; + + if ($from_type && $from_type->isSingleIntLiteral() && $length_type === null) { + $string_length = -$from_type->getSingleIntLiteral()->value; + } elseif ($length_type && $length_type->isSingleIntLiteral()) { + $string_length = $length_type->getSingleIntLiteral()->value; + } + + if ($string_length > 0) { + foreach ($stmt_right_type->getAtomicTypes() as $atomic_right_type) { + if ($atomic_right_type instanceof Type\Atomic\TLiteralString) { + if (strlen($atomic_right_type->value) !== $string_length) { + if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal + || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical + ) { + if ($atomic_right_type->from_docblock) { + if (IssueBuffer::accepts( + new \Psalm\Issue\DocblockTypeContradiction( + $atomic_right_type . ' string length is not ' . $string_length, + new CodeLocation($statements_analyzer, $stmt) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } else { + if (IssueBuffer::accepts( + new \Psalm\Issue\TypeDoesNotContainType( + $atomic_right_type . ' string length is not ' . $string_length, + new CodeLocation($statements_analyzer, $stmt) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } + } else { + if ($atomic_right_type->from_docblock) { + if (IssueBuffer::accepts( + new \Psalm\Issue\RedundantConditionGivenDocblockType( + $atomic_right_type . ' string length is never ' . $string_length, + new CodeLocation($statements_analyzer, $stmt) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } else { + if (IssueBuffer::accepts( + new \Psalm\Issue\RedundantCondition( + $atomic_right_type . ' string length is never ' . $string_length, + new CodeLocation($statements_analyzer, $stmt) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } + } + } + } + } + } + } + if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal && $stmt_left_type && $stmt_right_type diff --git a/src/psalm-language-server.php b/src/psalm-language-server.php index 7fd25256e..00b9aaa6b 100644 --- a/src/psalm-language-server.php +++ b/src/psalm-language-server.php @@ -93,20 +93,6 @@ array_map( error_log('Bad argument'); exit(1); } - } elseif (substr($arg, 0, 2) === '-' && $arg !== '-' && $arg !== '--') { - $arg_name = preg_replace('/=.*$/', '', substr($arg, 1)); - - if (!in_array($arg_name, $valid_short_options, true) - && !in_array($arg_name . ':', $valid_short_options, true) - ) { - fwrite( - STDERR, - 'Unrecognised argument "-' . $arg_name . '"' . PHP_EOL - . 'Type --help to see a list of supported arguments' . PHP_EOL - ); - error_log('Bad argument'); - exit(1); - } } }, $args diff --git a/src/psalm-refactor.php b/src/psalm-refactor.php index 164715705..f071ef4c5 100644 --- a/src/psalm-refactor.php +++ b/src/psalm-refactor.php @@ -84,17 +84,6 @@ array_map( ); exit(1); } - } elseif (substr($arg, 0, 2) === '-' && $arg !== '-' && $arg !== '--') { - $arg_name = preg_replace('/=.*$/', '', substr($arg, 1)); - - if (!in_array($arg_name, $valid_short_options) && !in_array($arg_name . ':', $valid_short_options)) { - fwrite( - STDERR, - 'Unrecognised argument "-' . $arg_name . '"' . PHP_EOL - . 'Type --help to see a list of supported arguments'. PHP_EOL - ); - exit(1); - } } }, $args diff --git a/src/psalter.php b/src/psalter.php index 911572f3d..7f12f5732 100644 --- a/src/psalter.php +++ b/src/psalter.php @@ -106,17 +106,6 @@ array_map( ); exit(1); } - } elseif (substr($arg, 0, 2) === '-' && $arg !== '-' && $arg !== '--') { - $arg_name = preg_replace('/=.*$/', '', substr($arg, 1)); - - if (!in_array($arg_name, $valid_short_options) && !in_array($arg_name . ':', $valid_short_options)) { - fwrite( - STDERR, - 'Unrecognised argument "-' . $arg_name . '"' . PHP_EOL - . 'Type --help to see a list of supported arguments'. PHP_EOL - ); - exit(1); - } } }, $args diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index 9fd37a687..57d1f3bc9 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -358,6 +358,17 @@ class BinaryOperationTest extends TestCase $a = ~true;', 'error_message' => 'InvalidOperand', ], + 'substrImpossible' => [ + ' 'TypeDoesNotContainType', + ], ]; } }