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

Explicitly ban overriding extend as part of a --config flag #10135

Merged
merged 2 commits into from Feb 26, 2024

Conversation

AlexWaygood
Copy link
Member

Summary

Relates to #10035. Attempting to override extend as part of a --config flag doesn't work because we currently apply the configuration overrides given in the --config flag after any configuration options found in any pyproject.toml/ruff.toml files have been parsed and validated. But this particular override affects which config files ruff should be looking at in the first place, so we'd need to look for it at an earlier stage in the process.

Currently we just silently accept the argument but just don't do anything with it; this PR changes that so that we explicitly error out if a user tries to override extend using the --config flag. Longer term, we could possibly think about reworking how --config works so that it is possible to override extend using --config; or, we could add a separate option so that people can override extend properly via the command line.

Test Plan

cargo test, and manual testing

Copy link
Member Author

Choose a reason for hiding this comment

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

This should also ban passing extend as part of a --config flag when doing ruff format, but I didn't bother adding an equivalent test in crates/ruff/tests/format.rs, as it just felt duplicative. Lmk if you disagree!

@MichaReiser MichaReiser added configuration Related to settings and configuration cli Related to the command-line interface labels Feb 26, 2024
crates/ruff/src/args.rs Outdated Show resolved Hide resolved
@MichaReiser
Copy link
Member

MichaReiser commented Feb 26, 2024

This is, in theory, a breaking change because it could break invocations like ruff check --config extend=test.toml. I think it's okay to move forward with this change, considering that it never worked the way the user intended.

Copy link

github-actions bot commented Feb 26, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 1 projects; 42 projects unchanged)

pandas-dev/pandas (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ pandas/_libs/json.pyi:7:5: PLR0917 Too many positional arguments (8/5)

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLR0917 1 1 0 0 0

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to read examples/How_to_handle_rate_limits.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: trailing comma at line 47 column 4

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to read examples/How_to_handle_rate_limits.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: trailing comma at line 47 column 4

@AlexWaygood AlexWaygood enabled auto-merge (squash) February 26, 2024 16:00
@AlexWaygood AlexWaygood merged commit c25f1cd into astral-sh:main Feb 26, 2024
17 checks passed
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants