Add support for ruff-ecosystem format comparisons with black (#8419)

Extends https://github.com/astral-sh/ruff/pull/8416 activating the
`black-and-ruff` and `black-then-ruff` formatter comparison modes for
ecosystem checks allowing us to compare changes to Black across the
ecosystem.
This commit is contained in:
Zanie Blue 2023-11-01 20:29:25 -05:00 committed by GitHub
parent 2f7e2a8de3
commit ebad36da06
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 75 additions and 48 deletions

View file

@ -37,6 +37,12 @@ Run `ruff format` ecosystem checks comparing with changes to code that is alread
ruff-ecosystem format ruff "./target/debug/ruff" --format-comparison ruff-then-ruff ruff-ecosystem format ruff "./target/debug/ruff" --format-comparison ruff-then-ruff
``` ```
Run `ruff format` ecosystem checks comparing with the Black formatter:
```shell
ruff-ecosystem format black ruff -v --cache python/checkouts --format-comparison black-and-ruff
```
The default output format is markdown, which includes nice summaries of the changes. You can use `--output-format json` to display the raw data — this is The default output format is markdown, which includes nice summaries of the changes. You can use `--output-format json` to display the raw data — this is
particularly useful when making changes to the ecosystem checks. particularly useful when making changes to the ecosystem checks.

View file

@ -24,7 +24,7 @@ from ruff_ecosystem.types import (
Comparison, Comparison,
Diff, Diff,
Result, Result,
RuffError, ToolError,
) )
if TYPE_CHECKING: if TYPE_CHECKING:
@ -500,7 +500,7 @@ async def ruff_check(
*, executable: Path, path: Path, name: str, options: CheckOptions *, executable: Path, path: Path, name: str, options: CheckOptions
) -> Sequence[str]: ) -> Sequence[str]:
"""Run the given ruff binary against the specified path.""" """Run the given ruff binary against the specified path."""
ruff_args = options.to_cli_args() ruff_args = options.to_ruff_args()
logger.debug(f"Checking {name} with {executable} " + " ".join(ruff_args)) logger.debug(f"Checking {name} with {executable} " + " ".join(ruff_args))
start = time.time() start = time.time()
@ -518,7 +518,7 @@ async def ruff_check(
logger.debug(f"Finished checking {name} with {executable} in {end - start:.2f}s") logger.debug(f"Finished checking {name} with {executable} in {end - start:.2f}s")
if proc.returncode != 0: if proc.returncode != 0:
raise RuffError(err.decode("utf8")) raise ToolError(err.decode("utf8"))
# Strip summary lines so the diff is only diagnostic lines # Strip summary lines so the diff is only diagnostic lines
lines = [ lines = [

View file

@ -46,32 +46,34 @@ def entrypoint():
tempfile.TemporaryDirectory() if not args.cache else nullcontext(args.cache) tempfile.TemporaryDirectory() if not args.cache else nullcontext(args.cache)
) )
ruff_baseline = args.ruff_baseline baseline_executable = args.baseline_executable
if not args.ruff_baseline.exists(): if not args.baseline_executable.exists():
ruff_baseline = get_executable_path(str(args.ruff_baseline)) baseline_executable = get_executable_path(str(args.baseline_executable))
if not ruff_baseline: if not baseline_executable:
print( print(
f"Could not find ruff baseline executable: {args.ruff_baseline}", f"Could not find ruff baseline executable: {args.baseline_executable}",
sys.stderr, sys.stderr,
) )
exit(1) exit(1)
logger.info( logger.info(
"Resolved baseline executable %s to %s", args.ruff_baseline, ruff_baseline "Resolved baseline executable %s to %s",
args.baseline_executable,
baseline_executable,
) )
ruff_comparison = args.ruff_comparison comparison_executable = args.comparison_executable
if not args.ruff_comparison.exists(): if not args.comparison_executable.exists():
ruff_comparison = get_executable_path(str(args.ruff_comparison)) comparison_executable = get_executable_path(str(args.comparison_executable))
if not ruff_comparison: if not comparison_executable:
print( print(
f"Could not find ruff comparison executable: {args.ruff_comparison}", f"Could not find ruff comparison executable: {args.comparison_executable}",
sys.stderr, sys.stderr,
) )
exit(1) exit(1)
logger.info( logger.info(
"Resolved comparison executable %s to %s", "Resolved comparison executable %s to %s",
args.ruff_comparison, args.comparison_executable,
ruff_comparison, comparison_executable,
) )
targets = DEFAULT_TARGETS targets = DEFAULT_TARGETS
@ -89,8 +91,8 @@ def entrypoint():
main_task = asyncio.ensure_future( main_task = asyncio.ensure_future(
main( main(
command=RuffCommand(args.ruff_command), command=RuffCommand(args.ruff_command),
ruff_baseline_executable=ruff_baseline, baseline_executable=baseline_executable,
ruff_comparison_executable=ruff_comparison, comparison_executable=comparison_executable,
targets=targets, targets=targets,
format=OutputFormat(args.output_format), format=OutputFormat(args.output_format),
project_dir=Path(cache), project_dir=Path(cache),
@ -160,11 +162,11 @@ def parse_args() -> argparse.Namespace:
help="The Ruff command to test", help="The Ruff command to test",
) )
parser.add_argument( parser.add_argument(
"ruff_baseline", "baseline_executable",
type=Path, type=Path,
) )
parser.add_argument( parser.add_argument(
"ruff_comparison", "comparison_executable",
type=Path, type=Path,
) )

View file

@ -15,7 +15,7 @@ from unidiff import PatchSet
from ruff_ecosystem import logger from ruff_ecosystem import logger
from ruff_ecosystem.markdown import markdown_project_section from ruff_ecosystem.markdown import markdown_project_section
from ruff_ecosystem.types import Comparison, Diff, Result, RuffError from ruff_ecosystem.types import Comparison, Diff, Result, ToolError
if TYPE_CHECKING: if TYPE_CHECKING:
from ruff_ecosystem.projects import ClonedRepository, FormatOptions from ruff_ecosystem.projects import ClonedRepository, FormatOptions
@ -151,14 +151,16 @@ async def format_then_format(
cloned_repo: ClonedRepository, cloned_repo: ClonedRepository,
) -> Sequence[str]: ) -> Sequence[str]:
# Run format to get the baseline # Run format to get the baseline
await ruff_format( await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(), executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path, path=cloned_repo.path,
name=cloned_repo.fullname, name=cloned_repo.fullname,
options=options, options=options,
) )
# Then get the diff from stdout # Then get the diff from stdout
diff = await ruff_format( diff = await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(), executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path, path=cloned_repo.path,
name=cloned_repo.fullname, name=cloned_repo.fullname,
@ -176,7 +178,8 @@ async def format_and_format(
cloned_repo: ClonedRepository, cloned_repo: ClonedRepository,
) -> Sequence[str]: ) -> Sequence[str]:
# Run format without diff to get the baseline # Run format without diff to get the baseline
await ruff_format( await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(), executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path, path=cloned_repo.path,
name=cloned_repo.fullname, name=cloned_repo.fullname,
@ -189,7 +192,8 @@ async def format_and_format(
# Then reset # Then reset
await cloned_repo.reset() await cloned_repo.reset()
# Then run format again # Then run format again
await ruff_format( await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(), executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path, path=cloned_repo.path,
name=cloned_repo.fullname, name=cloned_repo.fullname,
@ -200,8 +204,9 @@ async def format_and_format(
return diff return diff
async def ruff_format( async def format(
*, *,
formatter: Formatter,
executable: Path, executable: Path,
path: Path, path: Path,
name: str, name: str,
@ -209,16 +214,20 @@ async def ruff_format(
diff: bool = False, diff: bool = False,
) -> Sequence[str]: ) -> Sequence[str]:
"""Run the given ruff binary against the specified path.""" """Run the given ruff binary against the specified path."""
ruff_args = options.to_cli_args() args = (
logger.debug(f"Formatting {name} with {executable} " + " ".join(ruff_args)) options.to_ruff_args()
if formatter == Formatter.ruff
else options.to_black_args()
)
logger.debug(f"Formatting {name} with {executable} " + " ".join(args))
if diff: if diff:
ruff_args.append("--diff") args.append("--diff")
start = time.time() start = time.time()
proc = await create_subprocess_exec( proc = await create_subprocess_exec(
executable.absolute(), executable.absolute(),
*ruff_args, *args,
".", ".",
stdout=PIPE, stdout=PIPE,
stderr=PIPE, stderr=PIPE,
@ -230,7 +239,7 @@ async def ruff_format(
logger.debug(f"Finished formatting {name} with {executable} in {end - start:.2f}s") logger.debug(f"Finished formatting {name} with {executable} in {end - start:.2f}s")
if proc.returncode not in [0, 1]: if proc.returncode not in [0, 1]:
raise RuffError(err.decode("utf8")) raise ToolError(err.decode("utf8"))
lines = result.decode("utf8").splitlines() lines = result.decode("utf8").splitlines()
return lines return lines

View file

@ -29,8 +29,8 @@ class OutputFormat(Enum):
async def main( async def main(
command: RuffCommand, command: RuffCommand,
ruff_baseline_executable: Path, baseline_executable: Path,
ruff_comparison_executable: Path, comparison_executable: Path,
targets: list[Project], targets: list[Project],
project_dir: Path, project_dir: Path,
format: OutputFormat, format: OutputFormat,
@ -39,9 +39,11 @@ async def main(
raise_on_failure: bool = False, raise_on_failure: bool = False,
) -> None: ) -> None:
logger.debug("Using command %s", command.value) logger.debug("Using command %s", command.value)
logger.debug("Using baseline executable at %s", ruff_baseline_executable) logger.debug("Using baseline executable at %s", baseline_executable)
logger.debug("Using comparison executable at %s", ruff_comparison_executable) logger.debug("Using comparison executable at %s", comparison_executable)
logger.debug("Using checkout_dir directory %s", project_dir) logger.debug("Using checkout_dir directory %s", project_dir)
if format_comparison:
logger.debug("Using format comparison type %s", format_comparison.value)
logger.debug("Checking %s targets", len(targets)) logger.debug("Checking %s targets", len(targets))
# Limit parallelism to avoid high memory consumption # Limit parallelism to avoid high memory consumption
@ -56,8 +58,8 @@ async def main(
limited_parallelism( limited_parallelism(
clone_and_compare( clone_and_compare(
command, command,
ruff_baseline_executable, baseline_executable,
ruff_comparison_executable, comparison_executable,
target, target,
project_dir, project_dir,
format_comparison, format_comparison,
@ -98,8 +100,8 @@ async def main(
async def clone_and_compare( async def clone_and_compare(
command: RuffCommand, command: RuffCommand,
ruff_baseline_executable: Path, baseline_executable: Path,
ruff_comparison_executable: Path, comparison_executable: Path,
target: Project, target: Project,
project_dir: Path, project_dir: Path,
format_comparison: FormatComparison | None, format_comparison: FormatComparison | None,
@ -125,8 +127,8 @@ async def clone_and_compare(
try: try:
return await compare( return await compare(
ruff_baseline_executable, baseline_executable,
ruff_comparison_executable, comparison_executable,
options, options,
cloned_repo, cloned_repo,
**kwargs, **kwargs,

View file

@ -12,7 +12,7 @@ def markdown_project_section(
return markdown_details( return markdown_details(
summary=f'<a href="{project.repo.url}">{project.repo.fullname}</a> ({title})', summary=f'<a href="{project.repo.url}">{project.repo.fullname}</a> ({title})',
# Show the command used for the check # Show the command used for the check
preface="<pre>ruff " + " ".join(options.to_cli_args()) + "</pre>", preface="<pre>ruff " + " ".join(options.to_ruff_args()) + "</pre>",
content=content, content=content,
) )

View file

@ -49,7 +49,7 @@ class CommandOptions(Serializable, abc.ABC):
return type(self)(**{**dataclasses.asdict(self), **kwargs}) return type(self)(**{**dataclasses.asdict(self), **kwargs})
@abc.abstractmethod @abc.abstractmethod
def to_cli_args(self) -> list[str]: def to_ruff_args(self) -> list[str]:
pass pass
@ -70,7 +70,7 @@ class CheckOptions(CommandOptions):
# Limit the number of reported lines per rule # Limit the number of reported lines per rule
max_lines_per_rule: int | None = 50 max_lines_per_rule: int | None = 50
def to_cli_args(self) -> list[str]: def to_ruff_args(self) -> list[str]:
args = ["check", "--no-cache", "--exit-zero"] args = ["check", "--no-cache", "--exit-zero"]
if self.select: if self.select:
args.extend(["--select", self.select]) args.extend(["--select", self.select])
@ -88,13 +88,13 @@ class CheckOptions(CommandOptions):
@dataclass(frozen=True) @dataclass(frozen=True)
class FormatOptions(CommandOptions): class FormatOptions(CommandOptions):
""" """
Ruff format options. Format ecosystem check options.
""" """
preview: bool = False preview: bool = False
exclude: str = "" exclude: str = ""
def to_cli_args(self) -> list[str]: def to_ruff_args(self) -> list[str]:
args = ["format"] args = ["format"]
if self.exclude: if self.exclude:
args.extend(["--exclude", self.exclude]) args.extend(["--exclude", self.exclude])
@ -102,6 +102,14 @@ class FormatOptions(CommandOptions):
args.append("--preview") args.append("--preview")
return args return args
def to_black_args(self) -> list[str]:
args = []
if self.exclude:
args.extend(["--exclude", self.exclude])
if self.preview:
args.append("--preview")
return args
class ProjectSetupError(Exception): class ProjectSetupError(Exception):
"""An error setting up a project.""" """An error setting up a project."""

View file

@ -89,5 +89,5 @@ class Comparison(Serializable):
repo: ClonedRepository repo: ClonedRepository
class RuffError(Exception): class ToolError(Exception):
"""An error reported by Ruff.""" """An error reported by the checked executable."""