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 50a7fa76e4..9de4085291 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`` +3. ``--output-format=...`` + +Setting ``NO_COLOR`` (to any value) will disable colorized output, while +``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 new file mode 100644 index 0000000000..93451dd720 --- /dev/null +++ b/doc/whatsnew/fragments/3995.feature @@ -0,0 +1,6 @@ +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 diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 615cf7c351..7a6cdff788 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,14 +49,16 @@ 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.progress_reporters import ProgressReporter -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, @@ -70,6 +73,13 @@ MANAGER = astroid.MANAGER +NO_COLOR = "NO_COLOR" +FORCE_COLOR = "FORCE_COLOR" + +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__( @@ -251,6 +261,71 @@ def _load_reporter_by_class(reporter_class: str) -> type[BaseReporter]: } +def _handle_force_color_no_color( + original_reporters: Sequence[BaseReporter], +) -> list[reporters.BaseReporter]: + """ + Check ``NO_COLOR`` and ``FORCE_COLOR``, and return the modified reporter list. + + 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) + + if no_color and force_color: + warnings.warn( + WARN_BOTH_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + force_color = False + + final_reporters: list[BaseReporter] = [] + + for rep in original_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()) + + elif ( + force_color + # Need type explicit check here + and type(rep) is TextReporter # pylint: disable=unidiomatic-typecheck + and rep.out.buffer is sys.stdout.buffer + ): + warnings.warn( + WARN_FORCE_COLOR_SET, + ReporterWarning, + stacklevel=2, + ) + final_reporters.append(ColorizedTextReporter()) + + else: + final_reporters.append(rep) + + return final_reporters + + # pylint: disable=too-many-instance-attributes,too-many-public-methods class PyLinter( _ArgumentsManager, @@ -436,6 +511,8 @@ def _load_reporters(self, reporter_names: str) -> None: # Extend the lifetime of all opened output files close_output_files = stack.pop_all().close + sub_reporters = _handle_force_color_no_color(sub_reporters) + if len(sub_reporters) > 1 or output_files: self.set_reporter( reporters.MultiReporter( diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index c5487a8c6d..6b4cda1de8 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 bool(os.environ.get(env_var)) 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]) diff --git a/tests/lint/test_pylinter.py b/tests/lint/test_pylinter.py index bc12455354..af7739f9d9 100644 --- a/tests/lint/test_pylinter.py +++ b/tests/lint/test_pylinter.py @@ -2,17 +2,34 @@ # 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 from pathlib import Path -from typing import Any, NoReturn +from typing import Any, NamedTuple, 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, + 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 +85,101 @@ def test_open_pylinter_prefer_stubs(linter: PyLinter) -> None: assert MANAGER.prefer_stubs finally: MANAGER.prefer_stubs = False + + +class ReportersCombo(NamedTuple): + text_reporters: tuple[str, ...] + colorized_reporters: tuple[str, ...] + + +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( + "no_color", + [True, False], + ids=lambda no_color: f"{no_color=}", +) +@pytest.mark.parametrize( + "force_color", + [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, + no_color: 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 "") + + 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 diff --git a/tests/lint/test_utils.py b/tests/lint/test_utils.py index fa449374a9..5957208df9 100644 --- a/tests/lint/test_utils.py +++ b/tests/lint/test_utils.py @@ -8,7 +8,11 @@ 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 +60,26 @@ 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), + (False, True), + ("on", True), + ], + ids=repr, +) +def test_is_env_set_and_non_empty( + 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) is expected