Skip to content

Commit

Permalink
Add support for ruff-ecosystem format comparisons with black
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Nov 1, 2023
1 parent 82c8405 commit 54a3d68
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 50 deletions.
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
```

## Development

When developing, it can be useful to set the `--pdb` flag to drop into a debugger on failure:
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."""
4 changes: 2 additions & 2 deletions scripts/check_ecosystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ async def _get_commit(self: Self, checkout_dir: Path) -> str:
SUMMARY_LINE_RE = re.compile(r"^(Found \d+ error.*)|(.*potentially fixable with.*)$")


class RuffError(Exception):
class ToolError(Exception):
"""An error reported by ruff."""


Expand Down Expand Up @@ -200,7 +200,7 @@ async def check(
logger.debug(f"Finished checking {name} with {ruff} in {end - start:.2f}")

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

lines = [
line
Expand Down

0 comments on commit 54a3d68

Please sign in to comment.