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.

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

Disallow ksorting lists #7019

Merged
merged 6 commits into from Jan 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions config.xsd
Expand Up @@ -424,6 +424,8 @@
<xs:element name="RedundantCastGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantCondition" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantConditionGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantFunctionCall" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantFunctionCallGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantPropertyInitializationCheck" type="IssueHandlerType" minOccurs="0" />
<xs:element name="RedundantIdentityWithTrue" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ReferenceConstraintViolation" type="IssueHandlerType" minOccurs="0" />
Expand Down
2 changes: 2 additions & 0 deletions docs/running_psalm/error_levels.md
Expand Up @@ -140,6 +140,7 @@ These issues are treated as errors at level 2 and below.
- [PropertyNotSetInConstructor](issues/PropertyNotSetInConstructor.md)
- [RawObjectIteration](issues/RawObjectIteration.md)
- [RedundantConditionGivenDocblockType](issues/RedundantConditionGivenDocblockType.md)
- [RedundantFunctionCallGivenDocblockType](issues/RedundantFunctionCallGivenDocblockType.md)
- [ReferenceConstraintViolation](issues/ReferenceConstraintViolation.md)
- [UndefinedTrace](issues/UndefinedTrace.md)
- [UnresolvableInclude](issues/UnresolvableInclude.md)
Expand Down Expand Up @@ -209,6 +210,7 @@ These issues are treated as errors at level 4 and below.
- [NoInterfaceProperties](issues/NoInterfaceProperties.md)
- [PossibleRawObjectIteration](issues/PossibleRawObjectIteration.md)
- [RedundantCondition](issues/RedundantCondition.md)
- [RedundantFunctionCall](issues/RedundantFunctionCall.md)
- [RedundantPropertyInitializationCheck](issues/RedundantPropertyInitializationCheck.md)
- [StringIncrement](issues/StringIncrement.md)
- [TooManyArguments](issues/TooManyArguments.md)
Expand Down
2 changes: 2 additions & 0 deletions docs/running_psalm/issues.md
Expand Up @@ -200,6 +200,8 @@
- [RedundantCastGivenDocblockType](issues/RedundantCastGivenDocblockType.md)
- [RedundantCondition](issues/RedundantCondition.md)
- [RedundantConditionGivenDocblockType](issues/RedundantConditionGivenDocblockType.md)
- [RedundantFunctionCall](issues/RedundantFunctionCall.md)
- [RedundantFunctionCallGivenDocblockType](issues/RedundantFunctionCallGivenDocblockType.md)
- [RedundantIdentityWithTrue](issues/RedundantIdentityWithTrue.md)
- [RedundantPropertyInitializationCheck](issues/RedundantPropertyInitializationCheck.md)
- [ReferenceConstraintViolation](issues/ReferenceConstraintViolation.md)
Expand Down
12 changes: 12 additions & 0 deletions docs/running_psalm/issues/RedundantFunctionCall.md
@@ -0,0 +1,12 @@
# RedundantFunctionCall

Emitted when a function call (`array_values`, `strtolower`, `ksort` etc.) is redundant.

```php
<?php

$a = ['already', 'a', 'list'];

$redundant = array_values($a);
$alreadyLower = strtolower($redundant[0]);
```
@@ -0,0 +1,21 @@
# RedundantFunctionCallGivenDocblockType

Emitted when function call is redundant given information supplied in one or more docblocks.

This may be desired (e.g. when checking user input) so is distinct from RedundantFunctionCall, which only applies to non-docblock types.

```php
<?php

/**
* @param array{0: lowercase-string, 1: non-empty-list<lowercase-string>} $s
*
* @return lowercase-string
*/
function foo($s): string {
$redundantList = array_values($s);
$redundantSubList = array_values($s[1]);
$redundantLowercase = strtolower($redundantSubList[0]);
return $redundantLowercase;
}
```
4 changes: 4 additions & 0 deletions src/Psalm/Config.php
Expand Up @@ -1679,6 +1679,10 @@ public static function getParentIssueType(string $issue_type): ?string
return 'RedundantCondition';
}

if ($issue_type === 'RedundantFunctionCallGivenDocblockType') {
return 'RedundantFunctionCall';
}

if ($issue_type === 'RedundantCastGivenDocblockType') {
return 'RedundantCast';
}
Expand Down
Expand Up @@ -18,8 +18,8 @@
use Psalm\Issue\ForbiddenCode;
use Psalm\Issue\PossibleRawObjectIteration;
use Psalm\Issue\RawObjectIteration;
use Psalm\Issue\RedundantCast;
use Psalm\Issue\RedundantCastGivenDocblockType;
use Psalm\Issue\RedundantFunctionCall;
use Psalm\Issue\RedundantFunctionCallGivenDocblockType;
use Psalm\IssueBuffer;
use Psalm\Node\Expr\VirtualArray;
use Psalm\Node\Expr\VirtualArrayItem;
Expand Down Expand Up @@ -389,7 +389,7 @@ function (array $_): bool {
return;
}

if ($first_arg && $function_id === 'array_values') {
if ($first_arg && ($function_id === 'array_values' || $function_id === 'ksort')) {
$first_arg_type = $statements_analyzer->node_data->getType($first_arg->value);

if ($first_arg_type
Expand All @@ -401,16 +401,16 @@ function (array $_): bool {
) {
if ($first_arg_type->from_docblock) {
IssueBuffer::maybeAdd(
new RedundantCastGivenDocblockType(
'The call to array_values is unnecessary given the list docblock type '.$first_arg_type,
new RedundantFunctionCallGivenDocblockType(
"The call to $function_id is unnecessary given the list docblock type $first_arg_type",
new CodeLocation($statements_analyzer, $function_name)
),
$statements_analyzer->getSuppressedIssues()
);
} else {
IssueBuffer::maybeAdd(
new RedundantCast(
'The call to array_values is unnecessary, '.$first_arg_type.' is already a list',
new RedundantFunctionCall(
"The call to $function_id is unnecessary, $first_arg_type is already a list",
new CodeLocation($statements_analyzer, $function_name)
),
$statements_analyzer->getSuppressedIssues()
Expand All @@ -430,15 +430,15 @@ function (array $_): bool {
) {
if ($first_arg_type->from_docblock) {
IssueBuffer::maybeAdd(
new RedundantCastGivenDocblockType(
new RedundantFunctionCallGivenDocblockType(
'The call to strtolower is unnecessary given the docblock type',
new CodeLocation($statements_analyzer, $function_name)
),
$statements_analyzer->getSuppressedIssues()
);
} else {
IssueBuffer::maybeAdd(
new RedundantCast(
new RedundantFunctionCall(
'The call to strtolower is unnecessary',
new CodeLocation($statements_analyzer, $function_name)
),
Expand Down
9 changes: 9 additions & 0 deletions src/Psalm/Issue/RedundantFunctionCall.php
@@ -0,0 +1,9 @@
<?php

namespace Psalm\Issue;

class RedundantFunctionCall extends CodeIssue
{
public const ERROR_LEVEL = 4;
public const SHORTCODE = 280;
}
9 changes: 9 additions & 0 deletions src/Psalm/Issue/RedundantFunctionCallGivenDocblockType.php
@@ -0,0 +1,9 @@
<?php

namespace Psalm\Issue;

class RedundantFunctionCallGivenDocblockType extends CodeIssue
{
public const ERROR_LEVEL = 2;
public const SHORTCODE = 281;
}
4 changes: 2 additions & 2 deletions tests/ArrayAssignmentTest.php
Expand Up @@ -835,7 +835,7 @@ public function offsetSet($offset, $value): void
'keyedIntOffsetArrayValues' => [
'<?php
$a = ["hello", 5];
/** @psalm-suppress RedundantCast */
/** @psalm-suppress RedundantFunctionCall */
$a_values = array_values($a);
$a_keys = array_keys($a);',
'assertions' => [
Expand Down Expand Up @@ -2062,7 +2062,7 @@ function baz(array $bar) : void {
function foo(array $a) : array {
return array_values($a);
}',
'error_message' => 'RedundantCast',
'error_message' => 'RedundantFunctionCall',
],
'assignToListWithUpdatedForeachKey' => [
'<?php
Expand Down
2 changes: 1 addition & 1 deletion tests/Template/ClassTemplateTest.php
Expand Up @@ -2289,7 +2289,7 @@ public function __construct(array $elements) {
* @return static<U>
*/
public function map(callable $callback) {
/** @psalm-suppress RedundantCast */
/** @psalm-suppress RedundantFunctionCall */
return new static(array_values(array_map($callback, $this->elements)));
}
}
Expand Down