Skip to content

Conversation

@11happy
Copy link
Contributor

@11happy 11happy commented Nov 27, 2025

Summary

This PR implements syntax error where a default type parameter is followed by a non-default type parameter. #17412 (comment)

Test Plan

I have written inline tests as directed in #17412

@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 27, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Comment on lines 147 to 158
Stmt::ClassDef(ast::StmtClassDef {
type_params: Some(type_params),
..
}) => {
Self::duplicate_type_parameter_name(type_params, ctx);
Self::type_parameter_default_order(type_params, ctx);
}
Stmt::TypeAlias(ast::StmtTypeAlias {
type_params: Some(type_params),
..
}) => {
Self::duplicate_type_parameter_name(type_params, ctx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check needs to be applied to type aliases as well:

In [1]: type Int[T = int, U] = int
  Cell In[1], line 1
    type Int[T = int, U] = int
                      ^
SyntaxError: non-default type parameter 'U' follows default type parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

however some additional detection's are there for example -

type X[*Ts = x := int] = int

in type params:

type_params: TypeParams { range: 6..22, node_index: NodeIndex(None), type_params: [TypeVarTuple(TypeParamTypeVarTuple { node_index: NodeIndex(None), range: 7..14, name: Identifier { id: Name("Ts"), range: 8..10, node_index: NodeIndex(None) }, default: Some(Name(ExprName { node_index: NodeIndex(None), range: 13..14, id: Name("x"), ctx: Load })) }), TypeVar(TypeParamTypeVar { node_index: NodeIndex(None), range: 18..21, name: Identifier { id: Name("int"), range: 18..21, node_index: NodeIndex(None) }, bound: None, default: None })]

int is getting detected as seperate non default typevar, whereas I think should it be parsed as single expression ?

Comment on lines +636 to +637
// test_err type_parameter_default_order
// class C[T = int, U]: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add some cases where the default is not at the start of the list and it has multiple non-default type parameter following a type parameter with default like:

class C[T1, T2 = int, T3, T4]: ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@dhruvmanila dhruvmanila added the parser Related to the parser label Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parser Related to the parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants