diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index 39cc9b90ea..860beb4dab 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -5281,11 +5281,8 @@ export class Checker extends ParseTreeWalker { } else { const baseClasses = classType.shared.baseClasses.filter(isClass); if ( - (classType.shared.declaredMetaclass?.category !== TypeCategory.Class || - !ClassType.isBuiltIn(classType.shared.declaredMetaclass, 'ABCMeta')) && - !baseClasses.some( - (baseClass) => baseClass.shared.fullName === 'abc.ABC' || ClassType.isBuiltIn(baseClass, 'Protocol') - ) + !ClassType.derivesFromExplicitAbstract(classType) && + !baseClasses.some((baseClass) => ClassType.isBuiltIn(baseClass, 'Protocol')) ) { const errorMessage = classType.shared.mro.some( (baseClass) => isClass(baseClass) && ClassType.isBuiltIn(baseClass, 'Protocol') diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index 09e782fe94..57a554567f 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -10786,43 +10786,54 @@ export function createTypeEvaluator( if (ClassType.supportsAbstractMethods(expandedCallType)) { const abstractSymbols = getAbstractSymbols(expandedCallType); + const derivesFromExplicitAbstract = ClassType.derivesFromExplicitAbstract(expandedCallType); + + // check for abstract usages. + // only report an error if it's an exact type (ie. we know it's definitely not a subclass/impl) + if (!expandedCallType.priv.includeSubclasses && !isTypeVar(unexpandedCallType)) { + // Handle abstract classes with abstract methods (reportAbstractUsage) + if (abstractSymbols.length > 0) { + // If the class is abstract, it can't be instantiated. + const diagAddendum = new DiagnosticAddendum(); + const errorsToDisplay = 2; - if ( - abstractSymbols.length > 0 && - !expandedCallType.priv.includeSubclasses && - !isTypeVar(unexpandedCallType) - ) { - // If the class is abstract, it can't be instantiated. - const diagAddendum = new DiagnosticAddendum(); - const errorsToDisplay = 2; - - abstractSymbols.forEach((abstractMethod, index) => { - if (index === errorsToDisplay) { - diagAddendum.addMessage( - LocAddendum.memberIsAbstractMore().format({ - count: abstractSymbols.length - errorsToDisplay, - }) - ); - } else if (index < errorsToDisplay) { - if (isInstantiableClass(abstractMethod.classType)) { - const className = abstractMethod.classType.shared.name; + abstractSymbols.forEach((abstractMethod, index) => { + if (index === errorsToDisplay) { diagAddendum.addMessage( - LocAddendum.memberIsAbstract().format({ - type: className, - name: abstractMethod.symbolName, + LocAddendum.memberIsAbstractMore().format({ + count: abstractSymbols.length - errorsToDisplay, }) ); + } else if (index < errorsToDisplay) { + if (isInstantiableClass(abstractMethod.classType)) { + const className = abstractMethod.classType.shared.name; + diagAddendum.addMessage( + LocAddendum.memberIsAbstract().format({ + type: className, + name: abstractMethod.symbolName, + }) + ); + } } - } - }); + }); - addDiagnostic( - DiagnosticRule.reportAbstractUsage, - LocMessage.instantiateAbstract().format({ - type: expandedCallType.shared.name, - }) + diagAddendum.getString(), - errorNode - ); + addDiagnostic( + DiagnosticRule.reportAbstractUsage, + LocMessage.instantiateAbstract().format({ + type: expandedCallType.shared.name, + }) + diagAddendum.getString(), + errorNode + ); + } else if (derivesFromExplicitAbstract) { + // Handle abstract classes with no abstract methods (reportEmptyAbstractClass) + addDiagnostic( + DiagnosticRule.reportExplicitAbstractUsage, + LocMessage.instantiateAbstract().format({ + type: expandedCallType.shared.name, + }) + LocAddendum.classIsExplicitlyAbstract().getFormatString(), + errorNode + ); + } } } diff --git a/packages/pyright-internal/src/analyzer/types.ts b/packages/pyright-internal/src/analyzer/types.ts index b1695db2b5..b63a2b22a5 100644 --- a/packages/pyright-internal/src/analyzer/types.ts +++ b/packages/pyright-internal/src/analyzer/types.ts @@ -1175,6 +1175,20 @@ export namespace ClassType { return !!(classType.shared.flags & ClassTypeFlags.SupportsAbstractMethods); } + export function derivesFromExplicitAbstract(classType: ClassType): boolean { + // Check if ABC is in the direct base classes + const derivesDirectlyFromABC = classType.shared.baseClasses.some( + (baseClass) => isClass(baseClass) && baseClass.shared.fullName === 'abc.ABC' + ); + // Check if the class uses ABCMeta as its metaclass + const hasABCMetaMetaclass = + classType.shared.declaredMetaclass !== undefined && + isClass(classType.shared.declaredMetaclass) && + classType.shared.declaredMetaclass.shared.fullName === 'abc.ABCMeta'; + + return derivesDirectlyFromABC || hasABCMetaMetaclass; + } + export function isDataClass(classType: ClassType) { return !!classType.shared.dataClassBehaviors; } diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index 6dbf559fa5..73a8953201 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -189,6 +189,9 @@ export interface DiagnosticRuleSet { // Report use of abstract method or variable? reportAbstractUsage: DiagnosticLevel; + // Report instantiation of abstract class with no abstract methods? + reportExplicitAbstractUsage: DiagnosticLevel; + // Report argument type incompatibilities? reportArgumentType: DiagnosticLevel; @@ -481,6 +484,7 @@ export function getDiagLevelDiagnosticRules() { DiagnosticRule.reportDuplicateImport, DiagnosticRule.reportWildcardImportFromLibrary, DiagnosticRule.reportAbstractUsage, + DiagnosticRule.reportExplicitAbstractUsage, DiagnosticRule.reportArgumentType, DiagnosticRule.reportAssertTypeFailure, DiagnosticRule.reportAssignmentType, @@ -617,6 +621,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportDuplicateImport: 'none', reportWildcardImportFromLibrary: 'none', reportAbstractUsage: 'none', + reportExplicitAbstractUsage: 'none', reportArgumentType: 'none', reportAssertTypeFailure: 'none', reportAssignmentType: 'none', @@ -737,6 +742,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportDuplicateImport: 'none', reportWildcardImportFromLibrary: 'warning', reportAbstractUsage: 'error', + reportExplicitAbstractUsage: 'error', reportArgumentType: 'error', reportAssertTypeFailure: 'error', reportAssignmentType: 'error', @@ -857,6 +863,7 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet { reportDuplicateImport: 'none', reportWildcardImportFromLibrary: 'warning', reportAbstractUsage: 'error', + reportExplicitAbstractUsage: 'error', reportArgumentType: 'error', reportAssertTypeFailure: 'error', reportAssignmentType: 'error', @@ -976,6 +983,7 @@ export const getRecommendedDiagnosticRuleSet = (): DiagnosticRuleSet => ({ reportDuplicateImport: 'warning', reportWildcardImportFromLibrary: 'warning', reportAbstractUsage: 'error', + reportExplicitAbstractUsage: 'error', reportArgumentType: 'error', reportAssertTypeFailure: 'error', reportAssignmentType: 'error', @@ -1092,6 +1100,7 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({ reportDuplicateImport: 'error', reportWildcardImportFromLibrary: 'error', reportAbstractUsage: 'error', + reportExplicitAbstractUsage: 'error', reportArgumentType: 'error', reportAssertTypeFailure: 'error', reportAssignmentType: 'error', @@ -1209,6 +1218,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportDuplicateImport: 'error', reportWildcardImportFromLibrary: 'error', reportAbstractUsage: 'error', + reportExplicitAbstractUsage: 'error', reportArgumentType: 'error', reportAssertTypeFailure: 'error', reportAssignmentType: 'error', diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 7738588855..3efee237a0 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -38,6 +38,7 @@ export enum DiagnosticRule { reportDuplicateImport = 'reportDuplicateImport', reportWildcardImportFromLibrary = 'reportWildcardImportFromLibrary', reportAbstractUsage = 'reportAbstractUsage', + reportExplicitAbstractUsage = 'reportExplicitAbstractUsage', reportArgumentType = 'reportArgumentType', reportAssertTypeFailure = 'reportAssertTypeFailure', reportAssignmentType = 'reportAssignmentType', diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index a439f95ce4..add108be5d 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -1393,6 +1393,8 @@ export namespace Localizer { ); export const memberIsAbstractMore = () => new ParameterizedString<{ count: number }>(getRawString('DiagnosticAddendum.memberIsAbstractMore')); + export const classIsExplicitlyAbstract = () => + new ParameterizedString<{ count: number }>(getRawString('DiagnosticAddendum.classIsExplicitlyAbstract')); export const memberIsClassVarInProtocol = () => new ParameterizedString<{ name: string }>(getRawString('DiagnosticAddendum.memberIsClassVarInProtocol')); export const memberIsInitVar = () => 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 7a5a4ac91d..c99266ce5b 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -1934,6 +1934,9 @@ "message": "and {count} more...", "comment": "{StrEnds='...'}" }, + "classIsExplicitlyAbstract": { + "message": "class has no abstract members, but is explicitly denoted with \"ABC\" or \"ABCMeta\"" + }, "memberIsClassVarInProtocol": { "message": "\"{name}\" is defined as a ClassVar in protocol", "comment": "{Locked='ClassVar'}" diff --git a/packages/pyright-internal/src/tests/checker.test.ts b/packages/pyright-internal/src/tests/checker.test.ts index eca1df3f74..7919c4349c 100644 --- a/packages/pyright-internal/src/tests/checker.test.ts +++ b/packages/pyright-internal/src/tests/checker.test.ts @@ -128,6 +128,22 @@ test('AbstractClass11', () => { TestUtils.validateResults(analysisResults, 2); }); +test('AbstractClass12', () => { + const analysisResults = TestUtils.typeAnalyzeSampleFiles(['abstractClass12.py']); + + TestUtils.validateResultsButBased(analysisResults, { + // one error to validate the message, the rest use `pyright: ignore` + errors: [ + { + line: 11, + message: + 'Cannot instantiate abstract class "ExplicitlyAbstract"class has no abstract members, but is explicitly denoted with "ABC" or "ABCMeta"', + code: DiagnosticRule.reportExplicitAbstractUsage, + }, + ], + }); +}); + test('Constants1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['constants1.py']); diff --git a/packages/pyright-internal/src/tests/samples/abstractClass12.py b/packages/pyright-internal/src/tests/samples/abstractClass12.py new file mode 100644 index 0000000000..1f236f96b0 --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/abstractClass12.py @@ -0,0 +1,40 @@ +# this sample tests the type analyzer's ability to flag attempts +# to instantiate abstract base classes that have no abstract methods + +from abc import ABC, ABCMeta + + +class ExplicitlyAbstract(ABC): + """an abstract class with no abstract methods""" + +# this should generate an error because `AbstractFoo` +# is an abstract class even though it has no abstract methods +a = ExplicitlyAbstract() + +class IndirectlyAbstract(ExplicitlyAbstract): + """inherits from an explicitly abstract class""" + + +# this should not generate an error because IndirectlyAbstract +# doesn't directly inherit from ABC (only through AbstractFoo) +f = IndirectlyAbstract() + + +class NotAbstract: + """a regular class that doesn't derive from ABC""" + +# This should not generate an error because NotAbstract is not abstract +e = NotAbstract() + +class AbstractWithMetaclass(metaclass=ABCMeta): + """abstract class using ABCMeta metaclass with no abstract methods""" + +class ConcreteWithMetaclass(AbstractWithMetaclass): + """concrete subclass of AbstractWithMetaclass""" + + +# this should generate an error because AbstractWithMetaclass uses ABCMeta +i = AbstractWithMetaclass() # pyright: ignore[reportExplicitAbstractUsage] + +# this should not generate an error +j = ConcreteWithMetaclass() diff --git a/packages/pyright-internal/src/tests/samples/async1.py b/packages/pyright-internal/src/tests/samples/async1.py index 47abf92d1a..dedc71c0de 100644 --- a/packages/pyright-internal/src/tests/samples/async1.py +++ b/packages/pyright-internal/src/tests/samples/async1.py @@ -9,7 +9,7 @@ async def b(): yield i -cm = AsyncExitStack() +cm = AsyncExitStack() # pyright: ignore[reportExplicitAbstractUsage] # typeshed moment def func1(): diff --git a/packages/pyright-internal/src/tests/samples/property2.py b/packages/pyright-internal/src/tests/samples/property2.py index 3b87aec4bd..8f5544b083 100644 --- a/packages/pyright-internal/src/tests/samples/property2.py +++ b/packages/pyright-internal/src/tests/samples/property2.py @@ -21,7 +21,7 @@ def y(self) -> float: raise NotImplementedError -a = Foo() +a = Foo() # pyright: ignore[reportExplicitAbstractUsage] requires_int(a.x) a.x = 3 diff --git a/packages/pyright-internal/src/tests/samples/pseudoGeneric3.py b/packages/pyright-internal/src/tests/samples/pseudoGeneric3.py index 4cf33269a6..2c15192901 100644 --- a/packages/pyright-internal/src/tests/samples/pseudoGeneric3.py +++ b/packages/pyright-internal/src/tests/samples/pseudoGeneric3.py @@ -25,6 +25,6 @@ def __getattr__(self, attr): return self.__getattribute__(attr) -b1 = ClassB("test") +b1 = ClassB("test") # pyright: ignore[reportExplicitAbstractUsage] reveal_type(b1.value, expected_text="Unknown | Any | None") del b1.cache diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 4627bc1717..b20a761898 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -490,6 +490,23 @@ false ] }, + "reportExplicitAbstractUsage": { + "type": [ + "string", + "boolean" + ], + "description": "Diagnostics for an attempt to instantiate an abstract class that has no abstract members.", + "default": "error", + "enum": [ + "none", + "hint", + "information", + "warning", + "error", + true, + false + ] + }, "reportArgumentType": { "type": [ "string", diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index 23348336d9..00bcc8e18d 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -185,6 +185,11 @@ "title": "Controls reporting of attempted instantiation of abstract class", "default": "error" }, + "reportExplicitAbstractUsage": { + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of attempted instantiation of empty abstract class", + "default": "error" + }, "reportArgumentType": { "$ref": "#/definitions/diagnostic", "title": "Controls reporting of incompatible argument type", @@ -744,6 +749,9 @@ "reportAbstractUsage": { "$ref": "#/definitions/reportAbstractUsage" }, + "reportExplicitAbstractUsage": { + "$ref": "#/definitions/reportExplicitAbstractUsage" + }, "reportArgumentType": { "$ref": "#/definitions/reportArgumentType" },