Skip to content

Commit

Permalink
Fixed a bug that led to a false negative when using a non-TypedDict b…
Browse files Browse the repository at this point in the history
…ase class within a TypedDict class statement. This addresses #5937.
  • Loading branch information
msfterictraut committed Sep 13, 2023
1 parent 08bf803 commit de64cae
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 8 deletions.
28 changes: 21 additions & 7 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15935,13 +15935,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// a TypedDict, it is considered a TypedDict.
if (ClassType.isBuiltIn(argType, 'TypedDict') || ClassType.isTypedDictClass(argType)) {
classType.details.flags |= ClassTypeFlags.TypedDictClass;
} else if (ClassType.isTypedDictClass(classType) && !ClassType.isTypedDictClass(argType)) {
// Exempt Generic from this test. As of Python 3.11, generic TypedDict
// classes are supported.
if (!isInstantiableClass(argType) || !ClassType.isBuiltIn(argType, 'Generic')) {
// TypedDict classes must derive only from other TypedDict classes.
addError(Localizer.Diagnostic.typedDictBaseClass(), arg);
}
}

// Validate that the class isn't deriving from itself, creating a
Expand Down Expand Up @@ -16385,6 +16378,27 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions

// Synthesize TypedDict methods.
if (ClassType.isTypedDictClass(classType)) {
// TypedDict classes must derive only from other TypedDict classes.
let foundInvalidBaseClass = false;
const diag = new DiagnosticAddendum();

classType.details.baseClasses.forEach((baseClass) => {
if (
isClass(baseClass) &&
!ClassType.isTypedDictClass(baseClass) &&
!ClassType.isBuiltIn(baseClass, ['_TypedDict', 'Generic'])
) {
foundInvalidBaseClass = true;
diag.addMessage(
Localizer.DiagnosticAddendum.typedDictBaseClass().format({ type: baseClass.details.name })
);
}
});

if (foundInvalidBaseClass) {
addError(Localizer.Diagnostic.typedDictBaseClass() + diag.getString(), node.name);
}

synthesizeTypedDictClassMethods(
evaluatorInterface,
node,
Expand Down
2 changes: 2 additions & 0 deletions packages/pyright-internal/src/localization/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,8 @@ export namespace Localizer {
new ParameterizedString<{ type: string; name: string }>(
getRawString('DiagnosticAddendum.typeConstrainedTypeVar')
);
export const typedDictBaseClass = () =>
new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.typedDictBaseClass'));
export const typedDictFieldMissing = () =>
new ParameterizedString<{ name: string; type: string }>(
getRawString('DiagnosticAddendum.typedDictFieldMissing')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@
"typeAssignmentMismatch": "Type \"{sourceType}\" cannot be assigned to type \"{destType}\"",
"typeBound": "Type \"{sourceType}\" is incompatible with bound type \"{destType}\" for type variable \"{name}\"",
"typeConstrainedTypeVar": "Type \"{type}\" is incompatible with constrained type variable \"{name}\"",
"typedDictBaseClass": "Class \"{type}\" is not a TypedDict",
"typedDictFieldMissing": "\"{name}\" is missing from \"{type}\"",
"typedDictFieldNotReadOnly": "\"{name}\" is not read-only in \"{type}\"",
"typedDictFieldNotRequired": "\"{name}\" is not required in \"{type}\"",
Expand Down
6 changes: 6 additions & 0 deletions packages/pyright-internal/src/tests/samples/typedDict1.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,9 @@ class NotATD:
# base classes shouldn't be allowed for TD classes.
class TD6(TD3, NotATD):
pass


# This should generate an error because non-TypeDict
# base classes shouldn't be allowed for TD classes.
class TD7(NotATD, TypedDict):
pass
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/typeEvaluator2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1320,7 +1320,7 @@ test('Protocol44', () => {
test('TypedDict1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typedDict1.py']);

TestUtils.validateResults(analysisResults, 6);
TestUtils.validateResults(analysisResults, 7);
});

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

0 comments on commit de64cae

Please sign in to comment.