From 6235008784978ab75a0d2798e40d9b3baec69efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 21 Sep 2021 17:09:17 +0200 Subject: [PATCH 1/7] Refactor and typing of OutputLine --- pylint/testutils/output_line.py | 61 ++++++++++++++++++----------- tests/testutils/test_output_line.py | 27 +++++++------ 2 files changed, 53 insertions(+), 35 deletions(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index e82df9da70..ef30a3a8e5 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -2,10 +2,13 @@ # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE import collections -from typing import Any, NamedTuple +from typing import Any, Iterable, List, NamedTuple, Optional, Tuple, Union + +from astroid import nodes -from pylint import interfaces from pylint.constants import PY38_PLUS +from pylint.interfaces import HIGH, UNDEFINED, Confidence +from pylint.message.message import Message from pylint.testutils.constants import UPDATE_OPTION @@ -16,19 +19,30 @@ class MessageTest( ): """Used to test messages produced by pylint. Class name cannot start with Test as pytest doesn't allow constructors in test classes.""" - def __new__(cls, msg_id, line=None, node=None, args=None, confidence=None): + def __new__( + cls, + msg_id: str, + line: Optional[int] = None, + node: Optional[nodes.NodeNG] = None, + args: Any = None, + confidence: Optional[Confidence] = None, + ) -> "MessageTest": return tuple.__new__(cls, (msg_id, line, node, args, confidence)) - def __eq__(self, other): + def __eq__(self, other: object) -> bool: if isinstance(other, MessageTest): if self.confidence and other.confidence: return super().__eq__(other) - return self[:-1] == other[:-1] + return tuple(self[:-1]) == tuple(other[:-1]) return NotImplemented # pragma: no cover class MalformedOutputLineException(Exception): - def __init__(self, row, exception): + def __init__( + self, + row: Union[Tuple[str, ...], List[str], str], + exception: Exception, + ) -> None: example = "msg-symbolic-name:42:27:MyClass.my_function:The message" other_example = "msg-symbolic-name:7:42::The message" expected = [ @@ -62,13 +76,13 @@ def __init__(self, row, exception): class OutputLine(NamedTuple): symbol: str lineno: int - column: str - object: Any + column: int + object: str msg: str - confidence: interfaces.Confidence + confidence: str @classmethod - def from_msg(cls, msg): + def from_msg(cls, msg: Message) -> "OutputLine": column = cls.get_column(msg.column) return cls( msg.symbol, @@ -76,25 +90,28 @@ def from_msg(cls, msg): column, msg.obj or "", msg.msg.replace("\r\n", "\n"), - msg.confidence.name - if msg.confidence != interfaces.UNDEFINED - else interfaces.HIGH.name, + msg.confidence.name if msg.confidence != UNDEFINED else HIGH.name, ) @classmethod - def get_column(cls, column): - if not PY38_PLUS: - return "" # pragma: no cover - return str(column) + def get_column(cls, column: str) -> int: + if ( + not PY38_PLUS + ): # This mimicks behaviour of MessagesHandlerMixIn.add_one_message() + return 0 # pragma: no cover + return int(column) @classmethod - def from_csv(cls, row): + def from_csv(cls, row: Union[Tuple[str, ...], List[str], str]) -> "OutputLine": try: - confidence = row[5] if len(row) == 6 else interfaces.HIGH.name column = cls.get_column(row[2]) - return cls(row[0], int(row[1]), column, row[3], row[4], confidence) + if isinstance(row, Iterable) and len(row) == 6: + return cls(row[0], int(row[1]), column, row[3], row[4], row[5]) + if isinstance(row, Iterable) and len(row) == 5: + return cls(row[0], int(row[1]), column, row[3], row[4], HIGH.name) + raise IndexError except Exception as e: raise MalformedOutputLineException(row, e) from e - def to_csv(self): - return tuple(self) + def to_csv(self) -> Tuple[str, str, str, str, str, str]: + return tuple(str(i) for i in self) # type: ignore # pylint: disable=not-an-iterable diff --git a/tests/testutils/test_output_line.py b/tests/testutils/test_output_line.py index e4afa9e337..46135dd209 100644 --- a/tests/testutils/test_output_line.py +++ b/tests/testutils/test_output_line.py @@ -3,7 +3,7 @@ # pylint: disable=redefined-outer-name -from typing import Callable, List, Optional +from typing import Callable, Optional import pytest @@ -11,6 +11,7 @@ from pylint.interfaces import HIGH, INFERENCE, Confidence from pylint.message import Message from pylint.testutils.output_line import MalformedOutputLineException, OutputLine +from pylint.typing import MessageLocationTuple @pytest.fixture() @@ -19,14 +20,14 @@ def inner(confidence: Confidence = HIGH) -> Message: return Message( symbol="missing-docstring", msg_id="C0123", - location=[ # type: ignore + location=MessageLocationTuple( "abspath", "path", "module", "obj", - "line", - "column", - ], + 1, + 2, + ), msg="msg", confidence=confidence, ) @@ -37,11 +38,11 @@ def inner(confidence: Confidence = HIGH) -> Message: def test_output_line() -> None: output_line = OutputLine( symbol="missing-docstring", - lineno=0, - column="0", + lineno=1, + column=2, object="", msg="Missing docstring's bad.", - confidence=HIGH, + confidence=HIGH.name, ) assert output_line.symbol == "missing-docstring" @@ -56,10 +57,10 @@ def test_output_line_from_message(message: Callable) -> None: def test_output_line_to_csv(confidence: Confidence, message: Callable) -> None: output_line = OutputLine.from_msg(message(confidence)) csv = output_line.to_csv() - expected_column = "column" if PY38_PLUS else "" + expected_column = "2" if PY38_PLUS else "0" assert csv == ( "missing-docstring", - "line", + "1", expected_column, "obj", "msg", @@ -84,10 +85,10 @@ def test_output_line_from_csv_error() -> None: "confidence,expected_confidence", [[None, "HIGH"], ["INFERENCE", "INFERENCE"]] ) def test_output_line_from_csv( - confidence: Optional[str], expected_confidence: Confidence + confidence: Optional[str], expected_confidence: str ) -> None: if confidence: - proper_csv: List[str] = [ + proper_csv = [ "missing-docstring", "1", "2", @@ -98,7 +99,7 @@ def test_output_line_from_csv( else: proper_csv = ["missing-docstring", "1", "2", "obj", "msg"] output_line = OutputLine.from_csv(proper_csv) - expected_column = "2" if PY38_PLUS else "" + expected_column = 2 if PY38_PLUS else 0 assert output_line == OutputLine( symbol="missing-docstring", lineno=1, From c467240b68793c80789f686e7f4cf0c199095d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 22 Sep 2021 12:10:39 +0200 Subject: [PATCH 2/7] Update pylint/testutils/output_line.py --- pylint/testutils/output_line.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index ef30a3a8e5..17b0562584 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -97,7 +97,7 @@ def from_msg(cls, msg: Message) -> "OutputLine": def get_column(cls, column: str) -> int: if ( not PY38_PLUS - ): # This mimicks behaviour of MessagesHandlerMixIn.add_one_message() + ): # We check the column only for the new better ast parser introduced in python 3.8 return 0 # pragma: no cover return int(column) From dbd47f9bd6907c10980c03e8462aae3e84ce9d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 23 Sep 2021 00:15:47 +0200 Subject: [PATCH 3/7] Apply suggestions from code review Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- pylint/testutils/output_line.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index 17b0562584..1dd11d980e 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -40,7 +40,7 @@ def __eq__(self, other: object) -> bool: class MalformedOutputLineException(Exception): def __init__( self, - row: Union[Tuple[str, ...], List[str], str], + row: Union[Sequence[str], str], exception: Exception, ) -> None: example = "msg-symbolic-name:42:27:MyClass.my_function:The message" @@ -95,14 +95,13 @@ def from_msg(cls, msg: Message) -> "OutputLine": @classmethod def get_column(cls, column: str) -> int: - if ( - not PY38_PLUS - ): # We check the column only for the new better ast parser introduced in python 3.8 + if not PY38_PLUS: + # We check the column only for the new better ast parser introduced in python 3.8 return 0 # pragma: no cover return int(column) @classmethod - def from_csv(cls, row: Union[Tuple[str, ...], List[str], str]) -> "OutputLine": + def from_csv(cls, row: Union[Sequence[str], str]) -> "OutputLine": try: column = cls.get_column(row[2]) if isinstance(row, Iterable) and len(row) == 6: From 2a97b30f7257cf8c33e666b2af5c0b517166ae3e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 22 Sep 2021 22:16:32 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/testutils/output_line.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index 1dd11d980e..17a18017f3 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -2,7 +2,7 @@ # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE import collections -from typing import Any, Iterable, List, NamedTuple, Optional, Tuple, Union +from typing import Any, Iterable, NamedTuple, Optional, Tuple, Union from astroid import nodes @@ -95,7 +95,7 @@ def from_msg(cls, msg: Message) -> "OutputLine": @classmethod def get_column(cls, column: str) -> int: - if not PY38_PLUS: + if not PY38_PLUS: # We check the column only for the new better ast parser introduced in python 3.8 return 0 # pragma: no cover return int(column) From 136d3cf6afe3aa40bf85bc9dbfedc3ff287135c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 23 Sep 2021 00:20:41 +0200 Subject: [PATCH 5/7] Code review --- pylint/testutils/output_line.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index 17a18017f3..dfc529c0a6 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -2,7 +2,7 @@ # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE import collections -from typing import Any, Iterable, NamedTuple, Optional, Tuple, Union +from typing import Any, NamedTuple, Optional, Sequence, Tuple, Union from astroid import nodes @@ -83,7 +83,7 @@ class OutputLine(NamedTuple): @classmethod def from_msg(cls, msg: Message) -> "OutputLine": - column = cls.get_column(msg.column) + column = cls._get_column(msg.column) return cls( msg.symbol, msg.line, @@ -93,8 +93,8 @@ def from_msg(cls, msg: Message) -> "OutputLine": msg.confidence.name if msg.confidence != UNDEFINED else HIGH.name, ) - @classmethod - def get_column(cls, column: str) -> int: + @staticmethod + def _get_column(column: str) -> int: if not PY38_PLUS: # We check the column only for the new better ast parser introduced in python 3.8 return 0 # pragma: no cover @@ -103,11 +103,12 @@ def get_column(cls, column: str) -> int: @classmethod def from_csv(cls, row: Union[Sequence[str], str]) -> "OutputLine": try: - column = cls.get_column(row[2]) - if isinstance(row, Iterable) and len(row) == 6: - return cls(row[0], int(row[1]), column, row[3], row[4], row[5]) - if isinstance(row, Iterable) and len(row) == 5: - return cls(row[0], int(row[1]), column, row[3], row[4], HIGH.name) + column = cls._get_column(row[2]) + if isinstance(row, Sequence): + if len(row) == 6: + return cls(row[0], int(row[1]), column, row[3], row[4], row[5]) + if len(row) == 5: + return cls(row[0], int(row[1]), column, row[3], row[4], HIGH.name) raise IndexError except Exception as e: raise MalformedOutputLineException(row, e) from e From dc188a9117c8abb416c8d3d944b67b88ff0c0c9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 23 Sep 2021 00:42:43 +0200 Subject: [PATCH 6/7] Update pylint/testutils/output_line.py Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- pylint/testutils/output_line.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index dfc529c0a6..40d947e733 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -103,8 +103,8 @@ def _get_column(column: str) -> int: @classmethod def from_csv(cls, row: Union[Sequence[str], str]) -> "OutputLine": try: - column = cls._get_column(row[2]) if isinstance(row, Sequence): + column = cls._get_column(row[2]) if len(row) == 6: return cls(row[0], int(row[1]), column, row[3], row[4], row[5]) if len(row) == 5: From 9942f56b97f2a0e114b461ff4f3cd5b54081f928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 23 Sep 2021 00:47:45 +0200 Subject: [PATCH 7/7] Update pylint/testutils/output_line.py Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- pylint/testutils/output_line.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index 40d947e733..73dca5b01c 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -105,10 +105,10 @@ def from_csv(cls, row: Union[Sequence[str], str]) -> "OutputLine": try: if isinstance(row, Sequence): column = cls._get_column(row[2]) - if len(row) == 6: - return cls(row[0], int(row[1]), column, row[3], row[4], row[5]) if len(row) == 5: return cls(row[0], int(row[1]), column, row[3], row[4], HIGH.name) + if len(row) == 6: + return cls(row[0], int(row[1]), column, row[3], row[4], row[5]) raise IndexError except Exception as e: raise MalformedOutputLineException(row, e) from e