From c6ca5e554e6aa1989fe02ef4017ae535d29a6e8f Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Thu, 21 Sep 2023 21:53:57 +0300 Subject: [PATCH 01/16] Create `_is_env_set_and_non_empty` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint/lint/utils.py | 6 ++++++ tests/lint/test_utils.py | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index c5487a8c6d..f73f7248fa 100644 --- a/pylint/lint/utils.py +++ b/pylint/lint/utils.py @@ -5,6 +5,7 @@ from __future__ import annotations import contextlib +import os import platform import sys import traceback @@ -133,3 +134,8 @@ def augmented_sys_path(additional_paths: Sequence[str]) -> Iterator[None]: yield finally: sys.path[:] = original + + +def _is_env_set_and_non_empty(env_var: str) -> bool: + """Checks if env_var is set and non-empty.""" + return os.environ.get(env_var) not in ["", None] diff --git a/tests/lint/test_utils.py b/tests/lint/test_utils.py index fa449374a9..6d11adf797 100644 --- a/tests/lint/test_utils.py +++ b/tests/lint/test_utils.py @@ -4,11 +4,16 @@ import unittest.mock from pathlib import Path, PosixPath +from typing import Any import pytest from pylint.constants import full_version -from pylint.lint.utils import get_fatal_error_message, prepare_crash_report +from pylint.lint.utils import ( + _is_env_set_and_non_empty, + get_fatal_error_message, + prepare_crash_report, +) from pylint.testutils._run import _Run as Run @@ -56,3 +61,31 @@ def test_issue_template_on_fatal_errors(capsys: pytest.CaptureFixture) -> None: assert "Fatal error while checking" in captured.out assert "Please open an issue" in captured.out assert "Traceback" in captured.err + + +@pytest.mark.parametrize( + "value, expected", + [ + (None, False), + ("", False), + (0, True), + (1, True), + (2, True), + (False, True), + ("no", True), + ("off", True), + ("on", True), + (True, True), + ("yes", True), + ], + ids=repr, +) +def test_is_env_set_and_non_empty( + monkeypatch: pytest.MonkeyPatch, value: Any, expected: bool +) -> None: + """Test the function returns True if the environment variable is set and non-empty.""" + env_var = "TEST_VAR" + if value is not None: + monkeypatch.setenv(env_var, str(value)) + + assert _is_env_set_and_non_empty(env_var) == expected From aec5efbaa3519fc85e3f5a2c55c1dbeb59cafe57 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Thu, 21 Sep 2023 23:42:19 +0300 Subject: [PATCH 02/16] Second option for `_is_env_set_and_non_empty` --- pylint/lint/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index f73f7248fa..6b4cda1de8 100644 --- a/pylint/lint/utils.py +++ b/pylint/lint/utils.py @@ -138,4 +138,4 @@ def augmented_sys_path(additional_paths: Sequence[str]) -> Iterator[None]: def _is_env_set_and_non_empty(env_var: str) -> bool: """Checks if env_var is set and non-empty.""" - return os.environ.get(env_var) not in ["", None] + return bool(os.environ.get(env_var)) From b5b91c19e0861dbd04b8f8f011f902f3b88551e4 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Thu, 21 Sep 2023 23:42:04 +0300 Subject: [PATCH 03/16] Create `pylinter#_handle_force_color_no_color` Use the `_is_env_set_and_non_empty`, and the logic decided in the issue, to modify the reporter(s) list accordingly. At most, one reporter should be modified - the one going to `stdout`. Hooking before the `self.set_reporter` was deemed as the smallest incision. For no particular reason, it was decided that `_handle_force_color_no_color` would modify its input; so there would be no need for re-assigning the input we want to modify anyway. Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint/lint/pylinter.py | 77 +++++++++++++++++++++++++++++++++++- pylint/reporters/__init__.py | 4 ++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index cc4605b96a..ba3c501fc4 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -12,6 +12,7 @@ import sys import tokenize import traceback +import warnings from collections import defaultdict from collections.abc import Callable, Iterable, Iterator, Sequence from io import TextIOWrapper @@ -48,13 +49,15 @@ report_total_messages_stats, ) from pylint.lint.utils import ( + _is_env_set_and_non_empty, augmented_sys_path, get_fatal_error_message, prepare_crash_report, ) from pylint.message import Message, MessageDefinition, MessageDefinitionStore +from pylint.reporters import ReporterWarning from pylint.reporters.base_reporter import BaseReporter -from pylint.reporters.text import TextReporter +from pylint.reporters.text import ColorizedTextReporter, TextReporter from pylint.reporters.ureports import nodes as report_nodes from pylint.typing import ( DirectoryNamespaceDict, @@ -69,6 +72,14 @@ MANAGER = astroid.MANAGER +NO_COLOR = "NO_COLOR" +FORCE_COLOR = "FORCE_COLOR" +PY_COLORS = "PY_COLORS" + +WARN_FORCE_COLOR_SET = "FORCE_COLOR is set; ignoring `text` at stdout" +WARN_NO_COLOR_SET = "NO_COLOR is set; ignoring `colorized` at stdout" +WARN_BOTH_COLOR_SET = "Both NO_COLOR and FORCE_COLOR are set! (disabling colors)" + class GetAstProtocol(Protocol): def __call__( @@ -250,6 +261,68 @@ def _load_reporter_by_class(reporter_class: str) -> type[BaseReporter]: } +def _handle_force_color_no_color(reporter: list[reporters.BaseReporter]) -> None: + """ + Check ``NO_COLOR``, ``FORCE_COLOR``, ``PY_COLOR`` and modify the reporter list + accordingly. + + Rules are presented in this table: + +--------------+---------------+-----------------+------------------------------------------------------------+ + | `NO_COLOR` | `FORCE_COLOR` | `output-format` | Behavior | + +==============+===============+=================+============================================================+ + | `bool: True` | `bool: True` | colorized | not colorized + warnings (override + inconsistent env var) | + | `bool: True` | `bool: True` | / | not colorized + warnings (inconsistent env var) | + | unset | `bool: True` | colorized | colorized | + | unset | `bool: True` | / | colorized + warnings (override) | + | `bool: True` | unset | colorized | not colorized + warnings (override) | + | `bool: True` | unset | / | not colorized | + | unset | unset | colorized | colorized | + | unset | unset | / | not colorized | + +--------------+---------------+-----------------+------------------------------------------------------------+ + """ + no_color = _is_env_set_and_non_empty(NO_COLOR) + force_color = _is_env_set_and_non_empty(FORCE_COLOR) or _is_env_set_and_non_empty( + PY_COLORS + ) + + if no_color and force_color: + warnings.warn( + WARN_BOTH_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + force_color = False + + if no_color: + for idx, rep in enumerate(list(reporter)): + if not isinstance(rep, ColorizedTextReporter): + continue + + if rep.out.buffer is sys.stdout.buffer: + warnings.warn( + WARN_NO_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + reporter.pop(idx) + reporter.append(TextReporter()) + + elif force_color: + for idx, rep in enumerate(list(reporter)): + # pylint: disable=unidiomatic-typecheck # Want explicit type check + if type(rep) is not TextReporter: + continue + + if rep.out.buffer is sys.stdout.buffer: + warnings.warn( + WARN_FORCE_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + reporter.pop(idx) + reporter.append(ColorizedTextReporter()) + + # pylint: disable=too-many-instance-attributes,too-many-public-methods class PyLinter( _ArgumentsManager, @@ -435,6 +508,8 @@ def _load_reporters(self, reporter_names: str) -> None: # Extend the lifetime of all opened output files close_output_files = stack.pop_all().close + _handle_force_color_no_color(sub_reporters) + if len(sub_reporters) > 1 or output_files: self.set_reporter( reporters.MultiReporter( diff --git a/pylint/reporters/__init__.py b/pylint/reporters/__init__.py index c24ea4be88..b5c4e3818c 100644 --- a/pylint/reporters/__init__.py +++ b/pylint/reporters/__init__.py @@ -19,6 +19,10 @@ from pylint.lint.pylinter import PyLinter +class ReporterWarning(Warning): + """Warning class for reporters.""" + + def initialize(linter: PyLinter) -> None: """Initialize linter with reporters in this package.""" utils.register_plugins(linter, __path__[0]) From 7e12334bea50e5aaf70ff504ae91de346b19f53f Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:01:55 +0300 Subject: [PATCH 04/16] `test_handle_force_color_no_color` --- tests/lint/test_pylinter.py | 151 +++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 1 deletion(-) diff --git a/tests/lint/test_pylinter.py b/tests/lint/test_pylinter.py index bc12455354..3f1267791b 100644 --- a/tests/lint/test_pylinter.py +++ b/tests/lint/test_pylinter.py @@ -2,17 +2,36 @@ # For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE # Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt +from __future__ import annotations + +import io import os +import sys +import warnings from pathlib import Path from typing import Any, NoReturn from unittest import mock from unittest.mock import patch +import pytest from pytest import CaptureFixture -from pylint.lint.pylinter import MANAGER, PyLinter +from pylint.lint.pylinter import ( + FORCE_COLOR, + MANAGER, + NO_COLOR, + PY_COLORS, + WARN_BOTH_COLOR_SET, + PyLinter, + _handle_force_color_no_color, +) +from pylint.reporters.text import ColorizedTextReporter, TextReporter from pylint.utils import FileState +COLORIZED_REPORTERS = "colorized_reporters" +TEXT_REPORTERS = "text_reporters" +STDOUT_TEXT = "stdout" + def raise_exception(*args: Any, **kwargs: Any) -> NoReturn: raise ValueError @@ -68,3 +87,133 @@ def test_open_pylinter_prefer_stubs(linter: PyLinter) -> None: assert MANAGER.prefer_stubs finally: MANAGER.prefer_stubs = False + + +def pytest_generate_tests(metafunc: pytest.Metafunc) -> None: + if metafunc.function.__name__ != test_handle_force_color_no_color.__name__: + return + + if ( + TEXT_REPORTERS not in metafunc.fixturenames + or COLORIZED_REPORTERS not in metafunc.fixturenames + ): + warnings.warn( + f"Missing fixture {TEXT_REPORTERS} or {COLORIZED_REPORTERS} in" + f" {test_handle_force_color_no_color.function.__name__}??", + stacklevel=2, + ) + return + + parameters = [] + + reporter_combinations = [ + ("file", STDOUT_TEXT), + ("file",), + (STDOUT_TEXT,), + ("",), + ] + + for tr in list(reporter_combinations): + for cr in list(reporter_combinations): + tr = tuple(t for t in tr if t) + cr = tuple(t for t in cr if t) + + total_reporters = len(tr) + len(cr) + unique_reporters = len(set(tr + cr)) + + if total_reporters == 0: + continue + + if unique_reporters != total_reporters: + continue + + parameters.append((tuple(tr), tuple(cr))) + + metafunc.parametrize( + f"{TEXT_REPORTERS}, {COLORIZED_REPORTERS}", parameters, ids=repr + ) + + +@pytest.mark.parametrize( + "no_color", + [True, False], + ids=lambda no_color: f"{no_color=}", +) +@pytest.mark.parametrize( + "py_colors", + [True, False], + ids=lambda py_colors: f"{py_colors=}", +) +@pytest.mark.parametrize( + "force_color", + [True, False], + ids=lambda force_color: f"{force_color=}", +) +def test_handle_force_color_no_color( + monkeypatch: pytest.MonkeyPatch, + recwarn: pytest.WarningsRecorder, + no_color: bool, + py_colors: bool, + force_color: bool, + text_reporters: tuple[str], + colorized_reporters: tuple[str], +) -> None: + monkeypatch.setenv(NO_COLOR, "1" if no_color else "") + monkeypatch.setenv(FORCE_COLOR, "1" if force_color else "") + monkeypatch.setenv(PY_COLORS, "1" if py_colors else "") + + force_color = force_color or py_colors + + if STDOUT_TEXT in text_reporters or STDOUT_TEXT in colorized_reporters: + monkeypatch.setattr(sys, STDOUT_TEXT, io.TextIOWrapper(io.BytesIO())) + + reporters = [] + for reporter, group in ( + (TextReporter, text_reporters), + (ColorizedTextReporter, colorized_reporters), + ): + for name in group: + if name == STDOUT_TEXT: + reporters.append(reporter()) + if name == "file": + reporters.append(reporter(io.TextIOWrapper(io.BytesIO()))) + + _handle_force_color_no_color(reporters) + + if no_color and force_color: + # Both NO_COLOR and FORCE_COLOR are set; expecting a warning. + both_color_warning = [ + idx + for idx, w in enumerate(recwarn.list) + if WARN_BOTH_COLOR_SET in str(w.message) + ] + assert len(both_color_warning) == 1 + recwarn.list.pop(both_color_warning[0]) + + if no_color: + # No ColorizedTextReporter expected to be connected to stdout. + assert all( + not isinstance(rep, ColorizedTextReporter) + for rep in reporters + if rep.out.buffer is sys.stdout.buffer + ) + + if STDOUT_TEXT in colorized_reporters: + assert len(recwarn.list) == 1 # expect a warning for overriding stdout + else: + assert len(recwarn.list) == 0 # no warning expected + elif force_color: + # No TextReporter expected to be connected to stdout. + # pylint: disable=unidiomatic-typecheck # Want explicit type check. + assert all( + type(rep) is not TextReporter + for rep in reporters + if rep.out.buffer is sys.stdout.buffer + ) + + if STDOUT_TEXT in text_reporters: + assert len(recwarn.list) == 1 # expect a warning for overriding stdout + else: + assert len(recwarn.list) == 0 # no warning expected + else: + assert len(recwarn.list) == 0 # no warning expected From 7f38e761ae31d4e5ae8a392e909fe92bc3e363b5 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Wed, 4 Oct 2023 14:19:15 +0300 Subject: [PATCH 05/16] `towncrier`: `3995.feature` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- doc/whatsnew/fragments/3995.feature | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 doc/whatsnew/fragments/3995.feature diff --git a/doc/whatsnew/fragments/3995.feature b/doc/whatsnew/fragments/3995.feature new file mode 100644 index 0000000000..5187567d09 --- /dev/null +++ b/doc/whatsnew/fragments/3995.feature @@ -0,0 +1,5 @@ +Support for ``NO_COLOR`` and ``FORCE_COLOR`` (and ``PY_COLORS``, as an alias to ``FORCE_COLOR``) environment variables has been added. +When running `pylint`, the reporter that reports to ``stdout`` will be modified according to the requested mode. +The order is: ``NO_COLOR`` > ``FORCE_COLOR``, ``PY_COLORS`` > ``--output=...``. + +Closes #3995 (https://github.com/pylint-dev/pylint/issues/3995). From 4427cf3cdde5d2490a7498a4b9d80dfc96724fa1 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Sat, 7 Oct 2023 23:31:15 +0300 Subject: [PATCH 06/16] Documentation for https://github.com/pylint-dev/pylint/issues/3995 Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- doc/user_guide/usage/output.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/user_guide/usage/output.rst b/doc/user_guide/usage/output.rst index 50a7fa76e4..0de49a0cb3 100644 --- a/doc/user_guide/usage/output.rst +++ b/doc/user_guide/usage/output.rst @@ -21,6 +21,19 @@ a colorized report to stdout at the same time: --output-format=json:somefile.json,colorized +Environment Variables +'''''''''''''''''''''''''''' +The colorization of the output can also be controlled through environment +variables. The precedence for determining output format is as follows: + +1. ``NO_COLOR`` +2. ``FORCE_COLOR``, or ``PY_COLORS`` +3. ``--output-format=...`` + +Setting ``NO_COLOR`` (to any value) will disable colorized output, while +``FORCE_COLOR`` or ``PY_COLORS`` (to any value) will enable it, overriding +the ``--output-format`` option if specified. + Custom message formats '''''''''''''''''''''' From 51cf8d2fcbfa852449d3208b91eca9bbbbf61729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sun, 12 Jan 2025 16:17:28 +0100 Subject: [PATCH 07/16] Remove `PY_COLORS`, fix spelling and don't modify in place --- custom_dict.txt | 1 + doc/user_guide/usage/output.rst | 6 +-- doc/whatsnew/fragments/3995.feature | 7 ++-- pylint/lint/pylinter.py | 63 +++++++++++++---------------- tests/lint/test_pylinter.py | 10 ----- tests/lint/test_utils.py | 9 +---- 6 files changed, 38 insertions(+), 58 deletions(-) diff --git a/custom_dict.txt b/custom_dict.txt index 265c7cb881..1996266e1e 100644 --- a/custom_dict.txt +++ b/custom_dict.txt @@ -108,6 +108,7 @@ encodings endswith enum enums +env epilog epylint epytext diff --git a/doc/user_guide/usage/output.rst b/doc/user_guide/usage/output.rst index 0de49a0cb3..9de4085291 100644 --- a/doc/user_guide/usage/output.rst +++ b/doc/user_guide/usage/output.rst @@ -27,12 +27,12 @@ The colorization of the output can also be controlled through environment variables. The precedence for determining output format is as follows: 1. ``NO_COLOR`` -2. ``FORCE_COLOR``, or ``PY_COLORS`` +2. ``FORCE_COLOR`` 3. ``--output-format=...`` Setting ``NO_COLOR`` (to any value) will disable colorized output, while -``FORCE_COLOR`` or ``PY_COLORS`` (to any value) will enable it, overriding -the ``--output-format`` option if specified. +``FORCE_COLOR`` (to any value) will enable it, overriding the +``--output-format`` option if specified. Custom message formats diff --git a/doc/whatsnew/fragments/3995.feature b/doc/whatsnew/fragments/3995.feature index 5187567d09..37e66a9121 100644 --- a/doc/whatsnew/fragments/3995.feature +++ b/doc/whatsnew/fragments/3995.feature @@ -1,5 +1,6 @@ -Support for ``NO_COLOR`` and ``FORCE_COLOR`` (and ``PY_COLORS``, as an alias to ``FORCE_COLOR``) environment variables has been added. -When running `pylint`, the reporter that reports to ``stdout`` will be modified according to the requested mode. -The order is: ``NO_COLOR`` > ``FORCE_COLOR``, ``PY_COLORS`` > ``--output=...``. +Support for ``NO_COLOR`` and ``FORCE_COLOR`` environment variables has been added. +When running `pylint`, the reporter that reports to ``stdout`` will be modified according +to the requested mode. +The order is: ``NO_COLOR`` > ``FORCE_COLOR`` > ``--output=...``. Closes #3995 (https://github.com/pylint-dev/pylint/issues/3995). diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index ba3c501fc4..da68a6fbee 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -74,7 +74,6 @@ NO_COLOR = "NO_COLOR" FORCE_COLOR = "FORCE_COLOR" -PY_COLORS = "PY_COLORS" WARN_FORCE_COLOR_SET = "FORCE_COLOR is set; ignoring `text` at stdout" WARN_NO_COLOR_SET = "NO_COLOR is set; ignoring `colorized` at stdout" @@ -261,10 +260,9 @@ def _load_reporter_by_class(reporter_class: str) -> type[BaseReporter]: } -def _handle_force_color_no_color(reporter: list[reporters.BaseReporter]) -> None: +def _handle_force_color_no_color(reporters: list[reporters.BaseReporter]) -> list[reporters.BaseReporter]: """ - Check ``NO_COLOR``, ``FORCE_COLOR``, ``PY_COLOR`` and modify the reporter list - accordingly. + Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list accordingly. Rules are presented in this table: +--------------+---------------+-----------------+------------------------------------------------------------+ @@ -281,9 +279,7 @@ def _handle_force_color_no_color(reporter: list[reporters.BaseReporter]) -> None +--------------+---------------+-----------------+------------------------------------------------------------+ """ no_color = _is_env_set_and_non_empty(NO_COLOR) - force_color = _is_env_set_and_non_empty(FORCE_COLOR) or _is_env_set_and_non_empty( - PY_COLORS - ) + force_color = _is_env_set_and_non_empty(FORCE_COLOR) if no_color and force_color: warnings.warn( @@ -293,34 +289,31 @@ def _handle_force_color_no_color(reporter: list[reporters.BaseReporter]) -> None ) force_color = False - if no_color: - for idx, rep in enumerate(list(reporter)): - if not isinstance(rep, ColorizedTextReporter): - continue - - if rep.out.buffer is sys.stdout.buffer: - warnings.warn( - WARN_NO_COLOR_SET, - ReporterWarning, - stacklevel=2, - ) - reporter.pop(idx) - reporter.append(TextReporter()) - - elif force_color: - for idx, rep in enumerate(list(reporter)): - # pylint: disable=unidiomatic-typecheck # Want explicit type check - if type(rep) is not TextReporter: - continue + final_reporters: list[reporters.BaseReporter] = [] + + for idx, rep in enumerate(reporters): + if no_color and isinstance(rep, ColorizedTextReporter) and rep.out.buffer is sys.stdout.buffer: + warnings.warn( + WARN_NO_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + final_reporters.append(TextReporter()) + + + # pylint: disable=unidiomatic-typecheck # Want explicit type check + elif force_color and type(rep) is TextReporter and rep.out.buffer is sys.stdout.buffer: + warnings.warn( + WARN_FORCE_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + final_reporters.append(ColorizedTextReporter()) - if rep.out.buffer is sys.stdout.buffer: - warnings.warn( - WARN_FORCE_COLOR_SET, - ReporterWarning, - stacklevel=2, - ) - reporter.pop(idx) - reporter.append(ColorizedTextReporter()) + else: + final_reporters.append(rep) + + return final_reporters # pylint: disable=too-many-instance-attributes,too-many-public-methods @@ -508,7 +501,7 @@ def _load_reporters(self, reporter_names: str) -> None: # Extend the lifetime of all opened output files close_output_files = stack.pop_all().close - _handle_force_color_no_color(sub_reporters) + sub_reporters = _handle_force_color_no_color(sub_reporters) if len(sub_reporters) > 1 or output_files: self.set_reporter( diff --git a/tests/lint/test_pylinter.py b/tests/lint/test_pylinter.py index 3f1267791b..b7f2ce42e3 100644 --- a/tests/lint/test_pylinter.py +++ b/tests/lint/test_pylinter.py @@ -20,7 +20,6 @@ FORCE_COLOR, MANAGER, NO_COLOR, - PY_COLORS, WARN_BOTH_COLOR_SET, PyLinter, _handle_force_color_no_color, @@ -139,11 +138,6 @@ def pytest_generate_tests(metafunc: pytest.Metafunc) -> None: [True, False], ids=lambda no_color: f"{no_color=}", ) -@pytest.mark.parametrize( - "py_colors", - [True, False], - ids=lambda py_colors: f"{py_colors=}", -) @pytest.mark.parametrize( "force_color", [True, False], @@ -153,16 +147,12 @@ def test_handle_force_color_no_color( monkeypatch: pytest.MonkeyPatch, recwarn: pytest.WarningsRecorder, no_color: bool, - py_colors: bool, force_color: bool, text_reporters: tuple[str], colorized_reporters: tuple[str], ) -> None: monkeypatch.setenv(NO_COLOR, "1" if no_color else "") monkeypatch.setenv(FORCE_COLOR, "1" if force_color else "") - monkeypatch.setenv(PY_COLORS, "1" if py_colors else "") - - force_color = force_color or py_colors if STDOUT_TEXT in text_reporters or STDOUT_TEXT in colorized_reporters: monkeypatch.setattr(sys, STDOUT_TEXT, io.TextIOWrapper(io.BytesIO())) diff --git a/tests/lint/test_utils.py b/tests/lint/test_utils.py index 6d11adf797..04a883e94d 100644 --- a/tests/lint/test_utils.py +++ b/tests/lint/test_utils.py @@ -70,22 +70,17 @@ def test_issue_template_on_fatal_errors(capsys: pytest.CaptureFixture) -> None: ("", False), (0, True), (1, True), - (2, True), (False, True), - ("no", True), - ("off", True), ("on", True), - (True, True), - ("yes", True), ], ids=repr, ) def test_is_env_set_and_non_empty( - monkeypatch: pytest.MonkeyPatch, value: Any, expected: bool + monkeypatch: pytest.MonkeyPatch, value: object, expected: bool ) -> None: """Test the function returns True if the environment variable is set and non-empty.""" env_var = "TEST_VAR" if value is not None: monkeypatch.setenv(env_var, str(value)) - assert _is_env_set_and_non_empty(env_var) == expected + assert _is_env_set_and_non_empty(env_var) is expected From 6d558c74caa141f3ec9aee72749f19f76293faba Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 12 Jan 2025 15:18:18 +0000 Subject: [PATCH 08/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/lint/pylinter.py | 24 +++++++++++++++++------- tests/lint/test_utils.py | 1 - 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index da68a6fbee..ae3d2476e7 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -260,9 +260,12 @@ def _load_reporter_by_class(reporter_class: str) -> type[BaseReporter]: } -def _handle_force_color_no_color(reporters: list[reporters.BaseReporter]) -> list[reporters.BaseReporter]: +def _handle_force_color_no_color( + reporters: list[reporters.BaseReporter], +) -> list[reporters.BaseReporter]: """ - Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list accordingly. + Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list + accordingly. Rules are presented in this table: +--------------+---------------+-----------------+------------------------------------------------------------+ @@ -290,9 +293,13 @@ def _handle_force_color_no_color(reporters: list[reporters.BaseReporter]) -> lis force_color = False final_reporters: list[reporters.BaseReporter] = [] - + for idx, rep in enumerate(reporters): - if no_color and isinstance(rep, ColorizedTextReporter) and rep.out.buffer is sys.stdout.buffer: + if ( + no_color + and isinstance(rep, ColorizedTextReporter) + and rep.out.buffer is sys.stdout.buffer + ): warnings.warn( WARN_NO_COLOR_SET, ReporterWarning, @@ -300,9 +307,12 @@ def _handle_force_color_no_color(reporters: list[reporters.BaseReporter]) -> lis ) final_reporters.append(TextReporter()) - # pylint: disable=unidiomatic-typecheck # Want explicit type check - elif force_color and type(rep) is TextReporter and rep.out.buffer is sys.stdout.buffer: + elif ( + force_color + and type(rep) is TextReporter + and rep.out.buffer is sys.stdout.buffer + ): warnings.warn( WARN_FORCE_COLOR_SET, ReporterWarning, @@ -312,7 +322,7 @@ def _handle_force_color_no_color(reporters: list[reporters.BaseReporter]) -> lis else: final_reporters.append(rep) - + return final_reporters diff --git a/tests/lint/test_utils.py b/tests/lint/test_utils.py index 04a883e94d..5957208df9 100644 --- a/tests/lint/test_utils.py +++ b/tests/lint/test_utils.py @@ -4,7 +4,6 @@ import unittest.mock from pathlib import Path, PosixPath -from typing import Any import pytest From 8d8f9884ddf7ff4c984b0e6eb6d1a1cc1ac77028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sun, 12 Jan 2025 16:21:12 +0100 Subject: [PATCH 09/16] Follow up to fixes --- pylint/lint/pylinter.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index ae3d2476e7..c93fcf9d07 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -261,11 +261,10 @@ def _load_reporter_by_class(reporter_class: str) -> type[BaseReporter]: def _handle_force_color_no_color( - reporters: list[reporters.BaseReporter], + reporters: Sequence[BaseReporter], ) -> list[reporters.BaseReporter]: """ - Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list - accordingly. + Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list. Rules are presented in this table: +--------------+---------------+-----------------+------------------------------------------------------------+ @@ -294,7 +293,7 @@ def _handle_force_color_no_color( final_reporters: list[reporters.BaseReporter] = [] - for idx, rep in enumerate(reporters): + for rep in reporters: if ( no_color and isinstance(rep, ColorizedTextReporter) From bb874735d36f898569b0939a941062a3bbb9154f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sun, 12 Jan 2025 16:23:16 +0100 Subject: [PATCH 10/16] Another follow up --- pylint/lint/pylinter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index c93fcf9d07..e5d6f754c1 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -291,7 +291,7 @@ def _handle_force_color_no_color( ) force_color = False - final_reporters: list[reporters.BaseReporter] = [] + final_reporters: list[BaseReporter] = [] for rep in reporters: if ( From 68b07a4cee1f0501f8c3127024ecf4477c8078ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sun, 12 Jan 2025 21:54:19 +0100 Subject: [PATCH 11/16] Make sure linting CI passes --- pylint/lint/pylinter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index e5d6f754c1..fca5ee83e1 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -306,10 +306,10 @@ def _handle_force_color_no_color( ) final_reporters.append(TextReporter()) - # pylint: disable=unidiomatic-typecheck # Want explicit type check elif ( force_color - and type(rep) is TextReporter + # Need type explicit check here + and type(rep) is TextReporter # pylint: disable=unidiomatic-typecheck and rep.out.buffer is sys.stdout.buffer ): warnings.warn( From cf95d2723cf4eeb55f01b9f7ed0f8f1c9ef572b6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 12 Jan 2025 20:55:09 +0000 Subject: [PATCH 12/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/lint/pylinter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index fca5ee83e1..3c0d850b7c 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -309,7 +309,7 @@ def _handle_force_color_no_color( elif ( force_color # Need type explicit check here - and type(rep) is TextReporter # pylint: disable=unidiomatic-typecheck + and type(rep) is TextReporter # pylint: disable=unidiomatic-typecheck and rep.out.buffer is sys.stdout.buffer ): warnings.warn( From c09eb185839df11633d50376cb77c80bd15ed074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sun, 12 Jan 2025 22:00:35 +0100 Subject: [PATCH 13/16] Rename to 'original_reporters' --- pylint/lint/pylinter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 3c0d850b7c..aac7515fd8 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -261,7 +261,7 @@ def _load_reporter_by_class(reporter_class: str) -> type[BaseReporter]: def _handle_force_color_no_color( - reporters: Sequence[BaseReporter], + original_reporters: Sequence[BaseReporter], ) -> list[reporters.BaseReporter]: """ Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list. @@ -293,7 +293,7 @@ def _handle_force_color_no_color( final_reporters: list[BaseReporter] = [] - for rep in reporters: + for rep in original_reporters: if ( no_color and isinstance(rep, ColorizedTextReporter) From cd55074064d72a3391df5fd4c3427a488535d3e1 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Mon, 13 Jan 2025 21:55:36 +0100 Subject: [PATCH 14/16] Update doc/whatsnew/fragments/3995.feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- doc/whatsnew/fragments/3995.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/3995.feature b/doc/whatsnew/fragments/3995.feature index 37e66a9121..93451dd720 100644 --- a/doc/whatsnew/fragments/3995.feature +++ b/doc/whatsnew/fragments/3995.feature @@ -3,4 +3,4 @@ When running `pylint`, the reporter that reports to ``stdout`` will be modified to the requested mode. The order is: ``NO_COLOR`` > ``FORCE_COLOR`` > ``--output=...``. -Closes #3995 (https://github.com/pylint-dev/pylint/issues/3995). +Closes #3995 From 260b671b756da1f4ec0f4f5432f2bd63189d14ef Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Tue, 11 Feb 2025 10:46:52 +0200 Subject: [PATCH 15/16] Constantify `test_handle_force_color_no_color_reporters` Instead of runtime-generating it each time Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- tests/lint/test_pylinter.py | 62 ++++++++++++------------------------- 1 file changed, 19 insertions(+), 43 deletions(-) diff --git a/tests/lint/test_pylinter.py b/tests/lint/test_pylinter.py index b7f2ce42e3..af7739f9d9 100644 --- a/tests/lint/test_pylinter.py +++ b/tests/lint/test_pylinter.py @@ -7,9 +7,8 @@ import io import os import sys -import warnings from pathlib import Path -from typing import Any, NoReturn +from typing import Any, NamedTuple, NoReturn from unittest import mock from unittest.mock import patch @@ -88,49 +87,21 @@ def test_open_pylinter_prefer_stubs(linter: PyLinter) -> None: MANAGER.prefer_stubs = False -def pytest_generate_tests(metafunc: pytest.Metafunc) -> None: - if metafunc.function.__name__ != test_handle_force_color_no_color.__name__: - return +class ReportersCombo(NamedTuple): + text_reporters: tuple[str, ...] + colorized_reporters: tuple[str, ...] - if ( - TEXT_REPORTERS not in metafunc.fixturenames - or COLORIZED_REPORTERS not in metafunc.fixturenames - ): - warnings.warn( - f"Missing fixture {TEXT_REPORTERS} or {COLORIZED_REPORTERS} in" - f" {test_handle_force_color_no_color.function.__name__}??", - stacklevel=2, - ) - return - - parameters = [] - - reporter_combinations = [ - ("file", STDOUT_TEXT), - ("file",), - (STDOUT_TEXT,), - ("",), - ] - - for tr in list(reporter_combinations): - for cr in list(reporter_combinations): - tr = tuple(t for t in tr if t) - cr = tuple(t for t in cr if t) - - total_reporters = len(tr) + len(cr) - unique_reporters = len(set(tr + cr)) - if total_reporters == 0: - continue - - if unique_reporters != total_reporters: - continue - - parameters.append((tuple(tr), tuple(cr))) - - metafunc.parametrize( - f"{TEXT_REPORTERS}, {COLORIZED_REPORTERS}", parameters, ids=repr - ) +test_handle_force_color_no_color_reporters = ( + ReportersCombo(("file", "stdout"), ()), + ReportersCombo(("file",), ("stdout",)), + ReportersCombo(("file",), ()), + ReportersCombo(("stdout",), ("file",)), + ReportersCombo(("stdout",), ()), + ReportersCombo((), ("file", "stdout")), + ReportersCombo((), ("file",)), + ReportersCombo((), ("stdout",)), +) @pytest.mark.parametrize( @@ -143,6 +114,11 @@ def pytest_generate_tests(metafunc: pytest.Metafunc) -> None: [True, False], ids=lambda force_color: f"{force_color=}", ) +@pytest.mark.parametrize( + "text_reporters, colorized_reporters", + test_handle_force_color_no_color_reporters, + ids=repr, +) def test_handle_force_color_no_color( monkeypatch: pytest.MonkeyPatch, recwarn: pytest.WarningsRecorder, From eadd83a0f36061811bdcf508d362ae7a261ddc6a Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Sun, 23 Mar 2025 23:57:25 +0200 Subject: [PATCH 16/16] Update `doc/user_guide/usage/output.rst` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) --- doc/user_guide/usage/output.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user_guide/usage/output.rst b/doc/user_guide/usage/output.rst index 9de4085291..a9560876c2 100644 --- a/doc/user_guide/usage/output.rst +++ b/doc/user_guide/usage/output.rst @@ -30,8 +30,8 @@ variables. The precedence for determining output format is as follows: 2. ``FORCE_COLOR`` 3. ``--output-format=...`` -Setting ``NO_COLOR`` (to any value) will disable colorized output, while -``FORCE_COLOR`` (to any value) will enable it, overriding the +Setting ``NO_COLOR`` (to any non-empty value) will disable colorized output, +while ``FORCE_COLOR`` (to any non-empty value) will enable it, overriding the ``--output-format`` option if specified.