Treat convention as setting ignore, rather than select (#1611)

README.md

@@ -1493,53 +1493,21 @@ For example, if you're coming from `flake8-docstrings`, and your originating con
 `--docstring-convention=numpy`, you'd instead set `convention = "numpy"` in your `pyproject.toml`,
 as above.
 
-Note that setting a `convention` is equivalent to adding that convention's specific set of codes to
+Alongside `convention`, you'll want to explicitly enable the `D` error code class, like so:
-your `select`. For example, `convention = "numpy"` is equivalent to:
 
 ```toml
 [tool.ruff]
-# Enable all `D` errors except `D107`, `D203`, `D212`, `D213`, `D402`, `D413`, `D415`, `D416`,
-# and `D417`.
 select = [
-    "D100",
+    "D",
-    "D101",
-    "D102",
-    "D103",
-    "D104",
-    "D105",
-    "D106",
-    "D200",
-    "D201",
-    "D202",
-    "D204",
-    "D205",
-    "D206",
-    "D207",
-    "D208",
-    "D209",
-    "D210",
-    "D211",
-    "D214",
-    "D215",
-    "D300",
-    "D301",
-    "D400",
-    "D403",
-    "D404",
-    "D405",
-    "D406",
-    "D407",
-    "D408",
-    "D409",
-    "D410",
-    "D411",
-    "D412",
-    "D414",
-    "D418",
-    "D419",
 ]
+
+[tool.ruff.pydocstyle]
+convention = "google"
 ```
 
+Setting a `convention` force-disables any rules that are incompatible with that convention, no
+matter how they're provided, which avoids accidental incompatibilities and simplifies configuration.
+
 ### How can I tell what settings Ruff is using to check my code?
 
 Run `ruff /path/to/code.py --show-settings` to view the resolved settings for a given file.

flake8_to_ruff/src/converter.rs

@@ -74,7 +74,7 @@ pub fn convert(
                 .as_ref()
                 .map(|value| BTreeSet::from_iter(parser::parse_prefix_codes(value)))
         })
-        .unwrap_or_else(|| plugin::resolve_select(flake8, &plugins));
+        .unwrap_or_else(|| plugin::resolve_select(&plugins));
     let mut ignore = flake8
         .get("ignore")
         .and_then(|value| {
@@ -291,13 +291,6 @@ pub fn convert(
         }
     }
 
-    // Set default `convention`.
-    if plugins.contains(&Plugin::Flake8Docstrings) {
-        if pydocstyle.convention.is_none() {
-            pydocstyle.convention = Some(Convention::Pep257);
-        }
-    }
-
     // Deduplicate and sort.
     options.select = Some(Vec::from_iter(select));
     options.ignore = Some(Vec::from_iter(ignore));
@@ -697,6 +690,7 @@ mod tests {
             required_version: None,
             respect_gitignore: None,
             select: Some(vec![
+                CheckCodePrefix::D,
                 CheckCodePrefix::E,
                 CheckCodePrefix::F,
                 CheckCodePrefix::W,

flake8_to_ruff/src/plugin.rs

@@ -97,7 +97,7 @@ impl fmt::Debug for Plugin {
 }
 
 impl Plugin {
-    pub fn default(&self) -> CheckCodePrefix {
+    pub fn prefix(&self) -> CheckCodePrefix {
         match self {
             Plugin::Flake8Annotations => CheckCodePrefix::ANN,
             Plugin::Flake8Bandit => CheckCodePrefix::S,
@@ -125,48 +125,6 @@ impl Plugin {
             Plugin::Pyupgrade => CheckCodePrefix::UP,
         }
     }
-
-    pub fn select(&self, flake8: &HashMap<String, Option<String>>) -> Vec<CheckCodePrefix> {
-        match self {
-            Plugin::Flake8Annotations => vec![CheckCodePrefix::ANN],
-            Plugin::Flake8Bandit => vec![CheckCodePrefix::S],
-            Plugin::Flake8BlindExcept => vec![CheckCodePrefix::BLE],
-            Plugin::Flake8Bugbear => vec![CheckCodePrefix::B],
-            Plugin::Flake8Builtins => vec![CheckCodePrefix::A],
-            Plugin::Flake8Comprehensions => vec![CheckCodePrefix::C4],
-            Plugin::Flake8Datetimez => vec![CheckCodePrefix::DTZ],
-            Plugin::Flake8Debugger => vec![CheckCodePrefix::T1],
-            Plugin::Flake8Docstrings => {
-                // Use the user-provided docstring.
-                for key in ["docstring-convention", "docstring_convention"] {
-                    if let Some(Some(value)) = flake8.get(key) {
-                        match DocstringConvention::from_str(value) {
-                            Ok(convention) => return convention.select(),
-                            Err(e) => {
-                                eprintln!("Unexpected '{key}' value: {value} ({e}");
-                                return vec![];
-                            }
-                        }
-                    }
-                }
-                // Default to PEP257.
-                DocstringConvention::Pep257.select()
-            }
-            Plugin::Flake8Eradicate => vec![CheckCodePrefix::ERA],
-            Plugin::Flake8ErrMsg => vec![CheckCodePrefix::EM],
-            Plugin::Flake8ImplicitStrConcat => vec![CheckCodePrefix::ISC],
-            Plugin::Flake8Print => vec![CheckCodePrefix::T2],
-            Plugin::Flake8PytestStyle => vec![CheckCodePrefix::PT],
-            Plugin::Flake8Quotes => vec![CheckCodePrefix::Q],
-            Plugin::Flake8Return => vec![CheckCodePrefix::RET],
-            Plugin::Flake8Simplify => vec![CheckCodePrefix::SIM],
-            Plugin::Flake8TidyImports => vec![CheckCodePrefix::TID],
-            Plugin::McCabe => vec![CheckCodePrefix::C9],
-            Plugin::PandasVet => vec![CheckCodePrefix::PD],
-            Plugin::PEP8Naming => vec![CheckCodePrefix::N],
-            Plugin::Pyupgrade => vec![CheckCodePrefix::UP],
-        }
-    }
 }
 
 pub enum DocstringConvention {
@@ -190,23 +148,6 @@ impl FromStr for DocstringConvention {
     }
 }
 
-impl DocstringConvention {
-    fn select(&self) -> Vec<CheckCodePrefix> {
-        match self {
-            DocstringConvention::All => vec![CheckCodePrefix::D],
-            DocstringConvention::Pep257 => vec![
-                // Covered by the `convention` setting.
-            ],
-            DocstringConvention::Numpy => vec![
-                // Covered by the `convention` setting.
-            ],
-            DocstringConvention::Google => vec![
-                // Covered by the `convention` setting.
-            ],
-        }
-    }
-}
-
 /// Infer the enabled plugins based on user-provided options.
 ///
 /// For example, if the user specified a `mypy-init-return` setting, we should
@@ -356,7 +297,7 @@ pub fn infer_plugins_from_codes(codes: &BTreeSet<CheckCodePrefix>) -> Vec<Plugin
             if prefix
                 .codes()
                 .iter()
-                .any(|code| plugin.default().codes().contains(code))
+                .any(|code| plugin.prefix().codes().contains(code))
             {
                 return true;
             }
@@ -367,18 +308,9 @@ pub fn infer_plugins_from_codes(codes: &BTreeSet<CheckCodePrefix>) -> Vec<Plugin
 }
 
 /// Resolve the set of enabled `CheckCodePrefix` values for the given plugins.
-pub fn resolve_select(
+pub fn resolve_select(plugins: &[Plugin]) -> BTreeSet<CheckCodePrefix> {
-    flake8: &HashMap<String, Option<String>>,
-    plugins: &[Plugin],
-) -> BTreeSet<CheckCodePrefix> {
-    // Include default Pyflakes and pycodestyle checks.
     let mut select = BTreeSet::from([CheckCodePrefix::F, CheckCodePrefix::E, CheckCodePrefix::W]);
-
+    select.extend(plugins.iter().map(Plugin::prefix));
-    // Add prefix codes for every plugin.
-    for plugin in plugins {
-        select.extend(plugin.select(flake8));
-    }
-
     select
 }
 

src/pydocstyle/settings.rs

@@ -4,7 +4,7 @@ use ruff_macros::ConfigurationOptions;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
-use crate::registry::CheckCode;
+use crate::registry_gen::CheckCodePrefix;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Hash, JsonSchema)]
 #[serde(deny_unknown_fields, rename_all = "kebab-case")]
@@ -18,153 +18,55 @@ pub enum Convention {
 }
 
 impl Convention {
-    pub fn codes(&self) -> Vec<CheckCode> {
+    pub fn codes(&self) -> &'static [CheckCodePrefix] {
         match self {
-            Convention::Google => vec![
+            Convention::Google => &[
                 // All errors except D203, D204, D213, D215, D400, D401, D404, D406, D407, D408,
                 // D409 and D413.
-                CheckCode::D100,
+                CheckCodePrefix::D203,
-                CheckCode::D101,
+                CheckCodePrefix::D204,
-                CheckCode::D102,
+                CheckCodePrefix::D213,
-                CheckCode::D103,
+                CheckCodePrefix::D215,
-                CheckCode::D104,
+                CheckCodePrefix::D400,
-                CheckCode::D105,
+                CheckCodePrefix::D404,
-                CheckCode::D106,
+                CheckCodePrefix::D406,
-                CheckCode::D107,
+                CheckCodePrefix::D407,
-                CheckCode::D200,
+                CheckCodePrefix::D408,
-                CheckCode::D201,
+                CheckCodePrefix::D409,
-                CheckCode::D202,
+                CheckCodePrefix::D413,
-                // CheckCode::D203,
-                // CheckCode::D204,
-                CheckCode::D205,
-                CheckCode::D206,
-                CheckCode::D207,
-                CheckCode::D208,
-                CheckCode::D209,
-                CheckCode::D210,
-                CheckCode::D211,
-                CheckCode::D212,
-                // CheckCode::D213,
-                CheckCode::D214,
-                // CheckCode::D215,
-                CheckCode::D300,
-                CheckCode::D301,
-                // CheckCode::D400,
-                CheckCode::D402,
-                CheckCode::D403,
-                // CheckCode::D404,
-                CheckCode::D405,
-                // CheckCode::D406,
-                // CheckCode::D407,
-                // CheckCode::D408,
-                // CheckCode::D409,
-                CheckCode::D410,
-                CheckCode::D411,
-                CheckCode::D412,
-                // CheckCode::D413,
-                CheckCode::D414,
-                CheckCode::D415,
-                CheckCode::D416,
-                CheckCode::D417,
-                CheckCode::D418,
-                CheckCode::D419,
             ],
-            Convention::Numpy => vec![
+            Convention::Numpy => &[
                 // All errors except D107, D203, D212, D213, D402, D413, D415, D416, and D417.
-                CheckCode::D100,
+                CheckCodePrefix::D107,
-                CheckCode::D101,
+                CheckCodePrefix::D203,
-                CheckCode::D102,
+                CheckCodePrefix::D212,
-                CheckCode::D103,
+                CheckCodePrefix::D213,
-                CheckCode::D104,
+                CheckCodePrefix::D402,
-                CheckCode::D105,
+                CheckCodePrefix::D413,
-                CheckCode::D106,
+                CheckCodePrefix::D415,
-                // CheckCode::D107,
+                CheckCodePrefix::D416,
-                CheckCode::D200,
+                CheckCodePrefix::D417,
-                CheckCode::D201,
-                CheckCode::D202,
-                // CheckCode::D203,
-                CheckCode::D204,
-                CheckCode::D205,
-                CheckCode::D206,
-                CheckCode::D207,
-                CheckCode::D208,
-                CheckCode::D209,
-                CheckCode::D210,
-                CheckCode::D211,
-                // CheckCode::D212,
-                // CheckCode::D213,
-                CheckCode::D214,
-                CheckCode::D215,
-                CheckCode::D300,
-                CheckCode::D301,
-                CheckCode::D400,
-                // CheckCode::D402,
-                CheckCode::D403,
-                CheckCode::D404,
-                CheckCode::D405,
-                CheckCode::D406,
-                CheckCode::D407,
-                CheckCode::D408,
-                CheckCode::D409,
-                CheckCode::D410,
-                CheckCode::D411,
-                CheckCode::D412,
-                // CheckCode::D413,
-                CheckCode::D414,
-                // CheckCode::D415,
-                // CheckCode::D416,
-                // CheckCode::D417,
-                CheckCode::D418,
-                CheckCode::D419,
             ],
-            Convention::Pep257 => vec![
+            Convention::Pep257 => &[
                 // All errors except D203, D212, D213, D214, D215, D404, D405, D406, D407, D408,
                 // D409, D410, D411, D413, D415, D416 and D417.
-                CheckCode::D100,
+                CheckCodePrefix::D203,
-                CheckCode::D101,
+                CheckCodePrefix::D212,
-                CheckCode::D102,
+                CheckCodePrefix::D213,
-                CheckCode::D103,
+                CheckCodePrefix::D214,
-                CheckCode::D104,
+                CheckCodePrefix::D215,
-                CheckCode::D105,
+                CheckCodePrefix::D404,
-                CheckCode::D106,
+                CheckCodePrefix::D405,
-                CheckCode::D107,
+                CheckCodePrefix::D406,
-                CheckCode::D200,
+                CheckCodePrefix::D407,
-                CheckCode::D201,
+                CheckCodePrefix::D408,
-                CheckCode::D202,
+                CheckCodePrefix::D409,
-                // CheckCode::D203,
+                CheckCodePrefix::D410,
-                CheckCode::D204,
+                CheckCodePrefix::D411,
-                CheckCode::D205,
+                CheckCodePrefix::D413,
-                CheckCode::D206,
+                CheckCodePrefix::D415,
-                CheckCode::D207,
+                CheckCodePrefix::D416,
-                CheckCode::D208,
+                CheckCodePrefix::D417,
-                CheckCode::D209,
-                CheckCode::D210,
-                CheckCode::D211,
-                // CheckCode::D212,
-                // CheckCode::D213,
-                // CheckCode::D214,
-                // CheckCode::D215,
-                CheckCode::D300,
-                CheckCode::D301,
-                CheckCode::D400,
-                CheckCode::D402,
-                CheckCode::D403,
-                // CheckCode::D404,
-                // CheckCode::D405,
-                // CheckCode::D406,
-                // CheckCode::D407,
-                // CheckCode::D408,
-                // CheckCode::D409,
-                // CheckCode::D410,
-                // CheckCode::D411,
-                CheckCode::D412,
-                // CheckCode::D413,
-                CheckCode::D414,
-                // CheckCode::D415,
-                // CheckCode::D416,
-                // CheckCode::D417,
-                CheckCode::D418,
-                CheckCode::D419,
             ],
         }
     }

src/settings/mod.rs

@@ -3,11 +3,13 @@
 //! to external visibility or parsing.
 
 use std::hash::{Hash, Hasher};
+use std::iter;
 use std::path::{Path, PathBuf};
 
 use anyhow::{anyhow, Result};
 use colored::Colorize;
 use globset::{Glob, GlobMatcher, GlobSet};
+use itertools::Either::{Left, Right};
 use itertools::Itertools;
 use once_cell::sync::Lazy;
 use path_absolutize::path_dedot;
@@ -114,12 +116,6 @@ impl Settings {
                 .dummy_variable_rgx
                 .unwrap_or_else(|| DEFAULT_DUMMY_VARIABLE_RGX.clone()),
             enabled: validate_enabled(resolve_codes(
-                config
-                    .pydocstyle
-                    .as_ref()
-                    .and_then(|pydocstyle| pydocstyle.convention)
-                    .map(|convention| convention.codes())
-                    .unwrap_or_default(),
                 [CheckCodeSpec {
                     select: &config
                         .select
@@ -133,6 +129,22 @@ impl Settings {
                         .iter()
                         .zip(config.extend_ignore.iter())
                         .map(|(select, ignore)| CheckCodeSpec { select, ignore }),
+                )
+                .chain(
+                    // If a docstring convention is specified, force-disable any incompatible error
+                    // codes.
+                    if let Some(convention) = config
+                        .pydocstyle
+                        .as_ref()
+                        .and_then(|pydocstyle| pydocstyle.convention)
+                    {
+                        Left(iter::once(CheckCodeSpec {
+                            select: &[],
+                            ignore: convention.codes(),
+                        }))
+                    } else {
+                        Right(iter::empty())
+                    },
                 ),
             )),
             exclude: resolve_globset(config.exclude.unwrap_or_else(|| DEFAULT_EXCLUDE.clone()))?,
@@ -141,7 +153,6 @@ impl Settings {
             fix: config.fix.unwrap_or(false),
             fix_only: config.fix_only.unwrap_or(false),
             fixable: resolve_codes(
-                vec![],
                 [CheckCodeSpec {
                     select: &config.fixable.unwrap_or_else(|| CATEGORIES.to_vec()),
                     ignore: &config.unfixable.unwrap_or_default(),
@@ -392,11 +403,8 @@ struct CheckCodeSpec<'a> {
 
 /// Given a set of selected and ignored prefixes, resolve the set of enabled
 /// error codes.
-fn resolve_codes<'a>(
+fn resolve_codes<'a>(specs: impl Iterator<Item = CheckCodeSpec<'a>>) -> FxHashSet<CheckCode> {
-    baseline: Vec<CheckCode>,
+    let mut codes: FxHashSet<CheckCode> = FxHashSet::default();
-    specs: impl Iterator<Item = CheckCodeSpec<'a>>,
-) -> FxHashSet<CheckCode> {
-    let mut codes: FxHashSet<CheckCode> = FxHashSet::from_iter(baseline);
     for spec in specs {
         for specificity in [
             SuffixLength::None,
@@ -449,7 +457,6 @@ mod tests {
     #[test]
     fn check_codes() {
         let actual = resolve_codes(
-            vec![],
             [CheckCodeSpec {
                 select: &[CheckCodePrefix::W],
                 ignore: &[],
@@ -460,7 +467,6 @@ mod tests {
         assert_eq!(actual, expected);
 
         let actual = resolve_codes(
-            vec![],
             [CheckCodeSpec {
                 select: &[CheckCodePrefix::W6],
                 ignore: &[],
@@ -471,7 +477,6 @@ mod tests {
         assert_eq!(actual, expected);
 
         let actual = resolve_codes(
-            vec![],
             [CheckCodeSpec {
                 select: &[CheckCodePrefix::W],
                 ignore: &[CheckCodePrefix::W292],
@@ -482,7 +487,6 @@ mod tests {
         assert_eq!(actual, expected);
 
         let actual = resolve_codes(
-            vec![],
             [CheckCodeSpec {
                 select: &[CheckCodePrefix::W605],
                 ignore: &[CheckCodePrefix::W605],
@@ -493,7 +497,6 @@ mod tests {
         assert_eq!(actual, expected);
 
         let actual = resolve_codes(
-            vec![],
             [
                 CheckCodeSpec {
                     select: &[CheckCodePrefix::W],
@@ -510,7 +513,6 @@ mod tests {
         assert_eq!(actual, expected);
 
         let actual = resolve_codes(
-            vec![],
             [
                 CheckCodeSpec {
                     select: &[CheckCodePrefix::W],