Merge pull request #4071 from rust-lang/listing-attr-handling

infra: improve error handling for `Listing`

packages/mdbook-trpl-listing/src/lib.rs

@@ -195,7 +195,7 @@ impl<'e> ListingState<'e> {
     ) -> Result<(), String> {
         // We do not *keep* the version constructed here, just temporarily
         // construct it so the HTML parser, which expects properly closed tags
-        // to parse it as a *tag* rather than a *weird text node*, which accept
+        // to parse it as a *tag* rather than a *weird text node*, will accept
         // it and provide a useful view of it.
         let to_parse = tag.to_owned().to_string() + "</Listing>";
         let listing = Dom::parse(&to_parse)
@@ -212,21 +212,22 @@ impl<'e> ListingState<'e> {
             .try_fold(ListingBuilder::new(), |builder, (key, maybe_value)| {
                 match (key.as_str(), maybe_value) {
                     ("number", Some(value)) => Ok(builder.with_number(value)),
-                    ("number", None) => {
-                        Err(String::from("number attribute without value"))
-                    }
+
                     ("caption", Some(value)) => Ok(builder.with_caption(value)),
-                    ("caption", None) => {
-                        Err(String::from("caption attribute without value"))
-                    }
+
                     ("file-name", Some(value)) => {
                         Ok(builder.with_file_name(value))
                     }
-                    ("file-name", None) => {
-                        Err(String::from("file-name attribute without value"))
+
+                    (attr @ "file-name", None)
+                    | (attr @ "caption", None)
+                    | (attr @ "number", None) => {
+                        Err(format!("Missing value for attribute: '{attr}'"))
                     }
 
-                    _ => Ok(builder), // TODO: error on extra attrs?
+                    (attr, _) => {
+                        Err(format!("Unsupported attribute name: '{attr}'"))
+                    }
                 }
             })?
             .build();

packages/mdbook-trpl-listing/src/tests/mod.rs

@@ -188,5 +188,108 @@ fn main() {}
     );
 }
 
+#[test]
+fn with_unsupported_attr_name() {
+    let result = rewrite_listing(
+        "<Listing invalid-attr>
+
+```rust
+fn main() {}
+```
+
+</Listing>",
+        Mode::Default,
+    );
+
+    assert_eq!(
+        result,
+        Err(String::from("Unsupported attribute name: 'invalid-attr'"))
+    )
+}
+
+#[test]
+fn with_unsupported_attr_name_with_arg() {
+    let result = rewrite_listing(
+        r#"<Listing invalid-attr="123">
+
+```rust
+fn main() {}
+```
+
+</Listing>"#,
+        Mode::Default,
+    );
+
+    assert_eq!(
+        result,
+        Err(String::from("Unsupported attribute name: 'invalid-attr'"))
+    )
+}
+
+#[cfg(test)]
+mod missing_value {
+    use super::*;
+
+    #[test]
+    fn for_number() {
+        let result = rewrite_listing(
+            r#"<Listing number>
+
+```rust
+fn main() {}
+```
+
+</Listing>"#,
+            Mode::Default,
+        );
+
+        assert_eq!(
+            result,
+            Err(String::from("Missing value for attribute: 'number'"))
+        )
+    }
+
+    #[test]
+    fn for_caption() {
+        let result = rewrite_listing(
+            r#"<Listing caption>
+
+```rust
+fn main() {}
+```
+
+</Listing>"#,
+            Mode::Default,
+        );
+
+        assert_eq!(
+            result,
+            Err(String::from("Missing value for attribute: 'caption'"))
+        )
+    }
+
+    #[test]
+    fn for_file_name() {
+        let result = rewrite_listing(
+            r#"<Listing file-name>
+
+```rust
+fn main() {}
+```
+
+</Listing>"#,
+            Mode::Default,
+        );
+
+        assert_eq!(
+            result,
+            Err(String::from("Missing value for attribute: 'file-name'"))
+        )
+    }
+}
+
+#[test]
+fn missing_value() {}
+
 #[cfg(test)]
 mod config;

src/ch18-03-oo-design-patterns.md

@@ -114,7 +114,7 @@ the `content` field’s data is read. The `add_text` method is pretty
 straightforward, so let’s add the implementation in Listing 18-13 to the `impl
 Post` block:
 
-<Listing number="18-13" fie-name="src/lib.rs" caption="Implementing the `add_text` method to add text to a post’s `content`">
+<Listing number="18-13" file-name="src/lib.rs" caption="Implementing the `add_text` method to add text to a post’s `content`">
 
 ```rust,noplayground
 {{#rustdoc_include ../listings/ch18-oop/listing-18-13/src/lib.rs:here}}