From c3c2de964d8e20f37b696aa2bd3c1b6ae3099a58 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Wed, 28 Nov 2018 12:11:45 -0500 Subject: [PATCH 1/2] reject invalid external doc attributes Also, provide a suggestion for the correct syntax. --- src/libsyntax/ext/expand.rs | 32 ++++++++++++++++++-- src/test/ui/extern/external-doc-error.rs | 20 ++++++++++++ src/test/ui/extern/external-doc-error.stderr | 26 +++++++++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 06e7b4657849f..adf080a27a3fc 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use ast::{self, Block, Ident, NodeId, PatKind, Path}; +use ast::{self, Block, Ident, LitKind, NodeId, PatKind, Path}; use ast::{MacStmtStyle, StmtKind, ItemKind}; use attr::{self, HasAttrs}; use source_map::{ExpnInfo, MacroBang, MacroAttribute, dummy_spanned, respan}; @@ -1549,7 +1549,35 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { } } } else { - items.push(noop_fold_meta_list_item(it, self)); + let mut err = self.cx.struct_span_err( + it.span, + &format!("expected path to external documentation"), + ); + + // Check if the user erroneously used `doc(include(...))` syntax. + let literal = it.meta_item_list().and_then(|list| { + if list.len() == 1 { + list[0].literal().map(|literal| &literal.node) + } else { + None + } + }); + + let (path, applicability) = match &literal { + Some(LitKind::Str(path, ..)) => { + (path.to_string(), Applicability::MachineApplicable) + } + _ => (String::from(""), Applicability::HasPlaceholders), + }; + + err.span_suggestion_with_applicability( + it.span, + "provide a file path with `=`", + format!("include = \"{}\"", path), + applicability, + ); + + err.emit(); } } diff --git a/src/test/ui/extern/external-doc-error.rs b/src/test/ui/extern/external-doc-error.rs index 5c6f6e49b3d77..f21583ad7b219 100644 --- a/src/test/ui/extern/external-doc-error.rs +++ b/src/test/ui/extern/external-doc-error.rs @@ -5,4 +5,24 @@ #[doc(include = "not-a-file.md")] //~ ERROR: couldn't read pub struct SomeStruct; +#[doc(include)] +pub struct MissingPath; //~^ ERROR expected path + //~| HELP provide a file path with `=` + //~| SUGGESTION include = "" + +#[doc(include("../README.md"))] +pub struct InvalidPathSyntax; //~^ ERROR expected path + //~| HELP provide a file path with `=` + //~| SUGGESTION include = "../README.md" + +#[doc(include = 123)] +pub struct InvalidPathType; //~^ ERROR expected path + //~| HELP provide a file path with `=` + //~| SUGGESTION include = "" + +#[doc(include(123))] +pub struct InvalidPathSyntaxAndType; //~^ ERROR expected path + //~| HELP provide a file path with `=` + //~| SUGGESTION include = "" + fn main() {} diff --git a/src/test/ui/extern/external-doc-error.stderr b/src/test/ui/extern/external-doc-error.stderr index 5cc7551ee03aa..846f8ddfcb67b 100644 --- a/src/test/ui/extern/external-doc-error.stderr +++ b/src/test/ui/extern/external-doc-error.stderr @@ -4,5 +4,29 @@ error: couldn't read $DIR/not-a-file.md: $FILE_NOT_FOUND_MSG (os error 2) LL | #[doc(include = "not-a-file.md")] //~ ERROR: couldn't read | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: expected path to external documentation + --> $DIR/external-doc-error.rs:8:7 + | +LL | #[doc(include)] + | ^^^^^^^ help: provide a file path with `=`: `include = ""` + +error: expected path to external documentation + --> $DIR/external-doc-error.rs:13:7 + | +LL | #[doc(include("../README.md"))] + | ^^^^^^^^^^^^^^^^^^^^^^^ help: provide a file path with `=`: `include = "../README.md"` + +error: expected path to external documentation + --> $DIR/external-doc-error.rs:18:7 + | +LL | #[doc(include = 123)] + | ^^^^^^^^^^^^^ help: provide a file path with `=`: `include = ""` + +error: expected path to external documentation + --> $DIR/external-doc-error.rs:23:7 + | +LL | #[doc(include(123))] + | ^^^^^^^^^^^^ help: provide a file path with `=`: `include = ""` + +error: aborting due to 5 previous errors From 7f7045f84795a7e1c1fb0a0160bf3319368c09ba Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Wed, 28 Nov 2018 14:54:08 -0500 Subject: [PATCH 2/2] improve diagnostics for invalid external docs --- src/libsyntax/ext/expand.rs | 36 +++++++++++++------ src/test/ui/extern/auxiliary/invalid-utf8.txt | 1 + src/test/ui/extern/external-doc-error.rs | 8 +++-- src/test/ui/extern/external-doc-error.stderr | 24 ++++++++----- 4 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/extern/auxiliary/invalid-utf8.txt diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index adf080a27a3fc..44d5ae6b40d2c 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -1535,17 +1535,33 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { let item = attr::mk_list_item(DUMMY_SP, include_ident, include_info); items.push(dummy_spanned(ast::NestedMetaItemKind::MetaItem(item))); } - Err(ref e) if e.kind() == ErrorKind::InvalidData => { - self.cx.span_err( - at.span, - &format!("{} wasn't a utf-8 file", filename.display()), - ); - } Err(e) => { - self.cx.span_err( - at.span, - &format!("couldn't read {}: {}", filename.display(), e), - ); + let lit = it + .meta_item() + .and_then(|item| item.name_value_literal()) + .unwrap(); + + if e.kind() == ErrorKind::InvalidData { + self.cx + .struct_span_err( + lit.span, + &format!("{} wasn't a utf-8 file", filename.display()), + ) + .span_label(lit.span, "contains invalid utf-8") + .emit(); + } else { + let mut err = self.cx.struct_span_err( + lit.span, + &format!("couldn't read {}: {}", filename.display(), e), + ); + err.span_label(lit.span, "couldn't read file"); + + if e.kind() == ErrorKind::NotFound { + err.help("external doc paths are relative to the crate root"); + } + + err.emit(); + } } } } else { diff --git a/src/test/ui/extern/auxiliary/invalid-utf8.txt b/src/test/ui/extern/auxiliary/invalid-utf8.txt new file mode 100644 index 0000000000000..dc1115b82db40 --- /dev/null +++ b/src/test/ui/extern/auxiliary/invalid-utf8.txt @@ -0,0 +1 @@ +Ã( \ No newline at end of file diff --git a/src/test/ui/extern/external-doc-error.rs b/src/test/ui/extern/external-doc-error.rs index f21583ad7b219..e17dda65568e9 100644 --- a/src/test/ui/extern/external-doc-error.rs +++ b/src/test/ui/extern/external-doc-error.rs @@ -2,8 +2,12 @@ #![feature(external_doc)] -#[doc(include = "not-a-file.md")] //~ ERROR: couldn't read -pub struct SomeStruct; +#[doc(include = "not-a-file.md")] +pub struct SomeStruct; //~^ ERROR couldn't read + //~| HELP external doc paths are relative to the crate root + +#[doc(include = "auxiliary/invalid-utf8.txt")] +pub struct InvalidUtf8; //~^ ERROR wasn't a utf-8 file #[doc(include)] pub struct MissingPath; //~^ ERROR expected path diff --git a/src/test/ui/extern/external-doc-error.stderr b/src/test/ui/extern/external-doc-error.stderr index 846f8ddfcb67b..a3be3277de545 100644 --- a/src/test/ui/extern/external-doc-error.stderr +++ b/src/test/ui/extern/external-doc-error.stderr @@ -1,32 +1,40 @@ error: couldn't read $DIR/not-a-file.md: $FILE_NOT_FOUND_MSG (os error 2) - --> $DIR/external-doc-error.rs:5:1 + --> $DIR/external-doc-error.rs:5:17 | -LL | #[doc(include = "not-a-file.md")] //~ ERROR: couldn't read - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #[doc(include = "not-a-file.md")] + | ^^^^^^^^^^^^^^^ couldn't read file + | + = help: external doc paths are relative to the crate root + +error: $DIR/auxiliary/invalid-utf8.txt wasn't a utf-8 file + --> $DIR/external-doc-error.rs:9:17 + | +LL | #[doc(include = "auxiliary/invalid-utf8.txt")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ contains invalid utf-8 error: expected path to external documentation - --> $DIR/external-doc-error.rs:8:7 + --> $DIR/external-doc-error.rs:12:7 | LL | #[doc(include)] | ^^^^^^^ help: provide a file path with `=`: `include = ""` error: expected path to external documentation - --> $DIR/external-doc-error.rs:13:7 + --> $DIR/external-doc-error.rs:17:7 | LL | #[doc(include("../README.md"))] | ^^^^^^^^^^^^^^^^^^^^^^^ help: provide a file path with `=`: `include = "../README.md"` error: expected path to external documentation - --> $DIR/external-doc-error.rs:18:7 + --> $DIR/external-doc-error.rs:22:7 | LL | #[doc(include = 123)] | ^^^^^^^^^^^^^ help: provide a file path with `=`: `include = ""` error: expected path to external documentation - --> $DIR/external-doc-error.rs:23:7 + --> $DIR/external-doc-error.rs:27:7 | LL | #[doc(include(123))] | ^^^^^^^^^^^^ help: provide a file path with `=`: `include = ""` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors