From 2dd8a662ef6089d3d08bcbecce56dccd517e02e8 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Fri, 4 Nov 2022 11:26:38 -0400 Subject: [PATCH 1/3] Add flake8 plugin to check textwrap.dedent usage. --- build-support/flake8/.flake8 | 1 + build-support/flake8/dedent_use_checker.py | 61 ++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 build-support/flake8/dedent_use_checker.py diff --git a/build-support/flake8/.flake8 b/build-support/flake8/.flake8 index 3de5d18f462..354727d7af8 100644 --- a/build-support/flake8/.flake8 +++ b/build-support/flake8/.flake8 @@ -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 diff --git a/build-support/flake8/dedent_use_checker.py b/build-support/flake8/dedent_use_checker.py new file mode 100644 index 00000000000..8fca2183b18 --- /dev/null +++ b/build-support/flake8/dedent_use_checker.py @@ -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") From 3159ea021d678aea2d54e5659ed47fa791efe4e1 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Fri, 4 Nov 2022 12:19:17 -0400 Subject: [PATCH 2/3] add noqa for legitimate uses of dedent. --- .../backend/explorer/graphql/query/conftest.py | 2 +- .../backend/helm/subsystems/post_renderer.py | 2 +- src/python/pants/backend/helm/testutil.py | 2 +- .../backend/python/packaging/pyoxidizer/rules.py | 2 +- .../pants/backend/python/target_types_rules.py | 15 +++++++-------- .../pants/backend/python/typecheck/mypy/rules.py | 2 +- src/python/pants/backend/python/util_rules/pex.py | 2 +- src/python/pants/backend/scala/bsp/rules.py | 2 +- src/python/pants/backend/shell/shell_command.py | 2 +- src/python/pants/bin/pants_loader.py | 4 ++-- src/python/pants/bsp/goal.py | 14 +++++++------- .../pants/core/util_rules/system_binaries.py | 2 +- src/python/pants/engine/internals/selectors.py | 14 +++++++------- src/python/pants/engine/target.py | 2 +- src/python/pants/jvm/jdk_rules.py | 2 +- src/python/pants/jvm/resolve/coursier_setup.py | 6 +++--- src/python/pants/util/eval.py | 15 ++++++++------- 17 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/python/pants/backend/explorer/graphql/query/conftest.py b/src/python/pants/backend/explorer/graphql/query/conftest.py index 1a15ed47341..4891c56e100 100644 --- a/src/python/pants/backend/explorer/graphql/query/conftest.py +++ b/src/python/pants/backend/explorer/graphql/query/conftest.py @@ -3,7 +3,7 @@ from __future__ import annotations -from textwrap import dedent +from textwrap import dedent # noqa: PNT20 import pytest diff --git a/src/python/pants/backend/helm/subsystems/post_renderer.py b/src/python/pants/backend/helm/subsystems/post_renderer.py index 03740efafe2..3ea576308a6 100644 --- a/src/python/pants/backend/helm/subsystems/post_renderer.py +++ b/src/python/pants/backend/helm/subsystems/post_renderer.py @@ -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 diff --git a/src/python/pants/backend/helm/testutil.py b/src/python/pants/backend/helm/testutil.py index b20069631d7..42e3c88a567 100644 --- a/src/python/pants/backend/helm/testutil.py +++ b/src/python/pants/backend/helm/testutil.py @@ -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 diff --git a/src/python/pants/backend/python/packaging/pyoxidizer/rules.py b/src/python/pants/backend/python/packaging/pyoxidizer/rules.py index 18d3d67cebc..c77222dc062 100644 --- a/src/python/pants/backend/python/packaging/pyoxidizer/rules.py +++ b/src/python/pants/backend/python/packaging/pyoxidizer/rules.py @@ -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 diff --git a/src/python/pants/backend/python/target_types_rules.py b/src/python/pants/backend/python/target_types_rules.py index 0ac597d98be..500554fdab3 100644 --- a/src/python/pants/backend/python/target_types_rules.py +++ b/src/python/pants/backend/python/target_types_rules.py @@ -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 ( @@ -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}. + """ ) ) diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py index 438f5338a7f..e6cc748bfd3 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules.py @@ -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 diff --git a/src/python/pants/backend/python/util_rules/pex.py b/src/python/pants/backend/python/util_rules/pex.py index fb541d20ba0..2a97dc78568 100644 --- a/src/python/pants/backend/python/util_rules/pex.py +++ b/src/python/pants/backend/python/util_rules/pex.py @@ -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 diff --git a/src/python/pants/backend/scala/bsp/rules.py b/src/python/pants/backend/scala/bsp/rules.py index ca880194188..0b242994bc7 100644 --- a/src/python/pants/backend/scala/bsp/rules.py +++ b/src/python/pants/backend/scala/bsp/rules.py @@ -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. diff --git a/src/python/pants/backend/shell/shell_command.py b/src/python/pants/backend/shell/shell_command.py index c51c1d89dde..4982df6f4e6 100644 --- a/src/python/pants/backend/shell/shell_command.py +++ b/src/python/pants/backend/shell/shell_command.py @@ -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 diff --git a/src/python/pants/bin/pants_loader.py b/src/python/pants/bin/pants_loader.py index 86271d76990..b493485a6c2 100644 --- a/src/python/pants/bin/pants_loader.py +++ b/src/python/pants/bin/pants_loader.py @@ -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 @@ -18,6 +17,7 @@ RECURSION_LIMIT, ) from pants.bin.pants_runner import PantsRunner +from pants.util.strutil import softwrap class PantsLoader: @@ -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`. diff --git a/src/python/pants/bsp/goal.py b/src/python/pants/bsp/goal.py index d548dc4c9fe..95a5d944877 100644 --- a/src/python/pants/bsp/goal.py +++ b/src/python/pants/bsp/goal.py @@ -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) diff --git a/src/python/pants/core/util_rules/system_binaries.py b/src/python/pants/core/util_rules/system_binaries.py index b17c13eb917..e5382594b90 100644 --- a/src/python/pants/core/util_rules/system_binaries.py +++ b/src/python/pants/core/util_rules/system_binaries.py @@ -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 diff --git a/src/python/pants/engine/internals/selectors.py b/src/python/pants/engine/internals/selectors.py index e79a0c34cac..07be1aa9253 100644 --- a/src/python/pants/engine/internals/selectors.py +++ b/src/python/pants/engine/internals/selectors.py @@ -6,7 +6,6 @@ import ast import itertools from dataclasses import dataclass -from textwrap import dedent from typing import ( TYPE_CHECKING, Any, @@ -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") @@ -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) )} @@ -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 diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 2af107e82ad..5626a27a53d 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -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( [ diff --git a/src/python/pants/jvm/jdk_rules.py b/src/python/pants/jvm/jdk_rules.py index 4c7d01908d8..cf61f96d9f0 100644 --- a/src/python/pants/jvm/jdk_rules.py +++ b/src/python/pants/jvm/jdk_rules.py @@ -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} diff --git a/src/python/pants/jvm/resolve/coursier_setup.py b/src/python/pants/jvm/resolve/coursier_setup.py index facaf288867..d87e1310c26 100644 --- a/src/python/pants/jvm/resolve/coursier_setup.py +++ b/src/python/pants/jvm/resolve/coursier_setup.py @@ -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 @@ -66,7 +66,7 @@ """ ) -COURSIER_FETCH_WRAPPER_SCRIPT = textwrap.dedent( +COURSIER_FETCH_WRAPPER_SCRIPT = textwrap.dedent( # noqa: PNT20 """\ set -eux @@ -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 diff --git a/src/python/pants/util/eval.py b/src/python/pants/util/eval.py index a3a957a56f2..bb35ca839b5 100644 --- a/src/python/pants/util/eval.py +++ b/src/python/pants/util/eval.py @@ -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 @@ -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()} """ ) ) @@ -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()} """ ) ) From 571cc15060265495b7c199c0c2de758ec944a278 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Fri, 4 Nov 2022 12:33:42 -0400 Subject: [PATCH 3/3] Fix python deprecation message for the export goal. Before: 12:27:46.73 [WARN] DEPRECATED: exporting resolves without using the --resolve option is scheduled to be removed in version 2.23.0.dev0. 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., `./pants export --resolve=python-default --resolve=pytest` After: 12:31:47.95 [WARN] DEPRECATED: exporting resolves without using the --resolve option is scheduled to be removed in version 2.23.0.dev0. 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., ./pants export --resolve=python-default --resolve=pytest --- src/python/pants/backend/python/goals/export.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/python/goals/export.py b/src/python/pants/backend/python/goals/export.py index 68a9a5c3158..2ba84fc0059 100644 --- a/src/python/pants/backend/python/goals/export.py +++ b/src/python/pants/backend/python/goals/export.py @@ -5,7 +5,6 @@ import logging import os -import textwrap import uuid from collections import defaultdict from dataclasses import dataclass @@ -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)