Skip to content
Permalink
Browse files

Find issues with impossible property values

  • Loading branch information
muglug committed Dec 11, 2019
1 parent 612f74f commit 05783eb61617251ad6d381bcd74d0268cefd1bdd
@@ -747,7 +747,7 @@ public static function checkReturnType(
return false;
}

if ($classlike_storage && $context->self && $function->name) {
if ($classlike_storage && $context->self) {
$class_template_params = MethodCallAnalyzer::getClassTemplateParams(
$codebase,
$classlike_storage,
@@ -244,12 +244,6 @@ public function __construct(
$progress
);

if ($stdout_report_options
&& !in_array($stdout_report_options->format, Report::SUPPORTED_OUTPUT_TYPES, true)
) {
throw new \UnexpectedValueException('Unrecognised output format ' . $stdout_report_options->format);
}

$this->stdout_report_options = $stdout_report_options;
$this->generated_report_options = $generated_report_options;

@@ -624,8 +624,6 @@ function (Clause $c) use ($changed_var_ids) {
// to $outer_context don't mess with elseif/else blocks
$original_context = clone $outer_context;

$if_conditional_context->inside_conditional = true;

if ($internally_applied_if_cond_expr !== $cond
|| $externally_applied_if_cond_expr !== $cond
) {
@@ -635,10 +633,14 @@ function (Clause $c) use ($changed_var_ids) {
$referenced_var_ids = $outer_context->referenced_var_ids;
$if_conditional_context->referenced_var_ids = [];

$if_conditional_context->inside_conditional = true;

if (ExpressionAnalyzer::analyze($statements_analyzer, $cond, $if_conditional_context) === false) {
throw new \Psalm\Exception\ScopeAnalysisException();
}

$if_conditional_context->inside_conditional = false;

/** @var array<string, bool> */
$more_cond_referenced_var_ids = $if_conditional_context->referenced_var_ids;
$if_conditional_context->referenced_var_ids = array_merge(
@@ -688,24 +690,8 @@ function (Type\Union $_) {
// get all the var ids that were referened in the conditional, but not assigned in it
$cond_referenced_var_ids = array_diff_key($cond_referenced_var_ids, $cond_assigned_var_ids);

// remove all newly-asserted var ids too
$cond_referenced_var_ids = array_filter(
$cond_referenced_var_ids,
/**
* @param string $var_id
*
* @return bool
*/
function ($var_id) use ($pre_condition_vars_in_scope) {
return isset($pre_condition_vars_in_scope[$var_id]);
},
ARRAY_FILTER_USE_KEY
);

$cond_referenced_var_ids = array_merge($newish_var_ids, $cond_referenced_var_ids);

$if_conditional_context->inside_conditional = false;

return new \Psalm\Internal\Scope\IfConditionalScope(
$if_context,
$original_context,
@@ -2563,7 +2563,6 @@ protected static function hasClassExistsCheck(PhpParser\Node\Expr\FuncCall $stmt
$second_arg = $stmt->args[1]->value;

if ($second_arg instanceof PhpParser\Node\Expr\ConstFetch
&& $second_arg->name instanceof PhpParser\Node\Name
&& strtolower($second_arg->name->parts[0]) === 'true'
) {
return 2;
@@ -2592,7 +2591,6 @@ protected static function hasTraitExistsCheck(PhpParser\Node\Expr\FuncCall $stmt
$second_arg = $stmt->args[1]->value;

if ($second_arg instanceof PhpParser\Node\Expr\ConstFetch
&& $second_arg->name instanceof PhpParser\Node\Name
&& strtolower($second_arg->name->parts[0]) === 'true'
) {
return 2;
@@ -2726,7 +2724,6 @@ protected static function hasInArrayCheck(PhpParser\Node\Expr\FuncCall $stmt)
$second_arg = $stmt->args[2]->value;

if ($second_arg instanceof PhpParser\Node\Expr\ConstFetch
&& $second_arg->name instanceof PhpParser\Node\Name
&& strtolower($second_arg->name->parts[0]) === 'true'
) {
return true;
@@ -86,7 +86,6 @@ public static function analyze(
);

$pre_referenced_var_ids = $context->referenced_var_ids;
$original_vars_in_scope = $context->vars_in_scope;

$pre_assigned_var_ids = $context->assigned_var_ids;

@@ -116,20 +115,6 @@ public static function analyze(

$new_referenced_var_ids = array_diff_key($new_referenced_var_ids, $new_assigned_var_ids);

// remove all newly-asserted var ids too
$new_referenced_var_ids = array_filter(
$new_referenced_var_ids,
/**
* @param string $var_id
*
* @return bool
*/
function ($var_id) use ($original_vars_in_scope) {
return isset($original_vars_in_scope[$var_id]);
},
ARRAY_FILTER_USE_KEY
);

$context_clauses = array_merge($left_context->clauses, $left_clauses);

if ($left_context->reconciled_expression_clauses) {
@@ -120,6 +120,7 @@ public function getStatementsForFile($file_path, Progress $progress = null)

$existing_statements = $this->parser_cache_provider->loadExistingStatementsFromCache($file_path);

/** @psalm-suppress DocblockTypeContradiction */
if ($existing_statements && !$existing_statements[0] instanceof PhpParser\Node\Stmt) {
$existing_statements = null;
}
@@ -1565,12 +1565,8 @@ private static function reconcileTraversable(
if ($type->hasTraversableInterface($codebase)) {
$traversable_types[] = $type;
} elseif ($type instanceof Atomic\TIterable) {
if ($type->type_params) {
$clone_type = clone $type;
$traversable_types[] = new Atomic\TGenericObject('Traversable', $clone_type->type_params);
} else {
$traversable_types[] = new Atomic\TNamedObject('Traversable');
}
$clone_type = clone $type;
$traversable_types[] = new Atomic\TGenericObject('Traversable', $clone_type->type_params);
$did_remove_type = true;
} elseif ($type instanceof TObject) {
$traversable_types[] = new TNamedObject('Traversable');
@@ -1644,12 +1640,8 @@ private static function reconcileArray(

$did_remove_type = true;
} elseif ($type instanceof Atomic\TIterable) {
if ($type->type_params) {
$clone_type = clone $type;
$array_types[] = new TArray($clone_type->type_params);
} else {
$array_types[] = new TArray([Type::getArrayKey(), Type::getMixed()]);
}
$clone_type = clone $type;
$array_types[] = new TArray($clone_type->type_params);

$did_remove_type = true;
} else {
@@ -1736,12 +1728,8 @@ private static function reconcileList(

$did_remove_type = true;
} elseif ($type instanceof Atomic\TIterable) {
if ($type->type_params) {
$clone_type = clone $type;
$array_types[] = new TList($clone_type->type_params[1]);
} else {
$array_types[] = new TList(Type::getMixed());
}
$clone_type = clone $type;
$array_types[] = new TList($clone_type->type_params[1]);

$did_remove_type = true;
} else {
@@ -229,7 +229,6 @@ public static function reconcile(
$existing_var_type->removeType('iterable');

if ($iterable instanceof Atomic\TIterable
&& $iterable->type_params
&& (!$iterable->type_params[0]->isMixed() || !$iterable->type_params[1]->isMixed())
) {
$traversable = new Atomic\TGenericObject('Traversable', $iterable->type_params);
@@ -310,14 +310,10 @@ public static function findMatchingAtomicTypeForTemplate(
return $atomic_input_type;
}

if ($atomic_type->type_params) {
$atomic_type = new Atomic\TGenericObject(
'Traversable',
$atomic_type->type_params
);
} else {
$atomic_type = new Atomic\TNamedObject('Traversable');
}
$atomic_type = new Atomic\TGenericObject(
'Traversable',
$atomic_type->type_params
);
}

try {
@@ -2700,7 +2700,6 @@ public function getTranslatedFunctionParam(

$is_nullable = $param->default !== null &&
$param->default instanceof PhpParser\Node\Expr\ConstFetch &&
$param->default->name instanceof PhpParser\Node\Name &&
strtolower($param->default->name->parts[0]) === 'null';

$param_typehint = $param->type;
@@ -582,7 +582,16 @@ private static function groupImpossibilities(array $clauses, int &$complexity =
$new_clause_possibilities[$var] = [$impossible_type];
}

$new_clause = new Clause($new_clause_possibilities, false, true, true);
$new_clause = new Clause(
$new_clause_possibilities,
false,
true,
true,
[],
$clause->creating_object_id === $grouped_clause->creating_object_id
? $clause->creating_object_id
: null
);

$new_clauses[] = $new_clause;

@@ -597,7 +606,14 @@ private static function groupImpossibilities(array $clauses, int &$complexity =

foreach ($clause->impossibilities as $var => $impossible_types) {
foreach ($impossible_types as $impossible_type) {
$new_clause = new Clause([$var => [$impossible_type]]);
$new_clause = new Clause(
[$var => [$impossible_type]],
false,
true,
false,
[],
$clause->creating_object_id
);

$new_clauses[] = $new_clause;

@@ -988,7 +988,7 @@ public function getPosters($commenter, $numToGet=10) {
/**
* @psalm-suppress MixedOperand
*/
$a[$key] += 5;
$a[$key] += rand(0, 10);
}
$a["four"] = true;
@@ -2616,23 +2616,14 @@ function foo($value) : void {
}',
'error_message' => 'RedundantCondition',
],
'SKIPPED-catchRedundantConditionOnBinaryOpForwards' => [
'catchRedundantConditionOnBinaryOpForwards' => [
'<?php
class App {}
function test(App $app) : void {
if ($app || rand(0, 1)) {}
}',
'error_message' => 'RedundantCondition',
],
'SKIPPED-catchRedundantConditionOnBinaryOpBackwards' => [
'<?php
class App {}
function test(App $app) : void {
if (rand(0, 1) || $app) {}
}',
'error_message' => 'RedundantCondition',
'error_message' => 'TypeDoesNotContainType',
],
];
}
@@ -1136,7 +1136,43 @@ function a(bool $a, bool $b) : void {
}
}',
'error_message' => 'RedundantCondition',
]
],
'prohibitFalsyChecksOnPropertiesWithMethodCall' => [
'<?php
class RequestHeaders {
public function has(string $s) : bool {
return true;
}
}
class Request {
public RequestHeaders $headers;
public function __construct(RequestHeaders $headers) {
$this->headers = $headers;
}
}
function lag(Request $req) : void {
if ($req->headers && $req->headers->has("foo")) {}
}',
'error_message' => 'RedundantCondition',
],
'prohibitFalsyChecksOnPropertiesWithoutMethodCall' => [
'<?php
class RequestHeaders {}
class Request {
public RequestHeaders $headers;
public function __construct(RequestHeaders $headers) {
$this->headers = $headers;
}
}
function lag(Request $req) : void {
if ($req->headers) {}
}',
'error_message' => 'RedundantCondition',
],
];
}
}

0 comments on commit 05783eb

Please sign in to comment.
You can’t perform that action at this time.