pallet macro improve span

[gui1117]

this tries to change how code related to storage is expand, in the past it used the attribute span, now it tries to use the type span instead.

Actually I'm not sure what make more sense, here some thoughts

• seeing the error on the type alias itself seems more beautiful, but the error doesn't make that much more sense to me, because the type alias doesn't require the bound, only the usage of the type requires the bound.
• both span link to the correct item, so all in all I don't mind.
• pointing to the attribute means that it is the generated code for this attribute which requires the bond, which is not so meaningless IMHO.

[KiChjang]

I think the span that makes the most sense to me is pointing directly at the type where the error message is complaining about:

Suggested change

	23 | type Foo<T> = StorageValue<_, Bar>;
	| ^^^^^^^^^^^^ the trait `MaxEncodedLen` is not implemented for `Bar`
	23 | type Foo<T> = StorageValue<_, Bar>;
	| ^^^ the trait `MaxEncodedLen` is not implemented for `Bar`

but it feels like you opted for the storage type instead because it's impossible to determine which inner type the compiler is complaining about? If so, then perhaps pointing to the angle brackets would make more sense?

Suggested change

	23 | type Foo<T> = StorageValue<_, Bar>;
	| ^^^^^^^^^^^^ the trait `MaxEncodedLen` is not implemented for `Bar`
	23 | type Foo<T> = StorageValue<_, Bar>;
	| ^^^^^^^^ the trait `MaxEncodedLen` is not implemented for `Bar`

[gui1117]

I tried to put the span on the whole type, I don't know yet why it only covers the ident StorageValue.
I'm not sure we can point to a particular generic because other generic can potentially be wrong.

But I'm thinking the best error message is for a failing #[pallet::getter(fn ...)]

it looks like this:

error[E0277]: the trait bound `Foo: WrapperTypeDecode` is not satisfied
   --> frame/support/test/tests/pallet.rs:231:36
    |
231 |     pub type Bar<T> = StorageValue<_, Foo>;
    |                                       ^^^ the trait `WrapperTypeDecode` is not implemented for `Foo`
    |
    = note: required because of the requirements on the impl of `_::_parity_scale_codec::Decode` for `Foo`
    = note: required because of the requirements on the impl of `FullCodec` for `Foo`
    = note: required by `hidden_include::StorageValue::get`

If we can create dead code which triggers those error when the generic doesn't implement the wanted trait (so Value should implement FullCodec, Key also, Hasher etc...) then I think that would be the ideal.

[gui1117]

after thought, we can just write a code very similar to the code generated for the getter to get the wanted error message. Shouldn't be difficult.

EDIT: and we should probably do same for the bounds required by storage_info stuff also.

[gui1117]

I just realize that getter has better span because in the code we write: <storagealias<T> as StorageValue<Value>>::get so the error points to Value and it got the correct spans.

[gui1117]

but it feels like you opted for the storage type instead because it's impossible to determine which inner type the compiler is complaining about? If so, then perhaps pointing to the angle brackets would make more sense?

Yes exactly, that said on my nightly it uses the span of the whole type with its generics. but stable only points to the ident

[stale]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.