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 support for ruff-ecosystem format comparisons with black #8419

Merged
merged 2 commits into from
Nov 2, 2023
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
6 changes: 6 additions & 0 deletions python/ruff-ecosystem/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

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
particularly useful when making changes to the ecosystem checks.

Expand Down
6 changes: 3 additions & 3 deletions python/ruff-ecosystem/ruff_ecosystem/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
Comparison,
Diff,
Result,
RuffError,
ToolError,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -500,7 +500,7 @@ async def ruff_check(
*, executable: Path, path: Path, name: str, options: CheckOptions
) -> Sequence[str]:
"""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))

start = time.time()
Expand All @@ -518,7 +518,7 @@ async def ruff_check(
logger.debug(f"Finished checking {name} with {executable} in {end - start:.2f}s")

if proc.returncode != 0:
raise RuffError(err.decode("utf8"))
raise ToolError(err.decode("utf8"))

# Strip summary lines so the diff is only diagnostic lines
lines = [
Expand Down
36 changes: 19 additions & 17 deletions python/ruff-ecosystem/ruff_ecosystem/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,34 @@ def entrypoint():
tempfile.TemporaryDirectory() if not args.cache else nullcontext(args.cache)
)

ruff_baseline = args.ruff_baseline
if not args.ruff_baseline.exists():
ruff_baseline = get_executable_path(str(args.ruff_baseline))
if not ruff_baseline:
baseline_executable = args.baseline_executable
if not args.baseline_executable.exists():
baseline_executable = get_executable_path(str(args.baseline_executable))
if not baseline_executable:
print(
f"Could not find ruff baseline executable: {args.ruff_baseline}",
f"Could not find ruff baseline executable: {args.baseline_executable}",
sys.stderr,
)
exit(1)
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
if not args.ruff_comparison.exists():
ruff_comparison = get_executable_path(str(args.ruff_comparison))
if not ruff_comparison:
comparison_executable = args.comparison_executable
if not args.comparison_executable.exists():
comparison_executable = get_executable_path(str(args.comparison_executable))
if not comparison_executable:
print(
f"Could not find ruff comparison executable: {args.ruff_comparison}",
f"Could not find ruff comparison executable: {args.comparison_executable}",
sys.stderr,
)
exit(1)
logger.info(
"Resolved comparison executable %s to %s",
args.ruff_comparison,
ruff_comparison,
args.comparison_executable,
comparison_executable,
)

targets = DEFAULT_TARGETS
Expand All @@ -89,8 +91,8 @@ def entrypoint():
main_task = asyncio.ensure_future(
main(
command=RuffCommand(args.ruff_command),
ruff_baseline_executable=ruff_baseline,
ruff_comparison_executable=ruff_comparison,
baseline_executable=baseline_executable,
comparison_executable=comparison_executable,
targets=targets,
format=OutputFormat(args.output_format),
project_dir=Path(cache),
Expand Down Expand Up @@ -160,11 +162,11 @@ def parse_args() -> argparse.Namespace:
help="The Ruff command to test",
)
parser.add_argument(
"ruff_baseline",
"baseline_executable",
type=Path,
)
parser.add_argument(
"ruff_comparison",
"comparison_executable",
type=Path,
)

Expand Down
31 changes: 20 additions & 11 deletions python/ruff-ecosystem/ruff_ecosystem/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from ruff_ecosystem import logger
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:
from ruff_ecosystem.projects import ClonedRepository, FormatOptions
Expand Down Expand Up @@ -151,14 +151,16 @@ async def format_then_format(
cloned_repo: ClonedRepository,
) -> Sequence[str]:
# Run format to get the baseline
await ruff_format(
await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
)
# Then get the diff from stdout
diff = await ruff_format(
diff = await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
Expand All @@ -176,7 +178,8 @@ async def format_and_format(
cloned_repo: ClonedRepository,
) -> Sequence[str]:
# Run format without diff to get the baseline
await ruff_format(
await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
Expand All @@ -189,7 +192,8 @@ async def format_and_format(
# Then reset
await cloned_repo.reset()
# Then run format again
await ruff_format(
await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
Expand All @@ -200,25 +204,30 @@ async def format_and_format(
return diff


async def ruff_format(
async def format(
*,
formatter: Formatter,
executable: Path,
path: Path,
name: str,
options: FormatOptions,
diff: bool = False,
) -> Sequence[str]:
"""Run the given ruff binary against the specified path."""
ruff_args = options.to_cli_args()
logger.debug(f"Formatting {name} with {executable} " + " ".join(ruff_args))
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:
ruff_args.append("--diff")
args.append("--diff")

start = time.time()
proc = await create_subprocess_exec(
executable.absolute(),
*ruff_args,
*args,
".",
stdout=PIPE,
stderr=PIPE,
Expand All @@ -230,7 +239,7 @@ async def ruff_format(
logger.debug(f"Finished formatting {name} with {executable} in {end - start:.2f}s")

if proc.returncode not in [0, 1]:
raise RuffError(err.decode("utf8"))
raise ToolError(err.decode("utf8"))

lines = result.decode("utf8").splitlines()
return lines
Expand Down
22 changes: 12 additions & 10 deletions python/ruff-ecosystem/ruff_ecosystem/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class OutputFormat(Enum):

async def main(
command: RuffCommand,
ruff_baseline_executable: Path,
ruff_comparison_executable: Path,
baseline_executable: Path,
comparison_executable: Path,
targets: list[Project],
project_dir: Path,
format: OutputFormat,
Expand All @@ -39,9 +39,11 @@ async def main(
raise_on_failure: bool = False,
) -> None:
logger.debug("Using command %s", command.value)
logger.debug("Using baseline executable at %s", ruff_baseline_executable)
logger.debug("Using comparison executable at %s", ruff_comparison_executable)
logger.debug("Using baseline executable at %s", baseline_executable)
logger.debug("Using comparison executable at %s", comparison_executable)
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))

# Limit parallelism to avoid high memory consumption
Expand All @@ -56,8 +58,8 @@ async def limited_parallelism(coroutine: Awaitable[T]) -> T:
limited_parallelism(
clone_and_compare(
command,
ruff_baseline_executable,
ruff_comparison_executable,
baseline_executable,
comparison_executable,
target,
project_dir,
format_comparison,
Expand Down Expand Up @@ -98,8 +100,8 @@ async def limited_parallelism(coroutine: Awaitable[T]) -> T:

async def clone_and_compare(
command: RuffCommand,
ruff_baseline_executable: Path,
ruff_comparison_executable: Path,
baseline_executable: Path,
comparison_executable: Path,
target: Project,
project_dir: Path,
format_comparison: FormatComparison | None,
Expand All @@ -125,8 +127,8 @@ async def clone_and_compare(

try:
return await compare(
ruff_baseline_executable,
ruff_comparison_executable,
baseline_executable,
comparison_executable,
options,
cloned_repo,
**kwargs,
Expand Down
2 changes: 1 addition & 1 deletion python/ruff-ecosystem/ruff_ecosystem/markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def markdown_project_section(
return markdown_details(
summary=f'<a href="{project.repo.url}">{project.repo.fullname}</a> ({title})',
# 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,
)

Expand Down
16 changes: 12 additions & 4 deletions python/ruff-ecosystem/ruff_ecosystem/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def with_options(self: Self, **kwargs) -> Self:
return type(self)(**{**dataclasses.asdict(self), **kwargs})

@abc.abstractmethod
def to_cli_args(self) -> list[str]:
def to_ruff_args(self) -> list[str]:
pass


Expand All @@ -70,7 +70,7 @@ class CheckOptions(CommandOptions):
# Limit the number of reported lines per rule
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"]
if self.select:
args.extend(["--select", self.select])
Expand All @@ -88,20 +88,28 @@ def to_cli_args(self) -> list[str]:
@dataclass(frozen=True)
class FormatOptions(CommandOptions):
"""
Ruff format options.
Format ecosystem check options.
"""

preview: bool = False
exclude: str = ""

def to_cli_args(self) -> list[str]:
def to_ruff_args(self) -> list[str]:
args = ["format"]
if self.exclude:
args.extend(["--exclude", self.exclude])
if self.preview:
args.append("--preview")
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):
"""An error setting up a project."""
Expand Down
4 changes: 2 additions & 2 deletions python/ruff-ecosystem/ruff_ecosystem/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,5 @@ class Comparison(Serializable):
repo: ClonedRepository


class RuffError(Exception):
"""An error reported by Ruff."""
class ToolError(Exception):
"""An error reported by the checked executable."""
Loading