Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added missing check for the override of a property that is marked `@f… #9862

Merged
merged 1 commit into from
Feb 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading