Fixed bug that results in false positive reportInconsistentOverload…

… and `reportNoOverloadImplementation` errors when an overloaded decorator is applied to a non-overloaded function or method. This addresses #8246. (#8249)
microsoft · Jun 27, 2024 · f104e7b · f104e7b
1 parent 1e1e912
commit f104e7b
Show file tree

Hide file tree

Showing 2 changed files with 125 additions and 90 deletions.
diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
@@ -3092,106 +3092,123 @@ export class Checker extends ParseTreeWalker {
 
  private _reportInvalidOverload(name: string, symbol: Symbol) {
  const typedDecls = symbol.getTypedDeclarations();
- if (typedDecls.length >= 1) {
- const primaryDecl = typedDecls[0];
-
- if (primaryDecl.type === DeclarationType.Function) {
- const type = this._evaluator.getEffectiveTypeOfSymbol(symbol);
- const overloadedFunctions = isOverloadedFunction(type)
- ? OverloadedFunctionType.getOverloads(type)
- : isFunction(type) && FunctionType.isOverloaded(type)
- ? [type]
- : [];
-
- if (overloadedFunctions.length === 1) {
- // There should never be a single overload.
- this._evaluator.addDiagnostic(
- DiagnosticRule.reportInconsistentOverload,
- LocMessage.singleOverload().format({ name }),
- primaryDecl.node.name
- );
- }
+ if (typedDecls.length === 0) {
+ return;
+ }
 
- // If the file is not a stub and this is the first overload,
- // verify that there is an implementation.
- if (!this._fileInfo.isStubFile && overloadedFunctions.length > 0) {
- let implementationFunction: FunctionType | undefined;
- let exemptMissingImplementation = false;
+ const primaryDecl = typedDecls[0];
 
- if (isOverloadedFunction(type)) {
- implementationFunction = OverloadedFunctionType.getImplementation(type);
+ if (primaryDecl.type !== DeclarationType.Function) {
+ return;
+ }
 
- // If the implementation has no name, it was synthesized probably by a
- // decorator that used a callable with a ParamSpec that captured the
- // overloaded signature. We'll exempt it from this check.
- const overloads = OverloadedFunctionType.getOverloads(type);
- if (overloads.length > 0 && overloads[0].details.name === '') {
- exemptMissingImplementation = true;
- }
- } else if (isFunction(type) && !FunctionType.isOverloaded(type)) {
- implementationFunction = type;
+ const type = this._evaluator.getEffectiveTypeOfSymbol(symbol);
+ const overloadedFunctions = isOverloadedFunction(type)
+ ? OverloadedFunctionType.getOverloads(type)
+ : isFunction(type) && FunctionType.isOverloaded(type)
+ ? [type]
+ : [];
+
+ // If the implementation has no name, it was synthesized probably by a
+ // decorator that used a callable with a ParamSpec that captured the
+ // overloaded signature. We'll exempt it from this check.
+ if (isOverloadedFunction(type)) {
+ const overloads = OverloadedFunctionType.getOverloads(type);
+ if (overloads.length > 0 && overloads[0].details.name === '') {
+ return;
+ }
+ } else if (isFunction(type)) {
+ if (type.details.name === '') {
+ return;
+ }
+ }
+
+ if (overloadedFunctions.length === 1) {
+ // There should never be a single overload.
+ this._evaluator.addDiagnostic(
+ DiagnosticRule.reportInconsistentOverload,
+ LocMessage.singleOverload().format({ name }),
+ primaryDecl.node.name
+ );
+ }
+
+ // If the file is not a stub and this is the first overload,
+ // verify that there is an implementation.
+ if (this._fileInfo.isStubFile || overloadedFunctions.length === 0) {
+ return;
+ }
+
+ let implementationFunction: FunctionType | undefined;
+
+ if (isOverloadedFunction(type)) {
+ implementationFunction = OverloadedFunctionType.getImplementation(type);
+ } else if (isFunction(type) && !FunctionType.isOverloaded(type)) {
+ implementationFunction = type;
+ }
+
+ if (!implementationFunction) {
+ const containingClassNode = ParseTreeUtils.getEnclosingClassOrFunction(primaryDecl.node);
+ if (containingClassNode && containingClassNode.nodeType === ParseNodeType.Class) {
+ const classType = this._evaluator.getTypeOfClass(containingClassNode);
+ if (classType) {
+ if (ClassType.isProtocolClass(classType.classType)) {
+ return;
  }
 
- if (!implementationFunction) {
- const containingClassNode = ParseTreeUtils.getEnclosingClassOrFunction(primaryDecl.node);
- if (containingClassNode && containingClassNode.nodeType === ParseNodeType.Class) {
- const classType = this._evaluator.getTypeOfClass(containingClassNode);
- if (classType) {
- if (ClassType.isProtocolClass(classType.classType)) {
- exemptMissingImplementation = true;
- } else if (ClassType.supportsAbstractMethods(classType.classType)) {
- if (
- isOverloadedFunction(type) &&
- OverloadedFunctionType.getOverloads(type).every((overload) =>
- FunctionType.isAbstractMethod(overload)
- )
- ) {
- exemptMissingImplementation = true;
- }
- }
- }
+ if (ClassType.supportsAbstractMethods(classType.classType)) {
+ if (
+ isOverloadedFunction(type) &&
+ OverloadedFunctionType.getOverloads(type).every((overload) =>
+ FunctionType.isAbstractMethod(overload)
+ )
+ ) {
+ return;
  }
+ }
+ }
+ }
 
- // If this is a method within a protocol class, don't require that
- // there is an implementation.
- if (!exemptMissingImplementation) {
- this._evaluator.addDiagnostic(
- DiagnosticRule.reportNoOverloadImplementation,
- LocMessage.overloadWithoutImplementation().format({
- name: primaryDecl.node.name.value,
- }),
- primaryDecl.node.name
- );
- }
- } else if (isOverloadedFunction(type)) {
- // Verify that all overload signatures are assignable to implementation signature.
- OverloadedFunctionType.getOverloads(type).forEach((overload, index) => {
- const diag = new DiagnosticAddendum();
- if (!this._isLegalOverloadImplementation(overload, implementationFunction!, diag)) {
- if (implementationFunction!.details.declaration) {
- const diagnostic = this._evaluator.addDiagnostic(
- DiagnosticRule.reportInconsistentOverload,
- LocMessage.overloadImplementationMismatch().format({
- name,
- index: index + 1,
- }) + diag.getString(),
- implementationFunction!.details.declaration.node.name
- );
+ // If this is a method within a protocol class, don't require that
+ // there is an implementation.
+ this._evaluator.addDiagnostic(
+ DiagnosticRule.reportNoOverloadImplementation,
+ LocMessage.overloadWithoutImplementation().format({
+ name: primaryDecl.node.name.value,
+ }),
+ primaryDecl.node.name
+ );
 
- if (diagnostic && overload.details.declaration) {
- diagnostic.addRelatedInfo(
- LocAddendum.overloadSignature(),
- overload.details.declaration?.uri ?? primaryDecl.uri,
- overload.details.declaration?.range ?? primaryDecl.range
- );
- }
- }
- }
- });
+ return;
+ }
+
+ if (!isOverloadedFunction(type)) {
+ return;
+ }
+
+ // Verify that all overload signatures are assignable to implementation signature.
+ OverloadedFunctionType.getOverloads(type).forEach((overload, index) => {
+ const diag = new DiagnosticAddendum();
+ if (!this._isLegalOverloadImplementation(overload, implementationFunction!, diag)) {
+ if (implementationFunction!.details.declaration) {
+ const diagnostic = this._evaluator.addDiagnostic(
+ DiagnosticRule.reportInconsistentOverload,
+ LocMessage.overloadImplementationMismatch().format({
+ name,
+ index: index + 1,
+ }) + diag.getString(),
+ implementationFunction!.details.declaration.node.name
+ );
+
+ if (diagnostic && overload.details.declaration) {
+ diagnostic.addRelatedInfo(
+ LocAddendum.overloadSignature(),
+ overload.details.declaration?.uri ?? primaryDecl.uri,
+ overload.details.declaration?.range ?? primaryDecl.range
+ );
  }
  }
  }
- }
+ });
  }
 
  private _reportMultipleFinalDeclarations(name: string, symbol: Symbol, scopeType: ScopeType) {

diff --git a/packages/pyright-internal/src/tests/samples/overload4.py b/packages/pyright-internal/src/tests/samples/overload4.py
@@ -43,7 +43,7 @@ def func4(self, name: str) -> str: ...
 
 
 def deco1(
- _origin: Callable[P, T]
+ _origin: Callable[P, T],
 ) -> Callable[[Callable[..., Any]], Callable[P, T]]: ...
 
 
@@ -60,3 +60,21 @@ def func5(v: int | str) -> int | str: ...
 
 @deco1(func5)
 def func6(*args: Any, **kwargs: Any) -> Any: ...
+
+
+@overload
+def deco2() -> Callable[[Callable[P, T]], Callable[P, T | None]]: ...
+@overload
+def deco2(
+ x: Callable[[], T],
+) -> Callable[[Callable[P, T]], Callable[P, T]]: ...
+
+
+def deco2(
+ x: Callable[[], T | None] = lambda: None,
+) -> Callable[[Callable[P, T]], Callable[P, T | None]]: ...
+
+
+@deco2(x=dict)
+def func7() -> dict[str, str]:
+ return {}