feat: add a witness hint for struct_missing lint

[sandptel]

(PR for Idea Verification) Though given the current test_crates for struct_missing this should work. Yet I feel that it should also cover the cases when the struct contains fields? And if it does it should have witness output that would be something like let witness = |a: <type>, b: <type>| struct_missing::WillBeRemovedStruct{foo: a, bar: b}; for a struct pub struct WillBeRemovedStruct{foo: <type>, bar:<type>} that is being completely removed. Shouldn't it?

Also, extracting field names seems/is straightforward but field types is something I can't figure out how to extract. Any help?

[obi1kenobi]

(PR for Idea Verification) Though given the current test_crates for struct_missing this should work. Yet I feel that it should also cover the cases when the struct contains fields?

Yes, the witness also needs to work for structs that have fields 👍

And if it does it should have witness output that would be something like let witness = |a: <type>, b: <type>| struct_missing::WillBeRemovedStruct{foo: a, bar: b}; for a struct pub struct WillBeRemovedStruct{foo: <type>, bar:<type>} that is being completely removed. Shouldn't it?

Unfortunately, not all structs can be constructed with a literal. This includes structs that have private fields and #[non_exhaustive] structs.

This is the crux of why writing witnesses is hard! We need to come up with a way to demonstrate breakage that works for:

• structs with no fields
• structs that aren't constructible with a literal
• structs with generic parameters
• unit, tuple, and plain ({ }) structs

Solving problems like this requires thinking hard about all the possible ways one can use the given element of Rust — in this case, a struct.

Also, extracting field names seems/is straightforward but field types is something I can't figure out how to extract. Any help?

Unfortunately, I don't think field types are in our schema yet. We need to design what that schema would look like and then implement it.

That particular case isn't beginner-friendly, but if you'd like to work up to it over the span of a few weeks, I'd be happy to suggest an easier schema evolution task that's more suitable for starting out. No pressure, purely if that's something you're interested in!

[suaviloquence]

I think

fn downstream(value: path::to::Struct) {
    let path::to::Struct {..} = value;
}

works in all of the cases that you mentioned (modulo generic parameters in the function signature for simplicity), but it's not the most user-friendly/understandable for tuple structs/unit structs, so this would be more helpful as a full witness than a witness hint, I think. I agree that this one is maybe not the best first witness hint to add.

[obi1kenobi]

Maybe we just avoid overcomplicating it for the witness hint and just do use path::to::Struct instead?

In principle someone may have added an module by that name (enum/union/trait are impossible because of the ImplOwner check in the query). But:

• modules with uppercase names cause a lint by default, and
• we have fundamental tension between "all the edge cases" and "easy-to-understand hint."

If we're losing understandability in the hint then it feels like we're not serving our users' best interests. Obviously the full (non-hint) witness needs to work perfectly in all cases, but there we can also afford to e.g. pull public fields / check for other types by the same name / check if the struct is a unit, tuple, or plain etc. and then if need be generate a customized witness for each case.

[sandptel]

Unfortunately, I don't think field types are in our schema yet. We need to design what that schema would look like and then implement it.

There is another lint namely constructible_struct_adds_field which I was trying to work on before. Which would require feild types as well (let witness = |a: <type>, b: <type>| constructible_struct_adds_field::ExhaustivePlainStruct{foo: a, bar: b}; for the struct pub struct ExhaustivePlainStruct{foo: <type>, bar:<type>}). This lint seemed as a superset for that.

That particular case isn't beginner-friendly, but if you'd like to work up to it over the span of a few weeks

I feel that working on this would be worth the time :)

I'd be happy to suggest an easier schema evolution task that's more suitable for starting out. No pressure, purely if that's something you're interested in!

If the following can provide a taste of how the above problem (adding field type to schema) could be approached, I would be happy to look into both parallelly. No problems there.

[obi1kenobi]

There is another lint namely constructible_struct_adds_field which I was trying to work on before. Which would require feild types as well (let witness = |a: <type>, b: <type>| constructible_struct_adds_field::ExhaustivePlainStruct{foo: a, bar: b}; for the struct pub struct ExhaustivePlainStruct{foo: <type>, bar:<type>}). This lint seemed as a superset for that.

I'm a bit confused, are the types required there? I think the closure could infer its arguments in most cases (unless they are generic), and the struct literal instantiation doesn't seem like it would require types.

That particular case isn't beginner-friendly, but if you'd like to work up to it over the span of a few weeks

I feel that working on this would be worth the time :)

I'd be happy to suggest an easier schema evolution task that's more suitable for starting out. No pressure, purely if that's something you're interested in!

If the following can provide a taste of how the above problem (adding field type to schema) could be approached, I would be happy to look into both parallelly. No problems there.

Sounds good! As a starting point, let's have you try to add information on T: ?Sized bounds to generic type arguments in the schema.

First, I'd recommend familiarizing yourself with the ?Sized feature (nomicon link). The docs on it are a bit sparse, so you may have to do some independent searching.

In parallel, I'd recommend writing a lint or two, so you get familiarized with the query syntax and its use cases. Follow our contributing guide to get started, and pick out an A-lint E-mentor issue.

The schema that queries follow describes all the types of data we have access to. While reading about ?Sized, consider how we may represent that information on our GenericTypeParameter type. You'll find we've already modeled some related things there, such as synthetic generic types (as in fn example(x: impl Debug) {}). Then let me know what you've decided and how you'd design ?Sized!

[sandptel]

There is another lint namely constructible_struct_adds_field which I was trying to work on before. Which would require feild types as well (let witness = |a: <type>, b: <type>| constructible_struct_adds_field::ExhaustivePlainStruct{foo: a, bar: b}; for the struct pub struct ExhaustivePlainStruct{foo: <type>, bar:<type>}). This lint seemed as a superset for that.

I'm a bit confused, are the types required there? I think the closure could infer its arguments in most cases (unless they are generic), and the struct literal instantiation doesn't seem like it would require types.

Did this just in an attempt to make witness more informative. Well, if it's addition doesn't affect much, does the rest of witness look fine for constructible_struct_adds_field lint ?

Sounds good! As a starting point, let's have you try to add information on T: ?Sized bounds to generic type arguments in the schema.

First, I'd recommend familiarizing yourself with the ?Sized feature (nomicon link). The docs on it are a bit sparse, so you may have to do some independent searching.

In parallel, I'd recommend writing a lint or two, so you get familiarized with the query syntax and its use cases. Follow our contributing guide to get started, and pick out an A-lint E-mentor issue.

The schema that queries follow describes all the types of data we have access to. While reading about ?Sized, consider how we may represent that information on our GenericTypeParameter type. You'll find we've already modeled some related things there, such as synthetic generic types (as in fn example(x: impl Debug) {}). Then let me know what you've decided and how you'd design ?Sized!

Will update you on this 👍

[obi1kenobi]

Did this just in an attempt to make witness more informative. Well, if it's addition doesn't affect much, does the rest of witness look fine for constructible_struct_adds_field lint ?

I'm confused by the current witness code, and by the question 👆

Asking because you mention constructible_struct_adds_field but this PR touches struct_missing. For constructible_struct_adds_field, the current witness is broadly on the right track (but not correct yet), while for struct_missing the current witness is not correct.

[sandptel]

I'm confused by the current witness code, and by the question 👆

Asking because you mention constructible_struct_adds_field but this PR touches struct_missing. For constructible_struct_adds_field, the current witness is broadly on the right track (but not correct yet), while for struct_missing the current witness is not correct.

I am asking about constructible_struct_adds_field and the witness looks like this let witness = |a, b| constructible_struct_adds_field::ExhaustivePlainStruct{foo: a, bar: b};, whats wrong with that?

[obi1kenobi]

What are foo and bar in that example? If they are field names, can we reuse those field names as arguments in the closure and make the witness shorter?

[sandptel]

What are foo and bar in that example? If they are field names, can we reuse those field names as arguments in the closure and make the witness shorter?

Yes foo and bar are field names and can be reused. Something like this ? let witness = |foo, bar| constructible_struct_adds_field::ExhaustivePlainStruct { foo, bar };

[obi1kenobi]

Yeah, looks great!