From 5a228263b861dd5ed4f0b8d799337745b1d1a39b Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Tue, 1 Dec 2020 14:11:33 +0100
Subject: [PATCH 1/5] Clean up doc attributes check before adding more

---
 compiler/rustc_passes/src/check_attr.rs | 172 +++++++++++++-----------
 1 file changed, 93 insertions(+), 79 deletions(-)

diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index c9e02d56f4b..54621dae1ac 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/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) {
@@ -297,89 +297,103 @@ impl CheckAttrVisitor<'tcx> {
             .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 {
+        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 != ' '))
+        {
+            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_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;
+        }
+    }
+
+    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();
                             return false;
                         }
                     }

From dc10ccfe89d6c8b4ca7993afd66c5dd83f6712ee Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Tue, 1 Dec 2020 14:19:53 +0100
Subject: [PATCH 2/5] Add checks for #[doc(keyword = "...")] and move them into
 rustc_passes

---
 compiler/rustc_passes/src/check_attr.rs | 60 ++++++++++++++++++++++---
 src/librustdoc/clean/mod.rs             |  3 --
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index 54621dae1ac..fc97ca035b9 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@@ -287,24 +287,20 @@ 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, meta: &NestedMetaItem, hir_id: HirId, target: Target) -> bool {
-        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);
+            self.doc_attr_str_error(meta, "alias");
             return false;
         }
         if let Some(c) =
@@ -365,6 +361,49 @@ impl CheckAttrVisitor<'tcx> {
         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,
@@ -384,6 +423,7 @@ impl CheckAttrVisitor<'tcx> {
                 .emit();
             return false;
         }
+        true
     }
 
     fn check_doc_attrs(&self, attr: &Attribute, hir_id: HirId, target: Target) -> bool {
@@ -396,6 +436,12 @@ impl CheckAttrVisitor<'tcx> {
                         {
                             return false;
                         }
+                    } 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;
+                        }
                     }
                 }
             }
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index d294d8f02a8..cd2700be5a7 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -162,9 +162,6 @@ 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);

From 8a35b93c4d5a0071b99ecb7cf6cd91310222c3ff Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Tue, 1 Dec 2020 23:41:12 +0100
Subject: [PATCH 3/5] Add rustc_lexer as dependency to rustc_passes

---
 Cargo.lock                       | 1 +
 compiler/rustc_passes/Cargo.toml | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Cargo.lock b/Cargo.lock
index ce1f705bdff..85be4c392af 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4011,6 +4011,7 @@ dependencies = [
  "rustc_errors",
  "rustc_hir",
  "rustc_index",
+ "rustc_lexer",
  "rustc_middle",
  "rustc_serialize",
  "rustc_session",
diff --git a/compiler/rustc_passes/Cargo.toml b/compiler/rustc_passes/Cargo.toml
index df6667a29d5..c87799f1c2a 100644
--- a/compiler/rustc_passes/Cargo.toml
+++ b/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" }

From 9866136bec0040993f26e8c4b7316fb0a5dc94b5 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Tue, 1 Dec 2020 23:42:07 +0100
Subject: [PATCH 4/5] Add tests for #[doc(keyword = "...")] and update other
 doc attributes tests

---
 .../rustdoc-ui/check-doc-alias-attr.stderr    |  6 +++---
 src/test/ui/check-doc-alias-attr.stderr       |  6 +++---
 src/test/ui/doc-alias-crate-level.rs          |  5 ++++-
 src/test/ui/doc-alias-crate-level.stderr      | 14 +++++++++----
 src/test/ui/doc_keyword.rs                    | 12 +++++++++++
 src/test/ui/doc_keyword.stderr                | 20 +++++++++++++++++++
 6 files changed, 52 insertions(+), 11 deletions(-)
 create mode 100644 src/test/ui/doc_keyword.rs
 create mode 100644 src/test/ui/doc_keyword.stderr

diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.stderr b/src/test/rustdoc-ui/check-doc-alias-attr.stderr
index 3ba0ba61075..1c7fc83bb8d 100644
--- a/src/test/rustdoc-ui/check-doc-alias-attr.stderr
+++ b/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"))]
diff --git a/src/test/ui/check-doc-alias-attr.stderr b/src/test/ui/check-doc-alias-attr.stderr
index a92298d1633..dce325f9d38 100644
--- a/src/test/ui/check-doc-alias-attr.stderr
+++ b/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"))]
diff --git a/src/test/ui/doc-alias-crate-level.rs b/src/test/ui/doc-alias-crate-level.rs
index b1b17f2de8a..9b596ece5b5 100644
--- a/src/test/ui/doc-alias-crate-level.rs
+++ b/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;
diff --git a/src/test/ui/doc-alias-crate-level.stderr b/src/test/ui/doc-alias-crate-level.stderr
index 32b42cf9be7..b6437fad5d0 100644
--- a/src/test/ui/doc-alias-crate-level.stderr
+++ b/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
 
diff --git a/src/test/ui/doc_keyword.rs b/src/test/ui/doc_keyword.rs
new file mode 100644
index 00000000000..4c72e7e9684
--- /dev/null
+++ b/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() {}
diff --git a/src/test/ui/doc_keyword.stderr b/src/test/ui/doc_keyword.stderr
new file mode 100644
index 00000000000..d72a876163e
--- /dev/null
+++ b/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
+

From 15f9453a260ffc035ea3797c6939b1f61acd3e6d Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Wed, 2 Dec 2020 10:44:19 +0100
Subject: [PATCH 5/5] Remove check keyword identifier check from rustdoc

---
 src/librustdoc/clean/mod.rs | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index cd2700be5a7..a3c28be3137 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -169,19 +169,7 @@ impl Clean<ExternalCrate> for CrateNum {
                 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;
                         }
                     }