From 860be3d4e950df2cd953b8b21130c5b52e660af2 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 8 Jul 2024 17:24:56 -0700 Subject: [PATCH] refactor output format and config handling for cwd, more tests, reword docs --- docs/guide/configuration.rst | 29 +++++---- src/fixit/api.py | 44 ++++++------- src/fixit/cli.py | 38 +++++------- src/fixit/config.py | 62 +++++++------------ src/fixit/ftypes.py | 38 +++++------- src/fixit/tests/config.py | 117 +++++++++++++++++++++-------------- src/fixit/util.py | 11 ++++ 7 files changed, 172 insertions(+), 167 deletions(-) diff --git a/docs/guide/configuration.rst b/docs/guide/configuration.rst index 6a414a08..01f687a3 100644 --- a/docs/guide/configuration.rst +++ b/docs/guide/configuration.rst @@ -152,40 +152,43 @@ The main configuration table. :class:`~fixit.Formatter` interface. .. attribute:: output-format - :type: Literal['fixit', 'vscode', 'custom'] + :type: str Choose one of the presets for terminal output formatting. - Fixit only reads this option from the current working directory or from an explicitly specified config file. - - Defaults to fixit's output style. + This option is inferred based on the current working directory or from + an explicity specified config file -- subpath overrides will be ignored. Can be one of: - - ``fixit``: Fixit's default output format - - ``vscode``: A format that works well with Visual Studio Code - - ``custom``: Use your own format via :attr:`output-template` config option + - ``custom``: Specify your own format using the :attr:`output-template` + option below. + - ``fixit``: Fixit's default output format. + - ``vscode``: A format that provides clickable paths for Visual Studio Code. .. attribute:: output-template :type: str Sets the format of output printed to terminal. Python formatting is used in the background to fill in data. - Only active with ``output-format = 'custom'``. - Like :attr:`output-format`, this config variable is only read from the current working directory. + Only active with :attr:`output-format` set to ``custom``. - Defaults to ``{path}@{start_line}:{start_col} {rule_name}: {message}`` + This option is inferred based on the current working directory or from + an explicity specified config file -- subpath overrides will be ignored. Supported variables: + - ``message``: Message emitted by the applied rule. + - ``path``: Path to affected file. - - ``start_line``: Start line of affected code lines. + - ``result``: Raw :class:`~fixit.Result` object. + + - ``rule_name``: Name of the applied rule. - ``start_col``: Start column of affected code. - - ``rule_name``: Name of the applied rule + - ``start_line``: Start line of affected code. - - ``message``: Message emitted by the applied rule .. _rule-options: diff --git a/src/fixit/api.py b/src/fixit/api.py index fd9e1ba0..d9fb7420 100644 --- a/src/fixit/api.py +++ b/src/fixit/api.py @@ -14,7 +14,7 @@ import trailrunner from moreorless.click import echo_color_precomputed_diff -from .config import collect_rules, generate_config, output_formats_templates +from .config import collect_rules, generate_config from .engine import LintRunner from .format import format_module from .ftypes import ( @@ -22,7 +22,7 @@ FileContent, LintViolation, Options, - OutputFormatTypeInput, + OutputFormat, Result, STDIN, ) @@ -35,8 +35,8 @@ def print_result( *, show_diff: bool = False, stderr: bool = False, - output_format_type: OutputFormatTypeInput = "fixit", - output_template: str = output_formats_templates["fixit"], + output_format: OutputFormat = OutputFormat.fixit, + output_template: str = "", ) -> int: """ Print linting results in a simple format designed for human eyes. @@ -52,9 +52,6 @@ def print_result( except ValueError: pass - if output_format_type != "custom": - output_template = output_formats_templates[output_format_type] - if result.violation: rule_name = result.violation.rule_name start_line = result.violation.range.start.line @@ -62,17 +59,24 @@ def print_result( message = result.violation.message if result.violation.autofixable: message += " (has autofix)" - click.secho( - output_template.format( + + if output_format == OutputFormat.fixit: + line = f"{path}@{start_line}:{start_col} {rule_name}: {message}" + elif output_format == OutputFormat.vscode: + line = f"{path}:{start_line}:{start_col} {rule_name}: {message}" + elif output_format == OutputFormat.custom: + line = output_template.format( + message=message, path=path, - start_line=start_line, - start_col=start_col, + result=result, rule_name=rule_name, - message=message, - ), - fg="yellow", - err=stderr, - ) + start_col=start_col, + start_line=start_line, + ) + else: + raise NotImplementedError(f"output-format = {output_format!r}") + click.secho(line, fg="yellow", err=stderr) + if show_diff and result.violation.diff: echo_color_precomputed_diff(result.violation.diff) return True @@ -125,7 +129,7 @@ def fixit_bytes( clean = True for violation in runner.collect_violations(rules, config): clean = False - fix = yield Result(path, violation=violation) + fix = yield Result(path, violation) if fix or autofix: pending_fixes.append(violation) @@ -139,11 +143,7 @@ def fixit_bytes( except Exception as error: # TODO: this is not the right place to catch errors LOG.debug("Exception while linting", exc_info=error) - yield Result( - path, - violation=None, - error=(error, traceback.format_exc()), - ) + yield Result(path, violation=None, error=(error, traceback.format_exc())) return None diff --git a/src/fixit/cli.py b/src/fixit/cli.py index 791cba38..f36cb0f9 100644 --- a/src/fixit/cli.py +++ b/src/fixit/cli.py @@ -7,22 +7,15 @@ import sys import unittest from pathlib import Path -from typing import Dict, get_args, Optional, Sequence, Set, Type +from typing import Dict, Optional, Sequence, Set, Type import click from fixit import __version__ from .api import fixit_paths, print_result -from .config import collect_rules, generate_config, get_cwd_config, parse_rule -from .ftypes import ( - Config, - LSPOptions, - Options, - OutputFormatTypeInput, - QualifiedRule, - Tags, -) +from .config import collect_rules, generate_config, parse_rule +from .ftypes import Config, LSPOptions, Options, OutputFormat, QualifiedRule, Tags from .rule import LintRule from .testing import generate_lint_rule_test_cases from .util import capture @@ -82,15 +75,16 @@ def f(v: int) -> str: @click.option( "--output-format", "-o", - type=click.Choice(get_args(OutputFormatTypeInput), case_sensitive=False), + type=click.Choice([o.name for o in OutputFormat], case_sensitive=False), + show_choices=True, default=None, - help="Select output format type [fixit, vscode, json, custom]", + help="Select output format type", ) @click.option( "--output-template", type=str, - default=None, - help="Python format Template to use with output format 'custom'", + default="", + help="Python format template to use with output format 'custom'", ) def main( ctx: click.Context, @@ -98,8 +92,8 @@ def main( config_file: Optional[Path], tags: str, rules: str, - output_format: Optional[OutputFormatTypeInput], - output_template: Optional[str], + output_format: Optional[OutputFormat], + output_template: str, ) -> None: level = logging.WARNING if debug is not None: @@ -145,14 +139,14 @@ def lint( visited: Set[Path] = set() dirty: Set[Path] = set() autofixes = 0 - cwd_config = get_cwd_config(options=options) + config = generate_config(options=options) for result in fixit_paths(paths, options=options): visited.add(result.path) if print_result( result, show_diff=diff, - output_format_type=cwd_config.output_format, - output_template=cwd_config.output_template, + output_format=config.output_format, + output_template=config.output_template, ): dirty.add(result.path) if result.violation: @@ -208,7 +202,7 @@ def fix( generator = capture( fixit_paths(paths, autofix=autofix, options=options, parallel=False) ) - cwd_config = get_cwd_config(options=options) + config = generate_config(options=options) for result in generator: visited.add(result.path) # for STDIN, we need STDOUT to equal the fixed content, so @@ -217,8 +211,8 @@ def fix( result, show_diff=interactive or diff, stderr=is_stdin, - output_format_type=cwd_config.output_format, - output_template=cwd_config.output_template, + output_format=config.output_format, + output_template=config.output_template, ): dirty.add(result.path) if autofix and result.violation and result.violation.autofixable: diff --git a/src/fixit/config.py b/src/fixit/config.py index f219fd26..b2a31944 100644 --- a/src/fixit/config.py +++ b/src/fixit/config.py @@ -35,12 +35,10 @@ from .format import FORMAT_STYLES from .ftypes import ( Config, - CwdConfig, is_collection, is_sequence, Options, - OutputFormatType, - OutputFormatTypeInput, + OutputFormat, QualifiedRule, QualifiedRuleRegex, RawConfig, @@ -58,13 +56,6 @@ FIXIT_CONFIG_FILENAMES = ("fixit.toml", ".fixit.toml", "pyproject.toml") FIXIT_LOCAL_MODULE = "fixit.local" -CWD_CONFIG_KEYS = ("output-format", "output-template") - - -output_formats_templates: Dict[OutputFormatType, str] = { - "fixit": "{path}@{start_line}:{start_col} {rule_name}: {message}", - "vscode": "{path}:{start_line}:{start_col} {rule_name}: {message}", -} log = logging.getLogger(__name__) @@ -414,6 +405,8 @@ def merge_configs( rule_options: RuleOptionsTable = {} target_python_version: Optional[Version] = Version(platform.python_version()) target_formatter: Optional[str] = None + output_format: OutputFormat = OutputFormat.fixit + output_template: str = "" def process_subpath( subpath: Path, @@ -495,6 +488,17 @@ def process_subpath( else: enable_root_import = True + if value := data.pop("output-format", ""): + try: + output_format = OutputFormat(value) + except ValueError as e: + raise ConfigError( + "output-format: unknown value {value!r}", config=config + ) from e + + if value := data.pop("output-template", ""): + output_template = value + process_subpath( config.path.parent, enable=get_sequence(config, "enable"), @@ -525,8 +529,7 @@ def process_subpath( ) for key in data.keys(): - if key not in CWD_CONFIG_KEYS: - log.warning("unknown configuration option %r", key) + log.warning("unknown configuration option %r", key) return Config( path=path, @@ -537,16 +540,21 @@ def process_subpath( options=rule_options, python_version=target_python_version, formatter=target_formatter, + output_format=output_format, + output_template=output_template, ) def generate_config( - path: Path, root: Optional[Path] = None, *, options: Optional[Options] = None + path: Optional[Path] = None, + root: Optional[Path] = None, + *, + options: Optional[Options] = None, ) -> Config: """ Given a file path, walk upwards looking for and applying cascading configs """ - path = path.resolve() + path = (path or Path.cwd()).resolve() if root is not None: root = root.resolve() @@ -567,32 +575,6 @@ def generate_config( config.enable = list(options.rules) config.disable = [] - return config - - -def get_cwd_config(options: Optional[Options] = None) -> CwdConfig: - config = CwdConfig() - if options and options.config_file: - paths = [options.config_file] - else: - cwd = Path.cwd() - paths = locate_configs(cwd, cwd) - - raw_configs = read_configs(paths) - - output_format: Optional[OutputFormatTypeInput] = None - output_template: Optional[str] = None - for raw_config in raw_configs: - - if output_format is None: - output_format = raw_config.data.get("output-format") - if output_template is None: - output_template = raw_config.data.get("output-template") - - config.output_format = output_format or "fixit" - config.output_template = output_template or output_formats_templates["fixit"] - - if options: if options.output_format: config.output_format = options.output_format diff --git a/src/fixit/ftypes.py b/src/fixit/ftypes.py index cc6beac5..9e02aef1 100644 --- a/src/fixit/ftypes.py +++ b/src/fixit/ftypes.py @@ -6,6 +6,7 @@ import platform import re from dataclasses import dataclass, field +from enum import Enum from pathlib import Path from typing import ( Any, @@ -16,7 +17,6 @@ Dict, Iterable, List, - Literal, Optional, Sequence, Tuple, @@ -50,11 +50,14 @@ TimingsHook = Callable[[Timings], None] VisitorMethod = Callable[[CSTNode], None] -VisitHook = Callable[[str], ContextManager] -OutputFormatType = Literal["fixit", "vscode", "json"] -OutputFormatTypeInput = Literal[OutputFormatType, "custom"] +VisitHook = Callable[[str], ContextManager[None]] -Version + +class OutputFormat(str, Enum): + custom = "custom" + fixit = "fixit" + # json = "json" # TODO + vscode = "vscode" @dataclass(frozen=True) @@ -182,12 +185,12 @@ class Options: Command-line options to affect runtime behavior """ - debug: Optional[bool] - config_file: Optional[Path] + debug: Optional[bool] = None + config_file: Optional[Path] = None tags: Optional[Tags] = None rules: Sequence[QualifiedRule] = () - output_format: Optional[OutputFormatTypeInput] = None - output_template: Optional[str] = None + output_format: Optional[OutputFormat] = None + output_template: str = "" @dataclass @@ -230,22 +233,13 @@ class Config: # post-run processing formatter: Optional[str] = None - def __post_init__(self) -> None: - from .config import DEFAULT_OUTPUT_FORMAT - - self.path = self.path.resolve() - self.root = self.root.resolve() - - -@dataclass -class CwdConfig: - output_format: OutputFormatTypeInput = "fixit" + # output formatting options + output_format: OutputFormat = OutputFormat.fixit output_template: str = "" def __post_init__(self) -> None: - from .config import output_formats_templates - - self.output_template = output_formats_templates["fixit"] + self.path = self.path.resolve() + self.root = self.root.resolve() @dataclass diff --git a/src/fixit/tests/config.py b/src/fixit/tests/config.py index 678a9981..77defd9e 100644 --- a/src/fixit/tests/config.py +++ b/src/fixit/tests/config.py @@ -3,7 +3,6 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -import os from dataclasses import asdict from pathlib import Path from tempfile import TemporaryDirectory @@ -14,10 +13,18 @@ from click.testing import CliRunner from .. import config - from ..cli import main -from ..ftypes import Config, QualifiedRule, RawConfig, Tags, Version +from ..ftypes import ( + Config, + Options, + OutputFormat, + QualifiedRule, + RawConfig, + Tags, + Version, +) from ..rule import LintRule +from ..util import chdir class ConfigTest(TestCase): @@ -345,11 +352,12 @@ def test_merge_configs(self) -> None: self.assertEqual(expected, actual) def test_generate_config(self) -> None: - for name, path, root, expected in ( + for name, path, root, options, expected in ( ( "inner", self.inner / "foo.py", None, + None, Config( path=self.inner / "foo.py", root=self.inner, @@ -365,6 +373,7 @@ def test_generate_config(self) -> None: "outer", self.outer / "foo.py", None, + None, Config( path=self.outer / "foo.py", root=self.tdp, @@ -384,6 +393,7 @@ def test_generate_config(self) -> None: "outer with root", self.outer / "foo.py", self.outer, + None, Config( path=self.outer / "foo.py", root=self.outer, @@ -395,6 +405,7 @@ def test_generate_config(self) -> None: "other", self.tdp / "other" / "foo.py", None, + None, Config( path=self.tdp / "other" / "foo.py", root=self.tdp, @@ -416,6 +427,21 @@ def test_generate_config(self) -> None: "root", self.tdp / "foo.py", None, + None, + Config( + path=self.tdp / "foo.py", + root=self.tdp, + enable_root_import=True, + enable=[QualifiedRule("fixit.rules"), QualifiedRule("more.rules")], + disable=[QualifiedRule("fixit.rules.SomethingSpecific")], + python_version=Version("3.8"), + ), + ), + ( + "root with options", + self.tdp / "foo.py", + None, + Options(output_format=OutputFormat.custom, output_template="foo-bar"), Config( path=self.tdp / "foo.py", root=self.tdp, @@ -423,11 +449,13 @@ def test_generate_config(self) -> None: enable=[QualifiedRule("fixit.rules"), QualifiedRule("more.rules")], disable=[QualifiedRule("fixit.rules.SomethingSpecific")], python_version=Version("3.8"), + output_format=OutputFormat.custom, + output_template="foo-bar", ), ), ): with self.subTest(name): - actual = config.generate_config(path, root) + actual = config.generate_config(path, root, options=options) self.assertDictEqual(asdict(expected), asdict(actual)) def test_invalid_config(self) -> None: @@ -557,48 +585,15 @@ def collect_types(cfg: Config) -> List[Type[LintRule]]: ) self.assertListEqual([UseTypesFromTyping], rules) - def test_cwd_config(self) -> None: - prev_cwd = Path.cwd() - os.chdir(str(self.outer)) - try: - - cwd_config = config.get_cwd_config() - self.assertEqual("fixit", cwd_config.output_format) - - (self.inner / "fixit.toml").write_text( - "[tool.fixit]\noutput-format = 'custom'" - ) - - cwd_config = config.get_cwd_config() - self.assertEqual("fixit", cwd_config.output_format) - - (self.tdp / "pyproject.toml").write_text( - "[tool.fixit]\noutput-format = 'custom'" - ) - - cwd_config = config.get_cwd_config() - self.assertEqual("fixit", cwd_config.output_format) - - (self.outer / "pyproject.toml").write_text( - "[tool.fixit]\noutput-format = 'vscode'" - ) - - cwd_config = config.get_cwd_config() - self.assertEqual("vscode", cwd_config.output_format) - - os.chdir(str(self.inner)) - - cwd_config = config.get_cwd_config() - self.assertEqual("custom", cwd_config.output_format) - finally: - os.chdir(str(prev_cwd)) - def test_format_output(self) -> None: - prev_cwd = Path.cwd() - try: - os.chdir(str(self.tdp)) + with chdir(self.tdp): (self.tdp / "pyproject.toml").write_text( - "[tool.fixit]\noutput-format = 'vscode'\n" + dedent( + """ + [tool.fixit] + output-format = "vscode" + """ + ) ) runner = CliRunner(mix_stderr=False) @@ -624,7 +619,13 @@ def test_format_output(self) -> None: "{path}|{start_line}|{start_col} {rule_name}: {message}" ) (self.tdp / "pyproject.toml").write_text( - f"[tool.fixit]\noutput-format = 'custom'\noutput-template = '{custom_output_format}'" + dedent( + f""" + [tool.fixit] + output-format = 'custom' + output-template = '{custom_output_format}' + """ + ) ) with self.subTest("linting custom"): @@ -639,5 +640,25 @@ def test_format_output(self) -> None: ) self.assertRegex(result.output, custom_output_format_regex) - finally: - os.chdir(str(prev_cwd)) + with self.subTest("override output-format"): + result = runner.invoke( + main, + ["--output-format", "vscode", "lint", filepath.as_posix()], + catch_exceptions=True, + ) + self.assertRegex(result.output, output_format_regex) + + with self.subTest("override output-template"): + result = runner.invoke( + main, + [ + "--output-template", + "file {path} line {start_line} rule {rule_name}", + "lint", + filepath.as_posix(), + ], + catch_exceptions=True, + ) + self.assertRegex( + result.output, r"file .*f_string\.py line \d+ rule UseFstring" + ) diff --git a/src/fixit/util.py b/src/fixit/util.py index 613cdb81..44ce518f 100644 --- a/src/fixit/util.py +++ b/src/fixit/util.py @@ -3,6 +3,7 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. +import os import sys from contextlib import contextmanager from pathlib import Path @@ -87,3 +88,13 @@ def append_sys_path(path: Path) -> Generator[None, None, None]: # already there: do nothing, and don't remove it later else: yield + + +@contextmanager +def chdir(path: Path) -> Generator[None, None, None]: + cwd = Path.cwd() + try: + os.chdir(path) + yield + finally: + os.chdir(cwd)