Skip to content

Commit ab742ae

Browse files
committed
Closes #862
1 parent abbcdaf commit ab742ae

File tree

6 files changed

+121
-1
lines changed

6 files changed

+121
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ We used to have incremental versioning before `0.1.0`.
3838
- Enforces to use `.items()` in loops
3939
- Enforces using `.get()` over `key in dict` checks
4040
- Forbids to use and declare `float` keys in arrays and dictionaries
41+
- Forbids to use `a[len(a) - 1]` because it is just `a[-1]`
4142

4243
### Bugfixes
4344

tests/fixtures/noqa.py

+1
Original file line numberDiff line numberDiff line change
@@ -618,3 +618,4 @@ def consecutive_yields():
618618
if 'key' in some_dict:
619619
print(some_dict['key']) # noqa: WPS529
620620
print(other_dict[1.0]) # noqa: WPS449
621+
print(some_sized[len(some_sized) - 2]) # noqa: WPS530

tests/test_checker/test_noqa.py

+1
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@
214214
'WPS527': 1,
215215
'WPS528': 1,
216216
'WPS529': 1,
217+
'WPS530': 1,
217218

218219
'WPS600': 1,
219220
'WPS601': 1,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# -*- coding: utf-8 -*-
2+
3+
import pytest
4+
5+
from wemake_python_styleguide.violations.refactoring import (
6+
ImplicitNegativeIndexViolation,
7+
)
8+
from wemake_python_styleguide.visitors.ast.subscripts import CorrectKeyVisitor
9+
10+
11+
@pytest.mark.parametrize('code', [
12+
'some_list[len(some_list) - 1]',
13+
'some_list[len(some_list) - 1.0]',
14+
'some_list[len(some_list) - 5]',
15+
'some_list[len(some_list) - name]',
16+
'some_list[len(some_list) - call()]',
17+
'some_list[len(some_list) - name.attr]',
18+
'some_list[len(some_list) - name["a"]]',
19+
'attr.some_list[len(attr.some_list) - 1]',
20+
])
21+
def test_implicit_negative_index(
22+
assert_errors,
23+
parse_ast_tree,
24+
code,
25+
default_options,
26+
):
27+
"""Testing that implicit negative indexes are forbidden."""
28+
tree = parse_ast_tree(code)
29+
30+
visitor = CorrectKeyVisitor(default_options, tree=tree)
31+
visitor.run()
32+
33+
assert_errors(visitor, [ImplicitNegativeIndexViolation])
34+
35+
36+
@pytest.mark.parametrize('code', [
37+
'some_list[len(other_list) - 1]',
38+
'some_list[len(some_list) + 1]',
39+
'some_list[len(some_list)]',
40+
'some_list[sum(some_list) + 1]',
41+
'some_list[len(some_list) + 1 + 2]',
42+
'some_list[-1]',
43+
'some_list[1]',
44+
'some_list[-name]',
45+
'some_list[name.attr]',
46+
'some_list[call() + 1]',
47+
'some_list[some[key]]',
48+
])
49+
def test_regular_len_calls(
50+
assert_errors,
51+
parse_ast_tree,
52+
code,
53+
default_options,
54+
):
55+
"""Testing that explicit negative calls are fine."""
56+
tree = parse_ast_tree(code)
57+
58+
visitor = CorrectKeyVisitor(default_options, tree=tree)
59+
visitor.run()
60+
61+
assert_errors(visitor, [])

wemake_python_styleguide/violations/refactoring.py

+31
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
NotATupleArgumentViolation
4646
ImplicitItemsIteratorViolation
4747
ImplicitDictGetViolation
48+
ImplicitNegativeIndexViolation
4849
4950
Refactoring opportunities
5051
-------------------------
@@ -79,6 +80,7 @@
7980
.. autoclass:: NotATupleArgumentViolation
8081
.. autoclass:: ImplicitItemsIteratorViolation
8182
.. autoclass:: ImplicitDictGetViolation
83+
.. autoclass:: ImplicitNegativeIndexViolation
8284
8385
"""
8486

@@ -1149,3 +1151,32 @@ class ImplicitDictGetViolation(ASTViolation):
11491151

11501152
error_template = 'Found implicit `.get()` dict usage'
11511153
code = 529
1154+
1155+
1156+
@final
1157+
class ImplicitNegativeIndexViolation(ASTViolation):
1158+
"""
1159+
Forbids to use implicit negative indexes.
1160+
1161+
Reasoning:
1162+
There's no need in getting the length of an iterable
1163+
and then having a negative offset,
1164+
when you can specify negative indexes in the first place.
1165+
1166+
Solution:
1167+
Use negative indexes.
1168+
1169+
Example::
1170+
1171+
# Correct:
1172+
some_list[-1]
1173+
1174+
# Wrong:
1175+
some_list[len(some_list) - 1]
1176+
1177+
.. versionadded:: 0.13.0
1178+
1179+
"""
1180+
1181+
error_template = 'Found implicit negative index'
1182+
code = 530

wemake_python_styleguide/visitors/ast/subscripts.py

+26-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from typing_extensions import final
66

7-
from wemake_python_styleguide.logic import operators, slices, source
7+
from wemake_python_styleguide.logic import functions, operators, slices, source
88
from wemake_python_styleguide.violations import (
99
best_practices,
1010
consistency,
@@ -113,9 +113,11 @@ def visit_Subscript(self, node: ast.Subscript) -> None:
113113
114114
Raises:
115115
FloatKeyViolation
116+
ImplicitNegativeIndexViolation
116117
117118
"""
118119
self._check_float_key(node)
120+
self._check_len_call(node)
119121
self.generic_visit(node)
120122

121123
def _check_float_key(self, node: ast.Subscript) -> None:
@@ -127,6 +129,29 @@ def _check_float_key(self, node: ast.Subscript) -> None:
127129
if is_float_key:
128130
self.add_violation(best_practices.FloatKeyViolation(node))
129131

132+
def _check_len_call(self, node: ast.Subscript) -> None:
133+
is_len_call = (
134+
isinstance(node.slice, ast.Index) and
135+
isinstance(node.slice.value, ast.BinOp) and
136+
isinstance(node.slice.value.op, ast.Sub) and
137+
self._is_wrong_len(
138+
node.slice.value,
139+
source.node_to_string(node.value),
140+
)
141+
)
142+
143+
if is_len_call:
144+
self.add_violation(
145+
refactoring.ImplicitNegativeIndexViolation(node),
146+
)
147+
148+
def _is_wrong_len(self, node: ast.BinOp, element: str) -> bool:
149+
return (
150+
isinstance(node.left, ast.Call) and
151+
bool(functions.given_function_called(node.left, {'len'})) and
152+
source.node_to_string(node.left.args[0]) == element
153+
)
154+
130155
def _is_float_key(self, node: ast.Index) -> bool:
131156
real_node = operators.unwrap_unary_node(node.value)
132157
return (

0 commit comments

Comments
 (0)