From 38e548159e3eedb35e38a0d67359bd4a821f39b8 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Tue, 9 Jul 2024 18:20:21 -0400 Subject: [PATCH 1/4] Revert "Remove `--min_pyver_end_position`" This reverts commit 4b45192113654c9403c288824bff1ff01c29e061. --- .../contributor_guide/tests/writing_test.rst | 1 + doc/whatsnew/fragments/9774.other | 2 -- pylint/testutils/functional/test_file.py | 4 ++++ pylint/testutils/lint_module_test.py | 3 +++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/doc/development_guide/contributor_guide/tests/writing_test.rst b/doc/development_guide/contributor_guide/tests/writing_test.rst index 9a65994ab7..9ce9ca1f0d 100644 --- a/doc/development_guide/contributor_guide/tests/writing_test.rst +++ b/doc/development_guide/contributor_guide/tests/writing_test.rst @@ -61,6 +61,7 @@ test runner. The following options are currently supported: - "min_pyver": Minimal python version required to run the test - "max_pyver": Python version from which the test won't be run. If the last supported version is 3.9 this setting should be set to 3.10. +- "min_pyver_end_position": Minimal python version required to check the end_line and end_column attributes of the message - "requires": Packages required to be installed locally to run the test - "except_implementations": List of python implementations on which the test should not run - "exclude_platforms": List of operating systems on which the test should not run diff --git a/doc/whatsnew/fragments/9774.other b/doc/whatsnew/fragments/9774.other index 52d2f502c3..fd536768c1 100644 --- a/doc/whatsnew/fragments/9774.other +++ b/doc/whatsnew/fragments/9774.other @@ -1,6 +1,4 @@ Remove support for launching pylint with Python 3.8. Code that supports Python 3.8 can still be linted with the ``--py-version=3.8`` setting. -``--min_pyver_end_position`` in the functional test runner is no longer relevant and is removed. - Refs #9774 diff --git a/pylint/testutils/functional/test_file.py b/pylint/testutils/functional/test_file.py index 8b83e38485..37ba3a5fc6 100644 --- a/pylint/testutils/functional/test_file.py +++ b/pylint/testutils/functional/test_file.py @@ -22,6 +22,7 @@ class NoFileError(Exception): class TestFileOptions(TypedDict): min_pyver: tuple[int, ...] max_pyver: tuple[int, ...] + min_pyver_end_position: tuple[int, ...] requires: list[str] except_implementations: list[str] exclude_platforms: list[str] @@ -32,6 +33,7 @@ class TestFileOptions(TypedDict): POSSIBLE_TEST_OPTIONS = { "min_pyver", "max_pyver", + "min_pyver_end_position", "requires", "except_implementations", "exclude_platforms", @@ -45,6 +47,7 @@ class FunctionalTestFile: _CONVERTERS: dict[str, Callable[[str], tuple[int, ...] | list[str]]] = { "min_pyver": parse_python_version, "max_pyver": parse_python_version, + "min_pyver_end_position": parse_python_version, "requires": lambda s: [i.strip() for i in s.split(",")], "except_implementations": lambda s: [i.strip() for i in s.split(",")], "exclude_platforms": lambda s: [i.strip() for i in s.split(",")], @@ -58,6 +61,7 @@ def __init__(self, directory: str, filename: str) -> None: self.options: TestFileOptions = { "min_pyver": (2, 5), "max_pyver": (4, 0), + "min_pyver_end_position": (3, 8), "requires": [], "except_implementations": [], "exclude_platforms": [], diff --git a/pylint/testutils/lint_module_test.py b/pylint/testutils/lint_module_test.py index 04615f6d5a..522f8f1cfb 100644 --- a/pylint/testutils/lint_module_test.py +++ b/pylint/testutils/lint_module_test.py @@ -79,6 +79,9 @@ def __init__( self._linter._arg_parser.add_argument( "--max_pyver", type=parse_python_version, default=(4, 0) ) + self._linter._arg_parser.add_argument( + "--min_pyver_end_position", type=parse_python_version, default=(3, 8) + ) self._linter._arg_parser.add_argument( "--requires", type=lambda s: [i.strip() for i in s.split(",")], default=[] ) From 3092b422d41a978ef78296894a47e6386867ca96 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 10 Jul 2024 07:30:40 -0400 Subject: [PATCH 2/4] Revert "Simplify testutils" This reverts commit 833762fd6f3326d61b897ffa2dab6086b8c8edc4. --- pylint/testutils/lint_module_test.py | 20 ++++++++-- pylint/testutils/output_line.py | 33 ++++++++++++---- tests/testutils/test_output_line.py | 57 +++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/pylint/testutils/lint_module_test.py b/pylint/testutils/lint_module_test.py index 522f8f1cfb..48ee5a0b2f 100644 --- a/pylint/testutils/lint_module_test.py +++ b/pylint/testutils/lint_module_test.py @@ -11,13 +11,15 @@ from collections import Counter from io import StringIO from pathlib import Path -from typing import TextIO +from typing import Counter as CounterType +from typing import TextIO, Tuple import pytest from _pytest.config import Config from pylint import checkers from pylint.config.config_initialization import _config_initialization +from pylint.constants import IS_PYPY from pylint.lint import PyLinter from pylint.message.message import Message from pylint.testutils.constants import _EXPECTED_RE, _OPERATORS, UPDATE_OPTION @@ -31,7 +33,7 @@ from pylint.testutils.output_line import OutputLine from pylint.testutils.reporter_for_tests import FunctionalTestReporter -MessageCounter = Counter[tuple[int, str]] +MessageCounter = CounterType[Tuple[int, str]] PYLINTRC = Path(__file__).parent / "testing_pylintrc" @@ -103,6 +105,13 @@ def __init__( self._linter, args_list=args, config_file=rc_file, reporter=_test_reporter ) + self._check_end_position = ( + sys.version_info >= self._linter.config.min_pyver_end_position + ) + # TODO: PY3.9: PyPy supports end_lineno from 3.9 and above + if self._check_end_position and IS_PYPY: + self._check_end_position = sys.version_info >= (3, 9) # pragma: no cover + self._config = config def setUp(self) -> None: @@ -218,7 +227,8 @@ def _get_expected(self) -> tuple[MessageCounter, list[OutputLine]]: expected_msgs = Counter() with self._open_expected_file() as f: expected_output_lines = [ - OutputLine.from_csv(row) for row in csv.reader(f, "test") + OutputLine.from_csv(row, self._check_end_position) + for row in csv.reader(f, "test") ] return expected_msgs, expected_output_lines @@ -232,7 +242,9 @@ def _get_actual(self) -> tuple[MessageCounter, list[OutputLine]]: msg.symbol != "fatal" ), f"Pylint analysis failed because of '{msg.msg}'" received_msgs[msg.line, msg.symbol] += 1 - received_output_lines.append(OutputLine.from_msg(msg)) + received_output_lines.append( + OutputLine.from_msg(msg, self._check_end_position) + ) return received_msgs, received_output_lines def _runTest(self) -> None: diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index 3146c12c78..c979a049c3 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -5,13 +5,15 @@ from __future__ import annotations from collections.abc import Sequence -from typing import Any, NamedTuple +from typing import Any, NamedTuple, TypeVar from astroid import nodes from pylint.interfaces import UNDEFINED, Confidence from pylint.message.message import Message +_T = TypeVar("_T") + class MessageTest(NamedTuple): msg_id: str @@ -39,15 +41,17 @@ class OutputLine(NamedTuple): confidence: str @classmethod - def from_msg(cls, msg: Message) -> OutputLine: + def from_msg(cls, msg: Message, check_endline: bool = True) -> OutputLine: """Create an OutputLine from a Pylint Message.""" column = cls._get_column(msg.column) + end_line = cls._get_py38_none_value(msg.end_line, check_endline) + end_column = cls._get_py38_none_value(msg.end_column, check_endline) return cls( msg.symbol, msg.line, column, - msg.end_line, - msg.end_column, + end_line, + end_column, msg.obj or "", msg.msg.replace("\r\n", "\n"), msg.confidence.name, @@ -58,8 +62,19 @@ def _get_column(column: str | int) -> int: """Handle column numbers.""" return int(column) + @staticmethod + def _get_py38_none_value(value: _T, check_endline: bool) -> _T | None: + """Used to make end_line and end_column None as indicated by our version + compared to `min_pyver_end_position`. + """ + if not check_endline: + return None # pragma: no cover + return value + @classmethod - def from_csv(cls, row: Sequence[str] | str) -> OutputLine: + def from_csv( + cls, row: Sequence[str] | str, check_endline: bool = True + ) -> OutputLine: """Create an OutputLine from a comma separated list (the functional tests expected output .txt files). """ @@ -68,8 +83,12 @@ def from_csv(cls, row: Sequence[str] | str) -> OutputLine: try: line = int(row[1]) column = cls._get_column(row[2]) - end_line = cls._value_to_optional_int(row[3]) - end_column = cls._value_to_optional_int(row[4]) + end_line = cls._value_to_optional_int( + cls._get_py38_none_value(row[3], check_endline) + ) + end_column = cls._value_to_optional_int( + cls._get_py38_none_value(row[4], check_endline) + ) # symbol, line, column, end_line, end_column, node, msg, confidences assert len(row) == 8 return cls( diff --git a/tests/testutils/test_output_line.py b/tests/testutils/test_output_line.py index 07be2f2411..faffc8110f 100644 --- a/tests/testutils/test_output_line.py +++ b/tests/testutils/test_output_line.py @@ -70,13 +70,33 @@ def test_output_line_from_message(message: _MessageCallable) -> None: assert output_line.msg == "msg" assert output_line.confidence == "HIGH" + output_line_with_end = OutputLine.from_msg(message(), True) + assert output_line_with_end.symbol == "missing-docstring" + assert output_line_with_end.lineno == 1 + assert output_line_with_end.column == 2 + assert output_line_with_end.end_lineno == 1 + assert output_line_with_end.end_column == 3 + assert output_line_with_end.object == "obj" + assert output_line_with_end.msg == "msg" + assert output_line_with_end.confidence == "HIGH" + + output_line_without_end = OutputLine.from_msg(message(), False) + assert output_line_without_end.symbol == "missing-docstring" + assert output_line_without_end.lineno == 1 + assert output_line_without_end.column == 2 + assert output_line_without_end.end_lineno is None + assert output_line_without_end.end_column is None + assert output_line_without_end.object == "obj" + assert output_line_without_end.msg == "msg" + assert output_line_without_end.confidence == "HIGH" + @pytest.mark.parametrize("confidence", [HIGH, INFERENCE]) def test_output_line_to_csv(confidence: Confidence, message: _MessageCallable) -> None: """Test that the OutputLine NamedTuple is instantiated correctly with from_msg and then converted to csv. """ - output_line = OutputLine.from_msg(message(confidence)) + output_line = OutputLine.from_msg(message(confidence), True) csv = output_line.to_csv() assert csv == ( "missing-docstring", @@ -89,6 +109,19 @@ def test_output_line_to_csv(confidence: Confidence, message: _MessageCallable) - confidence.name, ) + output_line_without_end = OutputLine.from_msg(message(confidence), False) + csv = output_line_without_end.to_csv() + assert csv == ( + "missing-docstring", + "1", + "2", + "None", + "None", + "obj", + "msg", + confidence.name, + ) + def test_output_line_from_csv() -> None: """Test that the OutputLine NamedTuple is instantiated correctly with from_csv. @@ -107,3 +140,25 @@ def test_output_line_from_csv() -> None: msg="msg", confidence="HIGH", ) + output_line_with_end = OutputLine.from_csv(proper_csv, True) + assert output_line_with_end == OutputLine( + symbol="missing-docstring", + lineno=1, + column=2, + end_lineno=1, + end_column=None, + object="obj", + msg="msg", + confidence="HIGH", + ) + output_line_without_end = OutputLine.from_csv(proper_csv, False) + assert output_line_without_end == OutputLine( + symbol="missing-docstring", + lineno=1, + column=2, + end_lineno=None, + end_column=None, + object="obj", + msg="msg", + confidence="HIGH", + ) From 01c7f939ed4bf6cff6dbcc6073e87f670999505e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 10 Jul 2024 07:32:46 -0400 Subject: [PATCH 3/4] Remove PyPy special case, typing fixes --- pylint/testutils/lint_module_test.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pylint/testutils/lint_module_test.py b/pylint/testutils/lint_module_test.py index 48ee5a0b2f..37839c8908 100644 --- a/pylint/testutils/lint_module_test.py +++ b/pylint/testutils/lint_module_test.py @@ -11,15 +11,13 @@ from collections import Counter from io import StringIO from pathlib import Path -from typing import Counter as CounterType -from typing import TextIO, Tuple +from typing import TextIO import pytest from _pytest.config import Config from pylint import checkers from pylint.config.config_initialization import _config_initialization -from pylint.constants import IS_PYPY from pylint.lint import PyLinter from pylint.message.message import Message from pylint.testutils.constants import _EXPECTED_RE, _OPERATORS, UPDATE_OPTION @@ -33,7 +31,7 @@ from pylint.testutils.output_line import OutputLine from pylint.testutils.reporter_for_tests import FunctionalTestReporter -MessageCounter = CounterType[Tuple[int, str]] +MessageCounter = Counter[tuple[int, str]] PYLINTRC = Path(__file__).parent / "testing_pylintrc" @@ -108,9 +106,6 @@ def __init__( self._check_end_position = ( sys.version_info >= self._linter.config.min_pyver_end_position ) - # TODO: PY3.9: PyPy supports end_lineno from 3.9 and above - if self._check_end_position and IS_PYPY: - self._check_end_position = sys.version_info >= (3, 9) # pragma: no cover self._config = config From 797e273055e9f2557b11bb8813b89ada59a5d2d7 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 10 Jul 2024 17:26:58 -0400 Subject: [PATCH 4/4] Rename method to avoid py38 reference --- pylint/testutils/output_line.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index c979a049c3..fbe5b3bbce 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -44,8 +44,8 @@ class OutputLine(NamedTuple): def from_msg(cls, msg: Message, check_endline: bool = True) -> OutputLine: """Create an OutputLine from a Pylint Message.""" column = cls._get_column(msg.column) - end_line = cls._get_py38_none_value(msg.end_line, check_endline) - end_column = cls._get_py38_none_value(msg.end_column, check_endline) + end_line = cls._get_end_line_and_end_col(msg.end_line, check_endline) + end_column = cls._get_end_line_and_end_col(msg.end_column, check_endline) return cls( msg.symbol, msg.line, @@ -63,7 +63,7 @@ def _get_column(column: str | int) -> int: return int(column) @staticmethod - def _get_py38_none_value(value: _T, check_endline: bool) -> _T | None: + def _get_end_line_and_end_col(value: _T, check_endline: bool) -> _T | None: """Used to make end_line and end_column None as indicated by our version compared to `min_pyver_end_position`. """ @@ -84,10 +84,10 @@ def from_csv( line = int(row[1]) column = cls._get_column(row[2]) end_line = cls._value_to_optional_int( - cls._get_py38_none_value(row[3], check_endline) + cls._get_end_line_and_end_col(row[3], check_endline) ) end_column = cls._value_to_optional_int( - cls._get_py38_none_value(row[4], check_endline) + cls._get_end_line_and_end_col(row[4], check_endline) ) # symbol, line, column, end_line, end_column, node, msg, confidences assert len(row) == 8