Skip to content
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

Update reporters to (allow) use of end_line and end_column #5372

Merged
merged 17 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from 13 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
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ Release date: TBA

Closes #4982

* Add ability to add ``end_line`` and ``end_column`` to the ``--msg-template`` option.
With the standard ``TextReporter`` this will add the line and column number of the
end of a node to the output of Pylint. If these numbers are unknown, they are represented
by an empty string.

* Add ``end_line`` and ``end_column`` fields to the output of the ``JSONReporter``.

* Fix ``install graphiz`` message which isn't needed for puml output format.

* Fix ``simplify-boolean-expression`` when condition can be inferred as False.
Expand Down
7 changes: 7 additions & 0 deletions doc/user_guide/output.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ line
line number
column
column number
end_line
line number of the end of the node
end_column
column number of the end of the node
module
module name
obj
Expand Down Expand Up @@ -94,6 +98,9 @@ A few other examples:
The ``--msg-template`` option can only be combined with text-based reporters (``--output-format`` either unspecified or one of: parseable, colorized or msvs).
If both ``--output-format`` and ``--msg-template`` are specified, the ``--msg-template`` option will take precedence over the default line format defined by the reporter class.

If ``end_line`` or ``end_column`` are ``None``, they will be represented as an empty string
by the default ``TextReporter``.

.. _Python new format syntax: https://docs.python.org/2/library/string.html#formatstrings

Source code analysis section
Expand Down
7 changes: 7 additions & 0 deletions doc/whatsnew/2.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,10 @@ Other Changes
* Fix crash on ``open()`` calls when the ``mode`` argument is not a simple string.

Partially closes #5321

* Add ability to add ``end_line`` and ``end_column`` to the ``--msg-template`` option.
With the standard ``TextReporter`` this will add the line and column number of the
end of a node to the output of Pylint. If these numbers are unknown, they are represented
by an empty string.
Copy link
Member

Choose a reason for hiding this comment

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

I think we could also change the default for msg-template in 3.0. Do we want the default to have end line and end column ? This seems like something that is useful in an IDE but not a lot for a text output read by humans.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cdce8p Argued that we might leave the default output as is because of the potential to break many tools. I think this is a discussion which (if we want to have it anyway) we could leave until we are closer to the release of 3.0. With this merge everybody that wants to can use this new feature!


* Add ``end_line`` and ``end_column`` fields to the output of the ``JSONReporter``.
Copy link
Member

Choose a reason for hiding this comment

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

It look like a breaking change that should wait for #4741. We have a 3.0 alpha branch, might be the time to dust it off ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted! See #5380

2 changes: 2 additions & 0 deletions pylint/reporters/json_reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ def display_messages(self, layout: Optional["Section"]) -> None:
"obj": msg.obj,
"line": msg.line,
"column": msg.column,
"end_line": msg.end_line,
"end_column": msg.end_column,
"path": msg.path,
"symbol": msg.symbol,
"message": msg.msg or "",
Expand Down
30 changes: 28 additions & 2 deletions pylint/reporters/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
:colorized: an ANSI colorized text reporter
"""
import os
import re
import sys
import warnings
from typing import (
Expand Down Expand Up @@ -183,13 +184,38 @@ def __init__(self, output: Optional[TextIO] = None) -> None:
super().__init__(output)
self._modules: Set[str] = set()
self._template = self.line_format
self._fixed_template = self.line_format
"""The output format template with any unrecognized arguments removed"""

def on_set_current_module(self, module: str, filepath: Optional[str]) -> None:
self._template = str(self.linter.config.msg_template or self._template)
"""Set the format template to be used and check for unrecognized arguments."""
template = str(self.linter.config.msg_template or self._template)

# Return early if the template is the same as the previous one
if template == self._template:
return

# Set template to the currently selected template
self._template = template

# Check to see if all parameters in the template are attributes of the Message
arguments = re.findall(r"\{(.+?)(:.*)?\}", template)
for argument in arguments:
if argument[0] not in Message._fields:
warnings.warn(
f"Don't recognize the argument {argument[0]} in the --msg-template. "
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"Are you sure it is supported on the current version of pylint?"
)
template = re.sub(r"\{" + argument[0] + r"(:.*?)?\}", "", template)
self._fixed_template = template

def write_message(self, msg: Message) -> None:
"""Convenience method to write a formatted message with class default template"""
self.writeln(msg.format(self._template))
self_dict = msg._asdict()
for key in ("end_line", "end_column"):
self_dict[key] = self_dict[key] or ""

self.writeln(self._fixed_template.format(**self_dict))

def handle_message(self, msg: Message) -> None:
"""manage message of different type and in the context of path"""
Expand Down
80 changes: 63 additions & 17 deletions tests/unittest_reporters_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,69 @@
from pylint.reporters.ureports.nodes import EvaluationSection

expected_score_message = "Expected score message"
expected_result = [
[
("column", 0),
("line", 1),
("message", "Line too long (1/2)"),
("message-id", "C0301"),
("module", "0123"),
("obj", ""),
("path", "0123"),
("symbol", "line-too-long"),
("type", "convention"),
]
]


def test_simple_json_output_no_score() -> None:
report = get_linter_result(score=False)
"""Test JSON reporter with no score"""
message = {
"msg": "line-too-long",
"line": 1,
"args": (1, 2),
"end_line": None,
"end_column": None,
}
expected = [
[
("column", 0),
("end_column", None),
("end_line", None),
("line", 1),
("message", "Line too long (1/2)"),
("message-id", "C0301"),
("module", "0123"),
("obj", ""),
("path", "0123"),
("symbol", "line-too-long"),
("type", "convention"),
]
]
report = get_linter_result(score=False, message=message)
assert len(report) == 1
report_result = [sorted(report[0].items(), key=lambda item: item[0])]
assert report_result == expected


def test_simple_json_output_no_score_with_end_line() -> None:
"""Test JSON reporter with no score with end_line and end_column"""
message = {
"msg": "line-too-long",
"line": 1,
"args": (1, 2),
"end_line": 1,
"end_column": 4,
}
expected = [
[
("column", 0),
("end_column", 4),
("end_line", 1),
("line", 1),
("message", "Line too long (1/2)"),
("message-id", "C0301"),
("module", "0123"),
("obj", ""),
("path", "0123"),
("symbol", "line-too-long"),
("type", "convention"),
]
]
report = get_linter_result(score=False, message=message)
assert len(report) == 1
report_result = [sorted(report[0].items(), key=lambda item: item[0])]
assert report_result == expected_result
assert report_result == expected


def get_linter_result(score: bool) -> List[Dict[str, Any]]:
def get_linter_result(score: bool, message: Dict[str, Any]) -> List[Dict[str, Any]]:
output = StringIO()
reporter = JSONReporter(output)
linter = PyLinter(reporter=reporter)
Expand All @@ -56,7 +96,13 @@ def get_linter_result(score: bool) -> List[Dict[str, Any]]:
linter.config.score = score
linter.open()
linter.set_current_module("0123")
linter.add_message("line-too-long", line=1, args=(1, 2))
linter.add_message(
message["msg"],
line=message["line"],
args=message["args"],
end_lineno=message["end_line"],
end_col_offset=message["end_column"],
)
# we call those methods because we didn't actually run the checkers
if score:
reporter.display_reports(EvaluationSection(expected_score_message))
Expand Down
70 changes: 70 additions & 0 deletions tests/unittest_reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,72 @@ def test_template_option(linter):
assert output.getvalue() == "************* Module 0123\nC0301:001\nC0301:002\n"


def test_template_option_default(linter) -> None:
"""Test the default msg-template setting"""
output = StringIO()
linter.reporter.out = output
linter.open()
linter.set_current_module("my_module")
linter.add_message("C0301", line=1, args=(1, 2))
linter.add_message("line-too-long", line=2, args=(3, 4))

out_lines = output.getvalue().split("\n")
assert out_lines[1] == "my_module:1:0: C0301: Line too long (1/2) (line-too-long)"
assert out_lines[2] == "my_module:2:0: C0301: Line too long (3/4) (line-too-long)"


def test_template_option_end_line(linter) -> None:
"""Test the msg-template option with end_line and end_column"""
output = StringIO()
linter.reporter.out = output
linter.set_option(
"msg-template",
"{path}:{line}:{column}:{end_line}:{end_column}: {msg_id}: {msg} ({symbol})",
)
linter.open()
linter.set_current_module("my_mod")
linter.add_message("C0301", line=1, args=(1, 2))
linter.add_message(
"line-too-long", line=2, end_lineno=2, end_col_offset=4, args=(3, 4)
)

out_lines = output.getvalue().split("\n")
assert out_lines[1] == "my_mod:1:0::: C0301: Line too long (1/2) (line-too-long)"
assert out_lines[2] == "my_mod:2:0:2:4: C0301: Line too long (3/4) (line-too-long)"


def test_template_option_non_exisiting(linter) -> None:
"""Test the msg-template option with a non exisiting options.
This makes sure that this option remains backwards compatible as new
parameters do not break on previous versions"""
output = StringIO()
linter.reporter.out = output
linter.set_option(
"msg-template",
"{path}:{line}:{a_new_option}:({a_second_new_option:03d})",
)
linter.open()
with pytest.warns(UserWarning) as records:
linter.set_current_module("my_mod")
assert len(records) == 2
assert (
"Don't recognize the argument 'a_new_option'" in records[0].message.args[0]
)
assert (
"Don't recognize the argument 'a_second_new_option'"
in records[1].message.args[0]
)

linter.add_message("C0301", line=1, args=(1, 2))
linter.add_message(
"line-too-long", line=2, end_lineno=2, end_col_offset=4, args=(3, 4)
)

out_lines = output.getvalue().split("\n")
assert out_lines[1] == "my_mod:1::()"
assert out_lines[2] == "my_mod:2::()"


def test_deprecation_set_output(recwarn):
"""TODO remove in 3.0""" # pylint: disable=fixme
reporter = BaseReporter()
Expand Down Expand Up @@ -153,6 +219,8 @@ def test_multi_format_output(tmp_path):
' "obj": "",\n'
' "line": 1,\n'
' "column": 0,\n'
' "end_line": null,\n'
' "end_column": null,\n'
f' "path": {escaped_source_file},\n'
' "symbol": "missing-module-docstring",\n'
' "message": "Missing module docstring",\n'
Expand All @@ -164,6 +232,8 @@ def test_multi_format_output(tmp_path):
' "obj": "",\n'
' "line": 1,\n'
' "column": 0,\n'
' "end_line": null,\n'
' "end_column": null,\n'
f' "path": {escaped_source_file},\n'
' "symbol": "line-too-long",\n'
' "message": "Line too long (1/2)",\n'
Expand Down