Implement AstNode for Identifier

[dhruvmanila]

Summary

Follow-up to #13147, this PR implements the AstNode for Identifier. This makes it easier to create the NodeKey in red knot because it uses a generic method to construct the key from AnyNodeRef and is important for definitions that are created only on identifiers instead of ExprName.

Test Plan

cargo test and cargo clippy

[AlexWaygood]

Looks reasonable to me! @MichaReiser is probably the better reviewer on this, though

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_salesforce.ipynb:14:1:1: Expected an expression

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_salesforce.ipynb:14:1:1: Expected an expression

[carljm]

Nice! Does this also mean that we'd now be able to just store the identifier directly as an AstNodeRef in the MatchPatternDefinitionKind, instead of storing an index? If so, that would make it more similar to assignment statements, which might be nice when we implement unpacking.

Or it may turn out that indices are nicer to work with than the node references, in which case we'd rather change assignments to use indices instead :) So maybe this is just something to consider when working on actually implementing unpacking assignment.

[dhruvmanila]

Nice! Does this also mean that we'd now be able to just store the identifier directly as an AstNodeRef in the MatchPatternDefinitionKind, instead of storing an index? If so, that would make it more similar to assignment statements, which might be nice when we implement unpacking.

Yeah, although I think index might still be better to identify the type. This provides us the position of the identifier in the pattern, then we'll kinda zip through the subject / pattern to perform structural matching until we reach the index position. Without the index, we will have to compare each identifier in the pattern to determine the one that we're looking to determine the type for. Does this make sense? I can provide an example as well.

[carljm]

Without the index, we will have to compare each identifier in the pattern to determine the one that we're looking to determine the type for

It does make sense; that's what I was thinking of when I said "it may turn out indices are nicer to work with." My main thought is that when we do unpacking, we should probably settle on a preference for either indices or node refs for these unpacking targets, and make them all work as similarly as possible (whichever way we decide is best), because I don't see a strong rationale for them to be different.