Skip to content

Disable 80 col hard wrapping in mypy output #16488

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

Merged
merged 11 commits into from
Aug 19, 2022
19 changes: 18 additions & 1 deletion src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
from pants.engine.rules import Get, MultiGet, collect_rules, rule, rule_helper
from pants.engine.target import CoarsenedTargets, FieldSet, Target
from pants.engine.unions import UnionRule
from pants.option.global_options import GlobalOptions
from pants.util.logging import LogLevel
from pants.util.ordered_set import FrozenOrderedSet, OrderedSet
from pants.util.strutil import pluralize, shell_quote
Expand Down Expand Up @@ -144,6 +145,7 @@ async def mypy_typecheck_partition(
mkdir: MkdirBinary,
cp: CpBinary,
mv: MvBinary,
global_options: GlobalOptions,
) -> CheckResult:
# MyPy requires 3.5+ to run, but uses the typed-ast library to work with 2.7, 3.4, 3.5, 3.6,
# and 3.7. However, typed-ast does not understand 3.8+, so instead we must run MyPy with
Expand Down Expand Up @@ -319,6 +321,18 @@ async def mypy_typecheck_partition(
env = {
"PEX_EXTRA_SYS_PATH": ":".join(all_used_source_roots),
"MYPYPATH": ":".join(all_used_source_roots),
# Always emit colors to improve cache hit rates, the results are post-processed to match the
# global setting
"MYPY_FORCE_COLOR": "1",
# Mypy needs to know the terminal so it can use appropriate escape sequences. linux is a
# reasonable lowest common denominator for the sort of escapes mypy uses (NB. TERM=xterm
# uses some additional codes that colors.strip_color doesn't remove).
"TERM": "linux",
# Similarly, force a fixed terminal width. This is effectively infinite, disabling mypy's
# builtin truncation and line wrapping. Terminals do an acceptable job of soft-wrapping
# diagnostic text and source code is typically already hard-wrapped to a limited width.
# (Unique random number to make it easier to search for the source of this setting.)
"MYPY_FORCE_TERMINAL_WIDTH": "642092230765939",
}

process = await Get(
Expand All @@ -337,7 +351,10 @@ async def mypy_typecheck_partition(
result = await Get(FallibleProcessResult, Process, process)
report = await Get(Digest, RemovePrefix(result.output_digest, REPORT_DIR))
return CheckResult.from_fallible_process_result(
result, partition_description=partition.description(), report=report
result,
partition_description=partition.description(),
report=report,
strip_formatting=not global_options.colors,
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import annotations

import re
from textwrap import dedent

import pytest
Expand Down Expand Up @@ -872,3 +873,33 @@ def test_determine_python_files() -> None:
assert determine_python_files(["f.py", "f.pyi"]) == ("f.pyi",)
assert determine_python_files(["f.pyi", "f.py"]) == ("f.pyi",)
assert determine_python_files(["f.json"]) == ()


def test_colors_and_formatting(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
f"{PACKAGE}/f.py": dedent(
"""\
class incredibly_long_type_name_to_force_wrapping_if_mypy_wrapped_error_messages_12345678901234567890123456789012345678901234567890:
pass

x = incredibly_long_type_name_to_force_wrapping_if_mypy_wrapped_error_messages_12345678901234567890123456789012345678901234567890()
x.incredibly_long_attribute_name_to_force_wrapping_if_mypy_wrapped_error_messages_12345678901234567890123456789012345678901234567890
"""
),
f"{PACKAGE}/BUILD": "python_sources()",
}
)
tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="f.py"))

result = run_mypy(rule_runner, [tgt], extra_args=["--colors=true", "--mypy-args=--pretty"])

assert len(result) == 1
assert result[0].exit_code == 1
# all one line
assert re.search(
"error:.*incredibly_long_type_name.*incredibly_long_attribute_name", result[0].stdout
)
# at least one escape sequence that sets text color (red)
assert "\033[31m" in result[0].stdout
assert result[0].report == EMPTY_DIGEST
7 changes: 6 additions & 1 deletion src/python/pants/core/goals/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from dataclasses import dataclass
from typing import Any, Iterable, cast

import colors

from pants.core.goals.lint import REPORT_DIR as REPORT_DIR # noqa: F401
from pants.core.goals.style_request import (
StyleRequest,
Expand Down Expand Up @@ -46,10 +48,13 @@ def from_fallible_process_result(
*,
partition_description: str | None = None,
strip_chroot_path: bool = False,
strip_formatting: bool = False,
report: Digest = EMPTY_DIGEST,
) -> CheckResult:
def prep_output(s: bytes) -> str:
return strip_v2_chroot_path(s) if strip_chroot_path else s.decode()
chroot = strip_v2_chroot_path(s) if strip_chroot_path else s.decode()
formatting = cast(str, colors.strip_color(chroot)) if strip_formatting else chroot
return formatting

return CheckResult(
exit_code=process_result.exit_code,
Expand Down
37 changes: 36 additions & 1 deletion src/python/pants/core/goals/check_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from textwrap import dedent
from typing import Iterable, Optional, Sequence, Tuple, Type

import pytest

from pants.core.goals.check import (
Check,
CheckRequest,
Expand All @@ -18,7 +20,9 @@
)
from pants.core.util_rules.distdir import DistDir
from pants.engine.addresses import Address
from pants.engine.fs import Workspace
from pants.engine.fs import EMPTY_DIGEST, EMPTY_FILE_DIGEST, Workspace
from pants.engine.platform import Platform
from pants.engine.process import FallibleProcessResult, ProcessResultMetadata
from pants.engine.target import FieldSet, MultipleSourcesField, Target, Targets
from pants.engine.unions import UnionMembership
from pants.testutil.option_util import create_options_bootstrapper, create_subsystem
Expand Down Expand Up @@ -242,3 +246,34 @@ def test_streaming_output_partitions() -> None:

"""
)


@pytest.mark.parametrize(
("strip_chroot_path", "strip_formatting", "expected"),
[
(False, False, "\033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m"),
(False, True, "/var/pants-sandbox-123/red/path.py bold"),
(True, False, "\033[0;31mred/path.py\033[0m \033[1mbold\033[0m"),
(True, True, "red/path.py bold"),
],
)
def test_from_fallible_process_result_output_prepping(
strip_chroot_path: bool, strip_formatting: bool, expected: str
) -> None:
result = CheckResult.from_fallible_process_result(
FallibleProcessResult(
exit_code=0,
stdout=b"stdout \033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m",
stdout_digest=EMPTY_FILE_DIGEST,
stderr=b"stderr \033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m",
stderr_digest=EMPTY_FILE_DIGEST,
output_digest=EMPTY_DIGEST,
platform=Platform.current,
metadata=ProcessResultMetadata(0, "ran_locally", 0),
),
strip_chroot_path=strip_chroot_path,
strip_formatting=strip_formatting,
)

assert result.stdout == "stdout " + expected
assert result.stderr == "stderr " + expected