Show rule codes in shell tab completion (#7375)

## Summary I noticed that we have a custom parser for rule selectors, but it wasn't actually being used? This PR adds it back to our Clap setup and changes the parser to only show full categories and individual rules when tab-completing: <img width="1792" alt="Screen Shot 2023-09-13 at 9 13 38 PM" src="https://github.com/astral-sh/ruff/assets/1309177/028b18d2-8c92-49c1-b781-f24c9ae310f7"> <img width="1792" alt="Screen Shot 2023-09-13 at 9 13 40 PM" src="https://github.com/astral-sh/ruff/assets/1309177/fd598da5-78fb-412d-a69e-2a0963d479cd"> <img width="1792" alt="Screen Shot 2023-09-13 at 9 13 58 PM" src="https://github.com/astral-sh/ruff/assets/1309177/7c482b90-6e54-425c-ae23-fb50496a177a"> The previous implementation showed all codes, which I found too noisy: <img width="1792" alt="Screen Shot 2023-09-13 at 8 57 09 PM" src="https://github.com/astral-sh/ruff/assets/1309177/db370a0e-2a9f-4acd-b1e3-224a1f8e9ce5">
astral-sh · Sep 14, 2023 · f9e3ea2 · f9e3ea2
1 parent 6856d0b
commit f9e3ea2
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 44 deletions.
diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs
@@ -5,6 +5,8 @@
 //!
 //! [Ruff]: https://github.com/astral-sh/ruff
 
+#[cfg(feature = "clap")]
+pub use rule_selector::clap_completion::RuleSelectorParser;
 pub use rule_selector::RuleSelector;
 pub use rules::pycodestyle::rules::{IOError, SyntaxError};
 

diff --git a/crates/ruff/src/rule_selector.rs b/crates/ruff/src/rule_selector.rs
@@ -336,13 +336,14 @@ pub enum Specificity {
 }
 
 #[cfg(feature = "clap")]
-mod clap_completion {
+pub mod clap_completion {
     use clap::builder::{PossibleValue, TypedValueParser, ValueParserFactory};
     use strum::IntoEnumIterator;
 
     use crate::{
         codes::RuleCodePrefix,
         registry::{Linter, RuleNamespace},
+        rule_selector::is_single_rule_selector,
         RuleSelector,
     };
 
@@ -362,17 +363,29 @@ mod clap_completion {
 
         fn parse_ref(
             &self,
-            _cmd: &clap::Command,
-            _arg: Option<&clap::Arg>,
+            cmd: &clap::Command,
+            arg: Option<&clap::Arg>,
             value: &std::ffi::OsStr,
         ) -> Result<Self::Value, clap::Error> {
             let value = value
                 .to_str()
                 .ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?;
 
-            value
-                .parse()
-                .map_err(|e| clap::Error::raw(clap::error::ErrorKind::InvalidValue, e))
+            value.parse().map_err(|_| {
+                let mut error =
+                    clap::Error::new(clap::error::ErrorKind::ValueValidation).with_cmd(cmd);
+                if let Some(arg) = arg {
+                    error.insert(
+                        clap::error::ContextKind::InvalidArg,
+                        clap::error::ContextValue::String(arg.to_string()),
+                    );
+                }
+                error.insert(
+                    clap::error::ContextKind::InvalidValue,
+                    clap::error::ContextValue::String(value.to_string()),
+                );
+                error
+            })
         }
 
         fn possible_values(&self) -> Option<Box<dyn Iterator<Item = PossibleValue> + '_>> {
@@ -387,27 +400,34 @@ mod clap_completion {
                             RuleCodePrefix::iter()
                                 // Filter out rule gated behind `#[cfg(feature = "unreachable-code")]`, which is
                                 // off-by-default
-                                .filter(|p| {
-                                    format!("{}{}", p.linter().common_prefix(), p.short_code())
-                                        != "RUF014"
+                                .filter(|prefix| {
+                                    format!(
+                                        "{}{}",
+                                        prefix.linter().common_prefix(),
+                                        prefix.short_code()
+                                    ) != "RUF014"
                                 })
-                                .map(|p| {
-                                    let prefix = p.linter().common_prefix();
-                                    let code = p.short_code();
-
-                                    let mut rules_iter = p.rules();
-                                    let rule1 = rules_iter.next();
-                                    let rule2 = rules_iter.next();
-
-                                    let value = PossibleValue::new(format!("{prefix}{code}"));
-
-                                    if rule2.is_none() {
-                                        let rule1 = rule1.unwrap();
-                                        let name: &'static str = rule1.into();
-                                        value.help(name)
-                                    } else {
-                                        value
+                                .filter_map(|prefix| {
+                                    // Ex) `UP`
+                                    if prefix.short_code().is_empty() {
+                                        let code = prefix.linter().common_prefix();
+                                        let name = prefix.linter().name();
+                                        return Some(PossibleValue::new(code).help(name));
                                     }
+
+                                    // Ex) `UP004`
+                                    if is_single_rule_selector(&prefix) {
+                                        let rule = prefix.rules().next()?;
+                                        let code = format!(
+                                            "{}{}",
+                                            prefix.linter().common_prefix(),
+                                            prefix.short_code()
+                                        );
+                                        let name: &'static str = rule.into();
+                                        return Some(PossibleValue::new(code).help(name));
+                                    }
+
+                                    None
                                 }),
                         ),
                 ),

diff --git a/crates/ruff_cli/src/args.rs b/crates/ruff_cli/src/args.rs
@@ -1,17 +1,16 @@
 use std::path::PathBuf;
-use std::str::FromStr;
 
 use clap::{command, Parser};
 use regex::Regex;
-use ruff::line_width::LineLength;
 use rustc_hash::FxHashMap;
 
+use ruff::line_width::LineLength;
 use ruff::logging::LogLevel;
 use ruff::registry::Rule;
 use ruff::settings::types::{
     FilePattern, PatternPrefixPair, PerFileIgnore, PreviewMode, PythonVersion, SerializationFormat,
 };
-use ruff::RuleSelector;
+use ruff::{RuleSelector, RuleSelectorParser};
 use ruff_workspace::configuration::{Configuration, RuleSelection};
 use ruff_workspace::resolver::ConfigProcessor;
 
@@ -129,7 +128,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide_possible_values = true
     )]
@@ -139,7 +138,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide_possible_values = true
     )]
@@ -149,7 +148,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide_possible_values = true
     )]
@@ -159,7 +158,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide = true
     )]
@@ -191,7 +190,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide_possible_values = true
     )]
@@ -201,7 +200,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide_possible_values = true
     )]
@@ -211,7 +210,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide_possible_values = true
     )]
@@ -221,7 +220,7 @@ pub struct CheckCommand {
         long,
         value_delimiter = ',',
         value_name = "RULE_CODE",
-        value_parser = parse_rule_selector,
+        value_parser = RuleSelectorParser,
         help_heading = "Rule selection",
         hide = true
     )]
@@ -511,11 +510,6 @@ impl FormatCommand {
     }
 }
 
-fn parse_rule_selector(env: &str) -> Result<RuleSelector, std::io::Error> {
-    RuleSelector::from_str(env)
-        .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))
-}
-
 fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
     match (yes, no) {
         (true, false) => Some(true),

diff --git a/crates/ruff_cli/tests/integration_test.rs b/crates/ruff_cli/tests/integration_test.rs
@@ -451,7 +451,7 @@ fn preview_group_selector() {
     ----- stdout -----
 
     ----- stderr -----
-    error: invalid value 'PREVIEW' for '--select <RULE_CODE>': Unknown rule selector: `PREVIEW`
+    error: invalid value 'PREVIEW' for '--select <RULE_CODE>'
 
     For more information, try '--help'.
     "###);
@@ -470,7 +470,7 @@ fn preview_enabled_group_ignore() {
     ----- stdout -----
 
     ----- stderr -----
-    error: invalid value 'PREVIEW' for '--ignore <RULE_CODE>': Unknown rule selector: `PREVIEW`
+    error: invalid value 'PREVIEW' for '--ignore <RULE_CODE>'
 
     For more information, try '--help'.
     "###);

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -476,7 +476,7 @@ Ruff supports two command-line flags that alter its exit code behavior:
     `--exit-non-zero-on-fix` can result in a non-zero exit code even if no violations remain after
     autofixing.
 
-## Autocompletion
+## Shell autocompletion
 
 Ruff supports autocompletion for most shells. A shell-specific completion script can be generated
 by `ruff generate-shell-completion <SHELL>`, where `<SHELL>` is one of `bash`, `elvish`, `fig`, `fish`,