Added new "reportMissingTypeArgument" diagnostic rule and enabled it by default in "strict" mode. It generates a diagnostic when a generic class or generic type alias is used in an annotation with no type arguments provided. Also fixed a bunch of inaccurate settings defaults in pyright's package.json.

This commit is contained in:
Eric Traut 2020-09-01 15:59:31 -07:00
parent 7f7ff556bc
commit 9e46548fc5
11 changed files with 200 additions and 38 deletions

View File

@ -106,6 +106,8 @@ The following settings control pyrights diagnostic output (warnings or errors
**reportUnknownMemberType** [boolean or string, optional]: Generate or suppress diagnostics for class or instance variables that have an unknown type. The default value for this setting is 'none'.
**reportMissingTypeArgument** [boolean or string, optional]: Generate or suppress diagnostics when a generic class is used without providing explicit or implicit type arguments. The default value for this setting is 'none'.
**reportCallInDefaultInitializer** [boolean or string, optional]: Generate or suppress diagnostics for function calls within a default value initialization expression. Such calls can mask expensive operations that are performed at module initialization time. The default value for this setting is 'none'.
**reportUnnecessaryIsInstance** [boolean or string, optional]: Generate or suppress diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true or always false. Such calls are often indicative of a programming error. The default value for this setting is 'none'.

View File

@ -190,6 +190,7 @@ import {
removeFalsinessFromType,
removeTruthinessFromType,
requiresSpecialization,
requiresTypeArguments,
selfSpecializeClassType,
setTypeArgumentsRecursive,
specializeType,
@ -291,6 +292,12 @@ export const enum EvaluatorFlags {
// The Generic class type is allowed in this context. It is
// normally not allowed if ExpectingType is set.
GenericClassTypeAllowed = 1 << 11,
// In most cases where ExpectingType is set, generic classes
// with missing type args are reported to the user, but there
// are cases where it is legitimate to leave off missing
// type args, such as with the "bound" parameter in a TypeArg.
AllowMissingTypeArgs = 1 << 12,
}
interface EvaluatorUsage {
@ -2956,6 +2963,21 @@ export function createTypeEvaluator(importLookup: ImportLookup, printTypeFlags:
if (!(flags & EvaluatorFlags.DoNotSpecialize)) {
if (isClass(type)) {
if (
(flags & EvaluatorFlags.ExpectingType) !== 0 &&
(flags & EvaluatorFlags.AllowMissingTypeArgs) === 0
) {
if (requiresTypeArguments(type) && !type.typeArguments) {
addDiagnostic(
fileInfo.diagnosticRuleSet.reportMissingTypeArgument,
DiagnosticRule.reportGeneralTypeIssues,
Localizer.Diagnostic.typeArgsMissingForClass().format({
name: type.details.name,
}),
node
);
}
}
if (!type.typeArguments) {
type = createSpecializedClassType(type, undefined, flags, node);
}
@ -2966,6 +2988,21 @@ export function createTypeEvaluator(importLookup: ImportLookup, printTypeFlags:
if (typeType) {
type = typeType;
}
} else if (
(flags & EvaluatorFlags.ExpectingType) !== 0 &&
type.typeAliasInfo &&
type.typeAliasInfo.typeParameters &&
type.typeAliasInfo.typeParameters.length > 0 &&
!type.typeAliasInfo.typeArguments
) {
addDiagnostic(
fileInfo.diagnosticRuleSet.reportMissingTypeArgument,
DiagnosticRule.reportGeneralTypeIssues,
Localizer.Diagnostic.typeArgsMissingForAlias().format({
name: type.typeAliasInfo.aliasName,
}),
node
);
}
}
@ -5818,7 +5855,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, printTypeFlags:
argList[i].valueExpression || errorNode
);
} else {
const argType = getTypeForArgumentExpectingType(argList[i], getFileInfo(errorNode));
const argType = getTypeForArgumentExpectingType(
argList[i],
getFileInfo(errorNode),
/* allowMissingTypeArgs */ true
);
if (requiresSpecialization(argType)) {
addError(Localizer.Diagnostic.typeVarGeneric(), argList[i].valueExpression || errorNode);
}
@ -11771,7 +11812,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, printTypeFlags:
// used in cases where the argument is expected to be a type
// and therefore follows the normal rules of types (e.g. they
// can be forward-declared in stubs, etc.).
function getTypeForArgumentExpectingType(arg: FunctionArgument, fileInfo: AnalyzerFileInfo): Type {
function getTypeForArgumentExpectingType(
arg: FunctionArgument,
fileInfo: AnalyzerFileInfo,
allowMissingTypeArgs = false
): Type {
if (arg.type) {
return arg.type;
}
@ -11785,6 +11830,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, printTypeFlags:
flags |= EvaluatorFlags.AllowForwardReferences;
}
if (allowMissingTypeArgs) {
flags |= EvaluatorFlags.AllowMissingTypeArgs;
}
// If there was no defined type provided, there should always
// be a value expression from which we can retrieve the type.
return getTypeOfExpression(arg.valueExpression!, undefined, flags).type;

View File

@ -1526,6 +1526,36 @@ function _getGeneratorReturnTypeArgs(returnType: Type): Type[] | undefined {
return undefined;
}
export function requiresTypeArguments(classType: ClassType) {
if (classType.details.typeParameters.length > 0) {
// If there are type parameters, type arguments are needed.
// The exception is if type parameters have been synthesized
// for classes that have untyped constructors.
return !classType.details.typeParameters[0].isSynthesized;
}
// There are a few built-in special classes that require
// type arguments even though typeParameters is empty.
if (ClassType.isBuiltIn(classType)) {
const specialClasses = [
'Tuple',
'Callable',
'Generic',
'Type',
'Optional',
'Union',
'Final',
'Literal',
'Annotated',
];
if (specialClasses.some((t) => t === classType.details.name)) {
return true;
}
}
return false;
}
export function requiresSpecialization(type: Type, recursionCount = 0): boolean {
switch (type.category) {
case TypeCategory.Class: {

View File

@ -170,6 +170,9 @@ export interface DiagnosticRuleSet {
// Report usage of unknown input or return parameters?
reportUnknownMemberType: DiagnosticLevel;
// Report usage of generic class without explicit type arguments?
reportMissingTypeArgument: DiagnosticLevel;
// Report usage of function call within default value
// initialization expression?
reportCallInDefaultInitializer: DiagnosticLevel;
@ -252,6 +255,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportUnknownLambdaType,
DiagnosticRule.reportUnknownVariableType,
DiagnosticRule.reportUnknownMemberType,
DiagnosticRule.reportMissingTypeArgument,
DiagnosticRule.reportCallInDefaultInitializer,
DiagnosticRule.reportUnnecessaryIsInstance,
DiagnosticRule.reportUnnecessaryCast,
@ -310,6 +314,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnknownLambdaType: 'none',
reportUnknownVariableType: 'none',
reportUnknownMemberType: 'none',
reportMissingTypeArgument: 'none',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'none',
reportUnnecessaryCast: 'none',
@ -364,6 +369,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnknownLambdaType: 'none',
reportUnknownVariableType: 'none',
reportUnknownMemberType: 'none',
reportMissingTypeArgument: 'none',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'none',
reportUnnecessaryCast: 'none',
@ -418,6 +424,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnknownLambdaType: 'error',
reportUnknownVariableType: 'error',
reportUnknownMemberType: 'error',
reportMissingTypeArgument: 'error',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'error',
reportUnnecessaryCast: 'error',
@ -943,6 +950,13 @@ export class ConfigOptions {
defaultSettings.reportUnknownMemberType
),
// Read the "reportMissingTypeArgument" entry.
reportMissingTypeArgument: this._convertDiagnosticLevel(
configObj.reportMissingTypeArgument,
DiagnosticRule.reportMissingTypeArgument,
defaultSettings.reportMissingTypeArgument
),
// Read the "reportCallInDefaultInitializer" entry.
reportCallInDefaultInitializer: this._convertDiagnosticLevel(
configObj.reportCallInDefaultInitializer,

View File

@ -46,6 +46,7 @@ export enum DiagnosticRule {
reportUnknownLambdaType = 'reportUnknownLambdaType',
reportUnknownVariableType = 'reportUnknownVariableType',
reportUnknownMemberType = 'reportUnknownMemberType',
reportMissingTypeArgument = 'reportMissingTypeArgument',
reportCallInDefaultInitializer = 'reportCallInDefaultInitializer',
reportUnnecessaryIsInstance = 'reportUnnecessaryIsInstance',
reportUnnecessaryCast = 'reportUnnecessaryCast',

View File

@ -515,6 +515,10 @@ export namespace Localizer {
export const typeArgsExpectingNone = () => getRawString('Diagnostic.typeArgsExpectingNone');
export const typeArgsMismatchOne = () =>
new ParameterizedString<{ received: number }>(getRawString('Diagnostic.typeArgsMismatchOne'));
export const typeArgsMissingForAlias = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.typeArgsMissingForAlias'));
export const typeArgsMissingForClass = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.typeArgsMissingForClass'));
export const typeArgsTooMany = () =>
new ParameterizedString<{ expected: number; received: number }>(getRawString('Diagnostic.typeArgsTooMany'));
export const typeAssignmentMismatch = () =>

View File

@ -250,6 +250,8 @@
"typeAliasRedeclared": "\"{name}\" is declared as a TypeAlias and can be assigned only once",
"typeArgsExpectingNone": "Expected no type arguments",
"typeArgsMismatchOne": "Expected one type argument but received {received}",
"typeArgsMissingForAlias": "Expected type arguments for generic type alias \"{name}\"",
"typeArgsMissingForClass": "Expected type arguments for generic class \"{name}\"",
"typeArgsTooMany": "Too many type arguments provided; expected {expected} but received {received}",
"typeAssignmentMismatch": "Expression of type \"{sourceType}\" cannot be assigned to declared type \"{destType}\"",
"typeCallNotAllowed": "type() call should not be used in type annotation",

View File

@ -1503,6 +1503,19 @@ test('GenericTypes32', () => {
validateResults(analysisResults, 0);
});
test('GenericTypes33', () => {
const configOptions = new ConfigOptions('.');
// By default, reportMissingTypeArgument is disabled.
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['genericTypes33.py']);
validateResults(analysisResults, 0);
// Turn on errors.
configOptions.diagnosticRuleSet.reportMissingTypeArgument = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['genericTypes33.py'], configOptions);
validateResults(analysisResults, 3);
});
test('Protocol1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['protocol1.py']);

View File

@ -0,0 +1,30 @@
# This sample tests the reportMissingTypeArgument diagnostic rule.
from typing import Generic, Optional, TypeVar, Union
_T = TypeVar("_T")
class Class1(Generic[_T]):
pass
# This should generate an error when reportMissingTypeArgument is enabled.
class Class2(Class1):
pass
# This should not generate an error.
_T2 = TypeVar("_T2", bound=Class1)
# This should generate an error when reportMissingTypeArgument is enabled.
var1: Optional[Class1] = None
GenericTypeAlias = Union[Class1[_T], int]
# This should generate an error when reportMissingTypeArgument is enabled.
var2: Optional[GenericTypeAlias] = None

View File

@ -119,7 +119,7 @@
"reportMissingImports": {
"type": "string",
"description": "Diagnostics for imports that have no corresponding imported python file or type stub file.",
"default": "warning",
"default": "error",
"enum": [
"none",
"information",
@ -141,7 +141,7 @@
"reportMissingTypeStubs": {
"type": "string",
"description": "Diagnostics for imports that have no corresponding type stub file (either a typeshed file or a custom type stub). The type checker requires type stubs to do its best job at analysis.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -152,7 +152,7 @@
"reportImportCycles": {
"type": "string",
"description": "Diagnostics for cyclical import chains. These are not errors in Python, but they do slow down type analysis and often hint at architectural layering issues. Generally, they should be avoided.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -163,7 +163,7 @@
"reportUnusedImport": {
"type": "string",
"description": "Diagnostics for an imported symbol that is not referenced within that file.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -174,7 +174,7 @@
"reportUnusedClass": {
"type": "string",
"description": "Diagnostics for a class with a private name (starting with an underscore) that is not accessed.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -185,7 +185,7 @@
"reportUnusedFunction": {
"type": "string",
"description": "Diagnostics for a function or method with a private name (starting with an underscore) that is not accessed.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -196,7 +196,7 @@
"reportUnusedVariable": {
"type": "string",
"description": "Diagnostics for a variable that is not accessed.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -207,7 +207,7 @@
"reportDuplicateImport": {
"type": "string",
"description": "Diagnostics for an imported symbol or module that is imported more than once.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -218,7 +218,7 @@
"reportOptionalSubscript": {
"type": "string",
"description": "Diagnostics for an attempt to subscript (index) a variable with an Optional type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -229,7 +229,7 @@
"reportOptionalMemberAccess": {
"type": "string",
"description": "Diagnostics for an attempt to access a member of a variable with an Optional type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -240,7 +240,7 @@
"reportOptionalCall": {
"type": "string",
"description": "Diagnostics for an attempt to call a variable with an Optional type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -251,7 +251,7 @@
"reportOptionalIterable": {
"type": "string",
"description": "Diagnostics for an attempt to use an Optional type as an iterable value (e.g. within a for statement).",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -262,7 +262,7 @@
"reportOptionalContextManager": {
"type": "string",
"description": "Diagnostics for an attempt to use an Optional type as a context manager (as a parameter to a with statement).",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -273,7 +273,7 @@
"reportOptionalOperand": {
"type": "string",
"description": "Diagnostics for an attempt to use an Optional type as an operand to a binary or unary operator (like '+', '==', 'or', 'not').",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -284,7 +284,7 @@
"reportUntypedFunctionDecorator": {
"type": "string",
"description": "Diagnostics for function decorators that have no type annotations. These obscure the function type, defeating many type analysis features.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -295,7 +295,7 @@
"reportUntypedClassDecorator": {
"type": "string",
"description": "Diagnostics for class decorators that have no type annotations. These obscure the class type, defeating many type analysis features.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -306,7 +306,7 @@
"reportUntypedBaseClass": {
"type": "string",
"description": "Diagnostics for base classes whose type cannot be determined statically. These obscure the class type, defeating many type analysis features.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -317,7 +317,7 @@
"reportUntypedNamedTuple": {
"type": "string",
"description": "Diagnostics when “namedtuple” is used rather than “NamedTuple”. The former contains no type information, whereas the latter does.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -328,7 +328,7 @@
"reportPrivateUsage": {
"type": "string",
"description": "Diagnostics for incorrect usage of private or protected variables or functions. Protected class members begin with a single underscore _ and can be accessed only by subclasses. Private class members begin with a double underscore but do not end in a double underscore and can be accessed only within the declaring class. Variables and functions declared outside of a class are considered private if their names start with either a single or double underscore, and they cannot be accessed outside of the declaring module.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -339,7 +339,7 @@
"reportConstantRedefinition": {
"type": "string",
"description": "Diagnostics for attempts to redefine variables whose names are all-caps with underscores and numerals.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -350,7 +350,7 @@
"reportIncompatibleMethodOverride": {
"type": "string",
"description": "Diagnostics for methods that override a method of the same name in a base class in an incompatible manner (wrong number of parameters, incompatible parameter types, or incompatible return type).",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -361,7 +361,7 @@
"reportIncompatibleVariableOverride": {
"type": "string",
"description": "Diagnostics for overrides in subclasses that redefine a variable in an incompatible way.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -383,7 +383,7 @@
"reportUnknownParameterType": {
"type": "string",
"description": "Diagnostics for input or return parameters for functions or methods that have an unknown type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -394,7 +394,7 @@
"reportUnknownArgumentType": {
"type": "string",
"description": "Diagnostics for call arguments for functions or methods that have an unknown type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -405,7 +405,7 @@
"reportUnknownLambdaType": {
"type": "string",
"description": "Diagnostics for input or return parameters for lambdas that have an unknown type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -416,7 +416,7 @@
"reportUnknownVariableType": {
"type": "string",
"description": "Diagnostics for variables that have an unknown type..",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -427,7 +427,18 @@
"reportUnknownMemberType": {
"type": "string",
"description": "Diagnostics for class or instance variables that have an unknown type.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
"warning",
"error"
]
},
"reportMissingTypeArgument": {
"type": "string",
"description": "Diagnostics for generic class reference with missing type arguments.",
"default": "none",
"enum": [
"none",
"information",
@ -438,7 +449,7 @@
"reportCallInDefaultInitializer": {
"type": "string",
"description": "Diagnostics for function calls within a default value initialization expression. Such calls can mask expensive operations that are performed at module initialization time.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -449,7 +460,7 @@
"reportUnnecessaryIsInstance": {
"type": "string",
"description": "Diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true or always false. Such calls are often indicative of a programming error.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -460,7 +471,7 @@
"reportUnnecessaryCast": {
"type": "string",
"description": "Diagnostics for 'cast' calls that are statically determined to be unnecessary. Such calls are sometimes indicative of a programming error.",
"default": "information",
"default": "none",
"enum": [
"none",
"information",
@ -471,7 +482,7 @@
"reportAssertAlwaysTrue": {
"type": "string",
"description": "Diagnostics for 'assert' statement that will provably always assert. This can be indicative of a programming error.",
"default": "information",
"default": "warning",
"enum": [
"none",
"information",
@ -482,7 +493,7 @@
"reportSelfClsParameterName": {
"type": "string",
"description": "Diagnostics for a missing or misnamed “self” parameter in instance methods and “cls” parameter in class methods. Instance methods in metaclasses (classes that derive from “type”) are allowed to use “cls” for instance methods.",
"default": "information",
"default": "warning",
"enum": [
"none",
"information",
@ -493,7 +504,7 @@
"reportImplicitStringConcatenation": {
"type": "string",
"description": "Diagnostics for two or more string literals that follow each other, indicating an implicit concatenation. This is considered a bad practice and often masks bugs such as missing commas.",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -504,7 +515,7 @@
"reportInvalidStubStatement": {
"type": "string",
"description": "Diagnostics for type stub statements that do not conform to PEP 484",
"default": "warning",
"default": "none",
"enum": [
"none",
"information",
@ -515,7 +526,7 @@
"reportUndefinedVariable": {
"type": "string",
"description": "Diagnostics for a missing or misnamed “self” parameter in instance methods and “cls” parameter in class methods. Instance methods in metaclasses (classes that derive from “type”) are allowed to use “cls” for instance methods.",
"default": "warning",
"default": "error",
"enum": [
"none",
"information",
@ -526,7 +537,7 @@
"reportUnboundVariable": {
"type": "string",
"description": "Diagnostics for unbound and possibly unbound variables.",
"default": "information",
"default": "error",
"enum": [
"none",
"information",

View File

@ -303,6 +303,12 @@
"title": "Controls reporting class and instance variables whose types are unknown",
"default": "none"
},
"reportMissingTypeArgument": {
"$id": "#/properties/reportMissingTypeArgument",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting generic class reference with missing type arguments",
"default": "none"
},
"reportCallInDefaultInitializer": {
"$id": "#/properties/reportCallInDefaultInitializer",
"$ref": "#/definitions/diagnostic",