Reject @runtime_checkable on non-protocol classes

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

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');

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'}"

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

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']);