wit-parser: Add validation hooks for custom linting

[wilfreddenton]

#2418

There are two strategies I've explored for giving consumers access to source locations:

1. Make spans available on resolved items so consumers can inspect them after resolution. There are two ways I found to do this

   1. Spans on structs (wit-parser-spans-in-structs) - Add span: Option<Span> directly to TypeDef, Function, Interface, World
      • Pros: Direct access pattern; spans travel with the data automatically during merges
      • Cons: Breaking change to public struct layouts
   2. Spans in HashMaps (wit-parser-spans-in-resolve) - Store spans in HashMap<Id, Span> fields on Resolve
      • Pros: No changes to existing struct layouts (Resolve derives Default so not likely to break consumers)
      • Cons: Indirect lookup; requires explicit remapping during merges

2. Hook into internal validation (this PR)

   Keep spans private but let consumers hook into the resolution process via a Validator trait with callbacks.

   • Pros: No breaking changes; opt-in complexity; zero cost for users who don't need spans
   • Cons: Validation must happen during push_* calls, not after-the-fact inspection

This PR implements Strategy 2 because it avoids breaking changes and has zero cost for existing. However, I'm happy to go with one of the other strategies or something totally new that I haven't considered.

Here's an example of how a consumer would use this new features:

use wit_parser::{Error, Resolve, Span, Validator};

struct NoEmptyRecords;

impl Validator for NoEmptyRecords {
    fn validate_type(&mut self, resolve: &Resolve, id: TypeId, span: Span) -> Result<(), Error> {
        let ty = &resolve.types[id];
        if let TypeDefKind::Record(r) = &ty.kind {
            if r.fields.is_empty() {
                return Err(Error::new(span, "empty records are not allowed"));
            }
        }
        Ok(())
    }
}

let mut resolve = Resolve::default();
let mut validator = NoEmptyRecords;
resolve.push_source_with_validator("test.wit", source, &mut validator)?;

[fitzgen]

Redirecting review because I'm not familiar with wit-parser

[alexcrichton]

Apologies for the delay in reviewing this, it's been a busy week!

This is a conceptually large enough change that the design in this PR isn't a slam-dunk to me just yet, and it might still be worth considering some of the other options that you laid out. Could you describe your end goal with this a bit more to help shape the thinking there? For example what are you looking to do with spans after a Resolve is created?

[wilfreddenton]

No problem! Thanks for the response. I totally understand wanting to consider other options that's why I tried to explore and present a few. Not sure if you clicked the links but they lead to diffs with all the changes made for that strategy.

The wit-parser itself performs "validation" already on a wit file. Essentially just confirming that the file is written correctly in the writ "language". However a project may want to assert additional validation rules i.e. exports with a specific name must have this type signature, exports can only accept and return a certain subset of types, records and enums cannot be recursive, etc. This is currently possible to do by iterating through the various properties of the Resolve and testing rules agianst them. However, because the Span type is not public and the instances do not exist on the Resolve or its items, line-based location information cannot be included in invalidated rule error messages. The best thing I've been able to do is "... in the my-type record definition" which is OK but not ideal.

So the first strategy is about making Spans available to consuming code. It can be done by adding Span maps to the Resolve or by adding Span properties to all the Resolve items. They are both technically breaking changes but adding Span properties to all the items breaks more things because they don't derive Default; however, it is the more ergonomic of the two and does not create complications in the merge process.

The second strategy, the PR, is allowing consuming code to hook into the resolution process to add additional validation rules at a point in the lifecycle where the Spans exist. So only the Span type needs to be made public and the instances can continue to be consumed by resolution. This is purely additive and has no breaking changes but could be perceived as a more "invasive" change.

[tschneidereit]

FWIW, I'm working on something for which I'd like to have source span information available as well. Not for validation, but to enable jumping to the line an item is defined in.

More generally, making spans available seems like the strictly more expressive change, which might be worth considering.

[alexcrichton]

Ok thanks for the info @wilfreddenton, that helps me at least!

Given your use case, what @tschneidereit is describing, and my own experience, I might actually lean more towards including spans on items themselves rather than having side tables or just one pass where the spans are available. I think that'll be the most flexible in terms of what consumers can do with the information.

I agree though it's not an easy change, so I think it'd be worth to game it out first before fully committing to ensure it's reasonable. For the breaking change aspect that's ok, this crate has a new major version on all releases intentionally to enable this sort of development. For Default, going this route would I believe basically require that impl to exist. I think the most appropriate default value would be some sort of sentinel for "not present".

@wilfreddenton with those two problems in theory resolved, how would you feel about the span-in-item route?

[wilfreddenton]

Yes happy to go with the span-in-item route. My main concern with it was breaking changes. The strategy is pretty well explored here: main...wilfreddenton:wasm-tools:wit-parser-spans-in-structs

For Default, going this route would I believe basically require that impl to exist.

Are you saying that you want Defaults on all the item structs or just on the Span. Currently I have the new span property as Option<Span>. It seems like you would rather have span: Span and then have some default like Span { start: 0, end: 0, source_map: u32::MAX }? Would you mind explaining your reasoning here?

If we want Defaults on all the items then there's a few property types where the defaults are unclear:

1. TypeOwner could default toTypeOwner::None
2. TypeDefKind could default to TypeDefKind::Unknown (or maybe Type(Type::Bool)?)
3. FunctionKind coulde default to FunctionKind::Freestanding

Would you please provide a quick initial pass of feedback on that diff above? Then I can make the updates and either force push those commits into this PR or close this one and make a new one, whichever you prefer.