diff --git a/CHANGELOG.md b/CHANGELOG.md index 25a2d5832..2fe576532 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Ensure pycodestyle W503, line break before binary operator, is disabled (regression from 2.6.2). - Fix `check_contracts` typings so PyCharm static checking will work +- Fix `invalid-range-index` bug where valid range calls were flagged as invalid ## [2.6.2] - 2023-09-22 diff --git a/README.md b/README.md index 92be1e086..7d0720d8d 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ included as a library). In the Python interpreter, try running: Lorena Buciu, Simon Chen, Freeman Cheng, +Ivan Chepelev, Nigel Fong, Adam Gleizer, Ibrahim Hasan, diff --git a/python_ta/checkers/invalid_range_index_checker.py b/python_ta/checkers/invalid_range_index_checker.py index a1ac47cfc..66f8eb5f8 100644 --- a/python_ta/checkers/invalid_range_index_checker.py +++ b/python_ta/checkers/invalid_range_index_checker.py @@ -25,31 +25,36 @@ def visit_call(self, node: nodes.Call) -> None: # ignore the name if it's not a builtin (i.e. not defined in the # locals nor globals scope) if not (name in node.frame() or name in node.root()) and name == "range": - arg = node.args # the arguments of 'range' call + args = node.args # the arguments of 'range' call # guard nodes (e.g. Name) not properly handled by literal_eval. - if any([not isinstance(item, nodes.Const) for item in arg]): + if any([not isinstance(arg, (nodes.Const, nodes.UnaryOp)) for arg in args]): return - eval_parm = list(map(lambda z: literal_eval(z.as_string()), arg)) - # check if there is no args in 'range' call + eval_params = list(map(lambda z: literal_eval(z.as_string()), args)) + if ( - len(arg) == 0 - or not all([isinstance(c, int) for c in eval_parm]) - or (len(arg) == 1 and eval_parm[0] < 2) - or (len(arg) == 2 and eval_parm[1] - eval_parm[0] < 2) + len(args) == 0 + or len(args) > 3 + or not all([isinstance(c, int) for c in eval_params]) ): - args = "{}".format(node.lineno) - self.add_message("invalid-range-index", node=node, args=args) - - if len(arg) == 3: - if ( - abs(eval_parm[2]) >= abs(eval_parm[0] - eval_parm[1]) - or eval_parm[2] == 0 - or (eval_parm[0] > eval_parm[1] and eval_parm[2] < 0) - or (eval_parm[0] < eval_parm[1] and eval_parm[2] > 0) - ): - args = "{}".format(node.lineno) - self.add_message("invalid-range-index", node=node, args=args) + self.add_message("invalid-range-index", node=node, args=str(node.lineno)) + return + + # set positional and default arguments of range + start = eval_params[0] if len(args) > 1 else 0 + stop = eval_params[0] if len(args) == 1 else eval_params[1] + step = eval_params[2] if len(args) == 3 else 1 + + if not is_valid_range(start, stop, step): + self.add_message("invalid-range-index", node=node, args=str(node.lineno)) + + +def is_valid_range(start: int, stop: int, step: int) -> bool: + """Returns True if a range call with three arguments is valid. + We consider a range to be valid if it has more than one element.""" + if step == 0: + return False + return (stop - start) / step > 1 def register(linter: PyLinter) -> None: diff --git a/tests/test_custom_checkers/test_invalid_range_index_checker.py b/tests/test_custom_checkers/test_invalid_range_index_checker.py new file mode 100644 index 000000000..3c1f041e8 --- /dev/null +++ b/tests/test_custom_checkers/test_invalid_range_index_checker.py @@ -0,0 +1,121 @@ +import astroid +import pylint.testutils + +from python_ta.checkers.invalid_range_index_checker import InvalidRangeIndexChecker + + +class TestInvalidRangeIndexChecker(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = InvalidRangeIndexChecker + + def test_valid_range_one_arg(self): + src = """ + range(10) + """ + range_node = astroid.extract_node(src) + with self.assertNoMessages(): + self.checker.visit_call(range_node) + + def test_valid_range_two_arg(self): + src = """ + range(2, 8) + """ + range_node = astroid.extract_node(src) + with self.assertNoMessages(): + self.checker.visit_call(range_node) + + def test_valid_range_three_arg(self): + src = """ + range(2, 8, 2) + """ + range_node = astroid.extract_node(src) + with self.assertNoMessages(): + self.checker.visit_call(range_node) + + def test_valid_range_three_arg_negative(self): + src = """ + range(-10, -20, -2) + """ + range_node = astroid.extract_node(src) + with self.assertNoMessages(): + self.checker.visit_call(range_node) + + def test_invalid_range_empty(self): + src = """ + range() + """ + range_node = astroid.extract_node(src) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="invalid-range-index", + node=range_node, + args="2", + ), + ignore_position=True, + ): + self.checker.visit_call(range_node) + + def test_invalid_range_one_arg(self): + src = """ + range(-10) + """ + range_node = astroid.extract_node(src) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="invalid-range-index", + node=range_node, + args="2", + ), + ignore_position=True, + ): + self.checker.visit_call(range_node) + + def test_invalid_range_two_arg(self): + src = """ + range(5, 6) + """ + range_node = astroid.extract_node(src) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="invalid-range-index", + node=range_node, + args="2", + ), + ignore_position=True, + ): + self.checker.visit_call(range_node) + + def test_invalid_range_three_arg(self): + src = """ + range(2, 15, 20) + """ + range_node = astroid.extract_node(src) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="invalid-range-index", + node=range_node, + args="2", + ), + ignore_position=True, + ): + self.checker.visit_call(range_node) + + def test_invalid_range_zero_step(self): + src = """ + range(1, 10, 0) + """ + range_node = astroid.extract_node(src) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="invalid-range-index", + node=range_node, + args="2", + ), + ignore_position=True, + ): + self.checker.visit_call(range_node) + + +if __name__ == "__main__": + import pytest + + pytest.main(["test_invalid_range_index_checker.py"])