Merge pull request #7019 from zoonru/no_ksort_list

vimeo · Jan 2, 2022 · 36d5a2a · 36d5a2a
2 parents 697db76 + 546438b
commit 36d5a2a
Show file tree

Hide file tree

Showing 11 changed files with 73 additions and 12 deletions.
diff --git a/config.xsd b/config.xsd
@@ -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" />

diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md
@@ -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)
@@ -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)

diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
@@ -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)

diff --git a/docs/running_psalm/issues/RedundantFunctionCall.md b/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]);
+```
diff --git a/docs/running_psalm/issues/RedundantFunctionCallGivenDocblockType .md b/docs/running_psalm/issues/RedundantFunctionCallGivenDocblockType .md
@@ -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;
+}
+```
diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -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';
         }

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php
@@ -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;
@@ -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
@@ -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()
@@ -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)
                         ),

diff --git a/src/Psalm/Issue/RedundantFunctionCall.php b/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;
+}
diff --git a/src/Psalm/Issue/RedundantFunctionCallGivenDocblockType.php b/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;
+}
diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php
@@ -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' => [
@@ -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

diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php
@@ -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)));
                         }
                     }