Invalid span of a struct's generics in presence of a where clause but no parameter

[scampi]

This is an issue that was reported initially to rustfmt in rust-lang/rustfmt#3194.
The problematic code was based on the Generics.span field, which gave a wrong BytePos in case of a struct with a where clause but no parameter.

Problem

Below is an explanation of the problem, and there is a runnable example below to test it out.

Observed

With the code mod m { struct S where A: B; }, the field generics.span of the struct item is equal to Span { lo: BytePos(0), hi: BytePos(0), .. }; this points to the start of the input.

Expected

The span value should point to the position just after the struct ident, i.e., Span { lo: BytePos(16), hi: BytePos(16), .. }.

Versions

$ rustc --version --verbose
rustc 1.32.0-nightly (6f93e93af 2018-11-14)
binary: rustc
commit-hash: 6f93e93af6f823948cc13d2938957757c6486d88
commit-date: 2018-11-14
host: x86_64-unknown-linux-gnu
release: 1.32.0-nightly
LLVM version: 8.0

Example code

• tested with rustc-ap-syntax = "297.0.0"

extern crate syntax;

use syntax::ast::*;
use syntax::ext::quote::rt::FileName;
use syntax::parse;
use syntax::parse::ParseSess;
use syntax::source_map::FilePathMapping;

fn main() {
    // With source = "mod m { struct S<A> where A: B; }":
    // - the generics span is equal to `Span { lo: BytePos(16), hi: BytePos(19), .. }`
    //
    // With source = "mod m { struct S where A: B; }":
    // - the generics span is equal to `Span { lo: BytePos(0), hi: BytePos(0), .. }`
    // - it would be better if the span pointed to the portion just after the ident with the difference between `hi` and `lo` equal to 0, i.e., `Span { lo: BytePos(16), hi: BytePos(16), .. }`

    //let source = "mod m { struct S<A> where A: B; }";
    let source = "mod m { struct S where A: B; }";

    let session = ParseSess::new(FilePathMapping::empty());
    let file_name = FileName::Custom("source".to_string());

    syntax::with_globals(|| {
        let mut parser = parse::new_parser_from_source_str(&session, file_name, source.to_string());
        let result = parser.parse_crate_mod().unwrap();
        let item = &result.module.items[0];
        if let ItemKind::Mod(module) = &item.node {
            let item = &module.items[0];
            if let ItemKind::Struct(_data, generics) = &item.node {
                println!("generics.span = [{:?}]", generics.span);
                println!("generics.params = [{:?}]", generics.params.len());
            }
        }
    });
}

[scampi]

@nrc Looking at the example code once again, maybe having the generics' span being (0, 0) is ok given that generics.params is empty ?
Then, the use of the generics' span in rustfmt was wrong to begin with, and there is nothing to fix for that span as written in the Expected section of the issue.

[zackmdavis]

@scampi Thanks for the detailed report!

maybe having the generics' span being (0, 0) is ok given that generics.params is empty? Then, the use of the generics' span in rustfmt was wrong to begin with,

It may not be a very high priority if rustfmt has already found a better way to do what it needs to do, but I remember at least one situation where choosing a more reasonably-placed span for an "empty"/not-present AST node was useful, so I would argue that this is a legitimate bug.

[nrc]

Looking at the example code once again, maybe having the generics' span being (0, 0) is ok given that generics.params is empty ?

I'd prefer not to do this. IME, the more we hack spans, the more we regret it later on.