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

Avoid duplicating linter-formatter compatibility warnings #8292

Merged
merged 1 commit into from Oct 30, 2023

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Oct 28, 2023

Summary

Uses warn_user_once! instead of warn! to ensure that every warning is shown exactly once, regardless of whether there are duplicates in the list, or warnings that are raised by multiple configuration files.

Closes #8271.

Rule::IndentationWithInvalidMultipleComment,
]) && setting.formatter.indent_width.value() != 4
{
warn_user_once!("The `format.indent-width` option with a value other than 4 is incompatible with `E111` and `E114`. We recommend disabling these rules when using the formatter, which enforces a consistent indentation width. Alternatively, set the `format.indent-width` option to `4`.");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Gave this a custom warning based on the indent-width.

if setting.linter.rules.enabled(Rule::TabIndentation)
&& setting.formatter.indent_style.is_tab()
{
warn_user_once!("The `format.indent-style=\"tab\"` option is incompatible with `W191`. We recommend disabling these rules when using the formatter, which enforces a consistent indentation style. Alternatively, set the `format.indent-style` option to `\"space\"`.");
Copy link
Member Author

Choose a reason for hiding this comment

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

Gave this a custom warning based on the indent-style.

@charliermarsh charliermarsh added the formatter Related to the formatter label Oct 28, 2023
@github-actions
Copy link

github-actions bot commented Oct 28, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no linter changes.

.map(|rule| format!("`{}`", rule.noqa_code()))
.collect();
rule_names.sort();
warn_user_once!("The following rules may cause conflicts when used with the formatter: {}. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.", rule_names.join(", "));
Copy link
Member

Choose a reason for hiding this comment

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

Minor question — since warn_user_once works using the call site, does this mean if we find a second set of incompatible rules in a different pyproject file we will not emit a second message with the different set? Probably not common... but not ideal?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, for this specific warning, we iterate over all pyproject.toml and collect all incompatible rules, so they would all be flagged here.

(All the other warnings are static, this is the only warning with dynamic input.)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!

@MichaReiser
Copy link
Member

I'd like to double check the removal of the on Monday. I verified them all manually and believe that I was able to come up with a conflict

@charliermarsh
Copy link
Member Author

@MichaReiser - Sorry, you're right. I think we don't reformat module-level docstrings which is why I was seeing different results. If we re-indent a function or class docstring, we do indeed change from spaces to tabs. Will restore.

@charliermarsh
Copy link
Member Author

@MichaReiser - Restored.

crates/ruff_cli/src/commands/format.rs Outdated Show resolved Hide resolved
@charliermarsh charliermarsh force-pushed the charlie/warn-user-once branch 3 times, most recently from 1c72bbe to 2d903b9 Compare October 30, 2023 13:34
@charliermarsh charliermarsh merged commit 7323c12 into main Oct 30, 2023
17 checks passed
@charliermarsh charliermarsh deleted the charlie/warn-user-once branch October 30, 2023 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruff format conflict warnings are shown twice
3 participants