Improve invalid-slice-index and add invalid-slice-step (#7762)

Author: cdce8p

doc/data/messages/i/invalid-slice-step/bad.py

@@ -0,0 +1,3 @@
+LETTERS = ["a", "b", "c", "d"]
+
+LETTERS[::0]  # [invalid-slice-step]

doc/data/messages/i/invalid-slice-step/good.py

@@ -0,0 +1,3 @@
+LETTERS = ["a", "b", "c", "d"]
+
+LETTERS[::2]

doc/user_guide/checkers/features.rst

@@ -1155,6 +1155,9 @@ Typecheck checker Messages
 :invalid-slice-index (E1127): *Slice index is not an int, None, or instance with __index__*
   Used when a slice index is not an integer, None, or an object with an
   __index__ method.
+:invalid-slice-step (E1144): *Slice step cannot be 0*
+  Used when a slice step is 0 and the object doesn't implement a custom
+  __getitem__ method.
 :too-many-function-args (E1121): *Too many positional arguments for %s call*
   Used when a function call passes too many positional arguments.
 :unexpected-keyword-arg (E1123): *Unexpected keyword argument %r in %s call*

doc/user_guide/messages/messages_overview.rst

@@ -102,6 +102,7 @@ All messages in the error category:
    error/invalid-repr-returned
    error/invalid-sequence-index
    error/invalid-slice-index
+   error/invalid-slice-step
    error/invalid-slots
    error/invalid-slots-object
    error/invalid-star-assignment-target

doc/whatsnew/fragments/7762.false_negative

@@ -0,0 +1,4 @@
+Extend ``invalid-slice-index`` to emit an warning for invalid slice indices
+used with string and byte sequences, and range objects.
+
+Refs #7762

doc/whatsnew/fragments/7762.new_check

@@ -0,0 +1,4 @@
+Add ``invalid-slice-step`` check to warn about a slice step value of ``0``
+for common builtin sequences.
+
+Refs #7762

pylint/checkers/typecheck.py

@@ -48,7 +48,7 @@
     supports_membership_test,
     supports_setitem,
 )
-from pylint.interfaces import INFERENCE
+from pylint.interfaces import HIGH, INFERENCE
 from pylint.typing import MessageDefinitionTuple
 
 if sys.version_info >= (3, 8):
@@ -374,6 +374,12 @@ def _missing_member_hint(
         "(i.e. doesn't define __hash__ method).",
         {"old_names": [("E1140", "unhashable-dict-key")]},
     ),
+    "E1144": (
+        "Slice step cannot be 0",
+        "invalid-slice-step",
+        "Used when a slice step is 0 and the object doesn't implement "
+        "a custom __getitem__ method.",
+    ),
     "W1113": (
         "Keyword argument before variable positional arguments list "
         "in the definition of %s function",
@@ -1785,7 +1791,11 @@ def _check_invalid_slice_index(self, node: nodes.Slice) -> None:
                     pass
             invalid_slices_nodes.append(index)
 
-        if not invalid_slices_nodes:
+        invalid_slice_step = (
+            node.step and isinstance(node.step, nodes.Const) and node.step.value == 0
+        )
+
+        if not (invalid_slices_nodes or invalid_slice_step):
             return
 
         # Anything else is an error, unless the object that is indexed
@@ -1803,11 +1813,19 @@ def _check_invalid_slice_index(self, node: nodes.Slice) -> None:
                 astroid.objects.FrozenSet,
                 nodes.Set,
             )
-            if not isinstance(inferred, known_objects):
+            if not (
+                isinstance(inferred, known_objects)
+                or isinstance(inferred, nodes.Const)
+                and inferred.pytype() in {"builtins.str", "builtins.bytes"}
+                or isinstance(inferred, astroid.bases.Instance)
+                and inferred.pytype() == "builtins.range"
+            ):
                 # Might be an instance that knows how to handle this slice object
                 return
         for snode in invalid_slices_nodes:
             self.add_message("invalid-slice-index", node=snode)
+        if invalid_slice_step:
+            self.add_message("invalid-slice-step", node=node.step, confidence=HIGH)
 
     @only_required_for_messages("not-context-manager")
     def visit_with(self, node: nodes.With) -> None:
@@ -2064,6 +2082,7 @@ def visit_set(self, node: nodes.Set) -> None:
         "unhashable-member",
         "invalid-sequence-index",
         "invalid-slice-index",
+        "invalid-slice-step",
     )
     def visit_subscript(self, node: nodes.Subscript) -> None:
         self._check_invalid_sequence_index(node)

tests/functional/i/invalid/invalid_slice_index.py

@@ -1,6 +1,6 @@
 """Errors for invalid slice indices"""
 # pylint: disable=too-few-public-methods,missing-docstring,expression-not-assigned,unnecessary-pass
-
+# pylint: disable=pointless-statement
 
 TESTLIST = [1, 2, 3]
 
@@ -11,7 +11,10 @@ def function1():
 
 def function2():
     """strings used as indices"""
-    return TESTLIST['0':'1':]  # [invalid-slice-index,invalid-slice-index]
+    TESTLIST['0':'1':]  # [invalid-slice-index,invalid-slice-index]
+    ()['0':'1']  # [invalid-slice-index,invalid-slice-index]
+    ""["a":"z"]  # [invalid-slice-index,invalid-slice-index]
+    b""["a":"z"]  # [invalid-slice-index,invalid-slice-index]
 
 def function3():
     """class without __index__ used as index"""
@@ -22,10 +25,27 @@ class NoIndexTest:
 
     return TESTLIST[NoIndexTest()::]  # [invalid-slice-index]
 
+def invalid_step():
+    """0 is an invalid value for slice step with most builtin sequences."""
+    TESTLIST[::0]  # [invalid-slice-step]
+    [][::0]  # [invalid-slice-step]
+    ""[::0]  # [invalid-slice-step]
+    b""[::0]  # [invalid-slice-step]
+
+    class Custom:
+        def __getitem__(self, indices):
+            ...
+
+    Custom()[::0]   # no error -> custom __getitem__ method
+
+def invalid_slice_range():
+    range(5)['0':'1']  # [invalid-slice-index,invalid-slice-index]
+
+
 # Valid indices
 def function4():
     """integers used as indices"""
-    return TESTLIST[0:0:0] # no error
+    return TESTLIST[0:1:1]
 
 def function5():
     """None used as indices"""

tests/functional/i/invalid/invalid_slice_index.txt

@@ -1,5 +1,17 @@
 invalid-slice-index:10:20:10:22:function1:Slice index is not an int, None, or instance with __index__:UNDEFINED
 invalid-slice-index:10:23:10:25:function1:Slice index is not an int, None, or instance with __index__:UNDEFINED
-invalid-slice-index:14:20:14:23:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
-invalid-slice-index:14:24:14:27:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
-invalid-slice-index:23:20:23:33:function3:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:14:13:14:16:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:14:17:14:20:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:15:7:15:10:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:15:11:15:14:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:16:7:16:10:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:16:11:16:14:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:17:8:17:11:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:17:12:17:15:function2:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:26:20:26:33:function3:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-step:30:15:30:16:invalid_step:Slice step cannot be 0:HIGH
+invalid-slice-step:31:9:31:10:invalid_step:Slice step cannot be 0:HIGH
+invalid-slice-step:32:9:32:10:invalid_step:Slice step cannot be 0:HIGH
+invalid-slice-step:33:10:33:11:invalid_step:Slice step cannot be 0:HIGH
+invalid-slice-index:42:13:42:16:invalid_slice_range:Slice index is not an int, None, or instance with __index__:UNDEFINED
+invalid-slice-index:42:17:42:20:invalid_slice_range:Slice index is not an int, None, or instance with __index__:UNDEFINED