Implemented reportOptionalOperand switch.

Implemented stricter type checking related to binary and unary operations.
This commit is contained in:
Eric Traut 2019-04-19 23:33:01 -07:00
parent 7b4c47f7fc
commit ce813deed2
8 changed files with 157 additions and 25 deletions

View File

@ -115,6 +115,12 @@
"title": "Controls reporting of attempts to use an Optional type as a parameter to a with statement",
"default": "none"
},
"reportOptionalOperand": {
"$id": "#/properties/reportOptionalOperand",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting of attempts to use an Optional type as an operand for a binary or unary operator",
"default": "none"
},
"reportUntypedFunctionDecorator": {
"$id": "#/properties/reportUntypedFunctionDecorator",
"$ref": "#/definitions/diagnostic",

View File

@ -46,6 +46,8 @@ The following settings control pyright's diagnostic output (warnings or errors).
**reportOptionalContextManager** [boolean or string, optional]: Generate or suppress diagnostics for an attempt to use an Optional type as a context manager (as a parameter to a `with` statement). The default value for this setting is 'none'.
**reportOptionalOperand** [boolean or string, optional]: Generate or suppress diagnostics for an attempt to use an Optional type as an operand to a binary or unary operator (like '+', '==', 'or', 'not'). The default value for this setting is 'none'.
**reportUntypedFunctionDecorator** [boolean or string, optional]: Generate or suppress diagnostics for function decorators that have no type annotations. These obscure the function type, defeating many type analysis features.
**reportUntypedClassDecorator** [boolean or string, optional]: Generate or suppress diagnostics for class decorators that have no type annotations. These obscure the class type, defeating many type analysis features.

View File

@ -1828,12 +1828,22 @@ export class ExpressionEvaluator {
const unaryOperatorMap: { [operator: number]: string } = {
[OperatorType.Add]: '__pos__',
[OperatorType.Subtract]: '__neg__',
[OperatorType.Not]: '__not__',
[OperatorType.BitwiseInvert]: '__inv__'
[OperatorType.BitwiseInvert]: '__invert__'
};
let type: Type | undefined;
if (node.operator !== OperatorType.Not) {
if (TypeUtils.isOptionalType(exprType)) {
this._addDiagnostic(
this._configOptions.reportOptionalOperand,
`Operator '${ ParseTreeUtils.printOperator(node.operator) }' not ` +
`supported for 'None' type`,
node.expression);
exprType = (exprType as UnionType).removeOptional();
}
}
// __not__ always returns a boolean.
if (node.operator === OperatorType.Not) {
type = ScopeUtils.getBuiltInObject(this._scope, 'bool');
@ -1842,13 +1852,14 @@ export class ExpressionEvaluator {
type = exprType;
} else {
const magicMethodName = unaryOperatorMap[node.operator];
type = this._getTypeFromMagicMethodReturn(exprType, magicMethodName);
type = this._getTypeFromMagicMethodReturn(exprType, [],
magicMethodName, node);
}
if (!type) {
this._addError(`Operator '${ ParseTreeUtils.printOperator(node.operator) }'` +
` not supported for type '${ exprType.asString() }'`,
node.expression);
node);
type = UnknownType.create();
}
}
@ -1857,7 +1868,6 @@ export class ExpressionEvaluator {
}
private _getTypeFromBinaryExpression(node: BinaryExpressionNode): TypeResult {
let leftType = this.getType(node.leftExpression);
// Is this an AND operator? If so, we can assume that the
@ -1880,7 +1890,7 @@ export class ExpressionEvaluator {
[OperatorType.FloorDivide]: ['__floordiv__', '__rfloordiv__', true],
[OperatorType.Divide]: ['__truediv__', '__rtruediv__', true],
[OperatorType.Mod]: ['__mod__', '__rmod__', true],
[OperatorType.Power]: ['__power__', '__rpower__', true],
[OperatorType.Power]: ['__pow__', '__rpow__', true],
[OperatorType.MatrixMultiply]: ['__matmul__', '', false]
};
@ -1912,6 +1922,35 @@ export class ExpressionEvaluator {
let type: Type | undefined;
// Optional checks apply to all operations except for boolean operations.
if (booleanOperatorMap[node.operator] === undefined) {
if (TypeUtils.isOptionalType(leftType)) {
// Skip the optional error reporting for == and !=, since
// None is a valid operand for these operators.
if (node.operator !== OperatorType.Equals && node.operator !== OperatorType.NotEquals) {
this._addDiagnostic(
this._configOptions.reportOptionalOperand,
`Operator '${ ParseTreeUtils.printOperator(node.operator) }' not ` +
`supported for 'None' type`,
node.leftExpression);
}
leftType = (leftType as UnionType).removeOptional();
}
if (TypeUtils.isOptionalType(rightType)) {
// Skip the optional error reporting for == and !=, since
// None is a valid operand for these operators.
if (node.operator !== OperatorType.Equals && node.operator !== OperatorType.NotEquals) {
this._addDiagnostic(
this._configOptions.reportOptionalOperand,
`Operator '${ ParseTreeUtils.printOperator(node.operator) }' not ` +
`supported for 'None' type`,
node.rightExpression);
}
rightType = (rightType as UnionType).removeOptional();
}
}
if (arithmeticOperatorMap[node.operator]) {
if (leftType.isAny() || rightType.isAny()) {
type = UnknownType.create();
@ -1953,7 +1992,14 @@ export class ExpressionEvaluator {
// Handle the general case.
if (!type) {
const magicMethodName = arithmeticOperatorMap[node.operator][0];
type = this._getTypeFromMagicMethodReturn(leftType, magicMethodName);
type = this._getTypeFromMagicMethodReturn(leftType, [rightType],
magicMethodName, node);
if (!type) {
const altMagicMethodName = arithmeticOperatorMap[node.operator][1];
type = this._getTypeFromMagicMethodReturn(rightType, [leftType],
altMagicMethodName, node);
}
}
} else if (bitwiseOperatorMap[node.operator]) {
if (leftType.isAny() || rightType.isAny()) {
@ -1973,7 +2019,8 @@ export class ExpressionEvaluator {
// Handle the general case.
if (!type) {
const magicMethodName = bitwiseOperatorMap[node.operator][0];
type = this._getTypeFromMagicMethodReturn(leftType, magicMethodName);
type = this._getTypeFromMagicMethodReturn(leftType, [rightType],
magicMethodName, node);
}
} else if (comparisonOperatorMap[node.operator]) {
if (leftType.isAny() || rightType.isAny()) {
@ -1981,8 +2028,8 @@ export class ExpressionEvaluator {
} else {
const magicMethodName = comparisonOperatorMap[node.operator];
type = this._getTypeFromMagicMethodReturn(leftType, magicMethodName,
ScopeUtils.getBuiltInObject(this._scope, 'bool'));
type = this._getTypeFromMagicMethodReturn(leftType, [rightType],
magicMethodName, node);
}
} else if (booleanOperatorMap[node.operator]) {
if (node.operator === OperatorType.And) {
@ -2000,32 +2047,76 @@ export class ExpressionEvaluator {
if (!type) {
this._addError(`Operator '${ ParseTreeUtils.printOperator(node.operator) }' not ` +
`supported for type '${ leftType.asString() }'`,
node.leftExpression);
`supported for types '${ leftType.asString() }' and '${ rightType.asString() }'`,
node);
type = UnknownType.create();
}
return { type, node };
}
private _getTypeFromMagicMethodReturn(objType: Type, magicMethodName: string,
fallbackType: Type | undefined = UnknownType.create()): Type | undefined {
private _getTypeFromMagicMethodReturn(objType: Type, args: Type[],
magicMethodName: string, errorNode: ExpressionNode): Type | undefined {
return TypeUtils.doForSubtypes(objType, subtype => {
let magicMethodSupported = true;
// Create a helper lambda for object subtypes.
let handleObjectSubtype = (subtype: ObjectType) => {
let magicMethodType = this._getTypeFromClassMemberName(subtype.getClassType(),
magicMethodName, EvaluatorUsage.Get, MemberAccessFlags.SkipForMethodLookup);
if (magicMethodType) {
let functionArgs = args.map(arg => {
return {
argumentCategory: ArgumentCategory.Simple,
type: arg
};
});
const callMethodType = TypeUtils.bindFunctionToClassOrObject(subtype, magicMethodType);
let returnType: Type | undefined;
this._silenceDiagnostics(() => {
returnType = this._validateCallArguments(errorNode,
functionArgs, callMethodType, new TypeVarMap());
});
if (!returnType) {
magicMethodSupported = false;
}
return returnType;
}
magicMethodSupported = false;
return undefined;
};
let returnType = TypeUtils.doForSubtypes(objType, subtype => {
if (subtype.isAny()) {
return UnknownType.create();
}
if (subtype instanceof ObjectType) {
let magicMethodType = this._getTypeFromClassMemberName(subtype.getClassType(),
magicMethodName, EvaluatorUsage.Get, MemberAccessFlags.SkipForMethodLookup);
if (magicMethodType && magicMethodType instanceof FunctionType) {
return magicMethodType.getEffectiveReturnType();
return handleObjectSubtype(subtype);
} else if (subtype instanceof NoneType) {
// NoneType derives from 'object', so do the lookup on 'object'
// in this case.
const obj = ScopeUtils.getBuiltInObject(this._scope, 'object');
if (obj instanceof ObjectType) {
return handleObjectSubtype(obj);
}
}
return fallbackType;
magicMethodSupported = false;
return undefined;
});
if (!magicMethodSupported) {
return undefined;
}
return returnType;
}
private _getBuiltInClassTypes(names: string[]): (ClassType | undefined)[] {

View File

@ -625,12 +625,12 @@ export class TypeAnalyzer extends ParseTreeWalker {
node.withItems.forEach(item => {
let exprType = this._getTypeOfExpression(item.expression);
if (exprType instanceof UnionType && exprType.getTypes().some(t => t instanceof NoneType)) {
if (TypeUtils.isOptionalType(exprType)) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalContextManager,
`Object of type 'None' cannot be used with 'with'`,
node);
exprType = exprType.removeOptional();
exprType = (exprType as UnionType).removeOptional();
}
const enterMethodName = node.isAsync ? '__aenter__' : '__enter__';
@ -941,6 +941,12 @@ export class TypeAnalyzer extends ParseTreeWalker {
let specializedBaseClass = TypeUtils.specializeType(aliasClass, undefined);
specialClassType.addBaseClass(specializedBaseClass, false);
} else {
// The other classes derive from 'object'.
let objBaseClass = ScopeUtils.getBuiltInType(this._currentScope, 'object');
if (objBaseClass instanceof ClassType) {
specialClassType.addBaseClass(objBaseClass, false);
}
}
specialType = specialClassType;

View File

@ -54,6 +54,14 @@ export enum ClassMemberLookupFlags {
}
export class TypeUtils {
static isOptionalType(type: Type): boolean {
if (type instanceof UnionType) {
return type.getTypes().some(t => t instanceof NoneType);
}
return false;
}
// Calls a callback for each subtype and combines the results
// into a final type.
static doForSubtypes(type: Type, callback: (type: Type) => (Type | undefined)): Type {

View File

@ -103,6 +103,9 @@ export class ConfigOptions {
// Report attempts to use an Optional type in a "with" statement?
reportOptionalContextManager: DiagnosticLevel = 'none';
// Report attempts to use an Optional type in a binary or unary operation.
reportOptionalOperand: DiagnosticLevel = 'none';
// Report untyped function decorators that obscure the function type?
reportUntypedFunctionDecorator: DiagnosticLevel = 'none';
@ -252,6 +255,10 @@ export class ConfigOptions {
this.reportOptionalContextManager = this._convertDiagnosticLevel(
configObj.reportOptionalContextManager, 'reportOptionalContextManager', 'none');
// Read the "reportOptionalOperand" entry.
this.reportOptionalOperand = this._convertDiagnosticLevel(
configObj.reportOptionalOperand, 'reportOptionalOperand', 'none');
// Read the "reportUntypedFunctionDecorator" entry.
this.reportUntypedFunctionDecorator = this._convertDiagnosticLevel(
configObj.reportUntypedFunctionDecorator, 'reportUntypedFunctionDecorator', 'none');

View File

@ -53,3 +53,13 @@ if 1:
with cm as val:
pass
# If "reportOptionalOperand" is enabled,
# this should generate 3 errors.
e = None
if 1:
e = 4
e + 4
4 + e
e < 5
not e

View File

@ -130,7 +130,7 @@ test('Specialization1', () => {
test('Expressions1', () => {
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['expressions1.py']);
validateResults(analysisResults, 2);
validateResults(analysisResults, 3);
});
test('Expressions2', () => {
@ -230,8 +230,9 @@ test('Optional1', () => {
configOptions.reportOptionalCall = 'warning';
configOptions.reportOptionalIterable = 'warning';
configOptions.reportOptionalContextManager = 'warning';
configOptions.reportOptionalOperand = 'warning';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['optional1.py'], configOptions);
validateResults(analysisResults, 0, 5);
validateResults(analysisResults, 0, 8);
// Turn on errors.
configOptions.reportOptionalSubscript = 'error';
@ -239,8 +240,9 @@ test('Optional1', () => {
configOptions.reportOptionalCall = 'error';
configOptions.reportOptionalIterable = 'error';
configOptions.reportOptionalContextManager = 'error';
configOptions.reportOptionalOperand = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['optional1.py'], configOptions);
validateResults(analysisResults, 5);
validateResults(analysisResults, 8);
});
test('Private1', () => {