Skip to content

Conversation

@wilfreddenton
Copy link
Contributor

#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)?;

@wilfreddenton wilfreddenton requested a review from a team as a code owner January 16, 2026 16:27
@wilfreddenton wilfreddenton requested review from fitzgen and removed request for a team January 16, 2026 16:27
@wilfreddenton wilfreddenton changed the title Wit parser validation wit-parser: Add validation hooks for custom linting Jan 16, 2026
@wilfreddenton wilfreddenton marked this pull request as draft January 16, 2026 16:31
@fitzgen fitzgen requested review from pchickey and removed request for fitzgen January 16, 2026 19:29
@fitzgen
Copy link
Member

fitzgen commented Jan 16, 2026

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

@wilfreddenton wilfreddenton changed the title wit-parser: Add validation hooks for custom linting wit-parser: Add validation hooks for custom linting Jan 20, 2026
@wilfreddenton wilfreddenton marked this pull request as ready for review January 23, 2026 14:08
@alexcrichton alexcrichton requested review from alexcrichton and removed request for pchickey January 23, 2026 17:32
@alexcrichton
Copy link
Member

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
Copy link
Contributor Author

wilfreddenton commented Jan 23, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants