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 useless ifs #8916

Merged
merged 2 commits into from Dec 17, 2022
Merged

Remove useless ifs #8916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30a8f71
Remove useless ifs
jack-worman Dec 17, 2022
2f677f6
CS fixes
jack-worman Dec 17, 2022
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
15 changes: 5 additions & 10 deletions examples/plugins/FunctionCasingChecker.php
View file
Open in desktop
@@ -1,10 +1,9 @@
<?php

namespace Psalm\Example\Plugin;

use Exception;
use PhpParser;
use Psalm\Checker;
use Psalm\Checker\StatementsChecker;
use Psalm\CodeLocation;
use Psalm\Issue\PluginIssue;
use Psalm\IssueBuffer;
Expand Down Expand Up @@ -48,15 +47,13 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
}

if ($function_storage->cased_name !== (string)$expr->name) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new IncorrectFunctionCasing(
'Function is incorrectly cased, expecting ' . $function_storage->cased_name,
new CodeLocation($statements_source, $expr->name)
),
$statements_source->getSuppressedIssues()
)) {
// fall through
}
);
}
} catch (Exception $e) {
// can throw if storage is missing
Expand Down Expand Up @@ -88,15 +85,13 @@ public static function afterFunctionCallAnalysis(AfterFunctionCallAnalysisEvent
$function_name_parts = explode('\\', $function_storage->cased_name);

if (end($function_name_parts) !== end($expr->name->parts)) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new IncorrectFunctionCasing(
'Function is incorrectly cased, expecting ' . $function_storage->cased_name,
new CodeLocation($statements_source, $expr->name)
),
$statements_source->getSuppressedIssues()
)) {
// fall through
}
);
}
} catch (Exception $e) {
// can throw if storage is missing
Expand Down
10 changes: 4 additions & 6 deletions examples/plugins/PreventFloatAssignmentChecker.php
View file
Open in desktop
@@ -1,8 +1,8 @@
<?php

namespace Psalm\Example\Plugin;

use PhpParser;
use Psalm\Checker;
use Psalm\CodeLocation;
use Psalm\Issue\PluginIssue;
use Psalm\IssueBuffer;
Expand All @@ -17,7 +17,7 @@ class PreventFloatAssignmentChecker implements AfterExpressionAnalysisInterface
/**
* Called after an expression has been checked
*
* @return null|false
jack-worman marked this conversation as resolved.
Show resolved Hide resolved
* @return null
*/
public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $event): ?bool {
$expr = $event->getExpr();
Expand All @@ -26,15 +26,13 @@ public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $eve
&& ($expr_type = $statements_source->getNodeTypeProvider()->getType($expr->expr))
&& $expr_type->hasFloat()
) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new NoFloatAssignment(
'Don’t assign to floats',
new CodeLocation($statements_source, $expr)
),
$statements_source->getSuppressedIssues()
)) {
// fall through
}
);
}

return null;
Expand Down
7 changes: 2 additions & 5 deletions examples/plugins/StringChecker.php
View file
Open in desktop
Expand Up @@ -30,17 +30,14 @@ public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $eve
&& preg_match($class_or_class_method, $expr->value)
) {
$absolute_class = preg_split('/[:]/', $expr->value)[0];

if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new InvalidClass(
'Use ::class constants when representing class names',
new CodeLocation($statements_source, $expr),
$absolute_class
),
$statements_source->getSuppressedIssues()
)) {
// fall through
}
);
}
} elseif ($expr instanceof PhpParser\Node\Expr\BinaryOp\Concat
&& $expr->left instanceof PhpParser\Node\Expr\ClassConstFetch
Expand Down
13 changes: 4 additions & 9 deletions examples/plugins/composer-based/echo-checker/EchoChecker.php
View file
Open in desktop
Expand Up @@ -27,17 +27,14 @@ public static function afterStatementAnalysis(AfterStatementAnalysisEvent $event
$expr_type = $statements_source->getNodeTypeProvider()->getType($expr);

if (!$expr_type || $expr_type->hasMixed()) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new ArgumentTypeCoercion(
'Echo requires an unescaped string, ' . $expr_type . ' provided',
new CodeLocation($statements_source, $expr),
'echo'
),
$statements_source->getSuppressedIssues()
)) {
// keep soldiering on
}

);
continue;
}

Expand All @@ -47,16 +44,14 @@ public static function afterStatementAnalysis(AfterStatementAnalysisEvent $event
if ($type instanceof TString
&& !$type instanceof TLiteralString
) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new ArgumentTypeCoercion(
'Echo requires an unescaped string, ' . $expr_type . ' provided',
new CodeLocation($statements_source, $expr),
'echo'
),
$statements_source->getSuppressedIssues()
)) {
// keep soldiering on
}
);
}
}
}
Expand Down
26 changes: 10 additions & 16 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View file
Open in desktop
Expand Up @@ -447,13 +447,11 @@ public function analyze(
} elseif ($stmt instanceof PhpParser\Node\Stmt\Property) {
foreach ($stmt->props as $prop) {
if ($storage->is_enum) {
if (IssueBuffer::accepts(new NoEnumProperties(
IssueBuffer::maybeAdd(new NoEnumProperties(
'Enums cannot have properties',
new CodeLocation($this, $prop),
$fq_class_name
))) {
// fall through
}
));
continue;
}
if ($prop->default) {
Expand Down Expand Up @@ -2421,48 +2419,44 @@ private function checkEnum(): void
$seen_values = [];
foreach ($storage->enum_cases as $case_storage) {
if ($case_storage->value !== null && $storage->enum_type === null) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new InvalidEnumCaseValue(
'Case of a non-backed enum should not have a value',
$case_storage->stmt_location,
$storage->name
)
)) {
}
);
} elseif ($case_storage->value === null && $storage->enum_type !== null) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new InvalidEnumCaseValue(
'Case of a backed enum should have a value',
$case_storage->stmt_location,
$storage->name
)
)) {
}
);
} elseif ($case_storage->value !== null && $storage->enum_type !== null) {
if ((is_int($case_storage->value) && $storage->enum_type === 'string')
|| (is_string($case_storage->value) && $storage->enum_type === 'int')
) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new InvalidEnumCaseValue(
'Enum case value type should be ' . $storage->enum_type,
$case_storage->stmt_location,
$storage->name
)
)) {
}
);
}
}

if ($case_storage->value !== null) {
if (in_array($case_storage->value, $seen_values, true)) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new DuplicateEnumCaseValue(
'Enum case values should be unique',
$case_storage->stmt_location,
$storage->name
)
)) {
}
);
} else {
$seen_values[] = $case_storage->value;
}
Expand Down
48 changes: 24 additions & 24 deletions src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php
View file
Open in desktop
Expand Up @@ -583,27 +583,27 @@ public static function checkPropertyVisibility(
return $emit_issues ? null : true;

case self::VISIBILITY_PRIVATE:
if ($emit_issues && IssueBuffer::accepts(
new InaccessibleProperty(
'Cannot access private property ' . $property_id . ' from context ' . $context->self,
$code_location
),
$suppressed_issues
)) {
// fall through
if ($emit_issues) {
IssueBuffer::maybeAdd(
new InaccessibleProperty(
'Cannot access private property ' . $property_id . ' from context ' . $context->self,
$code_location
),
$suppressed_issues
);
}

return null;
case self::VISIBILITY_PROTECTED:
if (!$context->self) {
if ($emit_issues && IssueBuffer::accepts(
new InaccessibleProperty(
'Cannot access protected property ' . $property_id,
$code_location
),
$suppressed_issues
)) {
// fall through
if ($emit_issues) {
IssueBuffer::maybeAdd(
new InaccessibleProperty(
'Cannot access protected property ' . $property_id,
$code_location
),
$suppressed_issues
);
}

return null;
Expand All @@ -614,14 +614,14 @@ public static function checkPropertyVisibility(
}

if (!$codebase->classExtends($context->self, $appearing_property_class)) {
if ($emit_issues && IssueBuffer::accepts(
new InaccessibleProperty(
'Cannot access protected property ' . $property_id . ' from context ' . $context->self,
$code_location
),
$suppressed_issues
)) {
// fall through
if ($emit_issues) {
IssueBuffer::maybeAdd(
new InaccessibleProperty(
'Cannot access protected property ' . $property_id . ' from context ' . $context->self,
$code_location
),
$suppressed_issues
);
}

return null;
Expand Down
7 changes: 3 additions & 4 deletions src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
View file
Open in desktop
Expand Up @@ -361,13 +361,12 @@ public function analyze(

if ($storage->unused_docblock_params) {
foreach ($storage->unused_docblock_params as $param_name => $param_location) {
if (IssueBuffer::accepts(
IssueBuffer::maybeAdd(
new InvalidDocblockParamName(
'Incorrect param name $' . $param_name . ' in docblock for ' . $cased_method_id,
$param_location
)
)) {
}
);
}
}

Expand All @@ -388,7 +387,7 @@ public function analyze(
if ($overridden_storage->allow_named_arg_calls) {
IssueBuffer::maybeAdd(new MethodSignatureMismatch(
'Method ' . (string) $method_id . ' should accept named arguments '
. ' as ' . (string) $overridden_method_id . ' does',
. ' as ' . (string) $overridden_method_id . ' does',
$storage->location
));
}
Expand Down
12 changes: 5 additions & 7 deletions src/Psalm/Internal/Analyzer/MethodAnalyzer.php
View file
Open in desktop
Expand Up @@ -99,13 +99,13 @@ public static function checkStatic(
CodeLocation $code_location,
array $suppressed_issues,
?bool &$is_dynamic_this_method = false
): bool {
): void {
weirdan marked this conversation as resolved.
Show resolved Hide resolved
$codebase_methods = $codebase->methods;

if ($method_id->fq_class_name === 'Closure'
&& $method_id->method_name === 'fromcallable'
) {
return true;
return;
}

$original_method_id = $method_id;
Expand All @@ -114,7 +114,7 @@ public static function checkStatic(

if (!$method_id) {
if (InternalCallMapHandler::inCallMap((string) $original_method_id)) {
return true;
return;
}

throw new LogicException('Declaring method for ' . $original_method_id . ' should not be null');
Expand All @@ -134,7 +134,7 @@ public static function checkStatic(
),
$suppressed_issues
)) {
return false;
return;
}
} else {
$is_dynamic_this_method = true;
Expand All @@ -149,12 +149,10 @@ public static function checkStatic(
),
$suppressed_issues
)) {
return false;
return;
}
}
}

return true;
}

/**
Expand Down