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

replace config_overrides.patch_config by config_overrides.to_ruff_args #10436

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 27 additions & 21 deletions python/ruff-ecosystem/ruff_ecosystem/check.py
Expand Up @@ -21,6 +21,7 @@
markdown_plus_minus,
markdown_project_section,
)
from ruff_ecosystem.projects import ConfigOverrides
from ruff_ecosystem.types import (
Comparison,
Diff,
Expand All @@ -32,7 +33,6 @@
from ruff_ecosystem.projects import (
CheckOptions,
ClonedRepository,
ConfigOverrides,
Project,
)

Expand Down Expand Up @@ -486,24 +486,25 @@ async def compare_check(
config_overrides: ConfigOverrides,
cloned_repo: ClonedRepository,
) -> Comparison:
with config_overrides.patch_config(cloned_repo.path, options.preview):
async with asyncio.TaskGroup() as tg:
baseline_task = tg.create_task(
ruff_check(
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
),
)
comparison_task = tg.create_task(
ruff_check(
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
),
)
async with asyncio.TaskGroup() as tg:
baseline_task = tg.create_task(
ruff_check(
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
config_overrides=config_overrides,
),
)
comparison_task = tg.create_task(
ruff_check(
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
config_overrides=config_overrides,
),
)

baseline_output, comparison_output = (
baseline_task.result(),
Expand All @@ -516,10 +517,15 @@ async def compare_check(


async def ruff_check(
*, executable: Path, path: Path, name: str, options: CheckOptions
*,
executable: Path,
path: Path,
name: str,
options: CheckOptions,
config_overrides: ConfigOverrides,
) -> Sequence[str]:
"""Run the given ruff binary against the specified path."""
ruff_args = options.to_ruff_args()
ruff_args = options.to_ruff_args() + config_overrides.to_ruff_args(options.preview)
logger.debug(f"Checking {name} with {executable} " + " ".join(ruff_args))

start = time.time()
Expand Down
76 changes: 40 additions & 36 deletions python/ruff-ecosystem/ruff_ecosystem/format.py
Expand Up @@ -172,24 +172,26 @@ async def format_then_format(
config_overrides: ConfigOverrides,
cloned_repo: ClonedRepository,
) -> Sequence[str]:
with config_overrides.patch_config(cloned_repo.path, options.preview):
# Run format to get the baseline
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 format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
diff=True,
)
# Run format to get the baseline
await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
config_overrides=config_overrides,
)

# Then get the diff from stdout
diff = await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
config_overrides=config_overrides,
diff=True,
)
return diff


Expand All @@ -201,15 +203,15 @@ async def format_and_format(
config_overrides: ConfigOverrides,
cloned_repo: ClonedRepository,
) -> Sequence[str]:
with config_overrides.patch_config(cloned_repo.path, options.preview):
# Run format without diff to get the baseline
await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
)
# Run format without diff to get the baseline
await format(
formatter=baseline_formatter,
executable=ruff_baseline_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
config_overrides=config_overrides,
)

# Commit the changes
commit = await cloned_repo.commit(
Expand All @@ -218,15 +220,15 @@ async def format_and_format(
# Then reset
await cloned_repo.reset()

with config_overrides.patch_config(cloned_repo.path, options.preview):
# Then run format again
await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
)
# Then run format again
await format(
formatter=Formatter.ruff,
executable=ruff_comparison_executable.resolve(),
path=cloned_repo.path,
name=cloned_repo.fullname,
options=options,
config_overrides=config_overrides,
)

# Then get the diff from the commit
diff = await cloned_repo.diff(commit)
Expand All @@ -241,6 +243,7 @@ async def format(
path: Path,
name: str,
options: FormatOptions,
config_overrides: ConfigOverrides,
diff: bool = False,
) -> Sequence[str]:
"""Run the given ruff binary against the specified path."""
Expand All @@ -249,6 +252,7 @@ async def format(
if formatter == Formatter.ruff
else options.to_black_args()
)
args += config_overrides.to_ruff_args(options.preview)
logger.debug(f"Formatting {name} with {executable} " + " ".join(args))

if diff:
Expand Down
76 changes: 5 additions & 71 deletions python/ruff-ecosystem/ruff_ecosystem/projects.py
Expand Up @@ -5,7 +5,6 @@
from __future__ import annotations

import abc
import contextlib
import dataclasses
from asyncio import create_subprocess_exec
from dataclasses import dataclass, field
Expand All @@ -15,7 +14,6 @@
from subprocess import DEVNULL, PIPE
from typing import Any, Self

import tomli
import tomli_w

from ruff_ecosystem import logger
Expand Down Expand Up @@ -63,8 +61,6 @@ class ConfigOverrides(Serializable):

The key describes a member to override in the toml file; '.' may be used to indicate a
nested value e.g. `format.quote-style`.

If a Ruff configuration file does not exist and overrides are provided, it will be createad.
"""

always: dict[str, Any] = field(default_factory=dict)
Expand All @@ -86,83 +82,21 @@ def as_string():

return hash(as_string())

@contextlib.contextmanager
def patch_config(
self,
dirpath: Path,
preview: bool,
) -> None:
"""
Temporarily patch the Ruff configuration file in the given directory.
"""
dot_ruff_toml = dirpath / ".ruff.toml"
ruff_toml = dirpath / "ruff.toml"
pyproject_toml = dirpath / "pyproject.toml"

# Prefer `ruff.toml` over `pyproject.toml`
if dot_ruff_toml.exists():
path = dot_ruff_toml
base = []
elif ruff_toml.exists():
path = ruff_toml
base = []
else:
path = pyproject_toml
base = ["tool", "ruff"]

def to_ruff_args(self, preview: bool = False) -> list[str]:
overrides = {
**ALWAYS_CONFIG_OVERRIDES,
**self.always,
**(self.when_preview if preview else self.when_no_preview),
}

if not overrides:
yield
return

# Read the existing content if the file is present
if path.exists():
contents = path.read_text()
toml = tomli.loads(contents)
else:
contents = None
toml = {}

# Do not write a toml file if it does not exist and we're just nulling values
if all((value is None for value in overrides.values())):
yield
return

# Update the TOML, using `.` to descend into nested keys
args = []
for key, value in overrides.items():
if value is not None:
logger.debug(f"Setting {key}={value!r} in {path}")
else:
logger.debug(f"Restoring {key} to default in {path}")

target = toml
names = base + key.split(".")
for name in names[:-1]:
if name not in target:
target[name] = {}
target = target[name]

if value is None:
# Remove null values i.e. restore to default
target.pop(names[-1], None)
args.extend(["--config", f"{key} = ''"])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWaygood Here I'm trying to unset / restore to default some values.

This is the part of the PR I'm the most unsure about, do you know if that's the correct way to do it ?

else:
target[names[-1]] = value
args.extend(["--config", f"{key} = {value}"])

tomli_w.dump(toml, path.open("wb"))

try:
yield
finally:
# Restore the contents or delete the file
if contents is None:
path.unlink()
else:
path.write_text(contents)
return args


class RuffCommand(Enum):
Expand Down