Skip to content

Commit

Permalink
Added error reporting for the situation where a generic instance vari… (
Browse files Browse the repository at this point in the history
microsoft#7057)

* Added error reporting for the situation where a generic instance variable is accessed through a class object. This addresses microsoft#7051.

* Fixed style issue.
  • Loading branch information
erictraut committed Jan 20, 2024
1 parent f10455e commit 335255f
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 6 deletions.
41 changes: 37 additions & 4 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
let effectiveFlags = flags;

if (objectTypeIsInstantiable) {
effectiveFlags |= MemberAccessFlags.SkipInstanceMembers | MemberAccessFlags.SkipAttributeAccessOverride;
effectiveFlags |=
MemberAccessFlags.SkipInstanceMembers |
MemberAccessFlags.SkipAttributeAccessOverride |
MemberAccessFlags.DisallowGenericInstanceVariableAccess;
effectiveFlags &= ~MemberAccessFlags.SkipClassMembers;
} else {
effectiveFlags |= MemberAccessFlags.DisallowClassVarWrites;
Expand Down Expand Up @@ -5593,7 +5596,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// clear the flag that causes an error to be generated.
if (usage.method === 'set' && memberInfo.symbol.isClassVar() && isAccessedThroughObject) {
const selfClass = selfType ?? memberName === '__new__' ? undefined : classType;
const typeResult = getTypeOfMemberInternal(memberInfo, selfClass);
const typeResult = getTypeOfMemberInternal(errorNode, memberInfo, selfClass, flags);

if (typeResult) {
if (isDescriptorInstance(typeResult.type, /* requireSetter */ true)) {
Expand Down Expand Up @@ -5623,7 +5626,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}
}

const typeResult = getTypeOfMemberInternal(memberInfo, selfClass);
const typeResult = getTypeOfMemberInternal(errorNode, memberInfo, selfClass, flags);

type = typeResult?.type ?? UnknownType.create();
if (typeResult?.isIncomplete) {
Expand Down Expand Up @@ -21717,8 +21720,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}

function getTypeOfMemberInternal(
errorNode: ExpressionNode | undefined,
member: ClassMember,
selfClass: ClassType | TypeVarType | undefined
selfClass: ClassType | TypeVarType | undefined,
flags: MemberAccessFlags
): TypeResult | undefined {
if (isInstantiableClass(member.classType)) {
const typeResult = getEffectiveTypeOfSymbolForUsage(member.symbol);
Expand All @@ -21729,6 +21734,34 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// prior to specializing.
inferReturnTypeIfNecessary(typeResult.type);

if (
member.isInstanceMember &&
(flags & MemberAccessFlags.DisallowGenericInstanceVariableAccess) !== 0
) {
let isGenericNonCallable = false;

doForEachSubtype(typeResult.type, (subtype) => {
if (!isAnyOrUnknown(subtype) && !isFunction(subtype) && !isOverloadedFunction(subtype)) {
if (
requiresSpecialization(typeResult.type, {
ignoreSelf: true,
ignoreImplicitTypeArgs: true,
})
) {
isGenericNonCallable = true;
}
}
});

if (isGenericNonCallable && errorNode) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.genericInstanceVariableAccess(),
errorNode
);
}
}

return {
type: partiallySpecializeType(typeResult.type, member.classType, selfClass),
isIncomplete: !!typeResult.isIncomplete,
Expand Down
4 changes: 4 additions & 0 deletions packages/pyright-internal/src/analyzer/typeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ export const enum MemberAccessFlags {
// (__getattr__, etc.) may provide the missing attribute type.
// This disables this check.
SkipAttributeAccessOverride = 1 << 9,

// Report an error if a symbol is an instance variable whose
// type is parameterized by a class TypeVar.
DisallowGenericInstanceVariableAccess = 1 << 10,
}

export const enum ClassIteratorFlags {
Expand Down
1 change: 1 addition & 0 deletions packages/pyright-internal/src/localization/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ export namespace Localizer {
export const genericBaseClassNotAllowed = () => getRawString('Diagnostic.genericBaseClassNotAllowed');
export const genericClassAssigned = () => getRawString('Diagnostic.genericClassAssigned');
export const genericClassDeleted = () => getRawString('Diagnostic.genericClassDeleted');
export const genericInstanceVariableAccess = () => getRawString('Diagnostic.genericInstanceVariableAccess');
export const genericNotAllowed = () => getRawString('Diagnostic.genericNotAllowed');
export const genericTypeAliasBoundTypeVar = () =>
new ParameterizedString<{ names: string }>(getRawString('Diagnostic.genericTypeAliasBoundTypeVar'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@
"genericBaseClassNotAllowed": "\"Generic\" base class cannot be used with type parameter syntax",
"genericClassAssigned": "Generic class type cannot be assigned",
"genericClassDeleted": "Generic class type cannot be deleted",
"genericInstanceVariableAccess": "Access to generic instance variable through class is ambiguous",
"genericNotAllowed": "\"Generic\" is not valid in this context",
"genericTypeAliasBoundTypeVar": "Generic type alias within class cannot use bound type variables {names}",
"genericTypeArgMissing": "\"Generic\" requires at least one type argument",
Expand Down
32 changes: 32 additions & 0 deletions packages/pyright-internal/src/tests/samples/memberAccess1.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,35 @@ def method3(cls):
cls.method1,
expected_text="Decorator[Self@ClassF, (a: int, *, b: str), str]",
)


class ClassG(Generic[_T]):
x: _T
y: int

def __init__(self, label: _T | None = None) -> None:
...


ClassG[int].y = 1
ClassG[int].y
del ClassG[int].y

ClassG.y = 1
ClassG.y
del ClassG.y

# This should generate an error because x is generic.
ClassG[int].x = 1

# This should generate an error because x is generic.
ClassG[int].x

# This should generate an error because x is generic.
del ClassG[int].x

# This should generate an error because x is generic.
ClassG.x

# This should generate an error because x is generic.
del ClassG.x
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/samples/unions1.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Baz(Generic[T]):
qux: T | None

reveal_type(helper(a), expected_text="str | None")
reveal_type(Baz[str].qux, expected_text="str | None")
reveal_type(Baz[str]().qux, expected_text="str | None")


T = TypeVar("T")
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 @@ -448,7 +448,7 @@ test('FString5', () => {

test('MemberAccess1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['memberAccess1.py']);
TestUtils.validateResults(analysisResults, 0);
TestUtils.validateResults(analysisResults, 5);
});

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

0 comments on commit 335255f

Please sign in to comment.