Skip to content

Commit

Permalink
fix reportMissingSuperCall false positives when the class has a bas…
Browse files Browse the repository at this point in the history
…e class that does not define an __init__ or __init_subclass__ and `reportUnsafeMultipleInheritance` is enabled
  • Loading branch information
DetachHead committed May 9, 2024
1 parent 05ca5b5 commit 65e85a9
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 20 deletions.
25 changes: 8 additions & 17 deletions packages/pyright-internal/src/analyzer/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6876,26 +6876,17 @@ export class Checker extends ParseTreeWalker {
return;
}

// If the class is marked final, we can skip the "object" base class
// because we know that the `__init__` method in `object` doesn't do
// anything. It's not safe to do this if the class isn't final because
// it could be combined with other classes in a multi-inheritance
// situation that effectively adds new superclasses that we don't know
// about statically.
// If the class is marked final or reportUnsafeMultipleInheritance is enabled,
// we can skip the "object" base class because we know that the `__init__` method
// in `object` doesn't do anything. It's not safe to do this normally because it
// could be combined with other classes in a multi-inheritance situation that
// effectively adds new superclasses that we don't know about statically.
let effectiveFlags = MemberAccessFlags.SkipInstanceMembers | MemberAccessFlags.SkipOriginalClass;
if (ClassType.isFinal(classType)) {
effectiveFlags |= MemberAccessFlags.SkipObjectBaseClass;
}

// if reportUnsafeMultipleInheritance is enabled, we can be less strict
// with reportMissingSuperCall by not reporting it on classes that don't
// have a base class, because reportUnsafeMultipleInheritance prevents the
// possibility of another method being added to the MRO
if (
this._fileInfo.diagnosticRuleSet.reportUnsafeMultipleInheritance !== 'none' &&
this._getActualBaseClasses(classType).length < 1
ClassType.isFinal(classType) ||
this._fileInfo.diagnosticRuleSet.reportUnsafeMultipleInheritance !== 'none'
) {
return;
effectiveFlags |= MemberAccessFlags.SkipObjectBaseClass;
}

const methodMember = lookUpClassMember(classType, methodType.details.name, effectiveFlags);
Expand Down
88 changes: 88 additions & 0 deletions packages/pyright-internal/src/tests/samples/missingSuperBased.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# This sample tests the reportMissingSuperCall diagnostic check.

from typing import final


class ParentA:
pass


class ParentB:
# This should generate an error because it's missing a super().__init__ call.
def __init__(self):
pass


class ParentBPrime(ParentB):
pass


class ParentC:
pass


@final
class ParentD:
def __init__(self):
pass

def __init_subclass__(cls) -> None:
pass


class ChildA(ParentA, ParentB):
# This should generate an error.
def __init__(self):
pass

# This should generate an error.... NOT!!!!
def __init_subclass__(cls) -> None:
pass


class ChildB(ParentA, ParentB):
def __init__(self):
super().__init__()


class ChildC1(ParentA, ParentB):
def __init__(self):
ParentB.__init__(self)


class ChildC2(ParentA, ParentB):
def __init__(self):
ParentA.__init__(self)
ParentB.__init__(self)


class ChildCPrime(ParentA, ParentBPrime, ParentC):
def __init__(self):
super(ParentBPrime).__init__()


class ChildD(ParentC):
# This should generate an error.... said no one ever
def __init__(self):
pass


@final
class ChildE(ParentC):
def __init__(self):
pass

class Foo:
def __init_subclass__(cls) -> None:
pass

class Bar(Foo):
def __init_subclass__(cls) -> None: # error
pass

class Baz:
pass

class Qux(Baz):
def __init_subclass__(cls) -> None: # no error
pass
5 changes: 2 additions & 3 deletions packages/pyright-internal/src/tests/typeEvaluator2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,17 +257,16 @@ test('MissingSuper with reportUnsafeMultipleInheritance enabled', () => {

configOptions.diagnosticRuleSet.reportMissingSuperCall = 'error';
configOptions.diagnosticRuleSet.reportUnsafeMultipleInheritance = 'error';
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['missingSuper1.py'], configOptions);
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['missingSuperBased.py'], configOptions);
TestUtils.validateResultsButBased(analysisResults, {
errors: [
{ code: DiagnosticRule.reportMissingSuperCall, line: 34 },
{ code: DiagnosticRule.reportMissingSuperCall, line: 38 },
{ code: DiagnosticRule.reportUnsafeMultipleInheritance, line: 32 },
{ code: DiagnosticRule.reportUnsafeMultipleInheritance, line: 42 },
{ code: DiagnosticRule.reportUnsafeMultipleInheritance, line: 47 },
{ code: DiagnosticRule.reportUnsafeMultipleInheritance, line: 52 },
{ code: DiagnosticRule.reportUnsafeMultipleInheritance, line: 58 },
{ code: DiagnosticRule.reportMissingSuperCall, line: 65 },
{ code: DiagnosticRule.reportMissingSuperCall, line: 79 },
],
});
});
Expand Down

0 comments on commit 65e85a9

Please sign in to comment.