Skip to content

Commit

Permalink
Added missing check for the override of a property that is marked `@f…
Browse files Browse the repository at this point in the history
…inal`. This addresses #9795. (#9862)
  • Loading branch information
erictraut authored Feb 9, 2025
1 parent 5b87b44 commit 7d18c57
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 46 deletions.
59 changes: 38 additions & 21 deletions packages/pyright-internal/src/analyzer/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6688,27 +6688,7 @@ export class Checker extends ParseTreeWalker {
const diagAddendum = new DiagnosticAddendum();

// Determine whether this is an attempt to override a method marked @final.
let reportFinalMethodOverride = false;

// Private names (starting with double underscore) are exempt from this check.
if (!SymbolNameUtils.isPrivateName(memberName)) {
if (isFunction(baseType) && FunctionType.isFinal(baseType)) {
reportFinalMethodOverride = true;
} else if (isOverloaded(baseType)) {
const overloads = OverloadedType.getOverloads(baseType);
const impl = OverloadedType.getImplementation(baseType);

if (overloads.some((overload) => FunctionType.isFinal(overload))) {
reportFinalMethodOverride = true;
}

if (impl && isFunction(impl) && FunctionType.isFinal(impl)) {
reportFinalMethodOverride = true;
}
}
}

if (reportFinalMethodOverride) {
if (this._isFinalFunction(memberName, baseType)) {
const decl = getLastTypedDeclarationForSymbol(overrideSymbol);
if (decl && decl.type === DeclarationType.Function) {
const diag = this._evaluator.addDiagnostic(
Expand Down Expand Up @@ -7009,6 +6989,31 @@ export class Checker extends ParseTreeWalker {
}
}

private _isFinalFunction(name: string, type: Type) {
if (SymbolNameUtils.isPrivateName(name)) {
return false;
}

if (isFunction(type) && FunctionType.isFinal(type)) {
return true;
}

if (isOverloaded(type)) {
const overloads = OverloadedType.getOverloads(type);
const impl = OverloadedType.getImplementation(type);

if (overloads.some((overload) => FunctionType.isFinal(overload))) {
return true;
}

if (impl && isFunction(impl) && FunctionType.isFinal(impl)) {
return true;
}
}

return false;
}

private _validatePropertyOverride(
baseClassType: ClassType,
childClassType: ClassType,
Expand Down Expand Up @@ -7069,6 +7074,18 @@ export class Checker extends ParseTreeWalker {
}

return;
} else if (this._isFinalFunction(methodName, baseClassPropMethod)) {
const decl = getLastTypedDeclarationForSymbol(overrideSymbol);
if (decl && decl.type === DeclarationType.Function) {
this._evaluator.addDiagnostic(
DiagnosticRule.reportIncompatibleMethodOverride,
LocMessage.finalMethodOverride().format({
name: memberName,
className: baseClassType.shared.name,
}),
decl.node.d.name
);
}
}

const subclassMethodType = partiallySpecializeType(
Expand Down
70 changes: 46 additions & 24 deletions packages/pyright-internal/src/tests/samples/final2.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,39 @@ def __func6(self):
pass

@overload
def func7(self, x: int) -> int:
...
def func7(self, x: int) -> int: ...

@overload
def func7(self, x: str) -> str:
...
def func7(self, x: str) -> str: ...

@final
def func7(self, x: int | str) -> int | str:
...
def func7(self, x: int | str) -> int | str: ...

# This should generate an error because the implementation
# of func8 is marked as not final but this overload is.
@overload
@final
def func8(self, x: int) -> int:
...
def func8(self, x: int) -> int: ...

@overload
def func8(self, x: str) -> str:
...
def func8(self, x: str) -> str: ...

def func8(self, x: int | str) -> int | str:
...
def func8(self, x: int | str) -> int | str: ...

@final
@property
def prop1(self) -> int: ...

@property
@final
def prop2(self) -> int: ...

@property
def prop3(self) -> int: ...

@prop3.setter
@final
def prop3(self, value: int) -> None: ...


# This should generate an error because func3 is final.
Expand Down Expand Up @@ -102,35 +111,48 @@ def __func6(self):
pass

@overload
def func7(self, x: int) -> int:
...
def func7(self, x: int) -> int: ...

@overload
def func7(self, x: str) -> str:
...
def func7(self, x: str) -> str: ...

@final
# This should generate an error because func7 is
# defined as final.
def func7(self, x: int | str) -> int | str:
...
def func7(self, x: int | str) -> int | str: ...

# This should generate an error because prop1 is
# defined as final.
@property
def prop1(self) -> int: ...

# This should generate an error because prop2 is
# defined as final.
@property
def prop2(self) -> int: ...

@property
def prop3(self) -> int: ...

# This should generate an error because prop3's setter is
# defined as final.
@prop3.setter
@final
def prop3(self, value: int) -> None: ...


class Base4:
...
class Base4: ...


class Base5:
@final
def __init__(self, v: int) -> None:
...
def __init__(self, v: int) -> None: ...


class C(Base4, Base5):
# This should generate an error because it overrides Base5,
# and __init__ is marked final there.
def __init__(self) -> None:
...
def __init__(self) -> None: ...


# This should generate an error because @final isn't allowed on
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/typeEvaluator4.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test('Final1', () => {

test('Final2', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['final2.py']);
TestUtils.validateResults(analysisResults, 12);
TestUtils.validateResults(analysisResults, 15);
});

test('Final3', () => {
Expand Down

0 comments on commit 7d18c57

Please sign in to comment.