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

Add flake8 plugin to check textwrap.dedent usage. #17467

Merged
merged 4 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build-support/flake8/.flake8
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ extend-ignore:
[flake8:local-plugins]
extension =
PNT10 = bin_name_checker:check_for_hardcoded_pants_bin_name
PNT20 = dedent_use_checker:check_for_dedent_imports
61 changes: 61 additions & 0 deletions build-support/flake8/dedent_use_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
"""Prefer softwrap over dedent in non-test code."""

from __future__ import annotations

import ast
from pathlib import PurePath
from typing import Iterator


def check_for_dedent_imports(tree: ast.AST, filename: str) -> Iterator[tuple[int, int, str, None]]:
path = PurePath(filename)
if (
not filename.startswith("src/python")
or path.stem.startswith("test_")
or path.stem.endswith("_test")
):
return

violations: list[tuple[int, int, str, None]] = []

class Visitor(ast.NodeVisitor):
def visit_ImportFrom(self, node: ast.ImportFrom):
for alias in node.names:
if alias.name == "dedent":
violations.append(
(
node.lineno,
node.col_offset,
"PNT20 Don't import `textwrap.dedent`, import `pants.util.strutil.softwrap` instead",
None,
)
)

def visit_Call(self, node: ast.Call):
is_dedent_call = (
isinstance(node.func, ast.Attribute)
and isinstance(node.func.value, ast.Name)
and node.func.attr == "dedent"
and node.func.value.id == "textwrap"
)
if is_dedent_call:
violations.append(
(
node.lineno,
node.col_offset,
"PNT20 Don't call `textwrap.dedent`, call `softwrap` from `pants.util.strutil` instead",
None,
)
)
else:
for arg in node.args:
self.visit(arg)

Visitor().visit(tree)
yield from violations


setattr(check_for_dedent_imports, "name", __name__)
setattr(check_for_dedent_imports, "version", "0.0.0")
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from __future__ import annotations

from textwrap import dedent
from textwrap import dedent # noqa: PNT20

import pytest

Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/helm/subsystems/post_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import shlex
from dataclasses import dataclass
from pathlib import PurePath
from textwrap import dedent
from textwrap import dedent # noqa: PNT20
from typing import Any, Iterable, Mapping

import yaml
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/helm/testutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from __future__ import annotations

from textwrap import dedent
from textwrap import dedent # noqa: PNT20

from pants.backend.helm.util_rules.chart_metadata import DEFAULT_API_VERSION, ChartType

Expand Down
12 changes: 6 additions & 6 deletions src/python/pants/backend/python/goals/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import logging
import os
import textwrap
import uuid
from collections import defaultdict
from dataclasses import dataclass
Expand Down Expand Up @@ -377,12 +376,13 @@ async def export_virtualenvs(
warn_or_error(
"2.23.0.dev0",
"exporting resolves without using the --resolve option",
textwrap.dedent(
softwrap(
f"""
Use the --resolve flag one or more times to name the resolves you want to export,
and don't provide any target specs. E.g.,\n
`{bin_name()} export --resolve=python-default --resolve=pytest`
"""
Use the --resolve flag one or more times to name the resolves you want to export,
and don't provide any target specs. E.g.,

{bin_name()} export --resolve=python-default --resolve=pytest
"""
),
)
resolve_to_root_targets: DefaultDict[str, list[Target]] = defaultdict(list)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import os
from dataclasses import dataclass
from pathlib import PurePath
from textwrap import dedent
from textwrap import dedent # noqa: PNT20

from pants.backend.python.packaging.pyoxidizer.config import PyOxidizerConfig
from pants.backend.python.packaging.pyoxidizer.subsystem import PyOxidizer
Expand Down
15 changes: 7 additions & 8 deletions src/python/pants/backend/python/target_types_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from collections import defaultdict
from dataclasses import dataclass
from itertools import chain
from textwrap import dedent
from typing import DefaultDict, Dict, Generator, Optional, Tuple, cast

from pants.backend.python.dependency_inference.module_mapper import (
Expand Down Expand Up @@ -397,13 +396,13 @@ async def resolve_python_distribution_entry_points(
if category in ["console_scripts", "gui_scripts"] and not entry_point.function:
url = "https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html#the-console-scripts-entry-point"
raise InvalidEntryPoint(
dedent(
f"""\
Every entry point in `{category}` for {address} must end in the format `:my_func`,
but {name} set it to {entry_point.spec!r}. For example, set
`entry_points={{"{category}": {{"{name}": "{entry_point.module}:main}} }}`.
See {url}.
"""
softwrap(
f"""
Every entry point in `{category}` for {address} must end in the format
`:my_func`, but {name} set it to {entry_point.spec!r}. For example, set
`entry_points={{"{category}": {{"{name}": "{entry_point.module}:main}} }}`. See
{url}.
"""
)
)

Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import itertools
from dataclasses import dataclass
from hashlib import sha256
from textwrap import dedent
from textwrap import dedent # noqa: PNT20
from typing import Iterable, Optional, Tuple

import packaging
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import shlex
from dataclasses import dataclass
from pathlib import PurePath
from textwrap import dedent
from textwrap import dedent # noqa: PNT20
from typing import Iterable, Iterator, Mapping, TypeVar

import packaging.specifiers
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/scala/bsp/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ async def bsp_resolve_scala_metadata(
jdk = system_jdk

cmd = "leak_paths.sh"
leak_jdk_sandbox_paths = textwrap.dedent(
leak_jdk_sandbox_paths = textwrap.dedent( # noqa: PNT20
f"""\
# Script to leak JDK cache paths out of Coursier sandbox so that BSP can use them.

Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/shell/shell_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import re
import shlex
from dataclasses import dataclass
from textwrap import dedent
from textwrap import dedent # noqa: PNT20

from pants.backend.shell.builtin import BASH_BUILTIN_COMMANDS
from pants.backend.shell.shell_setup import ShellSetup
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/bin/pants_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import sys
import time
import warnings
from textwrap import dedent

from pants import ox
from pants.base.exiter import PANTS_FAILED_EXIT_CODE
Expand All @@ -18,6 +17,7 @@
RECURSION_LIMIT,
)
from pants.bin.pants_runner import PantsRunner
from pants.util.strutil import softwrap


class PantsLoader:
Expand Down Expand Up @@ -56,7 +56,7 @@ def ensure_locale(cls) -> None:
and os.environ.get(IGNORE_UNRECOGNIZED_ENCODING, None) is None
):
raise RuntimeError(
dedent(
softwrap(
f"""
Your system's preferred encoding is `{encoding}`, but Pants requires `UTF-8`.
Specifically, Python's `locale.getpreferredencoding()` must resolve to `UTF-8`.
Expand Down
14 changes: 7 additions & 7 deletions src/python/pants/bsp/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ def _setup_bsp_connection(

run_script_path = bsp_scripts_dir / "run-bsp.sh"
run_script_path.write_text(
textwrap.dedent(
textwrap.dedent( # noqa: PNT20
f"""\
#!/bin/sh
{run_script_env_lines_str}
exec 2>>{shlex.quote(str(bsp_logs_dir / 'stderr.log'))}
env 1>&2
exec {shlex.quote(bin_name())} --no-pantsd {self.name} --server
"""
#!/bin/sh
{run_script_env_lines_str}
exec 2>>{shlex.quote(str(bsp_logs_dir / 'stderr.log'))}
env 1>&2
exec {shlex.quote(bin_name())} --no-pantsd {self.name} --server
"""
)
)
run_script_path.chmod(0o755)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/core/util_rules/system_binaries.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import subprocess
from dataclasses import dataclass
from enum import Enum
from textwrap import dedent
from textwrap import dedent # noqa: PNT20
from typing import Iterable, Sequence

from pants.core.subsystems import python_bootstrap
Expand Down
14 changes: 7 additions & 7 deletions src/python/pants/engine/internals/selectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import ast
import itertools
from dataclasses import dataclass
from textwrap import dedent
from typing import (
TYPE_CHECKING,
Any,
Expand All @@ -26,6 +25,7 @@
PyGeneratorResponseGetMulti,
)
from pants.util.meta import frozen_after_init
from pants.util.strutil import softwrap

_Output = TypeVar("_Output")
_Input = TypeVar("_Input")
Expand Down Expand Up @@ -537,8 +537,8 @@ def render_arg(arg: Any) -> str | None:
)
if any(arg is None for arg in likely_args_exlicitly_passed):
raise ValueError(
dedent(
f"""\
softwrap(
f"""
Unexpected MultiGet None arguments: {', '.join(
map(str, likely_args_exlicitly_passed)
)}
Expand All @@ -550,13 +550,13 @@ def render_arg(arg: Any) -> str | None:
)

raise TypeError(
dedent(
f"""\
softwrap(
f"""
Unexpected MultiGet argument types: {', '.join(map(str, likely_args_exlicitly_passed))}

A MultiGet can be constructed in two ways:
1. MultiGet(Iterable[Get[T]]) -> Tuple[T]
2. MultiGet(Get[T1]], ...) -> Tuple[T1, T2, ...]
1. MultiGet(Iterable[Get[T]]) -> Tuple[T]
2. MultiGet(Get[T1]], ...) -> Tuple[T1, T2, ...]

The 1st form is intended for homogenous collections of Gets and emulates an
async `for ...` comprehension used to iterate over the collection in parallel and
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -2890,7 +2890,7 @@ def generate_multiple_sources_field_help_message(files_example: str) -> str:
def generate_file_based_overrides_field_help_message(
generated_target_name: str, example: str
) -> str:
example = textwrap.dedent(example.lstrip("\n"))
example = textwrap.dedent(example.lstrip("\n")) # noqa: PNT20
example = textwrap.indent(example, " " * 4)
return "\n".join(
[
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/jvm/jdk_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ def prefixed(arg: str) -> str:

# TODO: Locate `ln`.
version_comment = "\n".join(f"# {line}" for line in java_version.splitlines())
jdk_preparation_script = textwrap.dedent(
jdk_preparation_script = textwrap.dedent( # noqa: PNT20
f"""\
# pants javac script using Coursier {coursier_jdk_option}. `java -version`:"
{version_comment}
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/jvm/resolve/coursier_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from pants.util.ordered_set import FrozenOrderedSet
from pants.util.strutil import softwrap

COURSIER_POST_PROCESSING_SCRIPT = textwrap.dedent(
COURSIER_POST_PROCESSING_SCRIPT = textwrap.dedent( # noqa: PNT20
"""\
import json
import sys
Expand Down Expand Up @@ -66,7 +66,7 @@
"""
)

COURSIER_FETCH_WRAPPER_SCRIPT = textwrap.dedent(
COURSIER_FETCH_WRAPPER_SCRIPT = textwrap.dedent( # noqa: PNT20
"""\
set -eux

Expand All @@ -87,7 +87,7 @@

# TODO: Coursier renders setrlimit error line on macOS.
# see https://github.com/pantsbuild/pants/issues/13942.
POST_PROCESS_COURSIER_STDERR_SCRIPT = textwrap.dedent(
POST_PROCESS_COURSIER_STDERR_SCRIPT = textwrap.dedent( # noqa: PNT20
"""\
#!{python_path}
import sys
Expand Down
15 changes: 8 additions & 7 deletions src/python/pants/util/eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

from __future__ import annotations

from textwrap import dedent
from typing import Any

from pants.util.strutil import softwrap


def parse_expression(
val: str, acceptable_types: type | tuple[type, ...], name: str | None = None
Expand Down Expand Up @@ -43,11 +44,11 @@ def format_raw_value():
parsed_value = eval(val)
except Exception as e:
raise ValueError(
dedent(
f"""\
softwrap(
f"""
The {get_name()} cannot be evaluated as a literal expression: {e!r}
Given raw value:
{format_raw_value()}
{format_raw_value()}
"""
)
)
Expand All @@ -67,11 +68,11 @@ def iter_types(types):

expected_types = ", ".join(format_type(t) for t in iter_types(acceptable_types))
raise ValueError(
dedent(
f"""\
softwrap(
f"""
The {get_name()} is not of the expected type(s): {expected_types}:
Given the following raw value that evaluated to type {format_type(type(parsed_value))}:
{format_raw_value()}
{format_raw_value()}
"""
)
)
Expand Down