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

Colorized output should not modify msg objects #7214

Closed
tom65536 opened this issue Jul 21, 2022 · 6 comments · Fixed by #7620
Closed

Colorized output should not modify msg objects #7214

tom65536 opened this issue Jul 21, 2022 · 6 comments · Fixed by #7620
Labels
Downstream Bug 🪲 The problem happens in a lib depending on pylint, not pylint Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine
Milestone

Comments

@tom65536
Copy link

Bug description

I noticed this problem because I use colorized text output alongside with the pylint-gitlab plugin. I filed an issue there as well, but I agree with the author of the plugin that the bug should be fixed within PyLint.

I use the pylint-gitlab reporter and colorized text output in the same run of pylint (see below). The plugin receives the messages modified by the ColorizedTextReporter. This causes an exception (KeyError) because the plugin receives the ANSI-color-coded category \x1b[1mconvention\x1b[0m instead of simply convention.

The modification happens inside the handle_message method:

        msg.msg = colorize_ansi(msg.msg, msg_style)
        msg.symbol = colorize_ansi(msg.symbol, msg_style)
        msg.category = colorize_ansi(msg.category, msg_style)
        msg.C = colorize_ansi(msg.C, msg_style)

Suggested solution:

As I haven't found a way of controlling the order in which the reporters are processed (the behavior would not be a problem if one could ensure that the ColorizedTextReporter is called last), I would suggest that the reporter should be changed to work in a re-entrant manner: The ColorizedTextReporter should not modify the original fields. It should rather add some extra fields (e.g. colorized_category) or keep a list of copies of the original msg objects.

Configuration

No response

Command used

python -m pylint --exit-zero --output-format=pylint_gitlab.GitlabCodeClimateReporter:pylint-codeclimate.json,pylint_gitlab.GitlabPagesHtmlReporter:index.html,text:pylint.txt,colorized src

Pylint output

File "xxx\python-3.8.9\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "xxx\python-3.8.9\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "xxx\.nox\lint_pylint\lib\site-packages\pylint\__main__.py", line 10, in <module>
    pylint.run_pylint()
  File "xxx\.nox\lint_pylint\lib\site-packages\pylint\__init__.py", line 25, in run_pylint
    PylintRun(argv or sys.argv[1:])
  File "xxx\.nox\lint_pylint\lib\site-packages\pylint\lint\run.py", line 205, in __init__
    score_value = linter.generate_reports()
  File "xxx\.nox\lint_pylint\lib\site-packages\pylint\lint\pylinter.py", line 998, in generate_reports
    self.reporter.display_messages(report_nodes.Section())
  File "xxx\.nox\lint_pylint\lib\site-packages\pylint\reporters\multi_reporter.py", line 95, in display_messages
    rep.display_messages(layout)
  File "xxx\.nox\lint_pylint\lib\site-packages\pylint_gitlab\reporter.py", line 69, in display_messages
    "class": self._MSG_TYPES[message.category],
KeyError: '\x1b[1mconvention\x1b[0m'

Expected behavior

ANSI-colored output to the screen plus some generated files (pylint-codeclimate.json, index.html, pylint.txt)

Pylint version

pylint-2.14.x (the problem did not occur with pylint<=3.13.x)

OS / Environment

Tested on Linux and Windows

Additional dependencies

pylint-gitlab=1.0.0

@tom65536 tom65536 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 21, 2022
@Smixi
Copy link
Contributor

Smixi commented Oct 13, 2022

I have the exact same issue running pylint --ignore=.cache --output-format=colorized,pylint_gitlab.GitlabCodeClimateReporter:reports/py-lint.codeclimate.json,parseable:reports/py-lint.parseable.txt **/**.py

gives me :

Traceback (most recent call last):
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 782, in _lint_file
    check_astroid_module(module)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 1052, in check_astroid_module
    retval = self._check_astroid_module(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 1100, in _check_astroid_module
    token_checker.process_tokens(tokens)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/checkers/format.py", line 477, in process_tokens
    self.add_message("trailing-newlines", line=line_num)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/checkers/base_checker.py", line 164, in add_message
    self.linter.add_message(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 1307, in add_message
    self._add_one_message(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 1265, in _add_one_message
    self.reporter.handle_message(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/reporters/multi_reporter.py", line 80, in handle_message
    rep.handle_message(msg)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint_gitlab/reporter.py", line 130, in handle_message
    "severity": self._MSG_TYPES[msg.category],
KeyError: '\x1b[1mconvention\x1b[0m'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 747, in _lint_files
    self._lint_file(fileitem, module, check_astroid_module)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 784, in _lint_file
    raise astroid.AstroidError from e
astroid.exceptions.AstroidError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sami/python/venvs/checkpoint-devices/bin/pylint", line 8, in <module>
    sys.exit(run_pylint())
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/__init__.py", line 35, in run_pylint
    PylintRun(argv or sys.argv[1:])
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/run.py", line 207, in __init__
    linter.check(args)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 691, in check
    self._lint_files(ast_per_fileitem, check_astroid_module)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 754, in _lint_files
    self.add_message(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 1307, in add_message
    self._add_one_message(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/lint/pylinter.py", line 1265, in _add_one_message
    self.reporter.handle_message(
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint/reporters/multi_reporter.py", line 80, in handle_message
    rep.handle_message(msg)
  File "/home/sami/python/venvs/checkpoint-devices/lib/python3.10/site-packages/pylint_gitlab/reporter.py", line 130, in handle_message
    "severity": self._MSG_TYPES[msg.category],
KeyError: '\x1b[1;4;31mfatal\x1b[0m'

@tom65536 did you find any workaround ?

Notes: It does work with python 2.13.9, but breaking at 2.14.0

@tom65536
Copy link
Author

I stopped using the GitlabCodeClimateReporter plugin. It does not solve the problem but works for me.

@Pierre-Sassoulas
Copy link
Member

I did not have time to look into this yet, but a reproducer not using GitlabCodeClimateReporter would be helpful imo.

@Pierre-Sassoulas Pierre-Sassoulas added Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Downstream Bug 🪲 The problem happens in a lib depending on pylint, not pylint and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 13, 2022
@Smixi
Copy link
Contributor

Smixi commented Oct 13, 2022

I can try to create a reproductible example with a custom reporter and custom colorizer.

@Smixi
Copy link
Contributor

Smixi commented Oct 13, 2022

Actually, it @tom65536 already describe fully the problem.

A simple way to reproduce this is adding "colorized" in the list of formatter for the test "test_multi_format_output" here :

formats = ",".join(["json:" + str(json), "text", nop_format, "colorized"])

The rendered json will contain the ansi encoded values :

./tests/reporters/unittest_reporting.py::test_multi_format_output Failed: [undefined]assert '[\n    {\n  ...rect output\n' == '[\n    {\n  ...rect output\n'
    [
        {
  -         "type": "convention",
  +         "type": "\u001b[1mconvention\u001b[0m",
  ?                  +++++++++          +++++++++
            "module": "somemodule",
            "obj": "",...

I never contributed to this project, and the question is now, should colorized modify the output (some reporters rely on the color from colorized) or not ? Feels like any reporter must receive a copy of message, otherwise they could interract between each other ?

@Smixi
Copy link
Contributor

Smixi commented Oct 13, 2022

I propose to pass a copy to ensure no reporter can affect another reporter. Don't know, if this affect performance ?

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.5 milestone Oct 15, 2022
@cdce8p cdce8p modified the milestones: 2.15.5, 2.15.6 Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Downstream Bug 🪲 The problem happens in a lib depending on pylint, not pylint Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants