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

Take the FORCE_COLOR and NO_COLOR environment variables into account #9128

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
13 changes: 13 additions & 0 deletions doc/user_guide/usage/output.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
''''''''''''''''''''''
Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/3995.feature
Original file line number Diff line number Diff line change
@@ -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).
77 changes: 76 additions & 1 deletion pylint/lint/pylinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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__(
Expand Down Expand Up @@ -250,6 +261,68 @@
}


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) |

Check notice on line 273 in pylint/lint/pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

C0402

Wrong spelling of a word 'env' in a docstring:
| `bool: True` | `bool: True` | / | not colorized + warnings (inconsistent env var) |

Check notice on line 274 in pylint/lint/pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

C0402

Wrong spelling of a word 'env' in a docstring:
| 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,
Expand Down Expand Up @@ -435,6 +508,8 @@
# 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(
Expand Down
19 changes: 19 additions & 0 deletions pylint/lint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from __future__ import annotations

import contextlib
import os
import platform
import sys
import traceback
Expand Down Expand Up @@ -133,3 +134,21 @@ def augmented_sys_path(additional_paths: Sequence[str]) -> Iterator[None]:
yield
finally:
sys.path[:] = original


def _is_relative_to(self: Path, *other: Path) -> bool:
"""Checks if self is relative to other.

Backport of pathlib.Path.is_relative_to for Python <3.9
TODO: py39: Remove this backport and use stdlib function.
"""
try:
self.relative_to(*other)
return True
except ValueError:
return False


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))
4 changes: 4 additions & 0 deletions pylint/reporters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
142 changes: 141 additions & 1 deletion tests/lint/test_pylinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,35 @@
# 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,
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
Expand Down Expand Up @@ -50,21 +68,143 @@
assert any(m.symbol == "astroid-error" for m in linter.reporter.messages)


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


def test_open_pylinter_denied_modules(linter: PyLinter) -> None:
"""Test PyLinter open() adds ignored modules to Astroid manager deny list."""
MANAGER.module_denylist = {"mod1"}

Check failure on line 195 in tests/lint/test_pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

E0602

Undefined variable 'MANAGER'
try:
linter.config.ignored_modules = ["mod2", "mod3"]
linter.open()
assert MANAGER.module_denylist == {"mod1", "mod2", "mod3"}

Check failure on line 199 in tests/lint/test_pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

E0602

Undefined variable 'MANAGER'
finally:
MANAGER.module_denylist = set()

Check failure on line 201 in tests/lint/test_pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

E0602

Undefined variable 'MANAGER'


def test_open_pylinter_prefer_stubs(linter: PyLinter) -> None:
try:
linter.config.prefer_stubs = True
linter.open()
assert MANAGER.prefer_stubs

Check failure on line 208 in tests/lint/test_pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

E0602

Undefined variable 'MANAGER'
finally:
MANAGER.prefer_stubs = False

Check failure on line 210 in tests/lint/test_pylinter.py

View workflow job for this annotation

GitHub Actions / pylint

E0602

Undefined variable 'MANAGER'
Loading
Loading