Skip to content

Commit c3d429d

Browse files
dhruvmaniladhruvmanila
authored
[red-knot] Move name field on parameter kind (#16830)
## Summary Previously, the `name` field was on `Parameter` which required it to be always optional regardless of the parameter kind because a `typing.Callable` signature does not have name for the parameters. This is the case for positional-only parameters. This wasn't enforced at the type level which meant that downstream usages would have to unwrap on `name` even though it's guaranteed to be present. This commit moves the `name` field from `Parameter` to the `ParameterKind` variants and makes it optional only for `ParameterKind::PositionalOnly` variant while required for all other variants. One change that's now required is that a `Callable` form using a gradual form for parameter types (`...`) would have a default `args` and `kwargs` name used for variadic and keyword-variadic parameter kind respectively. This is also the case for invalid `Callable` type forms. I think this is fine as names are not relevant in this context but happy to make it optional even in variadic variants. ## Test Plan No new tests; make sure existing tests are passing.
1 parent ab3ec4d commit c3d429d

File tree

4 files changed

+262
-157
lines changed

4 files changed

+262
-157
lines changed

‎crates/red_knot_python_semantic/src/types.rs‎

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2351,32 +2351,38 @@ impl<'db> Type<'db> {
23512351
Signature::new(
23522352
Parameters::new([
23532353
Parameter::new(
2354-
Some(Name::new_static("instance")),
23552354
Some(Type::none(db)),
2356-
ParameterKind::PositionalOnly { default_ty: None },
2355+
ParameterKind::PositionalOnly {
2356+
name: Some(Name::new_static("instance")),
2357+
default_ty: None,
2358+
},
23572359
),
23582360
Parameter::new(
2359-
Some(Name::new_static("owner")),
23602361
Some(KnownClass::Type.to_instance(db)),
2361-
ParameterKind::PositionalOnly { default_ty: None },
2362+
ParameterKind::PositionalOnly {
2363+
name: Some(Name::new_static("owner")),
2364+
default_ty: None,
2365+
},
23622366
),
23632367
]),
23642368
None,
23652369
),
23662370
Signature::new(
23672371
Parameters::new([
23682372
Parameter::new(
2369-
Some(Name::new_static("instance")),
23702373
Some(not_none),
2371-
ParameterKind::PositionalOnly { default_ty: None },
2374+
ParameterKind::PositionalOnly {
2375+
name: Some(Name::new_static("instance")),
2376+
default_ty: None,
2377+
},
23722378
),
23732379
Parameter::new(
2374-
Some(Name::new_static("owner")),
23752380
Some(UnionType::from_elements(
23762381
db,
23772382
[KnownClass::Type.to_instance(db), Type::none(db)],
23782383
)),
23792384
ParameterKind::PositionalOnly {
2385+
name: Some(Name::new_static("owner")),
23802386
default_ty: Some(Type::none(db)),
23812387
},
23822388
),
@@ -2402,42 +2408,52 @@ impl<'db> Type<'db> {
24022408
Signature::new(
24032409
Parameters::new([
24042410
Parameter::new(
2405-
Some(Name::new_static("self")),
24062411
Some(KnownClass::FunctionType.to_instance(db)),
2407-
ParameterKind::PositionalOnly { default_ty: None },
2412+
ParameterKind::PositionalOnly {
2413+
name: Some(Name::new_static("self")),
2414+
default_ty: None,
2415+
},
24082416
),
24092417
Parameter::new(
2410-
Some(Name::new_static("instance")),
24112418
Some(Type::none(db)),
2412-
ParameterKind::PositionalOnly { default_ty: None },
2419+
ParameterKind::PositionalOnly {
2420+
name: Some(Name::new_static("instance")),
2421+
default_ty: None,
2422+
},
24132423
),
24142424
Parameter::new(
2415-
Some(Name::new_static("owner")),
24162425
Some(KnownClass::Type.to_instance(db)),
2417-
ParameterKind::PositionalOnly { default_ty: None },
2426+
ParameterKind::PositionalOnly {
2427+
name: Some(Name::new_static("owner")),
2428+
default_ty: None,
2429+
},
24182430
),
24192431
]),
24202432
None,
24212433
),
24222434
Signature::new(
24232435
Parameters::new([
24242436
Parameter::new(
2425-
Some(Name::new_static("self")),
24262437
Some(KnownClass::FunctionType.to_instance(db)),
2427-
ParameterKind::PositionalOnly { default_ty: None },
2438+
ParameterKind::PositionalOnly {
2439+
name: Some(Name::new_static("self")),
2440+
default_ty: None,
2441+
},
24282442
),
24292443
Parameter::new(
2430-
Some(Name::new_static("instance")),
24312444
Some(not_none),
2432-
ParameterKind::PositionalOnly { default_ty: None },
2445+
ParameterKind::PositionalOnly {
2446+
name: Some(Name::new_static("instance")),
2447+
default_ty: None,
2448+
},
24332449
),
24342450
Parameter::new(
2435-
Some(Name::new_static("owner")),
24362451
Some(UnionType::from_elements(
24372452
db,
24382453
[KnownClass::Type.to_instance(db), Type::none(db)],
24392454
)),
24402455
ParameterKind::PositionalOnly {
2456+
name: Some(Name::new_static("owner")),
24412457
default_ty: Some(Type::none(db)),
24422458
},
24432459
),
@@ -2464,9 +2480,9 @@ impl<'db> Type<'db> {
24642480
self,
24652481
Signature::new(
24662482
Parameters::new([Parameter::new(
2467-
Some(Name::new_static("o")),
24682483
Some(Type::any()),
24692484
ParameterKind::PositionalOnly {
2485+
name: Some(Name::new_static("o")),
24702486
default_ty: Some(Type::BooleanLiteral(false)),
24712487
},
24722488
)]),
@@ -2489,9 +2505,9 @@ impl<'db> Type<'db> {
24892505
[
24902506
Signature::new(
24912507
Parameters::new([Parameter::new(
2492-
Some(Name::new_static("o")),
24932508
Some(Type::any()),
24942509
ParameterKind::PositionalOnly {
2510+
name: Some(Name::new_static("o")),
24952511
default_ty: Some(Type::string_literal(db, "")),
24962512
},
24972513
)]),
@@ -2500,19 +2516,25 @@ impl<'db> Type<'db> {
25002516
Signature::new(
25012517
Parameters::new([
25022518
Parameter::new(
2503-
Some(Name::new_static("o")),
25042519
Some(Type::any()), // TODO: ReadableBuffer
2505-
ParameterKind::PositionalOnly { default_ty: None },
2520+
ParameterKind::PositionalOnly {
2521+
name: Some(Name::new_static("o")),
2522+
default_ty: None,
2523+
},
25062524
),
25072525
Parameter::new(
2508-
Some(Name::new_static("encoding")),
25092526
Some(KnownClass::Str.to_instance(db)),
2510-
ParameterKind::PositionalOnly { default_ty: None },
2527+
ParameterKind::PositionalOnly {
2528+
name: Some(Name::new_static("encoding")),
2529+
default_ty: None,
2530+
},
25112531
),
25122532
Parameter::new(
2513-
Some(Name::new_static("errors")),
25142533
Some(KnownClass::Str.to_instance(db)),
2515-
ParameterKind::PositionalOnly { default_ty: None },
2534+
ParameterKind::PositionalOnly {
2535+
name: Some(Name::new_static("errors")),
2536+
default_ty: None,
2537+
},
25162538
),
25172539
]),
25182540
Some(KnownClass::Str.to_instance(db)),
@@ -2535,28 +2557,36 @@ impl<'db> Type<'db> {
25352557
[
25362558
Signature::new(
25372559
Parameters::new([Parameter::new(
2538-
Some(Name::new_static("o")),
25392560
Some(Type::any()),
2540-
ParameterKind::PositionalOnly { default_ty: None },
2561+
ParameterKind::PositionalOnly {
2562+
name: Some(Name::new_static("o")),
2563+
default_ty: None,
2564+
},
25412565
)]),
25422566
Some(KnownClass::Type.to_instance(db)),
25432567
),
25442568
Signature::new(
25452569
Parameters::new([
25462570
Parameter::new(
2547-
Some(Name::new_static("o")),
25482571
Some(Type::any()),
2549-
ParameterKind::PositionalOnly { default_ty: None },
2572+
ParameterKind::PositionalOnly {
2573+
name: Some(Name::new_static("o")),
2574+
default_ty: None,
2575+
},
25502576
),
25512577
Parameter::new(
2552-
Some(Name::new_static("bases")),
25532578
Some(Type::any()),
2554-
ParameterKind::PositionalOnly { default_ty: None },
2579+
ParameterKind::PositionalOnly {
2580+
name: Some(Name::new_static("bases")),
2581+
default_ty: None,
2582+
},
25552583
),
25562584
Parameter::new(
2557-
Some(Name::new_static("dict")),
25582585
Some(Type::any()),
2559-
ParameterKind::PositionalOnly { default_ty: None },
2586+
ParameterKind::PositionalOnly {
2587+
name: Some(Name::new_static("dict")),
2588+
default_ty: None,
2589+
},
25602590
),
25612591
]),
25622592
Some(KnownClass::Type.to_instance(db)),

0 commit comments

Comments
 (0)