Improve rule config resolution

BREAKING_CHANGES.md

@@ -1,5 +1,37 @@
 # Breaking Changes
 
+## Unreleased
+
+### `select`, `extend-select`, `ignore`, and `extend-ignore` have new semantics ([#2312](https://github.com/charliermarsh/ruff/pull/2312))
+
+Previously, the interplay between `select` and its related options could lead to unexpected
+behavior. For example, `ruff --select E501 --ignore ALL` and `ruff --select E501 --extend-ignore
+ALL` behaved differently. (See [#2312](https://github.com/charliermarsh/ruff/pull/2312) for more
+examples.)
+
+When Ruff determines the enabled rule set, it has to reconcile `select` and `ignore` from a variety
+of sources, including the current `pyproject.toml`, any inherited `pyproject.toml` files, and the
+CLI.
+
+The new semantics are such that Ruff uses the "highest-priority" `select` as the basis for the rule
+set, and then applies any `extend-select`, `ignore`, and `extend-ignore` adjustments. CLI options
+are given higher priority than `pyproject.toml` options, and the current `pyproject.toml` file is
+given higher priority than any inherited `pyproject.toml` files.
+
+`extend-select` and `extend-ignore` are no longer given "top priority"; instead, they merely append
+to the `select` and `ignore` lists, as in Flake8.
+
+This change is largely backwards compatible -- most users should experience no change in behavior.
+However, as an example of a breaking change, consider the following:
+
+```toml
+[tool.ruff]
+ignore = ["F401"]
+```
+
+Running `ruff --select F` would previously have enabled all `F` rules, apart from `F401`. Now, it
+will enable all `F` rules, including `F401`, as the command line's `--select` resets the resolution.
+
 ## 0.0.237
 
 ### `--explain`, `--clean`, and `--generate-shell-completion` are now subcommands ([#2190](https://github.com/charliermarsh/ruff/pull/2190))

README.md

@@ -433,8 +433,6 @@ Rule selection:
           Comma-separated list of rule codes to disable
       --extend-select <RULE_CODE>
           Like --select, but adds additional rule codes on top of the selected ones
-      --extend-ignore <RULE_CODE>
-          Like --ignore, but adds additional rule codes on top of the ignored ones
       --per-file-ignores <PER_FILE_IGNORES>
           List of mappings from file pattern to code to exclude
       --fixable <RULE_CODE>
@@ -523,6 +521,33 @@ By default, Ruff will also skip any files that are omitted via `.ignore`, `.giti
 Files that are passed to `ruff` directly are always linted, regardless of the above criteria.
 For example, `ruff /path/to/excluded/file.py` will always lint `file.py`.
 
+### Rule resolution
+
+The set of enabled rules is controlled via the [`select`](#select) and [`ignore`](#ignore) settings,
+along with the [`extend-select`](#extend-select) and [`extend-ignore`](#extend-ignore) modifiers.
+
+To resolve the enabled rule set, Ruff may need to reconcile `select` and `ignore` from a variety
+of sources, including the current `pyproject.toml`, any inherited `pyproject.toml` files, and the
+CLI (e.g., `--select`).
+
+In those scenarios, Ruff uses the "highest-priority" `select` as the basis for the rule set, and
+then applies any `extend-select`, `ignore`, and `extend-ignore` adjustments. CLI options are given
+higher priority than `pyproject.toml` options, and the current `pyproject.toml` file is given higher
+priority than any inherited `pyproject.toml` files.
+
+For example, given the following `pyproject.toml` file:
+
+```toml
+[tool.ruff]
+select = ["E", "F"]
+ignore = ["F401"]
+```
+
+Running `ruff --select F401` would result in Ruff enforcing `F401`, and no other rules.
+
+Running `ruff --extend-select B` would result in Ruff enforcing the `E`, `F`, and `B` rules, with
+the exception of `F401`.
+
 ### Ignoring errors
 
 To omit a lint rule entirely, add it to the "ignore" list via [`ignore`](#ignore) or
@@ -2212,11 +2237,8 @@ extend-exclude = ["tests", "src/bad.py"]
 A list of rule codes or prefixes to ignore, in addition to those
 specified by `ignore`.
 
-Note that `extend-ignore` is applied after resolving rules from
-`ignore`/`select` and a less specific rule in `extend-ignore`
-would overwrite a more specific rule in `select`. It is
-recommended to only use `extend-ignore` when extending a
-`pyproject.toml` file via `extend`.
+This option has been DEPRECATED in favor of `ignore`
+since its usage is now interchangeable with `ignore`.
 
 **Default value**: `[]`
 
@@ -2237,12 +2259,6 @@ extend-ignore = ["F841"]
 A list of rule codes or prefixes to enable, in addition to those
 specified by `select`.
 
-Note that `extend-select` is applied after resolving rules from
-`ignore`/`select` and a less specific rule in `extend-select`
-would overwrite a more specific rule in `ignore`. It is
-recommended to only use `extend-select` when extending a
-`pyproject.toml` file via `extend`.
-
 **Default value**: `[]`
 
 **Type**: `Vec<RuleSelector>`

ruff.schema.json

@@ -66,18 +66,8 @@
         "type": "string"
       }
     },
-    "extend-ignore": {
-      "description": "A list of rule codes or prefixes to ignore, in addition to those specified by `ignore`.\n\nNote that `extend-ignore` is applied after resolving rules from `ignore`/`select` and a less specific rule in `extend-ignore` would overwrite a more specific rule in `select`. It is recommended to only use `extend-ignore` when extending a `pyproject.toml` file via `extend`.",
-      "type": [
-        "array",
-        "null"
-      ],
-      "items": {
-        "$ref": "#/definitions/RuleSelector"
-      }
-    },
     "extend-select": {
-      "description": "A list of rule codes or prefixes to enable, in addition to those specified by `select`.\n\nNote that `extend-select` is applied after resolving rules from `ignore`/`select` and a less specific rule in `extend-select` would overwrite a more specific rule in `ignore`. It is recommended to only use `extend-select` when extending a `pyproject.toml` file via `extend`.",
+      "description": "A list of rule codes or prefixes to enable, in addition to those specified by `select`.",
       "type": [
         "array",
         "null"

ruff_cli/src/args.rs

@@ -5,6 +5,7 @@ use regex::Regex;
 use ruff::logging::LogLevel;
 use ruff::registry::Rule;
 use ruff::resolver::ConfigProcessor;
+use ruff::settings::configuration::RuleSelection;
 use ruff::settings::types::{
     FilePattern, PatternPrefixPair, PerFileIgnore, PythonVersion, SerializationFormat,
 };
@@ -116,13 +117,13 @@ pub struct CheckArgs {
         help_heading = "Rule selection"
     )]
     pub extend_select: Option<Vec<RuleSelector>>,
-    /// Like --ignore, but adds additional rule codes on top of the ignored
-    /// ones.
+    /// Like --ignore. (Deprecated: You can just use --ignore instead.)
     #[arg(
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        help_heading = "Rule selection"
+        help_heading = "Rule selection",
+        hide = true
     )]
     pub extend_ignore: Option<Vec<RuleSelector>>,
     /// List of mappings from file pattern to code to exclude
@@ -442,18 +443,25 @@ impl ConfigProcessor for &Overrides {
         if let Some(fix_only) = &self.fix_only {
             config.fix_only = Some(*fix_only);
         }
-        if let Some(fixable) = &self.fixable {
-            config.fixable = Some(fixable.clone());
-        }
+        config.rule_selections.push(RuleSelection {
+            select: self.select.clone(),
+            ignore: self
+                .ignore
+                .iter()
+                .cloned()
+                .chain(self.extend_ignore.iter().cloned().into_iter())
+                .flatten()
+                .collect(),
+            extend_select: self.extend_select.clone().unwrap_or_default(),
+            fixable: self.fixable.clone(),
+            unfixable: self.unfixable.clone().unwrap_or_default(),
+        });
         if let Some(format) = &self.format {
             config.format = Some(*format);
         }
         if let Some(force_exclude) = &self.force_exclude {
             config.force_exclude = Some(*force_exclude);
         }
-        if let Some(ignore) = &self.ignore {
-            config.ignore = Some(ignore.clone());
-        }
         if let Some(line_length) = &self.line_length {
             config.line_length = Some(*line_length);
         }
@@ -463,38 +471,15 @@ impl ConfigProcessor for &Overrides {
         if let Some(respect_gitignore) = &self.respect_gitignore {
             config.respect_gitignore = Some(*respect_gitignore);
         }
-        if let Some(select) = &self.select {
-            config.select = Some(select.clone());
-        }
         if let Some(show_source) = &self.show_source {
             config.show_source = Some(*show_source);
         }
         if let Some(target_version) = &self.target_version {
             config.target_version = Some(*target_version);
         }
-        if let Some(unfixable) = &self.unfixable {
-            config.unfixable = Some(unfixable.clone());
-        }
         if let Some(update_check) = &self.update_check {
             config.update_check = Some(*update_check);
         }
-        // Special-case: `extend_ignore` and `extend_select` are parallel arrays, so
-        // push an empty array if only one of the two is provided.
-        match (&self.extend_ignore, &self.extend_select) {
-            (Some(extend_ignore), Some(extend_select)) => {
-                config.extend_ignore.push(extend_ignore.clone());
-                config.extend_select.push(extend_select.clone());
-            }
-            (Some(extend_ignore), None) => {
-                config.extend_ignore.push(extend_ignore.clone());
-                config.extend_select.push(Vec::new());
-            }
-            (None, Some(extend_select)) => {
-                config.extend_ignore.push(Vec::new());
-                config.extend_select.push(extend_select.clone());
-            }
-            (None, None) => {}
-        }
     }
 }
 

src/rule_selector.rs

@@ -6,6 +6,7 @@ use schemars::JsonSchema;
 use serde::de::{self, Visitor};
 use serde::{Deserialize, Serialize};
 use strum::IntoEnumIterator;
+use strum_macros::EnumIter;
 
 use crate::registry::{Rule, RuleCodePrefix, RuleIter};
 use crate::rule_redirects::get_redirect;
@@ -171,7 +172,7 @@ impl RuleSelector {
     }
 }
 
-#[derive(PartialEq, Eq, PartialOrd, Ord)]
+#[derive(EnumIter, PartialEq, Eq, PartialOrd, Ord)]
 pub(crate) enum Specificity {
     All,
     Linter,

src/settings/configuration.rs

@@ -27,13 +27,17 @@ use crate::settings::types::{
 };
 
 #[derive(Debug, Default)]
-pub struct Configuration {
+pub struct RuleSelection {
     pub select: Option<Vec<RuleSelector>>,
-    pub ignore: Option<Vec<RuleSelector>>,
-    pub extend_select: Vec<Vec<RuleSelector>>,
-    pub extend_ignore: Vec<Vec<RuleSelector>>,
+    pub ignore: Vec<RuleSelector>,
+    pub extend_select: Vec<RuleSelector>,
     pub fixable: Option<Vec<RuleSelector>>,
-    pub unfixable: Option<Vec<RuleSelector>>,
+    pub unfixable: Vec<RuleSelector>,
+}
+
+#[derive(Debug, Default)]
+pub struct Configuration {
+    pub rule_selections: Vec<RuleSelection>,
     pub per_file_ignores: Option<Vec<PerFileIgnore>>,
 
     pub allowed_confusables: Option<Vec<char>>,
@@ -88,6 +92,18 @@ impl Configuration {
 
     pub fn from_options(options: Options, project_root: &Path) -> Result<Self> {
         Ok(Configuration {
+            rule_selections: vec![RuleSelection {
+                select: options.select,
+                ignore: options
+                    .ignore
+                    .into_iter()
+                    .flatten()
+                    .chain(options.extend_ignore.into_iter().flatten())
+                    .collect(),
+                extend_select: options.extend_select.unwrap_or_default(),
+                fixable: options.fixable,
+                unfixable: options.unfixable.unwrap_or_default(),
+            }],
             allowed_confusables: options.allowed_confusables,
             builtins: options.builtins,
             cache_dir: options
@@ -132,15 +148,11 @@ impl Configuration {
                         .collect()
                 })
                 .unwrap_or_default(),
-            extend_ignore: vec![options.extend_ignore.unwrap_or_default()],
-            extend_select: vec![options.extend_select.unwrap_or_default()],
             external: options.external,
             fix: options.fix,
             fix_only: options.fix_only,
-            fixable: options.fixable,
             format: options.format,
             force_exclude: options.force_exclude,
-            ignore: options.ignore,
             ignore_init_module_imports: options.ignore_init_module_imports,
             line_length: options.line_length,
             namespace_packages: options
@@ -157,7 +169,6 @@ impl Configuration {
             }),
             required_version: options.required_version,
             respect_gitignore: options.respect_gitignore,
-            select: options.select,
             show_source: options.show_source,
             src: options
                 .src
@@ -166,7 +177,6 @@ impl Configuration {
             target_version: options.target_version,
             task_tags: options.task_tags,
             typing_modules: options.typing_modules,
-            unfixable: options.unfixable,
             update_check: options.update_check,
             // Plugins
             flake8_annotations: options.flake8_annotations,
@@ -194,6 +204,11 @@ impl Configuration {
     #[must_use]
     pub fn combine(self, config: Configuration) -> Self {
         Self {
+            rule_selections: config
+                .rule_selections
+                .into_iter()
+                .chain(self.rule_selections.into_iter())
+                .collect(),
             allowed_confusables: self.allowed_confusables.or(config.allowed_confusables),
             builtins: self.builtins.or(config.builtins),
             cache_dir: self.cache_dir.or(config.cache_dir),
@@ -205,23 +220,11 @@ impl Configuration {
                 .into_iter()
                 .chain(self.extend_exclude.into_iter())
                 .collect(),
-            extend_ignore: config
-                .extend_ignore
-                .into_iter()
-                .chain(self.extend_ignore.into_iter())
-                .collect(),
-            extend_select: config
-                .extend_select
-                .into_iter()
-                .chain(self.extend_select.into_iter())
-                .collect(),
             external: self.external.or(config.external),
             fix: self.fix.or(config.fix),
             fix_only: self.fix_only.or(config.fix_only),
-            fixable: self.fixable.or(config.fixable),
             format: self.format.or(config.format),
             force_exclude: self.force_exclude.or(config.force_exclude),
-            ignore: self.ignore.or(config.ignore),
             ignore_init_module_imports: self
                 .ignore_init_module_imports
                 .or(config.ignore_init_module_imports),
@@ -230,13 +233,11 @@ impl Configuration {
             per_file_ignores: self.per_file_ignores.or(config.per_file_ignores),
             required_version: self.required_version.or(config.required_version),
             respect_gitignore: self.respect_gitignore.or(config.respect_gitignore),
-            select: self.select.or(config.select),
             show_source: self.show_source.or(config.show_source),
             src: self.src.or(config.src),
             target_version: self.target_version.or(config.target_version),
             task_tags: self.task_tags.or(config.task_tags),
             typing_modules: self.typing_modules.or(config.typing_modules),
-            unfixable: self.unfixable.or(config.unfixable),
             update_check: self.update_check.or(config.update_check),
             // Plugins
             flake8_annotations: self.flake8_annotations.or(config.flake8_annotations),