Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28393,7 +28393,14 @@ export function createTypeEvaluator(
return;
}

const destParamInfo = destParamMap.get(srcParamInfo.param.name);
const destParamInfo =
destParamMap.get(srcParamInfo.param.name) ??
destParamDetails.params.find(
(paramInfo) =>
paramInfo.param.name === srcParamInfo.param.name &&
paramInfo.kind === ParamKind.Standard &&
paramInfo.param.category === ParamCategory.Simple
);
const paramDiag = diag?.createAddendum();
const srcParamType = srcParamInfo.type;

Expand Down Expand Up @@ -28523,6 +28530,43 @@ export function createTypeEvaluator(
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This check needs to account for overload-overlap mode. For example, (a: int) -> int and (*args: int, **kwargs: bool) -> str still overlap for the positional call f(1), even though their keyword forms do not. During PartialOverloadOverlap, this unconditional failure makes assignFunction return false and suppresses the incompatible-return diagnostic. Could we model the positional and keyword paths disjunctively (or otherwise preserve an overlap once a shared positional call exists) and add a regression case to overloadOverlap1.py?

});

// Positional-or-keyword dest parameters can also be passed by name.
// If the source has no matching named parameter, the keyword form must
// be compatible with the source **kwargs (when present).
destParamDetails.params.forEach((destParamInfo) => {
if (
destParamInfo.kind !== ParamKind.Standard ||
!destParamInfo.param.name ||
destParamInfo.param.category !== ParamCategory.Simple
) {
return;
}

const srcHasNamed = srcParamDetails.params.some(
(srcParamInfo) =>
srcParamInfo.param.name === destParamInfo.param.name &&
srcParamInfo.param.category === ParamCategory.Simple &&
srcParamInfo.kind !== ParamKind.Positional
);
if (srcHasNamed || srcParamDetails.kwargsIndex === undefined) {
return;
}

if (
!assignParam(
destParamInfo.type,
srcParamDetails.params[srcParamDetails.kwargsIndex].type,
destParamInfo.index,
diag?.createAddendum(),
constraints,
flags,
recursionCount
)
) {
canAssign = false;
Comment thread
rchiodo marked this conversation as resolved.
}
});

// If both src and dest have a "**kwargs" parameter, make sure their types are compatible.
if (srcParamDetails.kwargsIndex !== undefined && destParamDetails.kwargsIndex !== undefined) {
if (
Expand Down
37 changes: 37 additions & 0 deletions packages/pyright-internal/src/tests/samples/callbackProtocol12.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# This sample tests that a callable with *args/**kwargs is not assignable
# to a callable whose positional-or-keyword parameter cannot be passed
# by keyword to the source (typing spec callable assignability).

from typing import Protocol


class AcceptsArgsKwargs(Protocol):
def __call__(self, *args: int, **kwargs: bool) -> None: ...


class AcceptsKeywordOrPositional(Protocol):
def __call__(self, a: int) -> None: ...


def func1(cb: AcceptsArgsKwargs):
# This should generate an error because AcceptsKeywordOrPositional
# can be called as cb(a=10), which is not valid for **kwargs: bool.
x: AcceptsKeywordOrPositional = cb


class AcceptsKeywordOnly(Protocol):
def __call__(self, *args: int, a: bool = False) -> None: ...


def func2(cb: AcceptsKeywordOnly):
# This should generate an error because the keyword form of
# parameter "a" has type int on the dest and bool on the source.
y: AcceptsKeywordOrPositional = cb


def ok_cb(a: int) -> None:
pass


def func3(cb: AcceptsKeywordOrPositional):
z: AcceptsKeywordOrPositional = ok_cb

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we add positive cases that exercise each newly accepted path? Assigning (*args: int, **kwargs: int) to (a: int) covers the new **kwargs fallback, and assigning (*args: int, a: int) to (a: int) covers the named keyword-only lookup. The current ok_cb case is an exact-signature assignment and would pass before this change.

6 changes: 6 additions & 0 deletions packages/pyright-internal/src/tests/typeEvaluator2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ test('CallbackProtocol11', () => {
TestUtils.validateResults(analysisResults, 0);
});

test('CallbackProtocol12', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['callbackProtocol12.py']);

TestUtils.validateResults(analysisResults, 2);
});

test('Assignment1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['assignment1.py']);

Expand Down
Loading