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

Merge subsettings when extending configurations #4431

Merged
merged 6 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
16 changes: 16 additions & 0 deletions crates/ruff/src/rules/flake8_annotations/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use serde::{Deserialize, Serialize};
use ruff_macros::CacheKey;
use ruff_macros::ConfigurationOptions;

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Default, Serialize, Deserialize, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -93,3 +95,17 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
mypy_init_return: self.mypy_init_return.or(other.mypy_init_return),
suppress_dummy_args: self.suppress_dummy_args.or(other.suppress_dummy_args),
suppress_none_returning: self
.suppress_none_returning
.or(other.suppress_none_returning),
allow_star_arg_any: self.allow_star_arg_any.or(other.allow_star_arg_any),
ignore_fully_untyped: self.ignore_fully_untyped.or(other.ignore_fully_untyped),
Copy link
Member

@charliermarsh charliermarsh May 14, 2023

Choose a reason for hiding this comment

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

We may be able to code-gen these via a Derive macro (you could look at how ConfigurationOptions works as an example), if you're interested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not done any macro stuff before, but I can give it a go. Would this potentially run into an issue with flake8_gettext since this is using a Vec instead of an Option<T>? This is the only occurrence of not using Option<T> that I came across, so arguably I could just not use a derive for that instance, but it does strike me as slightly strange that this isn't an Option<T>. I see no reference/reasoning for the difference given in the associated commits/PR.

Copy link
Member

Choose a reason for hiding this comment

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

Is that the only field that's not Option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Derive macro ended up not being too difficult to implement once I'd figured out how it worked, so I've handled the Vec<T> field case as well. So now all Options structs have their CombinePluginOptions impls derived (at least the ones that had an impl in the first place).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that the only field that's not Option?

Yep thats the only field in all the plugin options thats not Option that I could find.

}
}
}
16 changes: 16 additions & 0 deletions crates/ruff/src/rules/flake8_bandit/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

fn default_tmp_dirs() -> Vec<String> {
["/tmp", "/var/tmp", "/dev/shm"]
.map(std::string::ToString::to_string)
Expand Down Expand Up @@ -87,3 +89,17 @@ impl Default for Settings {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
hardcoded_tmp_directory: self
.hardcoded_tmp_directory
.or(other.hardcoded_tmp_directory),
hardcoded_tmp_directory_extend: self
.hardcoded_tmp_directory_extend
.or(other.hardcoded_tmp_directory_extend),
check_typed_exception: self.check_typed_exception.or(other.check_typed_exception),
}
}
}
10 changes: 10 additions & 0 deletions crates/ruff/src/rules/flake8_bugbear/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Default, Serialize, Deserialize, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -46,3 +48,11 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
extend_immutable_calls: self.extend_immutable_calls.or(other.extend_immutable_calls),
}
}
}
10 changes: 10 additions & 0 deletions crates/ruff/src/rules/flake8_builtins/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Default, Serialize, Deserialize, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -41,3 +43,11 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
builtins_ignorelist: self.builtins_ignorelist.or(other.builtins_ignorelist),
}
}
}
12 changes: 12 additions & 0 deletions crates/ruff/src/rules/flake8_comprehensions/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Default, Serialize, Deserialize, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -45,3 +47,13 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
allow_dict_calls_with_keyword_arguments: self
.allow_dict_calls_with_keyword_arguments
.or(other.allow_dict_calls_with_keyword_arguments),
}
}
}
10 changes: 10 additions & 0 deletions crates/ruff/src/rules/flake8_errmsg/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -37,3 +39,11 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
max_string_length: self.max_string_length.or(other.max_string_length),
}
}
}
15 changes: 15 additions & 0 deletions crates/ruff/src/rules/flake8_gettext/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -69,3 +71,16 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
function_names: self.function_names.or(other.function_names),
extend_function_names: other
.extend_function_names
.into_iter()
.chain(self.extend_function_names.into_iter())
.collect(),
}
}
}
10 changes: 10 additions & 0 deletions crates/ruff/src/rules/flake8_implicit_str_concat/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -59,3 +61,11 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
allow_multiline: self.allow_multiline.or(other.allow_multiline),
}
}
}
13 changes: 13 additions & 0 deletions crates/ruff/src/rules/flake8_import_conventions/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

const CONVENTIONAL_ALIASES: &[(&str, &str)] = &[
("altair", "alt"),
("matplotlib", "mpl"),
Expand Down Expand Up @@ -132,3 +134,14 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
aliases: self.aliases.or(other.aliases),
extend_aliases: self.extend_aliases.or(other.extend_aliases),
banned_aliases: self.banned_aliases.or(other.banned_aliases),
banned_from: self.banned_from.or(other.banned_from),
}
}
}
24 changes: 24 additions & 0 deletions crates/ruff/src/rules/flake8_pytest_style/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

use super::types;

fn default_broad_exceptions() -> Vec<String> {
Expand Down Expand Up @@ -167,3 +169,25 @@ impl Default for Settings {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
fixture_parentheses: self.fixture_parentheses.or(other.fixture_parentheses),
parametrize_names_type: self.parametrize_names_type.or(other.parametrize_names_type),
parametrize_values_type: self
.parametrize_values_type
.or(other.parametrize_values_type),
parametrize_values_row_type: self
.parametrize_values_row_type
.or(other.parametrize_values_row_type),
raises_require_match_for: self
.raises_require_match_for
.or(other.raises_require_match_for),
raises_extend_require_match_for: self
.raises_extend_require_match_for
.or(other.raises_extend_require_match_for),
mark_parentheses: self.mark_parentheses.or(other.mark_parentheses),
}
}
}
13 changes: 13 additions & 0 deletions crates/ruff/src/rules/flake8_quotes/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
Expand Down Expand Up @@ -111,3 +113,14 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
inline_quotes: self.inline_quotes.or(other.inline_quotes),
multiline_quotes: self.multiline_quotes.or(other.multiline_quotes),
docstring_quotes: self.docstring_quotes.or(other.docstring_quotes),
avoid_escape: self.avoid_escape.or(other.avoid_escape),
}
}
}
9 changes: 9 additions & 0 deletions crates/ruff/src/rules/flake8_self/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

// By default, ignore the `namedtuple` methods and attributes, which are underscore-prefixed to
// prevent conflicts with field names.
const IGNORE_NAMES: [&str; 5] = ["_make", "_asdict", "_replace", "_fields", "_field_defaults"];
Expand Down Expand Up @@ -57,3 +59,10 @@ impl From<Settings> for Options {
}
}
}
impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
ignore_names: self.ignore_names.or(other.ignore_names),
}
}
}
11 changes: 11 additions & 0 deletions crates/ruff/src/rules/flake8_tidy_imports/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::ConfigurationOptions;

use crate::settings::configuration::CombinePluginOptions;

use super::banned_api::ApiBan;
use super::relative_imports::Strictness;
use super::Settings;
Expand Down Expand Up @@ -60,3 +62,12 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
ban_relative_imports: self.ban_relative_imports.or(other.ban_relative_imports),
banned_api: self.banned_api.or(other.banned_api),
}
}
}
17 changes: 17 additions & 0 deletions crates/ruff/src/rules/flake8_type_checking/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -99,3 +101,18 @@ impl From<Settings> for Options {
}
}
}

impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
strict: self.strict.or(other.strict),
exempt_modules: self.exempt_modules.or(other.exempt_modules),
runtime_evaluated_base_classes: self
.runtime_evaluated_base_classes
.or(other.runtime_evaluated_base_classes),
runtime_evaluated_decorators: self
.runtime_evaluated_decorators
.or(other.runtime_evaluated_decorators),
}
}
}
9 changes: 9 additions & 0 deletions crates/ruff/src/rules/flake8_unused_arguments/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};

use ruff_macros::{CacheKey, ConfigurationOptions};

use crate::settings::configuration::CombinePluginOptions;

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions)]
#[serde(
deny_unknown_fields,
Expand Down Expand Up @@ -41,3 +43,10 @@ impl From<Settings> for Options {
}
}
}
impl CombinePluginOptions for Options {
fn combine(self, other: Self) -> Self {
Self {
ignore_variadic_names: self.ignore_variadic_names.or(other.ignore_variadic_names),
}
}
}
Loading