Add span_end_line to lints with spans

[suaviloquence]

Adds end_line to all lints that check begin_line

This is a draft because I don't know how best to review the new spans. I'm totally okay doing this lint-by-lint (or couple-lints-by-lint), and I have only checked a few of the generated new test outputs so far.

I quickly made a tool suaviloquence/print_spans_tool to help with reviewing the new spans, but I'd love your thoughts on how best to make this reviewable.

[suaviloquence]

(I don't expect this to get merged all at once. I just want feedback on how to best add these changes and make sure they are correct)

[obi1kenobi]

I actually think this is completely fine to merge as-is, with just all the snapshot files updated. That's the beauty of using a query engine here. All these changes are guaranteed to be correct if all of the following hold:

• all the query edits are reasonable (they are!)
• the underlying data source is correct (if it isn't, we have much bigger issues whether or not this PR merges!)
• the query engine doesn't have bugs (again, if it isn't, much bigger issues regardless of this PR)
• the adapter code between the underlying data and the query is correct

Here's the adapter code in question that produces all the span info:
https://github.com/obi1kenobi/trustfall-rustdoc-adapter/blob/cbd8e99619e036e15ecb280bea146a24050979e6/src/adapter/properties.rs#L141-L152

It sure looks pretty reasonable to me! Starts use the start value and ends use the end value. Nothing seems out of place. So say you've vetted one span out of an abundance of caution, and it looks good. The odds of then finding a bug in any of the rest of the returned data points are astronomically low — low enough that it definitely isn't worth doing. Even if the objective was bug-hunting in cargo-semver-checks, there are more effective ways to bug-hunt.

So I'd say update all the snapshots in bulk, check one of them, and let's merge this :)

[suaviloquence]

I added the rest of the snapshots. My worry is that the span is on the "wrong" item (e.g., whether it should be on the function or attribute or both, which I don't think we have a good standard for right now?), and adding end_line would exacerbate that.

[obi1kenobi]

I added the rest of the snapshots. My worry is that the span is on the "wrong" item (e.g., whether it should be on the function or attribute or both, which I don't think we have a good standard for right now?), and adding end_line would exacerbate that.

It's entirely possible. But the easiest way to find the offending spans is to catch the "wrong"-ness, so I'm not particularly worried about it right now.

Another idea for the future that will be useful as we consider upgrading the user experience (including the GitHub Action!) is the ability to emit multiple spans, so we can highlight multiple things at once. Consider rustc and clippy's diagnostics that are like "first, , then which is a problem." Something similar would be useful for us eventually, where we can say e.g. "this trait previously wasn't sealed , but now it's sealed because of item ." Obviously, this is material for future PRs — just sharing an idea.

[obi1kenobi]

Ah you have to capture this snapshot using Rust 1.85+ because before that, static safety data isn't available and the results end up empty. Sorry!

Rust 1.85 is beta at the moment but will become stable in just 2 weeks, and I didn't want to reject a useful lint over it. It does mean that regenerating this particular snapshot is a bit annoying though — sorry about that!

[obi1kenobi]

cargo-semver-checks/src/query.rs

Line 757 in d69d820

&& rustc_version::version().is_ok_and(|version| version < Version::new(1, 85, 0))

is the relevant code for this.

[suaviloquence]

Yeah, I was using stable-generated rustdocs on nightly 🤦

[obi1kenobi]

In two weeks that will be fine to do once again, and I'm excited for it!