Node .tokens() is not correctly ordered for ContainedSpans

[JohnnyMorganz]

If you have an expression node contained in parentheses, e.g. (foo) in local x = (foo), and then call .tokens() on it, then it is expected that the output is in the order of LeftParens, foo, RightParens.

However, it is in the order LeftParens, RightParens, foo.

This issue also affects Node.surrounding_trivia(), as the surrounding trivia is expected to be (Leading Trivia of LeftParens, Trailing Trivia of RightParens), however since it is not correctly ordered, what is actually returned is (Leading Trivia of LeftParens, Trailing Trivia of foo).

[JohnnyMorganz]

This issue is also present for the following (listed here for reference in case this is relied upon by a downstream crate):

• TableConstructor *
• Field::ExpressionKey
• Index::Brackets *
• FunctionArgs::Parentheses *
• FunctionBody
• TypeInfo::Array *
• TypeInfo::Callback
• TypeInfo::Generic *
• TypeInfo::Table *
• TypeInfo::Typeof *
• TypeInfo::Tuple *
• IndexedTypeInfo::Generic *
• TypeFieldKey::IndexSignature *
• GenericDeclaration *

Items highlighted with (*) indicates that this issue also affects the node's surrounding trivia output

[Kampfkarren]

Some of these can be attributed to using the wrong attributes, i.e. not using #[visit(contains = "...")] or #[node(full_range)], see:

full-moon/full-moon/src/ast/mod.rs

Lines 215 to 221 in 814b7f9

	pub struct TableConstructor<'a> {
	#[cfg_attr(feature = "serde", serde(borrow))]
	#[node(full_range)]
	#[visit(contains = "fields")]
	braces: ContainedSpan<'a>,
	fields: Punctuated<'a, Field<'a>>,
	}

Most should probably just manually implement the trait though, I'm not a fan of the way proc macros have to be implemented in general.

[JohnnyMorganz]

Some of these can be attributed to using the wrong attributes, i.e. not using #[visit(contains = "...")] or #[node(full_range)],

From what I can tell, the #[visit(contains = "...")] attribute only affects the Visit derive macro, not Node, so .tokens() [and hence surrounding_trivia()] is still incorrect.
Similarly, #[node(full_range)] only affects the .range() method in the macro (https://github.com/Kampfkarren/full-moon/blob/main/full-moon-derive/src/node.rs#L117-L162), and is not used for implementing .tokens().

Should either of these two attributes be transformed to fix tokens() aswell? Or should a different attribute be created? Or should it just be implemented manually like you said instead

[Kampfkarren]

If the proc macro is to be kept, then a new attribute is preferred, but the proc macro is really hard to reason about from its code, at least to me. I think I prefer just defining it explicitly,

[JohnnyMorganz]

My only gripe with converting it to explicit definitions would be that there is a significant amount of boilerplate/repeated code. I guess it could be abstracted out into some normal macros? Not sure what's best