Inherited visibility has a misleading location

[jrvidal]

Not sure if this is a bug or a feature, but it surprised me while trying to calculate the "real span" of some items (as in "the span of this item ignoring its attributes").

Test case

use syn::spanned::Spanned;

fn main() {
    let item: syn::ItemExternCrate = syn::parse_str(
      "#[foo]
      extern crate foo;
    ").unwrap();
    
    println!("attribute = {:?}", (item.attrs[0].span().start(), item.attrs[0].span().end()));
    println!("vis       = {:?}", (item.vis.span().start(), item.vis.span().end()));
    println!("`extern`  = {:?}", (item.extern_token.span().start(), item.extern_token.span().end()));

    println!("");

    let item = syn::parse_str::<syn::ItemExternCrate>(
      "#[foo]
      pub extern crate foo;
    ").unwrap();
    
    println!("attribute = {:?}", (item.attrs[0].span().start(), item.attrs[0].span().end()));
    println!("vis       = {:?}", (item.vis.span().start(), item.vis.span().end()));
    println!("`extern`  = {:?}", (item.extern_token.span().start(), item.extern_token.span().end()));
}

Output:

attribute = (LineColumn { line: 1, column: 0 }, LineColumn { line: 1, column: 6 })
vis       = (LineColumn { line: 1, column: 0 }, LineColumn { line: 1, column: 0 })
`extern`  = (LineColumn { line: 2, column: 6 }, LineColumn { line: 2, column: 12 })

attribute = (LineColumn { line: 1, column: 0 }, LineColumn { line: 1, column: 6 })
vis       = (LineColumn { line: 2, column: 6 }, LineColumn { line: 2, column: 9 })
`extern`  = (LineColumn { line: 2, column: 10 }, LineColumn { line: 2, column: 16 })

I would expect... maybe an empty span just before the extern_token?

[dtolnay]

Rust associates spans only with tokens; every token has a span. If there isn't a token somewhere, there isn't a span that can represent the place where the token isn't.

[dtolnay]

I'll close this because I don't expect to diverge from the proc macro model on this.

For calculating the span without attributes, you should be able to use: ItemExternCrate { attrs: Vec::new(), ..item.clone() }.span()

[jrvidal]

For calculating the span without attributes, you should be able to use: ItemExternCrate { attrs: Vec::new(), ..item.clone() }.span()

Thanks 👍 I will try some workaround.

If there isn't a token somewhere, there isn't a span that can represent the place where the token isn't.

I don't expect to diverge from the proc macro model

Hm, OK, but by introducing an Inherited variant, we're already choosing to expose a fake, empty Span, aren't we? Would we diverge more from the proc macro model by choosing to move that fake span somewhere else?

[dtolnay]

Inherited isn't a token, it is an empty enum variant representing absence of any tokens. There isn't a span in it. https://docs.rs/syn/1.0.17/syn/enum.Visibility.html

[jrvidal]

Sure, but even if there is no span for Inherited, you can still call to <Visibility as Spanned>::span().

Anyway, after looking a bit how it is implemented under the hood, I realize now what you're saying. There's no easy way to have a custom, fake Span if they ultimately come from the generated tokens of ToTokens. And I see now that this gotcha it's perfectly documented in Spanned::span() 🤦‍♀️ Sorry about the noise.

[dtolnay]

More generally, whether or not ToTokens is involved in the current implementation, it's that there isn't any token to identify the place where the token isn't.

pub enum Visibility {
    ...
    Inherited,
}

impl Visibility {
    pub fn span(&self) -> Span {
        match self {
            ...
            Visibility::Inherited => ???
        }
    }
}

[jrvidal]

Out of curiosity, what's the reason to have an empty variant instead of Option<Visibility>?

[dtolnay]

Because it's always what you want. I don't know of any syntax that would want to parse a visibility but not allow the empty visibility.

[ratijas]

By the way, compiler explicitly claims to parse :vis macro meta-variable as "Possibly empty". Although, that has some restrictions at the moment (see rust-lang/rust#71422).