Skip to content

Commit

Permalink
Merge pull request #10750 from weirdan/update-master
Browse files Browse the repository at this point in the history
Update master from 5.x
  • Loading branch information
weirdan committed Feb 26, 2024
2 parents c488d40 + 22056c6 commit b940c7e
Show file tree
Hide file tree
Showing 37 changed files with 1,104 additions and 314 deletions.
4 changes: 2 additions & 2 deletions docs/running_psalm/issues/RiskyTruthyFalsyComparison.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# RiskyTruthyFalsyComparison

Emitted when comparing a value with multiple types that can both contain truthy and falsy values.
Emitted when comparing a value with multiple types, where at least one type can be only truthy or falsy and other types can contain both truthy and falsy values.

```php
<?php
Expand All @@ -22,7 +22,7 @@ function foo($arg) {

## Why this is bad

The truthy/falsy type of one variable is often forgotten and not handled explicitly causing hard to track down errors.
The truthy/falsy type of a variable is often forgotten and not handled explicitly causing hard to track down errors.

## How to fix

Expand Down
13 changes: 1 addition & 12 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="dev-master@4b2c6980c51129b066ae85c73798162865df5142">
<files psalm-version="dev-master@c488d40e6243536a42608bbe37245f07ebebe1ad">
<file src="examples/TemplateChecker.php">
<PossiblyUndefinedIntArrayOffset>
<code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
Expand Down Expand Up @@ -862,8 +862,6 @@
<code><![CDATA[!$context->self]]></code>
<code><![CDATA[!$fq_class_name]]></code>
<code><![CDATA[$class_storage->mixin_declaring_fqcln]]></code>
<code><![CDATA[$class_storage->parent_class]]></code>
<code><![CDATA[$class_storage->parent_class]]></code>
<code><![CDATA[$context->calling_method_id]]></code>
<code><![CDATA[$context->calling_method_id]]></code>
<code><![CDATA[$context->self]]></code>
Expand Down Expand Up @@ -1194,11 +1192,6 @@
<code><![CDATA[$migrated_source_fqcln]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Codebase/ConstantTypeResolver.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[$cond->value]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Codebase/Functions.php">
<PossiblyUndefinedIntArrayOffset>
<code><![CDATA[$stub]]></code>
Expand Down Expand Up @@ -1918,9 +1911,6 @@
<code><![CDATA[$combination->strings]]></code>
<code><![CDATA[$combination->strings]]></code>
<code><![CDATA[$combination->strings]]></code>
<code><![CDATA[$combination->value_types['string'] instanceof TNonFalsyString
? $type->value
: $type->value !== '']]></code>
<code><![CDATA[$shared_classlikes]]></code>
</RiskyTruthyFalsyComparison>
</file>
Expand Down Expand Up @@ -2227,7 +2217,6 @@
<RiskyTruthyFalsyComparison>
<code><![CDATA[!strpos($key, '$')]]></code>
<code><![CDATA[!strpos($key, '[')]]></code>
<code><![CDATA[$array_key_offset]]></code>
<code><![CDATA[$failed_reconciliation]]></code>
<code><![CDATA[strpos($base_key, '::')]]></code>
<code><![CDATA[strpos($key, '::')]]></code>
Expand Down
8 changes: 8 additions & 0 deletions src/Psalm/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -2197,6 +2197,10 @@ public function visitPreloadedStubFiles(Codebase $codebase, ?Progress $progress

foreach ($stub_files as $file_path) {
$file_path = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $file_path);
// fix mangled phar paths on Windows
if (strpos($file_path, 'phar:\\\\') === 0) {
$file_path = 'phar://'. substr($file_path, 7);
}
$codebase->scanner->addFileToDeepScan($file_path);
}

Expand Down Expand Up @@ -2282,6 +2286,10 @@ public function visitStubFiles(Codebase $codebase, ?Progress $progress = null):

foreach ($stub_files as $file_path) {
$file_path = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $file_path);
// fix mangled phar paths on Windows
if (strpos($file_path, 'phar:\\\\') === 0) {
$file_path = 'phar://' . substr($file_path, 7);
}
$codebase->scanner->addFileToDeepScan($file_path);
}

Expand Down
3 changes: 1 addition & 2 deletions src/Psalm/Internal/Analyzer/FileAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ public function analyze(
return;
}

$event = new BeforeFileAnalysisEvent($this, $this->context, $file_storage, $codebase);

$event = new BeforeFileAnalysisEvent($this, $this->context, $file_storage, $codebase, $stmts);
$codebase->config->eventDispatcher->dispatchBeforeFileAnalysis($event);

if ($codebase->alter_code) {
Expand Down
33 changes: 26 additions & 7 deletions src/Psalm/Internal/Analyzer/MethodAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Psalm\Issue\InvalidStaticInvocation;
use Psalm\Issue\MethodSignatureMustOmitReturnType;
use Psalm\Issue\NonStaticSelfCall;
use Psalm\Issue\UndefinedMagicMethod;
use Psalm\Issue\UndefinedMethod;
use Psalm\IssueBuffer;
use Psalm\StatementsSource;
Expand Down Expand Up @@ -114,8 +115,9 @@ public static function checkStatic(
}

$original_method_id = $method_id;
$with_pseudo = true;

$method_id = $codebase_methods->getDeclaringMethodId($method_id);
$method_id = $codebase_methods->getDeclaringMethodId($method_id, $with_pseudo);

if (!$method_id) {
if (InternalCallMapHandler::inCallMap((string) $original_method_id)) {
Expand All @@ -125,7 +127,7 @@ public static function checkStatic(
throw new LogicException('Declaring method for ' . $original_method_id . ' should not be null');
}

$storage = $codebase_methods->getStorage($method_id);
$storage = $codebase_methods->getStorage($method_id, $with_pseudo);

if (!$storage->is_static) {
if ($self_call) {
Expand Down Expand Up @@ -170,6 +172,7 @@ public static function checkMethodExists(
CodeLocation $code_location,
array $suppressed_issues,
?string $calling_method_id = null,
bool $with_pseudo = false,
): ?bool {
if ($codebase->methods->methodExists(
$method_id,
Expand All @@ -180,15 +183,31 @@ public static function checkMethodExists(
: null,
null,
$code_location->file_path,
true,
false,
$with_pseudo,
)) {
return true;
}

if (IssueBuffer::accepts(
new UndefinedMethod('Method ' . $method_id . ' does not exist', $code_location, (string) $method_id),
$suppressed_issues,
)) {
return false;
if ($with_pseudo) {
if (IssueBuffer::accepts(
new UndefinedMagicMethod(
'Magic method ' . $method_id . ' does not exist',
$code_location,
(string) $method_id,
),
$suppressed_issues,
)) {
return false;
}
} else {
if (IssueBuffer::accepts(
new UndefinedMethod('Method ' . $method_id . ' does not exist', $code_location, (string) $method_id),
$suppressed_issues,
)) {
return false;
}
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,16 +374,18 @@ public static function handleParadoxicalCondition(
&& !($stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical)
&& !($stmt instanceof PhpParser\Node\Expr\BooleanNot)) {
if (count($type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}

if (count($both_types->getAtomicTypes()) > 0) {
if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
use Psalm\Type\Union;
use UnexpectedValueException;

use function array_merge;
use function in_array;
use function strlen;

Expand Down Expand Up @@ -173,14 +172,6 @@ public static function analyze(
$removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);

if ($stmt_left_type && $stmt_left_type->parent_nodes) {
// numeric types can't be tainted html or has_quotes, neither can bool
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& $stmt_left_type->isSingle()
&& ($stmt_left_type->isInt() || $stmt_left_type->isFloat() || $stmt_left_type->isBool())
) {
$removed_taints = array_merge($removed_taints, array('html', 'has_quotes'));
}

foreach ($stmt_left_type->parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath(
$parent_node,
Expand All @@ -193,14 +184,6 @@ public static function analyze(
}

if ($stmt_right_type && $stmt_right_type->parent_nodes) {
// numeric types can't be tainted html or has_quotes, neither can bool
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& $stmt_right_type->isSingle()
&& ($stmt_right_type->isInt() || $stmt_right_type->isFloat() || $stmt_right_type->isBool())
) {
$removed_taints = array_merge($removed_taints, array('html', 'has_quotes'));
}

foreach ($stmt_right_type->parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath(
$parent_node,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,18 @@ public static function analyze(
$stmt_type = new TTrue($expr_type->from_docblock);
} else {
if (count($expr_type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $expr_type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}

if (count($both_types->getAtomicTypes()) > 0) {
if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Union;

use function array_merge;
use function count;
use function explode;
use function implode;
Expand Down Expand Up @@ -1524,19 +1523,19 @@ private static function processTaintedness(
return;
}

$event = new AddRemoveTaintsEvent($expr, $context, $statements_analyzer, $codebase);

$added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
$removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);

// numeric types can't be tainted html or has_quotes, neither can bool
// numeric types can't be tainted, neither can bool
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& $input_type->isSingle()
&& ($input_type->isInt() || $input_type->isFloat() || $input_type->isBool())
) {
$removed_taints = array_merge($removed_taints, array('html', 'has_quotes'));
return;
}

$event = new AddRemoveTaintsEvent($expr, $context, $statements_analyzer, $codebase);

$added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
$removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);

if ($function_param->type && $function_param->type->isString() && !$input_type->isString()) {
$input_type = CastAnalyzer::castStringAttempt(
$statements_analyzer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use Psalm\Type\Atomic\TNonEmptyArray;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Atomic\TString;
use Psalm\Type\TaintKind;
use Psalm\Type\Union;
use UnexpectedValueException;

Expand Down Expand Up @@ -641,9 +642,9 @@ private static function taintReturnType(
$pattern = substr($pattern, 2, -1);

if (self::simpleExclusion($pattern, $first_arg_value[0])) {
$removed_taints[] = 'html';
$removed_taints[] = 'has_quotes';
$removed_taints[] = 'sql';
$removed_taints[] = TaintKind::INPUT_HTML;
$removed_taints[] = TaintKind::INPUT_HAS_QUOTES;
$removed_taints[] = TaintKind::INPUT_SQL;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public static function analyze(
$fq_classlike_name = $method_id->fq_class_name;
$method_name = $method_id->method_name;

$with_pseudo = true;

if ($codebase_methods->visibility_provider->has($fq_classlike_name)) {
$method_visible = $codebase_methods->visibility_provider->isMethodVisible(
$source,
Expand All @@ -67,7 +69,7 @@ public static function analyze(
}
}

$declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id);
$declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id, $with_pseudo);

if (!$declaring_method_id) {
if ($method_name === '__construct'
Expand Down Expand Up @@ -111,7 +113,7 @@ public static function analyze(
return null;
}

$storage = $codebase->methods->getStorage($declaring_method_id);
$storage = $codebase->methods->getStorage($declaring_method_id, $with_pseudo);
$visibility = $storage->visibility;

if ($appearing_method_name
Expand Down

0 comments on commit b940c7e

Please sign in to comment.