docs(ast): improve AST node documentation

[re-taro]

partof #2803

I even wrote a VariableDeclaration.

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[overlookmotel]

Just want to say thanks loads for tackling this. Documentation isn't always the most "glamorous" work, but it is a priority for the project to be approachable to new contributors, and the AST is the core of Oxc, so this is really important work. So, yeah, just wanted say, really appreciate you working on this.

[overlookmotel]

@re-taro #4413 also added more doc comments to AST. Would be keen to get your changes merged too on top of that. Would you have time to rebase on latest master and resolve conflicts, and then we can get it merged?

[re-taro]

@overlookmotel

Yes, we can get on with it now. But it's still a work in progress, so is there a problem with merging it in stages?

[overlookmotel]

Thanks for coming back so swiftly. Making changes in stages is totally fine, as long as an intermediate stage still leaves Oxc in a working state (CI green). In fact, it's good! Shorter-lived branches = less merge conflicts, which takes up less time for PR authors, and we can all keep moving swiftly.

If something is incomplete, just make a note to that effect in the 1st comment on the PR (which becomes the comment on the commit when it's merged).

[re-taro]

ok! I'll get the conflicts out of ASAP and put it up for review!

[codspeed-hq]

CodSpeed Performance Report

Merging #4051 will not alter performance

_{Comparing re-taro:feat/add_node_docs (5fb665e) with main (e02bac3)}

Summary

✅ 32 untouched benchmarks

[overlookmotel]

Oh I'm sorry. I misunderstood what you meant by incomplete. I thought you meant that you intended to add comments to more AST nodes, but hadn't done them all yet.

I'm afraid Boshen's comment above I think does need addressing before we merge.

Just to make sure what Boshen was saying is clear, for e.g. IdentifierName, this would be ideal:

/// Identifier name.
///
/// Fundamental syntactic structure used for naming variables, functions, and properties.
/// It must start with a Unicode letter (including `$` and `_`) and can be followed by
/// Unicode letters, digits, `$`, or `_`.
///
/// See: [13.1 Identifiers](https://tc39.es/ecma262/#sec-identifiers)
///
/// ## Example
/// ```js
/// let foo = bar;
///     ^^^   ^^^
/// ```
#[ast(visit)]
/*...etc...*/
pub struct IdentifierName<'a> {

The first line of the comment should be short, and followed by a line break. That first line is the summary text that appears in docs.

On a few types, this PR removes some useful text which was there previously e.g. ObjectExpression. Could you please restore that text?

[Boshen]

The current style is good and will show what I want on cargo docs. Let's merge some of this and continue iterating on it.

[graphite-app]

Merge activity

• Aug 1, 5:09 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Aug 1, 5:09 AM EDT: Boshen added this pull request to the Graphite merge queue.
• Aug 1, 5:09 AM EDT: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (Stack merges are not currently supported for forked repositories. Please create a branch in the target repository in order to merge).
• Aug 1, 5:10 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Aug 1, 5:10 AM EDT: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (Stack merges are not currently supported for forked repositories. Please create a branch in the target repository in order to merge).

[re-taro]

fmm… can’t merge it?