New Singleton enum for PatternMatchSingleton node

[dhruvmanila]

Summary

This PR adds a new Singleton enum for the PatternMatchSingleton node.

Earlier the node was using the Constant enum but the value for this pattern can only be either None, True or False. With the coming PR to remove the Constant, this node required a new type to fill in.

This also has the benefit of narrowing the type down to only the possible values for the node as evident by the removal of unreachable.

Test Plan

Update the AST snapshots and run cargo test.

[dhruvmanila]

Current dependencies on/for this PR:

• main
  • PR Remove leftover constant tuple reference #8062
    • PR New Singleton enum for PatternMatchSingleton node #8063 👈
      • PR Split Constant to individual literal nodes #8064
        • PR Update string nodes for implicit concatenation #7927
          • PR Explicit as_str (no deref), add no allocation methods #8826
            • PR Remove #[allow(unused_variables)] from visitor methods #8828
              • PR New AST nodes for f-string elements #8835
                • PR Introduce StringLike enum #9016

This stack of pull requests is managed by Graphite.

[dhruvmanila]

I don't think this should be defined 🤔

[github-actions]

PR Check Results

Ecosystem

✅ ecosystem check detected no linter changes.

✅ ecosystem check detected no format changes.

[MichaReiser]

Nit: I find Singleton a confusing name. I assume it comes from Python's AST definition?

Or should this match the literal case? https://peps.python.org/pep-0622/#literal-patterns

[dhruvmanila]

I assume it comes from Python's AST definition?

Yes, and in turn ours as both uses the "Singleton" suffix.

It is for the literal pattern but only a subset of it - None, True, False. The string and number literal are represented using PatternMatchValue while the singletons are using PatternMatchSingleton.

https://play.ruff.rs/2834cfa6-4d0d-4aaf-bbd2-1bd722fdd2ba

[MichaReiser]

What about constant's in other positions? e.g. do we call this function when visiting a ExprConstant too?

[dhruvmanila]

Yes, we do:

ruff/crates/ruff_python_ast/src/node.rs

Lines 2681 to 2687 in db9d767

	fn visit_preorder<'a, V>(&'a self, visitor: &mut V)
	where
	V: PreorderVisitor<'a> + ?Sized,
	{
	let ast::ExprConstant { value, range: _ } = self;
	visitor.visit_constant(value);
	}

I'm not sure if there should be a visit_singleton or not. Do we define visitor methods for the enum values as well? Wouldn't they be accounted for when visiting the enclosing node? Looking at Constant, we do.

[MichaReiser]

I'm not sure. I guess it can be useful when the same non-nodes may have different parent nodes but I would be okay only having visitors for nodes. @charliermarsh should have more context on why visit_constant exists.

[MichaReiser]

I'm not a huge fan of the Singleton terminology because it only makes me thing about the singleton pattern but it is in line with Python's terminology. It also seem to make sense from a technical perspective (I didn't know that) because None, True, and False are indeed all singleton values 🤯

The one alternative I could think of is a KeywordLiteral.

[dhruvmanila]

It also seem to make sense from a technical perspective (I didn't know that) because None, True, and False are indeed all singleton values 🤯

Ah yes, I should've written that in the PR description.

The one alternative I could think of is a KeywordLiteral.

This might be confusing as other keywords are not part of it.