Skip to content

Commit

Permalink
Fixed bug that led to false negative when evaluating a call to a func…
Browse files Browse the repository at this point in the history
…tion with a recursive ParamSpec. This addresses the remainder of #6178. (#6262)
  • Loading branch information
erictraut authored Oct 29, 2023
1 parent 08a295a commit 863afb7
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 4 deletions.
51 changes: 47 additions & 4 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10155,8 +10155,12 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// with a ParamSpec and a Concatenate operator. PEP 612 indicates that
// all positional parameters specified in the Concatenate must be
// filled explicitly.
if (typeResult.type.details.paramSpec && paramIndex < positionParamLimitIndex) {
if (isTypeVar(argTypeResult.type) && argTypeResult.type.paramSpecAccess === 'args') {
if (paramIndex < positionParamLimitIndex) {
if (
isTypeVar(argTypeResult.type) &&
argTypeResult.type.paramSpecAccess === 'args' &&
paramDetails.params[paramIndex].param.category !== ParameterCategory.ArgsList
) {
if (!isDiagnosticSuppressedForNode(errorNode)) {
addDiagnostic(
AnalyzerNodeInfo.getFileInfo(errorNode).diagnosticRuleSet.reportGeneralTypeIssues,
Expand Down Expand Up @@ -11695,8 +11699,47 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
functionType.details.parameters.length === 0 &&
isTypeSame(functionType.details.paramSpec, paramSpec)
) {
// TODO - need to perform additional validation here.
return { argumentErrors: false, typeVarContexts: [srcTypeVarContext] };
// If there are any arguments other than *args: P.args or **kwargs: P.kwargs,
// report an error.
let sawArgs = false;
let sawKwargs = false;
let argumentErrors = false;
let argErrorNode: ExpressionNode | undefined;

for (const arg of argList) {
const argType = getTypeOfArgument(arg)?.type;
const isArgTypeCompatible = argType && (isTypeSame(argType, paramSpec) || isAnyOrUnknown(argType));

if (arg.argumentCategory === ArgumentCategory.UnpackedList && !sawArgs && isArgTypeCompatible) {
sawArgs = true;
} else if (
arg.argumentCategory === ArgumentCategory.UnpackedDictionary &&
!sawKwargs &&
isArgTypeCompatible
) {
sawKwargs = true;
} else {
argErrorNode = argErrorNode ?? arg.valueExpression;
argumentErrors = true;
}
}

if (!sawArgs || !sawKwargs) {
argumentErrors = true;
}

if (argumentErrors) {
addDiagnostic(
AnalyzerNodeInfo.getFileInfo(errorNode).diagnosticRuleSet.reportGeneralTypeIssues,
DiagnosticRule.reportGeneralTypeIssues,
Localizer.Diagnostic.paramSpecArgsMissing().format({
type: printType(functionType.details.paramSpec),
}),
argErrorNode ?? errorNode
);
}

return { argumentErrors, typeVarContexts: [srcTypeVarContext] };
}

const result = validateFunctionArgumentTypes(errorNode, matchResults, srcTypeVarContext, signatureTracker);
Expand Down
59 changes: 59 additions & 0 deletions packages/pyright-internal/src/tests/samples/paramSpec49.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# This sample tests the case where a function parameterized with a
# ParamSpec P is called with *args: P.args and **kwargs: P.kwargs.

from typing import Any, Generic, ParamSpec

P = ParamSpec("P")


class TaskDeclaration(Generic[P]):
pass


class Dispatcher:
def dispatch(
self,
task_declaration: TaskDeclaration[P],
count: int,
/,
*args: P.args,
**kwargs: P.kwargs,
) -> None:
pass


class Queue:
dispatcher: Dispatcher

def method1(self, stub: TaskDeclaration[P]) -> Any:
def inner0(*args: P.args, **kwargs: P.kwargs) -> None:
self.dispatcher.dispatch(stub, 1, *args, **kwargs)

def inner1(*args: P.args, **kwargs: P.kwargs) -> None:
self.dispatcher.dispatch(stub, 1, **kwargs, *args)

def inner2(*args: P.args, **kwargs: P.kwargs) -> None:
# This should generate an error because it's missing
# a positional argument for 'count'.
self.dispatcher.dispatch(stub, *args, **kwargs)

def inner3(*args: P.args, **kwargs: P.kwargs) -> None:
# This should generate an error because it has an
# additional positional argument.
self.dispatcher.dispatch(stub, 1, 1, *args, **kwargs)

def inner4(*args: P.args, **kwargs: P.kwargs) -> None:
# This should generate an error because it is missing
# the *args argument.
self.dispatcher.dispatch(stub, 1, **kwargs)

def inner5(*args: P.args, **kwargs: P.kwargs) -> None:
# This should generate an error because it is missing
# the *kwargs argument.
self.dispatcher.dispatch(stub, 1, *args)

def inner6(*args: P.args, **kwargs: P.kwargs) -> None:
# This should generate an error because it has an
# extra *args argument.
self.dispatcher.dispatch(stub, 1, *args, *args, **kwargs)

5 changes: 5 additions & 0 deletions packages/pyright-internal/src/tests/typeEvaluator4.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,11 @@ test('ParamSpec48', () => {
TestUtils.validateResults(results, 0);
});

test('ParamSpec49', () => {
const results = TestUtils.typeAnalyzeSampleFiles(['paramSpec49.py']);
TestUtils.validateResults(results, 5);
});

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

Expand Down

0 comments on commit 863afb7

Please sign in to comment.