Skip to content

Commit

Permalink
Fix docblock mixed escape hatch
Browse files Browse the repository at this point in the history 
revert #7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs.
It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all)

This is also a required preparation to fix some isset issues of #9759
  • Loading branch information
@kkmuffme
kkmuffme committed Nov 21, 2023
1 parent d94f7bd commit 0d7c5a2
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 101 deletions.
4 changes: 1 addition & 3 deletions src/Psalm/Internal/Type/NegatedAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,7 @@ public static function reconcile(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

return $existing_var_type;
Expand Down
69 changes: 17 additions & 52 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
use Psalm\Type\Atomic\TCallableString;
use Psalm\Type\Atomic\TClassConstant;
use Psalm\Type\Atomic\TClassString;
use Psalm\Type\Atomic\TEmptyMixed;
use Psalm\Type\Atomic\TFalse;
use Psalm\Type\Atomic\TFloat;
use Psalm\Type\Atomic\TGenericObject;
Expand Down Expand Up @@ -976,9 +975,7 @@ private static function reconcileHasMethod(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? new Union([new TEmptyMixed()])
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1071,9 +1068,7 @@ private static function reconcileString(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? new Union([new TEmptyMixed()])
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1165,9 +1160,7 @@ private static function reconcileInt(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? new Union([new TEmptyMixed()])
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1244,9 +1237,7 @@ private static function reconcileBool(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1329,9 +1320,7 @@ private static function reconcileFalse(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1414,9 +1403,7 @@ private static function reconcileTrue(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1489,9 +1476,7 @@ private static function reconcileScalar(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1582,9 +1567,7 @@ private static function reconcileNumeric(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1702,9 +1685,7 @@ private static function reconcileObject(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1760,9 +1741,7 @@ private static function reconcileResource(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1832,9 +1811,7 @@ private static function reconcileCountable(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1895,9 +1872,7 @@ private static function reconcileIterable(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1939,9 +1914,7 @@ private static function reconcileInArray(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

return $intersection;
Expand Down Expand Up @@ -2260,9 +2233,7 @@ private static function reconcileTraversable(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -2375,9 +2346,7 @@ private static function reconcileArray(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -2485,9 +2454,7 @@ private static function reconcileList(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -2725,9 +2692,7 @@ private static function reconcileCallable(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down
56 changes: 14 additions & 42 deletions src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ public static function reconcile(
}
}

return $existing_var_type->from_docblock
? Type::getNull()
: Type::getNever();
return Type::getNever();
}

return Type::getNull();
Expand Down Expand Up @@ -507,9 +505,7 @@ private static function reconcileBool(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -703,9 +699,7 @@ private static function reconcileNull(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -785,9 +779,7 @@ private static function reconcileFalse(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -867,9 +859,7 @@ private static function reconcileTrue(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -924,9 +914,7 @@ private static function reconcileFalsyOrEmpty(

$failed_reconciliation = 2;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

if ($redundant) {
Expand Down Expand Up @@ -1139,9 +1127,7 @@ private static function reconcileScalar(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1240,9 +1226,7 @@ private static function reconcileObject(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1336,9 +1320,7 @@ private static function reconcileNumeric(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1438,9 +1420,7 @@ private static function reconcileInt(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1535,9 +1515,7 @@ private static function reconcileFloat(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1641,9 +1619,7 @@ private static function reconcileString(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1746,9 +1722,7 @@ private static function reconcileArray(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down Expand Up @@ -1818,9 +1792,7 @@ private static function reconcileResource(

$failed_reconciliation = Reconciler::RECONCILIATION_EMPTY;

return $existing_var_type->from_docblock
? Type::getMixed()
: Type::getNever();
return Type::getNever();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/TypeReconciliation/ConditionalTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ function foo($length) {
}
}',
'assertions' => [],
'ignored_issues' => ['DocblockTypeContradiction'],
'ignored_issues' => ['DocblockTypeContradiction', 'TypeDoesNotContainType'],
],
'notInstanceof' => [
'code' => '<?php
Expand Down
13 changes: 10 additions & 3 deletions tests/UnusedVariableTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2142,12 +2142,19 @@ function foo(array $clips) : void {
* @param bool $b
*/
function validate($b, string $source) : void {
/** @var bool|string $b */
if (!is_bool($b)) {
$source = $b;
$b = false;
}
/**
* @psalm-suppress DocblockTypeContradiction
* @psalm-suppress MixedAssignment
* test to ensure $b is only type bool and not bool|string anymore
* after we set $b = false; inside the condition above
* @psalm-suppress TypeDoesNotContainType
*/
if (!is_bool($b)) {
$source = $b;
echo "this should not happen";
}
print_r($source);
Expand Down

0 comments on commit 0d7c5a2

Please sign in to comment.