Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove always true/false conditions #7229

Merged
merged 3 commits into from Dec 27, 2021
Merged

remove always true/false conditions #7229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9029e1d
remove always true/false conditions
orklah Dec 27, 2021
a9b1a42
fix
orklah Dec 27, 2021
3ae8098
fix unused and CS
orklah Dec 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 2 additions & 8 deletions src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
View file
Open in desktop
Expand Up @@ -41,7 +41,6 @@
use Psalm\Type\Atomic\Scalar;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TDependentListKey;
use Psalm\Type\Atomic\TEmpty;
use Psalm\Type\Atomic\TFalse;
use Psalm\Type\Atomic\TGenericObject;
use Psalm\Type\Atomic\TIntRange;
Expand Down Expand Up @@ -502,16 +501,11 @@ public static function checkIteratorType(

$always_non_empty_array = false;

if ($iterator_atomic_type instanceof Scalar ||
$iterator_atomic_type instanceof TVoid
) {
if ($iterator_atomic_type instanceof Scalar || $iterator_atomic_type instanceof TVoid) {
$invalid_iterator_types[] = $iterator_atomic_type->getKey();

$value_type = Type::getMixed();
} elseif ($iterator_atomic_type instanceof TObject ||
$iterator_atomic_type instanceof TMixed ||
$iterator_atomic_type instanceof TEmpty
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEmpty is already covered by Scalar on the previous if

) {
} elseif ($iterator_atomic_type instanceof TObject || $iterator_atomic_type instanceof TMixed) {
$has_valid_iterator = true;
$value_type = Type::getMixed();
$key_type = Type::getMixed();
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View file
Open in desktop
Expand Up @@ -188,7 +188,6 @@ public static function analyze(
if (!$atomic_key_type instanceof TString
&& !$atomic_key_type instanceof TInt
&& !$atomic_key_type instanceof TArrayKey
&& !$atomic_key_type instanceof TMixed
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TMixed is handled above

&& !$atomic_key_type instanceof TTemplateParam
&& !(
$atomic_key_type instanceof TObjectWithProperties
Expand Down
6 changes: 1 addition & 5 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View file
Open in desktop
Expand Up @@ -1214,11 +1214,7 @@ private static function analyzeDestructuringAssignment(
&& !$assign_value_type->hasArrayAccessInterface($codebase)
) {
if ($assign_value_type->hasArray()) {
if (($assign_value_atomic_type instanceof TFalse
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TNull is handled above

&& $assign_value_type->ignore_falsable_issues)
|| ($assign_value_atomic_type instanceof TNull
&& $assign_value_type->ignore_nullable_issues)
) {
if ($assign_value_atomic_type instanceof TFalse && $assign_value_type->ignore_falsable_issues) {
// do nothing
} elseif (IssueBuffer::accepts(
new PossiblyInvalidArrayAccess(
Expand Down
36 changes: 0 additions & 36 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/NonComparisonOpAnalyzer.php
View file
Open in desktop
Expand Up @@ -87,22 +87,6 @@ public static function analyze(
return;
}

if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\BitwiseXor) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BitwiseXor is handled above

if ($stmt_left_type->hasBool() || $stmt_right_type->hasBool()) {
$statements_analyzer->node_data->setType($stmt, Type::getInt());
}

BinaryOpAnalyzer::addDataFlow(
$statements_analyzer,
$stmt,
$stmt->left,
$stmt->right,
'xor'
);

return;
}

if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\LogicalXor) {
if ($stmt_left_type->hasBool() || $stmt_right_type->hasBool()) {
$statements_analyzer->node_data->setType($stmt, Type::getBool());
Expand Down Expand Up @@ -146,25 +130,5 @@ public static function analyze(

return;
}

if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\BitwiseOr) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BitwiseOr is handled above

ArithmeticOpAnalyzer::analyze(
$statements_analyzer,
$statements_analyzer->node_data,
$stmt->left,
$stmt->right,
$stmt,
$result_type,
$context
);

BinaryOpAnalyzer::addDataFlow(
$statements_analyzer,
$stmt,
$stmt->left,
$stmt->right,
'or'
);
}
}
}
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
View file
Open in desktop
Expand Up @@ -671,7 +671,7 @@ private static function getAnalyzeNamedExpression(

$function_call_info->function_exists = true;
$has_valid_function_call_type = true;
} elseif ($var_type_part instanceof TMixed || $var_type_part instanceof TTemplateParam) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TTemplateParam can't get here, there's a continue above: https://github.com/vimeo/psalm/pull/7229/files#diff-2b5a0d000f3703de9fd772b1a15dc0290371dcd5e742a2f6d583e44796199bd7L635

} elseif ($var_type_part instanceof TMixed) {
$has_valid_function_call_type = true;

IssueBuffer::maybeAdd(
Expand Down
6 changes: 0 additions & 6 deletions src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php
View file
Open in desktop
Expand Up @@ -169,12 +169,6 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
$function_call_arg->value->items[1]->value->value,
[]
);
} elseif ($variable_atomic_type instanceof TTemplateParamClass) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TTemplateParamClass is handled above with the same content in the conditional

$fake_method_call = new VirtualStaticCall(
$function_call_arg->value->items[0]->value,
$function_call_arg->value->items[1]->value->value,
[]
);
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php
View file
Open in desktop
Expand Up @@ -395,10 +395,6 @@ public static function isContainedBy(
}

foreach ($input_type_part->as->getAtomicTypes() as $input_as_type_part) {
if ($input_as_type_part instanceof TNull && $container_type_part instanceof TNull) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TNull was already returned here: https://github.com/vimeo/psalm/pull/7229/files#diff-e17ecd10daef9f0bed66b49fc187403cae341f5a3994fdf1f776f044103a3e66L128

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's for the container type part, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, so it goes $input_as_type_part instanceof TNull && false that is simplified to false, so the conditional is never met

continue;
}

if (self::isContainedBy(
$codebase,
$input_as_type_part,
Expand Down
4 changes: 1 addition & 3 deletions src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php
View file
Open in desktop
Expand Up @@ -89,9 +89,7 @@ public static function isContainedBy(
}

if ($container_type_part instanceof TNonspecificLiteralString
&& ($input_type_part instanceof TLiteralString
|| $input_type_part instanceof TNonspecificLiteralString
|| $input_type_part instanceof TNonEmptyNonspecificLiteralString)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TNonEmptyNonspecificLiteralString is a child of TNonspecificLiteralString just above

&& ($input_type_part instanceof TLiteralString || $input_type_part instanceof TNonspecificLiteralString)
) {
return true;
}
Expand Down
31 changes: 1 addition & 30 deletions src/Psalm/Internal/Type/TypeCombiner.php
View file
Open in desktop
Expand Up @@ -53,7 +53,6 @@
use Psalm\Type\Atomic\TString;
use Psalm\Type\Atomic\TTemplateParam;
use Psalm\Type\Atomic\TTemplateParamClass;
use Psalm\Type\Atomic\TTraitString;
use Psalm\Type\Atomic\TTrue;
use Psalm\Type\Union;
use UnexpectedValueException;
Expand Down Expand Up @@ -1050,42 +1049,14 @@ private static function scrapeStringProperties(

$combination->strings = null;
} else {
$has_non_literal_class_string = false;

$shared_classlikes = $codebase ? self::getSharedTypes($combination, $codebase) : [];

foreach ($combination->strings as $string_type) {
if (!$string_type instanceof TLiteralClassString) {
$has_non_literal_class_string = true;
break;
}
}

if ($has_non_literal_class_string ||
!$type instanceof TClassString
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$type instanceof TClassString was handled in another branch

) {
$combination->value_types[$type_key] = new TString();
} else {
if (isset($shared_classlikes[$type->as]) && $type->as_type) {
$combination->class_string_types[$type->as] = $type->as_type;
} else {
$combination->class_string_types['object'] = new TObject();
}
}
$combination->value_types[$type_key] = new TString();
}
} else {
$combination->value_types[$type_key] = $type;
}
} elseif (get_class($combination->value_types['string']) !== TString::class) {
if (get_class($type) === TString::class) {
$combination->value_types['string'] = $type;
} elseif ($combination->value_types['string'] instanceof TTraitString
&& $type instanceof TClassString
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

) {
$combination->value_types['trait-string'] = $combination->value_types['string'];
$combination->value_types['class-string'] = $type;

unset($combination->value_types['string']);
} elseif (get_class($combination->value_types['string']) !== get_class($type)) {
if (get_class($type) === TNonEmptyString::class
&& get_class($combination->value_types['string']) === TNumericString::class
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Type/Reconciler.php
View file
Open in desktop
Expand Up @@ -732,7 +732,6 @@ private static function getValueForKey(
if ($existing_key_type_part instanceof TNull) {
$class_property_type = Type::getNull();
} elseif ($existing_key_type_part instanceof TMixed
|| $existing_key_type_part instanceof TTemplateParam
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continued just above

|| $existing_key_type_part instanceof TObject
|| ($existing_key_type_part instanceof TNamedObject
&& strtolower($existing_key_type_part->value) === 'stdclass')
Expand Down
11 changes: 1 addition & 10 deletions tests/TestCase.php
View file
Open in desktop
Expand Up @@ -15,7 +15,6 @@
use Psalm\IssueBuffer;
use Psalm\Tests\Internal\Provider\FakeParserCacheProvider;
use Psalm\Type\Union;
use RuntimeException;
use Throwable;

use function array_filter;
Expand Down Expand Up @@ -151,15 +150,7 @@ public function analyzeFile($file_path, Context $context, bool $track_unused_sup
*/
protected function getTestName($withDataSet = true): string
{
$name = parent::getName($withDataSet);
/**
* @psalm-suppress TypeDoesNotContainNull PHPUnit 8.2 made it non-nullable again
*/
if (null === $name) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-nullable since 8.2 and Psalm 4 requires PHPunit 9

throw new RuntimeException('anonymous test - shouldn\'t happen');
}

return $name;
return $this->getName($withDataSet);
}

/**
Expand Down