Fix qualified lint completions ignoring the qualifier value

4 files changed, 119 insertions, 104 deletions

diff --git a/crates/ide_completion/src/completions/attribute.rs b/crates/ide_completion/src/completions/attribute.rs
index 6abc2a94c41..e139dcd40ef 100644
--- a/crates/ide_completion/src/completions/attribute.rs
+++ b/crates/ide_completion/src/completions/attribute.rs
@@ -4,7 +4,7 @@
 //! for built-in attributes.
 
 use hir::HasAttrs;
-use ide_db::helpers::generated_lints::{CLIPPY_LINTS, DEFAULT_LINTS, FEATURES};
+use ide_db::helpers::generated_lints::{CLIPPY_LINTS, DEFAULT_LINTS, FEATURES, RUSTDOC_LINTS};
 use itertools::Itertools;
 use once_cell::sync::Lazy;
 use rustc_hash::FxHashMap;
@@ -29,12 +29,16 @@ pub(crate) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext)
     };
     match (name_ref, attribute.token_tree()) {
         (Some(path), Some(token_tree)) => match path.text().as_str() {
-            "derive" => derive::complete_derive(acc, ctx, token_tree),
             "repr" => repr::complete_repr(acc, ctx, token_tree),
-            "feature" => lint::complete_lint(acc, ctx, token_tree, FEATURES),
+            "derive" => derive::complete_derive(acc, ctx, &parse_comma_sep_paths(token_tree)?),
+            "feature" => {
+                lint::complete_lint(acc, ctx, &parse_comma_sep_paths(token_tree)?, FEATURES)
+            }
             "allow" | "warn" | "deny" | "forbid" => {
-                lint::complete_lint(acc, ctx, token_tree.clone(), DEFAULT_LINTS);
-                lint::complete_lint(acc, ctx, token_tree, CLIPPY_LINTS);
+                let existing_lints = parse_comma_sep_paths(token_tree)?;
+                lint::complete_lint(acc, ctx, &existing_lints, DEFAULT_LINTS);
+                lint::complete_lint(acc, ctx, &existing_lints, CLIPPY_LINTS);
+                lint::complete_lint(acc, ctx, &existing_lints, RUSTDOC_LINTS);
             }
             "cfg" => {
                 cfg::complete_cfg(acc, ctx);
diff --git a/crates/ide_completion/src/completions/attribute/derive.rs b/crates/ide_completion/src/completions/attribute/derive.rs
index 2824b2a2c2a..e460a91102c 100644
--- a/crates/ide_completion/src/completions/attribute/derive.rs
+++ b/crates/ide_completion/src/completions/attribute/derive.rs
@@ -14,60 +14,57 @@ use crate::{
 pub(super) fn complete_derive(
     acc: &mut Completions,
     ctx: &CompletionContext,
-    derive_input: ast::TokenTree,
+    existing_derives: &[ast::Path],
 ) {
-    if let Some(existing_derives) = super::parse_comma_sep_paths(derive_input.clone()) {
-        let core = FamousDefs(&ctx.sema, ctx.krate).core();
-        let existing_derives: FxHashSet<_> = existing_derives
-            .into_iter()
-            .filter_map(|path| ctx.scope.speculative_resolve_as_mac(&path))
-            .filter(|mac| mac.kind() == MacroKind::Derive)
-            .collect();
+    let core = FamousDefs(&ctx.sema, ctx.krate).core();
+    let existing_derives: FxHashSet<_> = existing_derives
+        .into_iter()
+        .filter_map(|path| ctx.scope.speculative_resolve_as_mac(&path))
+        .filter(|mac| mac.kind() == MacroKind::Derive)
+        .collect();
 
-        for (name, mac) in get_derives_in_scope(ctx) {
-            if existing_derives.contains(&mac) {
-                continue;
-            }
+    for (name, mac) in get_derives_in_scope(ctx) {
+        if existing_derives.contains(&mac) {
+            continue;
+        }
 
-            let name = name.to_smol_str();
-            let label;
-            let (label, lookup) = match core.zip(mac.module(ctx.db).map(|it| it.krate())) {
-                // show derive dependencies for `core`/`std` derives
-                Some((core, mac_krate)) if core == mac_krate => {
-                    if let Some(derive_completion) = DEFAULT_DERIVE_DEPENDENCIES
-                        .iter()
-                        .find(|derive_completion| derive_completion.label == name)
-                    {
-                        let mut components = vec![derive_completion.label];
-                        components.extend(derive_completion.dependencies.iter().filter(
-                            |&&dependency| {
-                                !existing_derives
-                                    .iter()
-                                    .filter_map(|it| it.name(ctx.db))
-                                    .any(|it| it.to_smol_str() == dependency)
-                            },
-                        ));
-                        let lookup = components.join(", ");
-                        label = components.iter().rev().join(", ");
-                        (label.as_str(), Some(lookup))
-                    } else {
-                        (&*name, None)
-                    }
+        let name = name.to_smol_str();
+        let label;
+        let (label, lookup) = match core.zip(mac.module(ctx.db).map(|it| it.krate())) {
+            // show derive dependencies for `core`/`std` derives
+            Some((core, mac_krate)) if core == mac_krate => {
+                if let Some(derive_completion) = DEFAULT_DERIVE_DEPENDENCIES
+                    .iter()
+                    .find(|derive_completion| derive_completion.label == name)
+                {
+                    let mut components = vec![derive_completion.label];
+                    components.extend(derive_completion.dependencies.iter().filter(
+                        |&&dependency| {
+                            !existing_derives
+                                .iter()
+                                .filter_map(|it| it.name(ctx.db))
+                                .any(|it| it.to_smol_str() == dependency)
+                        },
+                    ));
+                    let lookup = components.join(", ");
+                    label = components.iter().rev().join(", ");
+                    (label.as_str(), Some(lookup))
+                } else {
+                    (&*name, None)
                 }
-                _ => (&*name, None),
-            };
-
-            let mut item =
-                CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
-            item.kind(CompletionItemKind::Attribute);
-            if let Some(docs) = mac.docs(ctx.db) {
-                item.documentation(docs);
             }
-            if let Some(lookup) = lookup {
-                item.lookup_by(lookup);
-            }
-            item.add_to(acc);
+            _ => (&*name, None),
+        };
+
+        let mut item = CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
+        item.kind(CompletionItemKind::Attribute);
+        if let Some(docs) = mac.docs(ctx.db) {
+            item.documentation(docs);
+        }
+        if let Some(lookup) = lookup {
+            item.lookup_by(lookup);
         }
+        item.add_to(acc);
     }
 }
 
diff --git a/crates/ide_completion/src/completions/attribute/lint.rs b/crates/ide_completion/src/completions/attribute/lint.rs
index b7ad06d2f08..18942f8beb3 100644
--- a/crates/ide_completion/src/completions/attribute/lint.rs
+++ b/crates/ide_completion/src/completions/attribute/lint.rs
@@ -11,60 +11,56 @@ use crate::{
 pub(super) fn complete_lint(
     acc: &mut Completions,
     ctx: &CompletionContext,
-    derive_input: ast::TokenTree,
+    existing_lints: &[ast::Path],
     lints_completions: &[Lint],
 ) {
-    if let Some(existing_lints) = super::parse_comma_sep_paths(derive_input) {
-        for &Lint { label, description } in lints_completions {
-            let (qual, name) = {
-                // FIXME: change `Lint`'s label to not store a path in it but split the prefix off instead?
-                let mut parts = label.split("::");
-                let ns_or_label = match parts.next() {
-                    Some(it) => it,
-                    None => continue,
-                };
-                let label = parts.next();
-                match label {
-                    Some(label) => (Some(ns_or_label), label),
-                    None => (None, ns_or_label),
-                }
+    let is_qualified = ctx.previous_token_is(T![:]);
+    for &Lint { label, description } in lints_completions {
+        let (qual, name) = {
+            // FIXME: change `Lint`'s label to not store a path in it but split the prefix off instead?
+            let mut parts = label.split("::");
+            let ns_or_label = match parts.next() {
+                Some(it) => it,
+                None => continue,
             };
-            let lint_already_annotated = existing_lints
-                .iter()
-                .filter_map(|path| {
-                    let q = path.qualifier();
-                    if q.as_ref().and_then(|it| it.qualifier()).is_some() {
-                        return None;
-                    }
-                    Some((q.and_then(|it| it.as_single_name_ref()), path.segment()?.name_ref()?))
-                })
-                .any(|(q, name_ref)| {
-                    let qualifier_matches = match (q, qual) {
-                        (None, None) => true,
-                        (None, Some(_)) => false,
-                        (Some(_), None) => false,
-                        (Some(q), Some(ns)) => q.text() == ns,
-                    };
-                    qualifier_matches && name_ref.text() == name
-                });
-            if lint_already_annotated {
-                continue;
+            let label = parts.next();
+            match label {
+                Some(label) => (Some(ns_or_label), label),
+                None => (None, ns_or_label),
             }
-            let insert = match (qual, ctx.previous_token_is(T![:])) {
-                (Some(qual), false) => format!("{}::{}", qual, name),
-                // user is completing a qualified path but this completion has no qualifier
-                // so discard this completion
-                // FIXME: This is currently very hacky and will propose odd completions if
-                // we add more qualified (tool) completions other than clippy
-                (None, true) => continue,
-                _ => name.to_owned(),
-            };
-            let mut item =
-                CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
-            item.kind(CompletionItemKind::Attribute)
-                .insert_text(insert)
-                .documentation(hir::Documentation::new(description.to_owned()));
-            item.add_to(acc)
+        };
+        if qual.is_none() && is_qualified {
+            // qualified completion requested, but this lint is unqualified
+            continue;
+        }
+        let lint_already_annotated = existing_lints
+            .iter()
+            .filter_map(|path| {
+                let q = path.qualifier();
+                if q.as_ref().and_then(|it| it.qualifier()).is_some() {
+                    return None;
+                }
+                Some((q.and_then(|it| it.as_single_name_ref()), path.segment()?.name_ref()?))
+            })
+            .any(|(q, name_ref)| {
+                let qualifier_matches = match (q, qual) {
+                    (None, None) => true,
+                    (None, Some(_)) => false,
+                    (Some(_), None) => false,
+                    (Some(q), Some(ns)) => q.text() == ns,
+                };
+                qualifier_matches && name_ref.text() == name
+            });
+        if lint_already_annotated {
+            continue;
         }
+        let label = match qual {
+            Some(qual) if !is_qualified => format!("{}::{}", qual, name),
+            _ => name.to_owned(),
+        };
+        let mut item = CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
+        item.kind(CompletionItemKind::Attribute)
+            .documentation(hir::Documentation::new(description.to_owned()));
+        item.add_to(acc)
     }
 }
diff --git a/crates/ide_completion/src/tests/attribute.rs b/crates/ide_completion/src/tests/attribute.rs
index 6a37b53cf58..9f86fc50a22 100644
--- a/crates/ide_completion/src/tests/attribute.rs
+++ b/crates/ide_completion/src/tests/attribute.rs
@@ -693,11 +693,29 @@ mod lint {
     #[test]
     fn lint_clippy_qualified() {
         check_edit(
-            "clippy::as_conversions",
+            "as_conversions",
             r#"#[allow(clippy::$0)] struct Test;"#,
             r#"#[allow(clippy::as_conversions)] struct Test;"#,
         );
     }
+
+    #[test]
+    fn lint_rustdoc_unqualified() {
+        check_edit(
+            "rustdoc::bare_urls",
+            r#"#[allow($0)] struct Test;"#,
+            r#"#[allow(rustdoc::bare_urls)] struct Test;"#,
+        );
+    }
+
+    #[test]
+    fn lint_rustdoc_qualified() {
+        check_edit(
+            "bare_urls",
+            r#"#[allow(rustdoc::$0)] struct Test;"#,
+            r#"#[allow(rustdoc::bare_urls)] struct Test;"#,
+        );
+    }
 }
 
 mod repr {