Skip to content

Commit

Permalink
Fixed a spec conformance issue relating to callable assignability whe…
Browse files Browse the repository at this point in the history
…n the dest type uses an unpacked TypedDict and the source does not accept `**kwargs`. The typing spec indicates that this should fail because TypedDicts are not closed. This addresses #9807. (#9824)
  • Loading branch information
erictraut authored Feb 6, 2025
1 parent 2782fe3 commit d5ca5e9
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 11 deletions.
24 changes: 20 additions & 4 deletions packages/pyright-internal/src/analyzer/parameterUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
import { ParamCategory } from '../parser/parseNodes';
import { isDunderName } from './symbolNameUtils';
import {
AnyType,
ClassType,
FunctionParam,
FunctionParamFlags,
FunctionType,
isAnyOrUnknown,
isClassInstance,
isNever,
isParamSpec,
isPositionOnlySeparator,
isTypeSame,
Expand Down Expand Up @@ -72,10 +74,19 @@ export interface ParamListDetails {
paramSpec?: TypeVarType;
}

export interface ParamListDetailsOptions {
// Should we disallow extra keyword arguments to be passed
// if the function uses a **kwargs annotated with a (non-closed)
// unpacked TypedDict? By default, this is allowed, but PEP 692
// suggests that this should be disallowed for calls whereas it
// explicitly says this is allowed for callable assignment rules.
disallowExtraKwargsForTd?: boolean;
}

// Examines the input parameters within a function signature and creates a
// "virtual list" of parameters, stripping out any markers and expanding
// any *args with unpacked tuples.
export function getParamListDetails(type: FunctionType): ParamListDetails {
export function getParamListDetails(type: FunctionType, options?: ParamListDetailsOptions): ParamListDetails {
const result: ParamListDetails = {
firstPositionOrKeywordIndex: 0,
positionParamCount: 0,
Expand Down Expand Up @@ -265,16 +276,21 @@ export function getParamListDetails(type: FunctionType): ParamListDetails {
);
});

if (paramType.shared.typedDictEntries.extraItems) {
const extraItemsType = paramType.shared.typedDictEntries.extraItems?.valueType ?? AnyType.create();

// Unless the TypedDict is completely closed (i.e. is not allowed to
// have any extra items), add a virtual **kwargs parameter to represent
// any additional items.
if (!isNever(extraItemsType) && !options?.disallowExtraKwargsForTd) {
addVirtualParam(
FunctionParam.create(
ParamCategory.KwargsDict,
paramType.shared.typedDictEntries.extraItems.valueType,
extraItemsType,
FunctionParamFlags.TypeDeclared,
'kwargs'
),
index,
paramType.shared.typedDictEntries.extraItems.valueType
extraItemsType
);

result.kwargsIndex = result.params.length - 1;
Expand Down
6 changes: 3 additions & 3 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10628,7 +10628,7 @@ export function createTypeEvaluator(
overloadIndex: number
): MatchArgsToParamsResult {
const overload = typeResult.type;
const paramDetails = getParamListDetails(overload);
const paramDetails = getParamListDetails(overload, { disallowExtraKwargsForTd: true });
const paramSpec = FunctionType.getParamSpecFromArgsKwargs(overload);

let argIndex = 0;
Expand Down Expand Up @@ -11207,8 +11207,8 @@ export function createTypeEvaluator(

validateArgTypeParams.push({
paramCategory: ParamCategory.KwargsDict,
paramType: kwargsParam.declaredType,
requiresTypeVarMatching: requiresSpecialization(kwargsParam.declaredType),
paramType: kwargsParam.type,
requiresTypeVarMatching: requiresSpecialization(kwargsParam.type),
argument: {
argCategory: ArgCategory.UnpackedDictionary,
typeResult: { type: extraItemsType },
Expand Down
8 changes: 8 additions & 0 deletions packages/pyright-internal/src/tests/samples/kwargsUnpack1.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,11 @@ def func6(a: int, /, **kwargs: Unpack[TD3]):


func6(1, a=2)


def func7(*, v1: int, v3: str, v2: str = "") -> None: ...


# This should generate an error because func7 doesn't
# accept additional keyword arguments.
v7: TDProtocol6 = func7
11 changes: 9 additions & 2 deletions packages/pyright-internal/src/tests/samples/typedDictClosed2.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,18 @@ class Movie2(TypedDict, extra_items=int):
name: str


def func2(**kwargs: Unpack[Movie2]) -> None:
...
def func2(**kwargs: Unpack[Movie2]) -> None: ...


func2(name="")

# It's not clear whether this is allowed from the spec,
# but based on a reading of PEP 692, extra (non-specified)
# items should not be allowed as explicit keyword arguments.
# This is consistent with the original TypedDict PEP
# that disallows extra items to be present in a literal dict
# expression that is assigned to a TypedDict type.
# We'll therefore assume this should generate an error.
func2(name="", foo=1)

# This should generate an error.
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/typeEvaluator1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ test('Function10', () => {
test('KwargsUnpack1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['kwargsUnpack1.py']);

TestUtils.validateResults(analysisResults, 12);
TestUtils.validateResults(analysisResults, 13);
});

test('FunctionMember1', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/typeEvaluator5.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ test('TypedDictClosed2', () => {
configOptions.diagnosticRuleSet.enableExperimentalFeatures = true;

const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typedDictClosed2.py'], configOptions);
TestUtils.validateResults(analysisResults, 4);
TestUtils.validateResults(analysisResults, 5);
});

test('TypedDictClosed3', () => {
Expand Down

0 comments on commit d5ca5e9

Please sign in to comment.