Skip to content

Add end_lineno and end_col_offset to MessageLocationTuple #5343

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

Merged
merged 3 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 5 additions & 1 deletion pylint/checkers/base_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,12 @@ def add_message(
args: Any = None,
confidence: Optional[Confidence] = None,
col_offset: Optional[int] = None,
end_lineno: Optional[int] = None,
end_col_offset: Optional[int] = None,
) -> None:
self.linter.add_message(msgid, line, node, args, confidence, col_offset)
self.linter.add_message(
msgid, line, node, args, confidence, col_offset, end_lineno, end_col_offset
)

def check_consistency(self):
"""Check the consistency of msgid.
Expand Down
44 changes: 38 additions & 6 deletions pylint/lint/pylinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1408,14 +1408,30 @@ def _add_one_message(
args: Optional[Any],
confidence: Optional[interfaces.Confidence],
col_offset: Optional[int],
end_lineno: Optional[int],
end_col_offset: Optional[int],
) -> None:
"""After various checks have passed a single Message is
passed to the reporter and added to stats"""
message_definition.check_message_definition(line, node)
if line is None and node is not None:
line = node.fromlineno
if col_offset is None and hasattr(node, "col_offset"):
col_offset = node.col_offset # type: ignore[union-attr]

# Look up "location" data of node if not yet supplied
if node:
if not line:
line = node.fromlineno
# pylint: disable=fixme
# TODO: Initialize col_offset on every node (can be None) -> astroid
if not col_offset and hasattr(node, "col_offset"):
col_offset = node.col_offset
# pylint: disable=fixme
# TODO: Initialize end_lineno on every node (can be None) -> astroid
# See https://github.com/PyCQA/astroid/issues/1273
if not end_lineno and hasattr(node, "end_lineno"):
end_lineno = node.end_lineno
# pylint: disable=fixme
# TODO: Initialize end_col_offset on every node (can be None) -> astroid
if not end_col_offset and hasattr(node, "end_col_offset"):
end_col_offset = node.end_col_offset

# should this message be displayed
if not self.is_message_enabled(message_definition.msgid, line, confidence):
Expand Down Expand Up @@ -1458,7 +1474,14 @@ def _add_one_message(
message_definition.msgid,
message_definition.symbol,
MessageLocationTuple(
abspath, path, module or "", obj, line or 1, col_offset or 0
abspath,
path,
module or "",
obj,
line or 1,
col_offset or 0,
end_lineno,
end_col_offset,
Comment on lines +1481 to +1484
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking again about this. Although I'm not 100% there yet, setting end_lineno and end_col_offset to None seems like the best we can do.

We probably shouldn't touch line or 1 and col_offset or 0 though as this will break tools.

),
msg,
confidence,
Expand All @@ -1473,6 +1496,8 @@ def add_message(
args: Optional[Any] = None,
confidence: Optional[interfaces.Confidence] = None,
col_offset: Optional[int] = None,
end_lineno: Optional[int] = None,
end_col_offset: Optional[int] = None,
) -> None:
"""Adds a message given by ID or name.

Expand All @@ -1487,7 +1512,14 @@ def add_message(
message_definitions = self.msgs_store.get_message_definitions(msgid)
for message_definition in message_definitions:
self._add_one_message(
message_definition, line, node, args, confidence, col_offset
message_definition,
line,
node,
args,
confidence,
col_offset,
end_lineno,
end_col_offset,
)

def add_ignored_message(
Expand Down
8 changes: 7 additions & 1 deletion pylint/message/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"obj",
"line",
"column",
"end_line",
"end_column",
],
)

Expand Down Expand Up @@ -59,7 +61,10 @@ def __new__(
cls,
msg_id: str,
symbol: str,
location: Union[Tuple[str, str, str, str, int, int], MessageLocationTuple],
location: Union[
Tuple[str, str, str, str, int, int],
MessageLocationTuple,
],
msg: str,
confidence: Optional[Confidence],
) -> "Message":
Expand All @@ -68,6 +73,7 @@ def __new__(
"In pylint 3.0, Messages will only accept a MessageLocationTuple as location parameter",
DeprecationWarning,
)
location = location + (None, None) # type: ignore[assignment] # Temporary fix until deprecation
return _MsgBase.__new__(
cls,
msg_id,
Expand Down
4 changes: 4 additions & 0 deletions pylint/testutils/unittest_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ def add_message(
args: Any = None,
confidence: Optional[Confidence] = None,
col_offset: Optional[int] = None,
end_lineno: Optional[int] = None,
end_col_offset: Optional[int] = None,
) -> None:
# Do not test col_offset for now since changing Message breaks everything
# pylint: disable=fixme
# TODO: Test end_lineno and end_col_offset :)
self._messages.append(MessageTest(msg_id, line, node, args, confidence))

@staticmethod
Expand Down
2 changes: 2 additions & 0 deletions pylint/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class MessageLocationTuple(NamedTuple):
obj: str
line: int
column: int
end_line: Optional[int] = None
end_column: Optional[int] = None


class ManagedMessage(NamedTuple):
Expand Down
20 changes: 18 additions & 2 deletions tests/lint/unittest_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,14 @@ def test_addmessage(linter: PyLinter) -> None:
description="Warning without any associated confidence level.",
),
location=MessageLocationTuple(
abspath="0123", path="0123", module="0123", obj="", line=1, column=0
abspath="0123",
path="0123",
module="0123",
obj="",
line=1,
column=0,
end_line=None,
end_column=None,
),
)
assert linter.reporter.messages[1] == Message(
Expand All @@ -512,7 +519,14 @@ def test_addmessage(linter: PyLinter) -> None:
description="Warning without any associated confidence level.",
),
location=MessageLocationTuple(
abspath="0123", path="0123", module="0123", obj="", line=2, column=0
abspath="0123",
path="0123",
module="0123",
obj="",
line=2,
column=0,
end_line=None,
end_column=None,
),
)

Expand Down Expand Up @@ -616,6 +630,8 @@ def test_analyze_explicit_script(linter: PyLinter) -> None:
obj="",
line=2,
column=0,
end_line=None,
end_column=None,
),
)

Expand Down
4 changes: 4 additions & 0 deletions tests/message/unittest_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def build_message(
obj="4",
line=5,
column=6,
end_line=5,
end_column=9,
)
w1234_location_values = MessageLocationTuple(
abspath="7",
Expand All @@ -42,6 +44,8 @@ def build_message(
obj="10",
line=11,
column=12,
end_line=11,
end_column=14,
)
expected = (
"2:5:6: E1234: Duplicate keyword argument %r in %s call (duplicate-keyword-arg)"
Expand Down
7 changes: 1 addition & 6 deletions tests/testutils/test_output_line.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ def inner(confidence: Confidence = HIGH) -> Message:
symbol="missing-docstring",
msg_id="C0123",
location=MessageLocationTuple(
"abspath",
"path",
"module",
"obj",
1,
2,
"abspath", "path", "module", "obj", 1, 2, 1, 3
),
msg="msg",
confidence=confidence,
Expand Down