From f395226f931b2f969431431a2c5b63dbf737c3b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 17:38:15 +0100 Subject: [PATCH 01/14] Update reporters to (allow) use end_line and end_column --- ChangeLog | 7 +++ doc/user_guide/output.rst | 7 +++ doc/whatsnew/2.12.rst | 7 +++ pylint/reporters/json_reporter.py | 2 + pylint/reporters/text.py | 13 ++++- tests/unittest_reporters_json.py | 80 ++++++++++++++++++++++++------- tests/unittest_reporting.py | 34 +++++++++++++ 7 files changed, 132 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index 10adc33792..db7e114be5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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. diff --git a/doc/user_guide/output.rst b/doc/user_guide/output.rst index 52959d3c71..4a2d6d0999 100644 --- a/doc/user_guide/output.rst +++ b/doc/user_guide/output.rst @@ -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 @@ -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 formatter. + .. _Python new format syntax: https://docs.python.org/2/library/string.html#formatstrings Source code analysis section diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index 9e8bfbb30a..8aa96c998d 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -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. + +* Add ``end_line`` and ``end_column`` fields to the output of the ``JSONReporter``. diff --git a/pylint/reporters/json_reporter.py b/pylint/reporters/json_reporter.py index 87949c1c44..2c7bac45cf 100644 --- a/pylint/reporters/json_reporter.py +++ b/pylint/reporters/json_reporter.py @@ -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 "", diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 42bd6b2ea3..b6f2ee3d5a 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -89,6 +89,9 @@ class MessageStyle(NamedTuple): "cyan": "36", "white": "37", } +PRINT_AS_EMPTY_STRING = {"end_line", "end_column"} +"""Set of Message attributes that should be printed as an +empty string when they are None""" def _get_ansi_code(msg_style: MessageStyle) -> str: @@ -177,6 +180,8 @@ class TextReporter(BaseReporter): __implements__ = IReporter name = "text" extension = "txt" + # pylint: disable=fixme + # TODO: Add end_line and end_column to standard line format of TextReporter line_format = "{path}:{line}:{column}: {msg_id}: {msg} ({symbol})" def __init__(self, output: Optional[TextIO] = None) -> None: @@ -189,7 +194,13 @@ def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: 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, value in self_dict.items(): + # pylint: disable=fixme + # TODO: Add column to list of attributes to be printed as an empty string + if value is None and key in PRINT_AS_EMPTY_STRING: + self_dict[key] = "" + self.writeln(self._template.format(**self_dict)) def handle_message(self, msg: Message) -> None: """manage message of different type and in the context of path""" diff --git a/tests/unittest_reporters_json.py b/tests/unittest_reporters_json.py index ed6ed74a4d..de484d52be 100644 --- a/tests/unittest_reporters_json.py +++ b/tests/unittest_reporters_json.py @@ -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), + ("line", 1), + ("end_line", None), + ("end_column", None), + ("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), + ("line", 1), + ("end_line", 1), + ("end_column", 4), + ("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) @@ -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)) diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 7400be9e56..ac24ede402 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -57,6 +57,40 @@ 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_deprecation_set_output(recwarn): """TODO remove in 3.0""" # pylint: disable=fixme reporter = BaseReporter() From 837b65f94843e178552d4b45e5adda391f9f69e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 23:01:03 +0100 Subject: [PATCH 02/14] Update tests and spelling --- doc/user_guide/output.rst | 2 +- tests/unittest_reporters_json.py | 8 ++++---- tests/unittest_reporting.py | 4 ++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/user_guide/output.rst b/doc/user_guide/output.rst index 4a2d6d0999..bb2a339f06 100644 --- a/doc/user_guide/output.rst +++ b/doc/user_guide/output.rst @@ -99,7 +99,7 @@ The ``--msg-template`` option can only be combined with text-based reporters (`` 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 formatter. +by the default ``TextReporter``. .. _Python new format syntax: https://docs.python.org/2/library/string.html#formatstrings diff --git a/tests/unittest_reporters_json.py b/tests/unittest_reporters_json.py index de484d52be..fb5598d255 100644 --- a/tests/unittest_reporters_json.py +++ b/tests/unittest_reporters_json.py @@ -39,9 +39,9 @@ def test_simple_json_output_no_score() -> None: expected = [ [ ("column", 0), - ("line", 1), - ("end_line", None), ("end_column", None), + ("end_line", None), + ("line", 1), ("message", "Line too long (1/2)"), ("message-id", "C0301"), ("module", "0123"), @@ -69,9 +69,9 @@ def test_simple_json_output_no_score_with_end_line() -> None: expected = [ [ ("column", 0), - ("line", 1), - ("end_line", 1), ("end_column", 4), + ("end_line", 1), + ("line", 1), ("message", "Line too long (1/2)"), ("message-id", "C0301"), ("module", "0123"), diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index ac24ede402..d1d55a9775 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -187,6 +187,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' @@ -198,6 +200,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' From 9792d04db510f4e2b3c773e6fceb2bcdfba1b019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 23:19:28 +0100 Subject: [PATCH 03/14] Apply suggestions from code review Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- ChangeLog | 2 +- doc/user_guide/output.rst | 2 +- doc/whatsnew/2.12.rst | 2 +- pylint/reporters/text.py | 4 ---- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index db7e114be5..2bdfc4ed88 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,7 +17,7 @@ Release date: TBA * 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 + 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``. diff --git a/doc/user_guide/output.rst b/doc/user_guide/output.rst index bb2a339f06..1aa18f3b31 100644 --- a/doc/user_guide/output.rst +++ b/doc/user_guide/output.rst @@ -98,7 +98,7 @@ 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 +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 diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index 8aa96c998d..e6a8ccce9f 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -201,7 +201,7 @@ Other Changes * 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 + 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``. diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index b6f2ee3d5a..e020004779 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -180,8 +180,6 @@ class TextReporter(BaseReporter): __implements__ = IReporter name = "text" extension = "txt" - # pylint: disable=fixme - # TODO: Add end_line and end_column to standard line format of TextReporter line_format = "{path}:{line}:{column}: {msg_id}: {msg} ({symbol})" def __init__(self, output: Optional[TextIO] = None) -> None: @@ -196,8 +194,6 @@ def write_message(self, msg: Message) -> None: """Convenience method to write a formatted message with class default template""" self_dict = msg._asdict() for key, value in self_dict.items(): - # pylint: disable=fixme - # TODO: Add column to list of attributes to be printed as an empty string if value is None and key in PRINT_AS_EMPTY_STRING: self_dict[key] = "" self.writeln(self._template.format(**self_dict)) From 45e7bd007e22b9399a8fd0267164ac7b709d2ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 22 Nov 2021 23:26:36 +0100 Subject: [PATCH 04/14] Code review --- pylint/reporters/text.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index e020004779..05e98f8b50 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -193,9 +193,8 @@ def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: def write_message(self, msg: Message) -> None: """Convenience method to write a formatted message with class default template""" self_dict = msg._asdict() - for key, value in self_dict.items(): - if value is None and key in PRINT_AS_EMPTY_STRING: - self_dict[key] = "" + for key in PRINT_AS_EMPTY_STRING: + self_dict[key] = self_dict[key] or "" self.writeln(self._template.format(**self_dict)) def handle_message(self, msg: Message) -> None: From d980fb87873d732579867cb8613763080b34561d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 00:17:02 +0100 Subject: [PATCH 05/14] Code review --- pylint/reporters/text.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 05e98f8b50..f04d825d3f 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -89,9 +89,6 @@ class MessageStyle(NamedTuple): "cyan": "36", "white": "37", } -PRINT_AS_EMPTY_STRING = {"end_line", "end_column"} -"""Set of Message attributes that should be printed as an -empty string when they are None""" def _get_ansi_code(msg_style: MessageStyle) -> str: @@ -193,7 +190,7 @@ def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: def write_message(self, msg: Message) -> None: """Convenience method to write a formatted message with class default template""" self_dict = msg._asdict() - for key in PRINT_AS_EMPTY_STRING: + for key in ("end_line", "end_column"): self_dict[key] = self_dict[key] or "" self.writeln(self._template.format(**self_dict)) From 1ed7e173ac25744f042c0b3d519d9baf42e48014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 00:27:19 +0100 Subject: [PATCH 06/14] Code review --- pylint/reporters/text.py | 12 ++++++++++++ tests/unittest_reporting.py | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index f04d825d3f..47d8cb3472 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -24,6 +24,7 @@ :colorized: an ANSI colorized text reporter """ import os +import re import sys import warnings from typing import ( @@ -183,6 +184,7 @@ def __init__(self, output: Optional[TextIO] = None) -> None: super().__init__(output) self._modules: Set[str] = set() self._template = self.line_format + self._checked_template = False def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self._template = str(self.linter.config.msg_template or self._template) @@ -192,6 +194,16 @@ def write_message(self, msg: Message) -> None: self_dict = msg._asdict() for key in ("end_line", "end_column"): self_dict[key] = self_dict[key] or "" + + # Check to see if all parameters in the template are attributes of the Message + if not self._checked_template: + template = self._template + parameters = re.findall(r"\{(.+?)\}", template) + for parameter in parameters: + if parameter not in self_dict: + template = template.replace(f"{{{parameter}}}", "") + self._template = template + self._checked_template = True self.writeln(self._template.format(**self_dict)) def handle_message(self, msg: Message) -> None: diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index d1d55a9775..3c77babb42 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -91,6 +91,28 @@ def test_template_option_end_line(linter) -> None: 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})", + ) + 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::()" + assert out_lines[2] == "my_mod:2::()" + + def test_deprecation_set_output(recwarn): """TODO remove in 3.0""" # pylint: disable=fixme reporter = BaseReporter() From 35fd523f62d8b8b2dd95eb4f1359995764fcd198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 00:58:48 +0100 Subject: [PATCH 07/14] Fix mistake with regex --- pylint/reporters/text.py | 6 +++--- tests/unittest_reporting.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 47d8cb3472..d93f081c04 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -198,10 +198,10 @@ def write_message(self, msg: Message) -> None: # Check to see if all parameters in the template are attributes of the Message if not self._checked_template: template = self._template - parameters = re.findall(r"\{(.+?)\}", template) + parameters = re.findall(r"\{(.+?)(:.*)?\}", template) for parameter in parameters: - if parameter not in self_dict: - template = template.replace(f"{{{parameter}}}", "") + if parameter[0] not in self_dict: + template = re.sub(r"\{" + parameter[0] + r"(:.*?)?\}", "", template) self._template = template self._checked_template = True self.writeln(self._template.format(**self_dict)) diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 3c77babb42..8a6bce5f64 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -99,7 +99,7 @@ def test_template_option_non_exisiting(linter) -> None: linter.reporter.out = output linter.set_option( "msg-template", - "{path}:{line}:{a_new_option}:({a_second_new_option})", + "{path}:{line}:{a_new_option}:({a_second_new_option:03d})", ) linter.open() linter.set_current_module("my_mod") From 45e0033ee152ba021fe2001bc5ec1f6dbdfb0720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 01:05:02 +0100 Subject: [PATCH 08/14] Move the overwrite --- pylint/reporters/text.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index d93f081c04..aba5b2129c 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -184,26 +184,24 @@ def __init__(self, output: Optional[TextIO] = None) -> None: super().__init__(output) self._modules: Set[str] = set() self._template = self.line_format - self._checked_template = False def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self._template = str(self.linter.config.msg_template or self._template) + # Check to see if all parameters in the template are attributes of the Message + template = self._template + parameters = re.findall(r"\{(.+?)(:.*)?\}", template) + for parameter in parameters: + if parameter[0] not in Message._fields: + template = re.sub(r"\{" + parameter[0] + r"(:.*?)?\}", "", template) + self._template = template + def write_message(self, msg: Message) -> None: """Convenience method to write a formatted message with class default template""" self_dict = msg._asdict() for key in ("end_line", "end_column"): self_dict[key] = self_dict[key] or "" - # Check to see if all parameters in the template are attributes of the Message - if not self._checked_template: - template = self._template - parameters = re.findall(r"\{(.+?)(:.*)?\}", template) - for parameter in parameters: - if parameter[0] not in self_dict: - template = re.sub(r"\{" + parameter[0] + r"(:.*?)?\}", "", template) - self._template = template - self._checked_template = True self.writeln(self._template.format(**self_dict)) def handle_message(self, msg: Message) -> None: From 0a6e6d607df6bb4283aa8a9e847ef5c608dc993b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 09:05:51 +0100 Subject: [PATCH 09/14] Code review --- pylint/reporters/text.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index aba5b2129c..88edc2f6fd 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -184,17 +184,26 @@ 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 parameters 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 parameters.""" + 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 - template = self._template parameters = re.findall(r"\{(.+?)(:.*)?\}", template) for parameter in parameters: if parameter[0] not in Message._fields: template = re.sub(r"\{" + parameter[0] + r"(:.*?)?\}", "", template) - self._template = template + self._fixed_template = template def write_message(self, msg: Message) -> None: """Convenience method to write a formatted message with class default template""" @@ -202,7 +211,7 @@ def write_message(self, msg: Message) -> None: for key in ("end_line", "end_column"): self_dict[key] = self_dict[key] or "" - self.writeln(self._template.format(**self_dict)) + 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""" From 3c3f1410abc5e05589f6d6ed5e2379ee3ccfe2e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 09:16:38 +0100 Subject: [PATCH 10/14] Add warning --- pylint/reporters/text.py | 16 ++++++++++------ tests/unittest_reporting.py | 13 ++++++++++++- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 88edc2f6fd..40fd3f141d 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -185,10 +185,10 @@ def __init__(self, output: Optional[TextIO] = None) -> None: self._modules: Set[str] = set() self._template = self.line_format self._fixed_template = self.line_format - """The output format template with any unrecognized parameters removed""" + """The output format template with any unrecognized arguments removed""" def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: - """Set the format template to be used and check for unrecognized parameters.""" + """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 @@ -199,10 +199,14 @@ def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self._template = template # Check to see if all parameters in the template are attributes of the Message - parameters = re.findall(r"\{(.+?)(:.*)?\}", template) - for parameter in parameters: - if parameter[0] not in Message._fields: - template = re.sub(r"\{" + parameter[0] + r"(:.*?)?\}", "", template) + 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. " + "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: diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 8a6bce5f64..5252d4295b 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -102,7 +102,18 @@ def test_template_option_non_exisiting(linter) -> None: "{path}:{line}:{a_new_option}:({a_second_new_option:03d})", ) linter.open() - linter.set_current_module("my_mod") + with pytest.warns(UserWarning) as records: + linter.set_current_module("my_mod") + assert len(records) == 2 + assert ( + records[0].message.args[0] + == "Don't recognize the argument a_new_option in the --msg-template. Are you sure it is supported on the current version of pylint?" + ) + assert ( + records[1].message.args[0] + == "Don't recognize the argument a_second_new_option in the --msg-template. Are you sure it is supported on the current version of pylint?" + ) + 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) From b24dd1c31542e23b31674577590383ca35eef9be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 09:58:17 +0100 Subject: [PATCH 11/14] Apply suggestions from code review Co-authored-by: Pierre Sassoulas --- tests/unittest_reporting.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 5252d4295b..424733b4e9 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -105,14 +105,8 @@ def test_template_option_non_exisiting(linter) -> None: with pytest.warns(UserWarning) as records: linter.set_current_module("my_mod") assert len(records) == 2 - assert ( - records[0].message.args[0] - == "Don't recognize the argument a_new_option in the --msg-template. Are you sure it is supported on the current version of pylint?" - ) - assert ( - records[1].message.args[0] - == "Don't recognize the argument a_second_new_option in the --msg-template. Are you sure it is supported on the current version of pylint?" - ) + 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( From 6ea66c6e2a28c708e829e27ea97231ad74b6853d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 23 Nov 2021 08:59:05 +0000 Subject: [PATCH 12/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unittest_reporting.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 424733b4e9..51bb0f8739 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -105,8 +105,13 @@ def test_template_option_non_exisiting(linter) -> None: 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] + 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( From a1bdf7c1ad401d4bb0390276d4007dacb6d63030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 23 Nov 2021 12:51:41 +0100 Subject: [PATCH 13/14] Update pylint/reporters/text.py --- pylint/reporters/text.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 40fd3f141d..54d375f1c6 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -203,7 +203,7 @@ def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: 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. " + f"Don't recognize the argument '{argument[0]}' in the --msg-template. " "Are you sure it is supported on the current version of pylint?" ) template = re.sub(r"\{" + argument[0] + r"(:.*?)?\}", "", template) From 6e912254dbc1e2e8b89f4e80cd859df42c07aa14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 24 Nov 2021 08:33:45 +0100 Subject: [PATCH 14/14] Revert changes to ``JSONReporter`` --- ChangeLog | 2 - doc/whatsnew/2.12.rst | 2 - pylint/reporters/json_reporter.py | 2 - tests/unittest_reporters_json.py | 80 +++++++------------------------ tests/unittest_reporting.py | 4 -- 5 files changed, 17 insertions(+), 73 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2bdfc4ed88..810de3d491 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,8 +20,6 @@ Release date: TBA 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. diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index e6a8ccce9f..0d8affd1d2 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -203,5 +203,3 @@ Other Changes 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``. diff --git a/pylint/reporters/json_reporter.py b/pylint/reporters/json_reporter.py index 2c7bac45cf..87949c1c44 100644 --- a/pylint/reporters/json_reporter.py +++ b/pylint/reporters/json_reporter.py @@ -39,8 +39,6 @@ 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 "", diff --git a/tests/unittest_reporters_json.py b/tests/unittest_reporters_json.py index fb5598d255..ed6ed74a4d 100644 --- a/tests/unittest_reporters_json.py +++ b/tests/unittest_reporters_json.py @@ -25,69 +25,29 @@ from pylint.reporters.ureports.nodes import EvaluationSection expected_score_message = "Expected score message" - - -def test_simple_json_output_no_score() -> None: - """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"), - ] +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"), ] - 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) +def test_simple_json_output_no_score() -> None: + report = get_linter_result(score=False) assert len(report) == 1 report_result = [sorted(report[0].items(), key=lambda item: item[0])] - assert report_result == expected + assert report_result == expected_result -def get_linter_result(score: bool, message: Dict[str, Any]) -> List[Dict[str, Any]]: +def get_linter_result(score: bool) -> List[Dict[str, Any]]: output = StringIO() reporter = JSONReporter(output) linter = PyLinter(reporter=reporter) @@ -96,13 +56,7 @@ def get_linter_result(score: bool, message: Dict[str, Any]) -> List[Dict[str, An linter.config.score = score linter.open() linter.set_current_module("0123") - linter.add_message( - message["msg"], - line=message["line"], - args=message["args"], - end_lineno=message["end_line"], - end_col_offset=message["end_column"], - ) + linter.add_message("line-too-long", line=1, args=(1, 2)) # we call those methods because we didn't actually run the checkers if score: reporter.display_reports(EvaluationSection(expected_score_message)) diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 5252d4295b..e98eadfb90 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -220,8 +220,6 @@ 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' @@ -233,8 +231,6 @@ 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'