Skip to content

Commit

Permalink
Thread Span information for stability errors instead
Browse files Browse the repository at this point in the history
  • Loading branch information
alexcrichton committed Jan 31, 2025
1 parent c02ae21 commit 31048e8
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 28 deletions.
72 changes: 46 additions & 26 deletions crates/wit-parser/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::fmt;
use std::mem;
use std::path::{Path, PathBuf};

use anyhow::{anyhow, bail, ensure, Context, Result};
use anyhow::{anyhow, bail, Context, Result};
use id_arena::{Arena, Id};
use indexmap::{IndexMap, IndexSet};
use semver::Version;
Expand Down Expand Up @@ -1778,7 +1778,31 @@ package {name} is defined in two different locations:\n\
}
}

fn include_stability(&self, stability: &Stability, pkg_id: &PackageId) -> Result<bool> {
/// Returns whether the `stability` annotation contained within `pkg_id`
/// should be included or not.
///
/// The `span` provided here is an optional span pointing to the item that
/// is annotated with `stability`.
///
/// Returns `Ok(true)` if the item is included, or `Ok(false)` if the item
/// is not.
///
/// # Errors
///
/// Returns an error if the `pkg_id` isn't annotated with sufficient version
/// information to have a `stability` annotation. For example if `pkg_id`
/// has no version listed then an error will be returned if `stability`
/// mentions a version.
fn include_stability(
&self,
stability: &Stability,
pkg_id: &PackageId,
span: Option<Span>,
) -> Result<bool> {
let err = |msg: String| match span {
Some(span) => Error::new(span, msg).into(),
None => anyhow::Error::msg(msg),
};
Ok(match stability {
Stability::Unknown => true,
// NOTE: deprecations are intentionally omitted -- an existing
Expand All @@ -1797,22 +1821,24 @@ package {name} is defined in two different locations:\n\

// Use of feature gating with version specifiers inside a
// package that is not versioned is not allowed
let package_version = p.name.version.as_ref().with_context(|| {
format!(
let package_version = p.name.version.as_ref().ok_or_else(|| {
err(format!(
"package [{}] contains a feature gate with a version \
specifier, so it must have a version",
p.name
)
))
})?;

// If the version on the feature gate is:
// - released, then we can include it
// - unreleased, then we must check the feature (if present)
ensure!(
since <= package_version,
"feature gate cannot reference unreleased version {since} of package [{}] (current version {package_version})",
p.name
);
if since > package_version {
return Err(err(format!(
"feature gate cannot reference unreleased version \
{since} of package [{}] (current version {package_version})",
p.name
)));
}

true
}
Expand Down Expand Up @@ -2627,7 +2653,7 @@ impl Remap {
.skip(foreign_types)
{
if !resolve
.include_stability(&ty.stability, &pkgid)
.include_stability(&ty.stability, &pkgid, Some(*span))
.with_context(|| {
format!(
"failed to process feature gate for type [{}] in package [{}]",
Expand Down Expand Up @@ -2677,7 +2703,7 @@ impl Remap {
.skip(foreign_interfaces)
{
if !resolve
.include_stability(&iface.stability, &pkgid)
.include_stability(&iface.stability, &pkgid, Some(span.span))
.with_context(|| {
format!(
"failed to process feature gate for interface [{}] in package [{}]",
Expand Down Expand Up @@ -2735,7 +2761,7 @@ impl Remap {
.skip(foreign_worlds)
{
if !resolve
.include_stability(&world.stability, &pkgid)
.include_stability(&world.stability, &pkgid, Some(span.span))
.with_context(|| {
format!(
"failed to process feature gate for world [{}] in package [{}]",
Expand Down Expand Up @@ -3130,8 +3156,9 @@ impl Remap {
assert_eq!(iface.functions.len(), spans.funcs.len());
}
for (i, (func_name, func)) in iface.functions.iter_mut().enumerate() {
let span = spans.map(|s| s.funcs[i]);
if !resolve
.include_stability(&func.stability, iface_pkg_id)
.include_stability(&func.stability, iface_pkg_id, span)
.with_context(|| {
format!(
"failed to process feature gate for function [{func_name}] in package [{}]",
Expand All @@ -3141,15 +3168,14 @@ impl Remap {
{
continue;
}
let span = spans.map(|s| s.funcs[i]);
self.update_function(resolve, func, span)
.with_context(|| format!("failed to update function `{}`", func.name))?;
}

// Filter out all of the existing functions in interface which fail the
// `include_stability()` check, as they shouldn't be available.
for (name, func) in mem::take(&mut iface.functions) {
if resolve.include_stability(&func.stability, iface_pkg_id)? {
if resolve.include_stability(&func.stability, iface_pkg_id, None)? {
iface.functions.insert(name, func);
}
}
Expand Down Expand Up @@ -3225,20 +3251,14 @@ impl Remap {
if let WorldItem::Type(id) = &mut item {
*id = self.map_type(*id, Some(*span))?;
}
self.update_world_key(&mut name, Some(*span))?;
let stability = item.stability(resolve);
if !resolve
.include_stability(stability, pkg_id)
.with_context(|| {
format!(
"failed to process imported world item type [{}] in package [{}]",
resolve.name_world_key(&name),
resolve.packages[*pkg_id].name,
)
})?
.include_stability(stability, pkg_id, Some(*span))
.with_context(|| format!("failed to process world item in `{}`", world.name))?
{
continue;
}
self.update_world_key(&mut name, Some(*span))?;
match &mut item {
WorldItem::Interface { id, .. } => {
*id = self.map_interface(*id, Some(*span))?;
Expand Down Expand Up @@ -3271,7 +3291,7 @@ impl Remap {
.zip(&include_names)
{
if !resolve
.include_stability(&stability, pkg_id)
.include_stability(&stability, pkg_id, Some(*span))
.with_context(|| {
format!(
"failed to process feature gate for included world [{}] in package [{}]",
Expand Down
2 changes: 1 addition & 1 deletion crates/wit-parser/tests/ui/streams-and-futures.wit.json
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,4 @@
"worlds": {}
}
]
}
}
6 changes: 5 additions & 1 deletion tests/cli/bad-stability.wit.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
error: failed to process imported world item type [d:e/[email protected]] in package [a:b]
error: failed to process world item in `c`

Caused by:
0: package [a:b] contains a feature gate with a version specifier, so it must have a version
--> tests/cli/bad-stability.wit:7:14
|
7 | import d:e/[email protected];
| ^----
4 changes: 4 additions & 0 deletions tests/cli/since-on-future-package.wit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@ error: failed to process feature gate for function [b] in package [test:invalid@

Caused by:
0: feature gate cannot reference unreleased version 0.1.1 of package [test:[email protected]] (current version 0.1.0)
--> tests/cli/since-on-future-package.wit:9:3
|
9 | b: func(s: string) -> string;
| ^

0 comments on commit 31048e8

Please sign in to comment.