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

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

Merged
merged 2 commits into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/data/messages/i/invalid-slice-step/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
LETTERS = ["a", "b", "c", "d"]

LETTERS[::0] # [invalid-slice-step]
3 changes: 3 additions & 0 deletions doc/data/messages/i/invalid-slice-step/good.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
LETTERS = ["a", "b", "c", "d"]

LETTERS[::2]
3 changes: 3 additions & 0 deletions doc/user_guide/checkers/features.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,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*
Expand Down
1 change: 1 addition & 0 deletions doc/user_guide/messages/messages_overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/7762.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Extend ``invalid-slice-index`` to emit an warning for invalid slice indices
used with string and byte sequences.

Refs #7762
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/7762.new_check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Add ``invalid-slice-step`` check to warn about a slice step value of ``0``
for common builtin sequences.

Refs #7762
23 changes: 20 additions & 3 deletions pylint/checkers/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1799,7 +1805,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
Expand All @@ -1819,11 +1829,17 @@ 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"}
):
# 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:
Expand Down Expand Up @@ -2080,6 +2096,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)
Expand Down
23 changes: 20 additions & 3 deletions tests/functional/i/invalid/invalid_slice_index.py
Original file line number Diff line number Diff line change
@@ -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]

Expand All @@ -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"""
Expand All @@ -22,10 +25,24 @@ 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


# 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"""
Expand Down
16 changes: 13 additions & 3 deletions tests/functional/i/invalid/invalid_slice_index.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
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