diff --git a/packages/pyright-internal/src/analyzer/decorators.ts b/packages/pyright-internal/src/analyzer/decorators.ts index a5e47dd8eda6..c0193f45fc73 100644 --- a/packages/pyright-internal/src/analyzer/decorators.ts +++ b/packages/pyright-internal/src/analyzer/decorators.ts @@ -409,7 +409,20 @@ export function applyClassDecorator( } if (FunctionType.isBuiltIn(decoratorType, 'runtime_checkable')) { - originalClassType.shared.flags |= ClassTypeFlags.RuntimeCheckable; + // Class decorators are applied bottom-up, so validate the class type + // that this decorator actually receives rather than the original + // (undecorated) class. + const decoratedClassType = isInstantiableClass(inputClassType) ? inputClassType : originalClassType; + + if (!ClassType.isProtocolClass(decoratedClassType)) { + evaluator.addDiagnostic( + DiagnosticRule.reportGeneralTypeIssues, + LocMessage.runtimeCheckableNotProtocol(), + decoratorNode.d.expr + ); + } else { + originalClassType.shared.flags |= ClassTypeFlags.RuntimeCheckable; + } // Don't call getTypeOfDecorator for runtime_checkable. It appears // frequently in stubs, and it's a waste of time to validate its diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index 95ab0685b8b6..09c1dc4a37ca 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -900,6 +900,7 @@ export namespace Localizer { getRawString('Diagnostic.returnTypeMismatch') ); export const returnTypeUnknown = () => getRawString('Diagnostic.returnTypeUnknown'); + export const runtimeCheckableNotProtocol = () => getRawString('Diagnostic.runtimeCheckableNotProtocol'); export const returnTypePartiallyUnknown = () => new ParameterizedString<{ returnType: string }>(getRawString('Diagnostic.returnTypePartiallyUnknown')); export const revealLocalsArgs = () => getRawString('Diagnostic.revealLocalsArgs'); diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json index e65ef2a23ed5..e54d5f989de8 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -1174,6 +1174,10 @@ "returnTypeMismatch": "Type \"{exprType}\" is not assignable to return type \"{returnType}\"", "returnTypePartiallyUnknown": "Return type, \"{returnType}\", is partially unknown", "returnTypeUnknown": "Return type is unknown", + "runtimeCheckableNotProtocol": { + "message": "@runtime_checkable can be applied only to a Protocol class", + "comment": "{Locked='@runtime_checkable','Protocol'}" + }, "revealLocalsArgs": { "message": "Expected no arguments for \"reveal_locals\" call", "comment": "{Locked='reveal_locals'}" diff --git a/packages/pyright-internal/src/tests/samples/protocol54.py b/packages/pyright-internal/src/tests/samples/protocol54.py new file mode 100644 index 000000000000..44d966b501ef --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/protocol54.py @@ -0,0 +1,49 @@ +# This sample tests that @runtime_checkable can be applied only to +# classes that are protocols (Protocol must appear in the base list). + +from typing import Protocol, runtime_checkable + + +@runtime_checkable +class P1(Protocol): + def foo(self) -> int: ... + + +# This should generate an error because a subclass of a protocol is +# not itself a protocol unless Protocol is listed as a base class. +@runtime_checkable +class P2(P1): + def bar(self) -> str: ... + + +@runtime_checkable +class P3(P1, Protocol): + def bar(self) -> str: ... + + +# This should generate an error because C1 is not a protocol. +@runtime_checkable +class C1: + def foo(self) -> int: ... + + +# Class decorators are applied bottom-up, so runtime_checkable receives +# the class produced by the decorator below it. +def replace_with_protocol(cls: type) -> type[P1]: ... + + +def replace_with_non_protocol(cls: type) -> type[C1]: ... + + +@runtime_checkable +@replace_with_protocol +class C2: + pass + + +# This should generate an error because the decorator below +# runtime_checkable replaces the class with a non-protocol class. +@runtime_checkable +@replace_with_non_protocol +class P4(Protocol): + pass diff --git a/packages/pyright-internal/src/tests/typeEvaluator7.test.ts b/packages/pyright-internal/src/tests/typeEvaluator7.test.ts index f9b1ba182a63..97b1d3e78664 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator7.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator7.test.ts @@ -8,6 +8,8 @@ * arbitrarily among multiple files so they can run in parallel. */ +import assert from 'assert'; + import { ConfigOptions } from '../common/configOptions'; import { pythonVersion3_10, @@ -628,6 +630,17 @@ test('Protocol53', () => { TestUtils.validateResults(analysisResults2, 8); }); +test('Protocol54', () => { + const analysisResults = TestUtils.typeAnalyzeSampleFiles(['protocol54.py']); + + TestUtils.validateResults(analysisResults, 3); + + // Verify that the errors are reported on the expected `@runtime_checkable` + // decorators rather than on some other (unrelated) declaration. + const errorLines = analysisResults[0].errors.map((diag) => diag.range.start.line).sort((a, b) => a - b); + assert.deepStrictEqual(errorLines, [13, 24, 45]); +}); + test('ProtocolExplicit1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['protocolExplicit1.py']);