From 7971e0b0eeb579ad1bd56ef8aef7f144b96c05bd Mon Sep 17 00:00:00 2001 From: Jaap Roes Date: Thu, 7 Sep 2023 15:41:22 +0200 Subject: [PATCH] Add `extend-ignore-names` for `flake8-self` (#7194) ## Summary Add a configuration option to extend the list of names that can be accessed without triggering SLF001. Fixes issue #7018 ## Test Plan Manually tested by creating a python file (`test.py`): ```python def foo(obj): obj._meta ``` and a `ruff.toml` file: ```toml select = ["SLF"] [flake8-self] extend-ignore-names = ["_meta"] ``` Then running `cargo run -p ruff_cli -- check test.py --no-cache` (once with the `extend-ignore-names` line comment out) to see if the configuration option works. --------- Co-authored-by: Charlie Marsh --- .../fixtures/flake8_self/SLF001_extended.py | 10 +++ crates/ruff/src/rules/flake8_self/mod.rs | 16 ++++ ...les__flake8_self__tests__ignore_names.snap | 18 +++++ crates/ruff_workspace/src/options.rs | 73 +++++++++++++++++-- ruff.schema.json | 10 +++ 5 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_self/SLF001_extended.py create mode 100644 crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__ignore_names.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_self/SLF001_extended.py b/crates/ruff/resources/test/fixtures/flake8_self/SLF001_extended.py new file mode 100644 index 0000000000..f628137f77 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_self/SLF001_extended.py @@ -0,0 +1,10 @@ +def foo(obj): + obj._meta # OK + + +def foo(obj): + obj._asdict # SLF001 + + +def foo(obj): + obj._bar # SLF001 diff --git a/crates/ruff/src/rules/flake8_self/mod.rs b/crates/ruff/src/rules/flake8_self/mod.rs index 3866aa49c4..8c9ab60d7e 100644 --- a/crates/ruff/src/rules/flake8_self/mod.rs +++ b/crates/ruff/src/rules/flake8_self/mod.rs @@ -11,6 +11,7 @@ mod tests { use test_case::test_case; use crate::registry::Rule; + use crate::rules::flake8_self; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -24,4 +25,19 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test] + fn ignore_names() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_self/SLF001_extended.py"), + &settings::Settings { + flake8_self: flake8_self::settings::Settings { + ignore_names: vec!["_meta".to_string()], + }, + ..settings::Settings::for_rule(Rule::PrivateMemberAccess) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__ignore_names.snap b/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__ignore_names.snap new file mode 100644 index 0000000000..6d3072d4e8 --- /dev/null +++ b/crates/ruff/src/rules/flake8_self/snapshots/ruff__rules__flake8_self__tests__ignore_names.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/src/rules/flake8_self/mod.rs +--- +SLF001_extended.py:6:5: SLF001 Private member accessed: `_asdict` + | +5 | def foo(obj): +6 | obj._asdict # SLF001 + | ^^^^^^^^^^^ SLF001 + | + +SLF001_extended.py:10:5: SLF001 Private member accessed: `_bar` + | + 9 | def foo(obj): +10 | obj._bar # SLF001 + | ^^^^^^^^ SLF001 + | + + diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index dc71122798..6b53f94d34 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -1240,16 +1240,26 @@ pub struct Flake8SelfOptions { )] /// A list of names to ignore when considering `flake8-self` violations. pub ignore_names: Option>, + #[option( + default = r#"[]"#, + value_type = "list[str]", + example = r#"extend-ignore-names = ["_base_manager", "_default_manager", "_meta"]"# + )] + /// Additional names to ignore when considering `flake8-self` violations, + /// in addition to those included in `ignore-names`. + pub extend_ignore_names: Option>, } impl Flake8SelfOptions { pub fn into_settings(self) -> flake8_self::settings::Settings { + let defaults = flake8_self::settings::Settings::default(); flake8_self::settings::Settings { - ignore_names: self.ignore_names.unwrap_or_else(|| { - flake8_self::settings::IGNORE_NAMES - .map(String::from) - .to_vec() - }), + ignore_names: self + .ignore_names + .unwrap_or(defaults.ignore_names) + .into_iter() + .chain(self.extend_ignore_names.unwrap_or_default()) + .collect(), } } } @@ -2216,3 +2226,56 @@ impl PyUpgradeOptions { } } } + +#[cfg(test)] +mod tests { + use crate::options::Flake8SelfOptions; + use ruff::rules::flake8_self; + + #[test] + fn flake8_self_options() { + let default_settings = flake8_self::settings::Settings::default(); + + // Uses defaults if no options are specified. + let options = Flake8SelfOptions { + ignore_names: None, + extend_ignore_names: None, + }; + let settings = options.into_settings(); + assert_eq!(settings.ignore_names, default_settings.ignore_names); + + // Uses ignore_names if specified. + let options = Flake8SelfOptions { + ignore_names: Some(vec!["_foo".to_string()]), + extend_ignore_names: None, + }; + let settings = options.into_settings(); + assert_eq!(settings.ignore_names, vec!["_foo".to_string()]); + + // Appends extend_ignore_names to defaults if only extend_ignore_names is specified. + let options = Flake8SelfOptions { + ignore_names: None, + extend_ignore_names: Some(vec!["_bar".to_string()]), + }; + let settings = options.into_settings(); + assert_eq!( + settings.ignore_names, + default_settings + .ignore_names + .into_iter() + .chain(["_bar".to_string()]) + .collect::>() + ); + + // Appends extend_ignore_names to ignore_names if both are specified. + let options = Flake8SelfOptions { + ignore_names: Some(vec!["_foo".to_string()]), + extend_ignore_names: Some(vec!["_bar".to_string()]), + }; + let settings = options.into_settings(); + assert_eq!( + settings.ignore_names, + vec!["_foo".to_string(), "_bar".to_string()] + ); + } +} diff --git a/ruff.schema.json b/ruff.schema.json index bce237bcb1..855add9dbc 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1033,6 +1033,16 @@ "Flake8SelfOptions": { "type": "object", "properties": { + "extend-ignore-names": { + "description": "Additional names to ignore when considering `flake8-self` violations, in addition to those included in `ignore-names`.", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, "ignore-names": { "description": "A list of names to ignore when considering `flake8-self` violations.", "type": [