Auto merge of #79613 - GuillaumeGomez:doc-keyword-checks, r=oli-obk

Add checks for #[doc(keyword = "...")] attribute

The goal here is to extend check for `#[doc(keyword = "...")]`.

cc `@jyn514`
r? `@oli-obk`

Cargo.lock

@@ -4011,6 +4011,7 @@ dependencies = [
  "rustc_errors",
  "rustc_hir",
  "rustc_index",
+ "rustc_lexer",
  "rustc_middle",
  "rustc_serialize",
  "rustc_session",

compiler/rustc_passes/Cargo.toml

@@ -18,3 +18,4 @@ rustc_ast = { path = "../rustc_ast" }
 rustc_serialize = { path = "../rustc_serialize" }
 rustc_span = { path = "../rustc_span" }
 rustc_trait_selection = { path = "../rustc_trait_selection" }
+rustc_lexer = { path = "../rustc_lexer" }

compiler/rustc_passes/src/check_attr.rs

@@ -78,7 +78,7 @@ impl CheckAttrVisitor<'tcx> {
             } else if self.tcx.sess.check_name(attr, sym::track_caller) {
                 self.check_track_caller(&attr.span, attrs, span, target)
             } else if self.tcx.sess.check_name(attr, sym::doc) {
-                self.check_doc_alias(attr, hir_id, target)
+                self.check_doc_attrs(attr, hir_id, target)
             } else if self.tcx.sess.check_name(attr, sym::no_link) {
                 self.check_no_link(&attr, span, target)
             } else if self.tcx.sess.check_name(attr, sym::export_name) {
@@ -287,99 +287,159 @@ impl CheckAttrVisitor<'tcx> {
         }
     }
 
-    fn doc_alias_str_error(&self, meta: &NestedMetaItem) {
+    fn doc_attr_str_error(&self, meta: &NestedMetaItem, attr_name: &str) {
         self.tcx
             .sess
             .struct_span_err(
                 meta.span(),
-                "doc alias attribute expects a string: #[doc(alias = \"0\")]",
+                &format!("doc {0} attribute expects a string: #[doc({0} = \"a\")]", attr_name),
             )
             .emit();
     }
 
-    fn check_doc_alias(&self, attr: &Attribute, hir_id: HirId, target: Target) -> bool {
+    fn check_doc_alias(&self, meta: &NestedMetaItem, hir_id: HirId, target: Target) -> bool {
+        let doc_alias = meta.value_str().map(|s| s.to_string()).unwrap_or_else(String::new);
+        if doc_alias.is_empty() {
+            self.doc_attr_str_error(meta, "alias");
+            return false;
+        }
+        if let Some(c) =
+            doc_alias.chars().find(|&c| c == '"' || c == '\'' || (c.is_whitespace() && c != ' '))
+        {
+            self.tcx
+                .sess
+                .struct_span_err(
+                    meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
+                    &format!("{:?} character isn't allowed in `#[doc(alias = \"...\")]`", c,),
+                )
+                .emit();
+            return false;
+        }
+        if doc_alias.starts_with(' ') || doc_alias.ends_with(' ') {
+            self.tcx
+                .sess
+                .struct_span_err(
+                    meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
+                    "`#[doc(alias = \"...\")]` cannot start or end with ' '",
+                )
+                .emit();
+            return false;
+        }
+        if let Some(err) = match target {
+            Target::Impl => Some("implementation block"),
+            Target::ForeignMod => Some("extern block"),
+            Target::AssocTy => {
+                let parent_hir_id = self.tcx.hir().get_parent_item(hir_id);
+                let containing_item = self.tcx.hir().expect_item(parent_hir_id);
+                if Target::from_item(containing_item) == Target::Impl {
+                    Some("type alias in implementation block")
+                } else {
+                    None
+                }
+            }
+            Target::AssocConst => {
+                let parent_hir_id = self.tcx.hir().get_parent_item(hir_id);
+                let containing_item = self.tcx.hir().expect_item(parent_hir_id);
+                // We can't link to trait impl's consts.
+                let err = "associated constant in trait implementation block";
+                match containing_item.kind {
+                    ItemKind::Impl { of_trait: Some(_), .. } => Some(err),
+                    _ => None,
+                }
+            }
+            _ => None,
+        } {
+            self.tcx
+                .sess
+                .struct_span_err(
+                    meta.span(),
+                    &format!("`#[doc(alias = \"...\")]` isn't allowed on {}", err),
+                )
+                .emit();
+            return false;
+        }
+        true
+    }
+
+    fn check_doc_keyword(&self, meta: &NestedMetaItem, hir_id: HirId) -> bool {
+        let doc_keyword = meta.value_str().map(|s| s.to_string()).unwrap_or_else(String::new);
+        if doc_keyword.is_empty() {
+            self.doc_attr_str_error(meta, "keyword");
+            return false;
+        }
+        match self.tcx.hir().expect_item(hir_id).kind {
+            ItemKind::Mod(ref module) => {
+                if !module.item_ids.is_empty() {
+                    self.tcx
+                        .sess
+                        .struct_span_err(
+                            meta.span(),
+                            "`#[doc(keyword = \"...\")]` can only be used on empty modules",
+                        )
+                        .emit();
+                    return false;
+                }
+            }
+            _ => {
+                self.tcx
+                    .sess
+                    .struct_span_err(
+                        meta.span(),
+                        "`#[doc(keyword = \"...\")]` can only be used on modules",
+                    )
+                    .emit();
+                return false;
+            }
+        }
+        if !rustc_lexer::is_ident(&doc_keyword) {
+            self.tcx
+                .sess
+                .struct_span_err(
+                    meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
+                    &format!("`{}` is not a valid identifier", doc_keyword),
+                )
+                .emit();
+            return false;
+        }
+        true
+    }
+
+    fn check_attr_crate_level(
+        &self,
+        meta: &NestedMetaItem,
+        hir_id: HirId,
+        attr_name: &str,
+    ) -> bool {
+        if CRATE_HIR_ID == hir_id {
+            self.tcx
+                .sess
+                .struct_span_err(
+                    meta.span(),
+                    &format!(
+                        "`#![doc({} = \"...\")]` isn't allowed as a crate level attribute",
+                        attr_name,
+                    ),
+                )
+                .emit();
+            return false;
+        }
+        true
+    }
+
+    fn check_doc_attrs(&self, attr: &Attribute, hir_id: HirId, target: Target) -> bool {
         if let Some(mi) = attr.meta() {
             if let Some(list) = mi.meta_item_list() {
                 for meta in list {
                     if meta.has_name(sym::alias) {
-                        if !meta.is_value_str() {
-                            self.doc_alias_str_error(meta);
-                            return false;
-                        }
-                        let doc_alias =
-                            meta.value_str().map(|s| s.to_string()).unwrap_or_else(String::new);
-                        if doc_alias.is_empty() {
-                            self.doc_alias_str_error(meta);
-                            return false;
-                        }
-                        if let Some(c) = doc_alias
-                            .chars()
-                            .find(|&c| c == '"' || c == '\'' || (c.is_whitespace() && c != ' '))
+                        if !self.check_attr_crate_level(meta, hir_id, "alias")
+                            || !self.check_doc_alias(meta, hir_id, target)
                         {
-                            self.tcx
-                                .sess
-                                .struct_span_err(
-                                    meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
-                                    &format!(
-                                        "{:?} character isn't allowed in `#[doc(alias = \"...\")]`",
-                                        c,
-                                    ),
-                                )
-                                .emit();
                             return false;
                         }
-                        if doc_alias.starts_with(' ') || doc_alias.ends_with(' ') {
-                            self.tcx
-                                .sess
-                                .struct_span_err(
-                                    meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
-                                    "`#[doc(alias = \"...\")]` cannot start or end with ' '",
-                                )
-                                .emit();
-                            return false;
-                        }
-                        if let Some(err) = match target {
-                            Target::Impl => Some("implementation block"),
-                            Target::ForeignMod => Some("extern block"),
-                            Target::AssocTy => {
-                                let parent_hir_id = self.tcx.hir().get_parent_item(hir_id);
-                                let containing_item = self.tcx.hir().expect_item(parent_hir_id);
-                                if Target::from_item(containing_item) == Target::Impl {
-                                    Some("type alias in implementation block")
-                                } else {
-                                    None
-                                }
-                            }
-                            Target::AssocConst => {
-                                let parent_hir_id = self.tcx.hir().get_parent_item(hir_id);
-                                let containing_item = self.tcx.hir().expect_item(parent_hir_id);
-                                // We can't link to trait impl's consts.
-                                let err = "associated constant in trait implementation block";
-                                match containing_item.kind {
-                                    ItemKind::Impl { of_trait: Some(_), .. } => Some(err),
-                                    _ => None,
-                                }
-                            }
-                            _ => None,
-                        } {
-                            self.tcx
-                                .sess
-                                .struct_span_err(
-                                    meta.span(),
-                                    &format!("`#[doc(alias = \"...\")]` isn't allowed on {}", err),
-                                )
-                                .emit();
-                            return false;
-                        }
-                        if CRATE_HIR_ID == hir_id {
-                            self.tcx
-                                .sess
-                                .struct_span_err(
-                                    meta.span(),
-                                    "`#![doc(alias = \"...\")]` isn't allowed as a crate \
-                                     level attribute",
-                                )
-                                .emit();
+                    } else if meta.has_name(sym::keyword) {
+                        if !self.check_attr_crate_level(meta, hir_id, "keyword")
+                            || !self.check_doc_keyword(meta, hir_id)
+                        {
                             return false;
                         }
                     }

src/librustdoc/clean/mod.rs

@@ -162,29 +162,14 @@ impl Clean<ExternalCrate> for CrateNum {
                 .collect()
         };
 
-        let get_span =
-            |attr: &ast::NestedMetaItem| Some(attr.meta_item()?.name_value_literal()?.span);
-
         let as_keyword = |res: Res| {
             if let Res::Def(DefKind::Mod, def_id) = res {
                 let attrs = cx.tcx.get_attrs(def_id).clean(cx);
                 let mut keyword = None;
                 for attr in attrs.lists(sym::doc) {
                     if attr.has_name(sym::keyword) {
                         if let Some(v) = attr.value_str() {
-                            let k = v.to_string();
-                            if !rustc_lexer::is_ident(&k) {
-                                let sp = get_span(&attr).unwrap_or_else(|| attr.span());
-                                cx.tcx
-                                    .sess
-                                    .struct_span_err(
-                                        sp,
-                                        &format!("`{}` is not a valid identifier", v),
-                                    )
-                                    .emit();
-                            } else {
-                                keyword = Some(k);
-                            }
+                            keyword = Some(v.to_string());
                             break;
                         }
                     }

src/test/rustdoc-ui/check-doc-alias-attr.stderr

@@ -1,16 +1,16 @@
-error: doc alias attribute expects a string: #[doc(alias = "0")]
+error: doc alias attribute expects a string: #[doc(alias = "a")]
   --> $DIR/check-doc-alias-attr.rs:6:7
    |
 LL | #[doc(alias)]
    |       ^^^^^
 
-error: doc alias attribute expects a string: #[doc(alias = "0")]
+error: doc alias attribute expects a string: #[doc(alias = "a")]
   --> $DIR/check-doc-alias-attr.rs:7:7
    |
 LL | #[doc(alias = 0)]
    |       ^^^^^^^^^
 
-error: doc alias attribute expects a string: #[doc(alias = "0")]
+error: doc alias attribute expects a string: #[doc(alias = "a")]
   --> $DIR/check-doc-alias-attr.rs:8:7
    |
 LL | #[doc(alias("bar"))]

src/test/ui/check-doc-alias-attr.stderr

@@ -1,16 +1,16 @@
-error: doc alias attribute expects a string: #[doc(alias = "0")]
+error: doc alias attribute expects a string: #[doc(alias = "a")]
   --> $DIR/check-doc-alias-attr.rs:7:7
    |
 LL | #[doc(alias)]
    |       ^^^^^
 
-error: doc alias attribute expects a string: #[doc(alias = "0")]
+error: doc alias attribute expects a string: #[doc(alias = "a")]
   --> $DIR/check-doc-alias-attr.rs:8:7
    |
 LL | #[doc(alias = 0)]
    |       ^^^^^^^^^
 
-error: doc alias attribute expects a string: #[doc(alias = "0")]
+error: doc alias attribute expects a string: #[doc(alias = "a")]
   --> $DIR/check-doc-alias-attr.rs:9:7
    |
 LL | #[doc(alias("bar"))]

src/test/ui/doc-alias-crate-level.rs

@@ -4,4 +4,7 @@
 
 #![crate_type = "lib"]
 
-#![doc(alias = "shouldn't work!")] //~ ERROR
+#![doc(alias = "not working!")] //~ ERROR
+
+#[doc(alias = "shouldn't work!")] //~ ERROR
+pub struct Foo;

src/test/ui/doc-alias-crate-level.stderr

@@ -1,8 +1,14 @@
 error: '\'' character isn't allowed in `#[doc(alias = "...")]`
-  --> $DIR/doc-alias-crate-level.rs:7:16
+  --> $DIR/doc-alias-crate-level.rs:9:15
    |
-LL | #![doc(alias = "shouldn't work!")]
-   |                ^^^^^^^^^^^^^^^^^
+LL | #[doc(alias = "shouldn't work!")]
+   |               ^^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: `#![doc(alias = "...")]` isn't allowed as a crate level attribute
+  --> $DIR/doc-alias-crate-level.rs:7:8
+   |
+LL | #![doc(alias = "not working!")]
+   |        ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
 

src/test/ui/doc_keyword.rs

@@ -0,0 +1,12 @@
+#![crate_type = "lib"]
+#![feature(doc_keyword)]
+
+#![doc(keyword = "hello")] //~ ERROR
+
+#[doc(keyword = "hell")] //~ ERROR
+mod foo {
+    fn hell() {}
+}
+
+#[doc(keyword = "hall")] //~ ERROR
+fn foo() {}

src/test/ui/doc_keyword.stderr

@@ -0,0 +1,20 @@
+error: `#[doc(keyword = "...")]` can only be used on empty modules
+  --> $DIR/doc_keyword.rs:6:7
+   |
+LL | #[doc(keyword = "hell")]
+   |       ^^^^^^^^^^^^^^^^
+
+error: `#[doc(keyword = "...")]` can only be used on modules
+  --> $DIR/doc_keyword.rs:11:7
+   |
+LL | #[doc(keyword = "hall")]
+   |       ^^^^^^^^^^^^^^^^
+
+error: `#![doc(keyword = "...")]` isn't allowed as a crate level attribute
+  --> $DIR/doc_keyword.rs:4:8
+   |
+LL | #![doc(keyword = "hello")]
+   |        ^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+