Skip to content

Commit

Permalink
Improve previous import/export diagnostic.
Browse files Browse the repository at this point in the history
The error now includes the previous span.
  • Loading branch information
peterhuene committed Nov 23, 2023
1 parent c775bd6 commit 649a326
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/wac-parser/src/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn detect_invalid_input(source: &str) -> Result<(), (Error, SourceSpan)> {
/// Represents a WAC token.
#[derive(Logos, Debug, Clone, Copy, PartialEq, Eq)]
#[logos(error = Error)]
#[logos(skip r"[ \t\n\f]+")]
#[logos(skip r"[ \t\r\n\f]+")]
#[logos(subpattern id = r"%?[a-z][a-z0-9]*(-[a-z][a-z0-9]*)*")]
#[logos(subpattern package_name = r"(?&id)(:(?&id))+")]
#[logos(subpattern semver = r"[0-9a-zA-Z-\.\+]+")]
Expand Down
5 changes: 4 additions & 1 deletion crates/wac-parser/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,11 @@ pub enum Error {
/// The kind of extern name.
kind: ExternKind,
/// The span where the error occurred.
#[label(primary, "duplicate name `{name}`")]
#[label(primary, "duplicate {kind} name `{name}`")]
span: SourceSpan,
/// The span where the error occurred.
#[label("previous {kind} here")]
previous: SourceSpan,
},
/// An invalid extern name was encountered.
#[error("{kind} name `{name}` is not valid")]
Expand Down
40 changes: 30 additions & 10 deletions crates/wac-parser/src/resolution/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ struct Import<'a> {
item: ItemId,
}

struct Export {
/// The span where the export was first introduced.
span: SourceSpan,
/// The exported item.
item: ItemId,
}

struct State<'a> {
resolver: Option<Box<dyn PackageResolver>>,
scopes: Vec<Scope>,
Expand All @@ -47,6 +54,8 @@ struct State<'a> {
/// The map of imported items.
/// This is used to keep track of implicit imports and merge them together.
imports: IndexMap<String, Import<'a>>,
/// The map of exported items.
exports: IndexMap<String, Export>,
}

impl<'a> State<'a> {
Expand All @@ -59,6 +68,7 @@ impl<'a> State<'a> {
package_map: Default::default(),
aliases: Default::default(),
imports: Default::default(),
exports: Default::default(),
}
}

Expand Down Expand Up @@ -143,15 +153,13 @@ impl<'a> State<'a> {
pub struct AstResolver<'a> {
document: &'a ast::Document<'a>,
definitions: Definitions,
exports: IndexMap<String, ItemId>,
}

impl<'a> AstResolver<'a> {
pub fn new(document: &'a ast::Document<'a>) -> Self {
Self {
document,
definitions: Default::default(),
exports: Default::default(),
}
}

Expand Down Expand Up @@ -183,7 +191,11 @@ impl<'a> AstResolver<'a> {
.into_iter()
.map(|(k, v)| (k, v.item))
.collect(),
exports: self.exports,
exports: state
.exports
.into_iter()
.map(|(k, v)| (k, v.item))
.collect(),
})
}

Expand Down Expand Up @@ -268,6 +280,7 @@ impl<'a> AstResolver<'a> {
name: name.to_owned(),
kind: ExternKind::Import,
span,
previous: existing.span,
});
}
_ => unreachable!(),
Expand Down Expand Up @@ -298,13 +311,19 @@ impl<'a> AstResolver<'a> {
) -> ResolutionResult<()> {
log::debug!("resolving type statement");

let (name, item) = match stmt {
ast::TypeStatement::Interface(i) => (i.id.string, self.interface_decl(state, i)?),
ast::TypeStatement::World(w) => (w.id.string, self.world_decl(state, w)?),
ast::TypeStatement::Type(t) => (t.id().string, self.type_decl(state, t)?),
let (id, item) = match stmt {
ast::TypeStatement::Interface(i) => (i.id, self.interface_decl(state, i)?),
ast::TypeStatement::World(w) => (w.id, self.world_decl(state, w)?),
ast::TypeStatement::Type(t) => (*t.id(), self.type_decl(state, t)?),
};

let prev = self.exports.insert(name.to_owned(), item);
let prev = state.exports.insert(
id.string.to_owned(),
Export {
span: id.span,
item,
},
);
assert!(prev.is_none());
Ok(())
}
Expand Down Expand Up @@ -387,15 +406,16 @@ impl<'a> AstResolver<'a> {
}
}

if self.exports.contains_key(name) {
if let Some(existing) = state.exports.get(name) {
return Err(Error::DuplicateExternName {
name: name.to_owned(),
kind: ExternKind::Export,
span,
previous: existing.span,
});
}

let prev = self.exports.insert(name.to_owned(), item);
let prev = state.exports.insert(name.to_owned(), Export { span, item });
assert!(prev.is_none());

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ failed to resolve document

× duplicate export `x`
╭─[tests/resolution/fail/export-duplicate-name.wac:5:15]
3 │ import f: func();
4 │ export f with "x";
· ─┬─
· ╰── previous export here
5 │ export f with "x";
· ─┬─
· ╰── duplicate name `x`
· ╰── duplicate export name `x`
╰────
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ failed to resolve document

× duplicate import `foo`
╭─[tests/resolution/fail/import-duplicate-name.wac:4:15]
2 │
3 │ import x with "foo": func();
· ──┬──
· ╰── previous import here
4 │ import y with "foo": interface {};
· ──┬──
· ╰── duplicate name `foo`
· ╰── duplicate import name `foo`
╰────

0 comments on commit 649a326

Please sign in to comment.