From bbde2d623940da533e8d38bce2f72ca3bb5c7756 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Fri, 30 Aug 2019 17:26:55 -0400 Subject: [PATCH] Add support for @psalm-external-mutation-free --- .../Expression/Call/FunctionCallAnalyzer.php | 4 +- .../Expression/Call/MethodCallAnalyzer.php | 18 +++++++- .../Expression/Call/NewAnalyzer.php | 5 ++ .../Visitor/CheckTrivialExprVisitor.php | 10 +++- .../UnusedVariableManipulationTest.php | 43 +++++++++++++++++ tests/UnusedCodeTest.php | 46 +++++++++++++++++++ 6 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index 3ec2a935d5a..07da8e5831f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -619,7 +619,7 @@ public static function analyze( ); } - if ($context->mutation_free || $codebase->find_unused_variables) { + if ($context->mutation_free || $context->external_mutation_free || $codebase->find_unused_variables) { $callmap_function_pure = $function_id && $in_call_map ? $codebase->functions->isCallMapFunctionPure($codebase, $function_id, $stmt->args) : null; @@ -628,7 +628,7 @@ public static function analyze( && !$function_storage->pure) || ($callmap_function_pure === false) ) { - if ($context->mutation_free) { + if ($context->mutation_free || $context->external_mutation_free) { if (IssueBuffer::accepts( new ImpureFunctionCall( 'Cannot call an impure function from a mutation-free context', diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php index f202cdc5b6c..dcfa0370bc6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php @@ -1226,7 +1226,23 @@ function (PhpParser\Node\Arg $arg) { )) { // fall through } - } elseif ($method_storage->mutation_free + } elseif ($context->external_mutation_free + && !$method_storage->mutation_free + && $fq_class_name !== $context->self + ) { + if (IssueBuffer::accepts( + new ImpureMethodCall( + 'Cannot call an possibly-mutating method ' + . $method_id . ' from a mutation-free context', + new CodeLocation($source, $stmt->name) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } elseif (($method_storage->mutation_free + || ($method_storage->external_mutation_free + && isset($stmt->var->external_mutation_free))) && $codebase->find_unused_variables && !$context->inside_conditional && !$context->inside_unset diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index 96efd59d536..d898ab782bf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -306,6 +306,11 @@ public static function analyze( ) { $storage = $codebase->classlike_storage_provider->get($fq_class_name); + if ($storage->external_mutation_free) { + /** @psalm-suppress UndefinedPropertyAssignment */ + $stmt->external_mutation_free = true; + } + // if we're not calling this constructor via new static() if ($storage->abstract && !$can_extend) { if (IssueBuffer::accepts( diff --git a/src/Psalm/Internal/Visitor/CheckTrivialExprVisitor.php b/src/Psalm/Internal/Visitor/CheckTrivialExprVisitor.php index fd713f8eda5..c9ce2fc631a 100644 --- a/src/Psalm/Internal/Visitor/CheckTrivialExprVisitor.php +++ b/src/Psalm/Internal/Visitor/CheckTrivialExprVisitor.php @@ -38,7 +38,15 @@ private function checkNonTrivialExpr(PhpParser\Node\Expr $node) || $node instanceof PhpParser\Node\Expr\YieldFrom || $node instanceof PhpParser\Node\Expr\New_ ) { - if ($node instanceof PhpParser\Node\Expr\FuncCall && isset($node->pure)) { + if (($node instanceof PhpParser\Node\Expr\FuncCall + || $node instanceof PhpParser\Node\Expr\MethodCall + || $node instanceof PhpParser\Node\Expr\StaticCall) + && isset($node->pure) + ) { + return false; + } + + if ($node instanceof PhpParser\Node\Expr\New_ && isset($node->external_mutation_free)) { return false; } diff --git a/tests/FileManipulation/UnusedVariableManipulationTest.php b/tests/FileManipulation/UnusedVariableManipulationTest.php index 94430eb9f1e..d15ef2baae4 100644 --- a/tests/FileManipulation/UnusedVariableManipulationTest.php +++ b/tests/FileManipulation/UnusedVariableManipulationTest.php @@ -510,6 +510,49 @@ function foo() : void { ['UnusedVariable'], true, ], + + 'removeLongUnusedAssignment' => [ + 'foo = $foo; + } + + public function setFoo(string $foo) : void { + $this->foo = $foo; + } + } + + function foo() : void { + $a = (new A("hello"))->setFoo("goodbye"); + }', + 'foo = $foo; + } + + public function setFoo(string $foo) : void { + $this->foo = $foo; + } + } + + function foo() : void { + }', + '7.1', + ['UnusedVariable'], + true, + ], ]; } } diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index 4fd509dc63e..c88764f4332 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -552,6 +552,30 @@ public function bar(): void { (new Foo)->bar();' ], + 'usedMethodCallForExternalMutationFreeClass' => [ + 'foo = $foo; + } + + public function setFoo(string $foo) : void { + $this->foo = $foo; + } + + public function getFoo() : string { + return $this->foo; + } + } + + $a = new A("hello"); + $a->setFoo($a->getFoo() . "cool");', + ], ]; } @@ -747,6 +771,28 @@ public function handle(): void (new A())->handle();', 'error_message' => 'UnusedProperty', ], + 'unusedMethodCallForExternalMutationFreeClass' => [ + 'foo = $foo; + } + + public function setFoo(string $foo) : void { + $this->foo = $foo; + } + } + + function foo() : void { + (new A("hello"))->setFoo("goodbye"); + }', + 'error_message' => 'UnusedMethodCall', + ], ]; } }