Implemented type checks for property setters and deleters.

README.md

@@ -85,7 +85,6 @@ Pyright is a work in progress. The following functionality is not yet finished.
 * Validate that overridden methods in subclass have same signature as base class methods
 * Add support for type hints on var-arg parameters
 * Add support for NoReturn type
-* Revamp support for properties - model with Descriptor protocol, detect missing setter
 * Add support for f-strings
 * Provide switch that reports circular import dependencies
 * Add numeric codes to diagnostics and a configuration mechanism for disabling errors by code

server/src/analyzer/expressionEvaluator.ts

@@ -476,11 +476,15 @@ export class ExpressionEvaluator {
         } else if (baseType instanceof ClassType) {
             type = this._validateTypeFromClassMemberAccess(node.memberName,
                 baseType, usage, MemberAccessFlags.SkipInstanceMembers);
-            type = this._bindFunctionToClassOrObject(baseType, type);
+            if (type) {
+                type = this._bindFunctionToClassOrObject(baseType, type);
+            }
         } else if (baseType instanceof ObjectType) {
             type = this._validateTypeFromClassMemberAccess(
                 node.memberName, baseType.getClassType(), usage, MemberAccessFlags.None);
-            type = this._bindFunctionToClassOrObject(baseType, type);
+            if (type) {
+                type = this._bindFunctionToClassOrObject(baseType, type);
+            }
         } else if (baseType instanceof ModuleType) {
             let memberInfo = baseType.getFields().get(memberName);
             if (memberInfo) {
@@ -541,7 +545,7 @@ export class ExpressionEvaluator {
             }
 
             this._addError(
-                `Cannot ${ operationName } '${ memberName }' for type '${ baseType.asString() }'`,
+                `Cannot ${ operationName } member '${ memberName }' for type '${ baseType.asString() }'`,
                 node.memberName);
             type = UnknownType.create();
         }
@@ -626,7 +630,7 @@ export class ExpressionEvaluator {
     // A wrapper around _getTypeFromClassMemberName that reports
     // errors if the member name is not found.
     private _validateTypeFromClassMemberAccess(memberNameNode: NameNode,
-            classType: ClassType, usage: EvaluatorUsage, flags: MemberAccessFlags) {
+            classType: ClassType, usage: EvaluatorUsage, flags: MemberAccessFlags): Type | undefined {
 
         // If this is a special type (like "List") that has an alias
         // class (like "list"), switch to the alias, which defines
@@ -640,15 +644,7 @@ export class ExpressionEvaluator {
         let type = this._getTypeFromClassMemberName(
             memberName, classType, usage, flags);
 
-        if (type) {
-            return type;
-        }
-
-        this._addError(
-            `'${ memberName }' is not a known member of '${ classType.getObjectName() }'`,
-            memberNameNode);
-
-        return UnknownType.create();
+        return type;
     }
 
     private _getTypeFromClassMemberName(memberName: string, classType: ClassType,
@@ -679,7 +675,16 @@ export class ExpressionEvaluator {
 
             if (!(flags & MemberAccessFlags.SkipGetCheck)) {
                 if (type instanceof PropertyType) {
-                    type = conditionallySpecialize(type.getEffectiveReturnType(), classType);
+                    if (usage === EvaluatorUsage.Get) {
+                        type = conditionallySpecialize(type.getEffectiveReturnType(), classType);
+                    } else if (usage === EvaluatorUsage.Set) {
+                        // The type isn't important for set or delete usage.
+                        // We just need to return some defined type.
+                        return type.hasSetter() ? AnyType.create() : undefined;
+                    } else {
+                        assert(usage === EvaluatorUsage.Delete);
+                        return type.hasDeleter() ? AnyType.create() : undefined;
+                    }
                 } else if (type instanceof ObjectType) {
                     // See if there's a magic "__get__", "__set__", or "__delete__"
                     // method on the object.
@@ -696,9 +701,15 @@ export class ExpressionEvaluator {
                     const memberClassType = type.getClassType();
                     let getMember = TypeUtils.lookUpClassMember(memberClassType, accessMethodName, false);
                     if (getMember) {
-                        const getType = TypeUtils.getEffectiveTypeOfMember(getMember);
-                        if (getType instanceof FunctionType) {
-                            type = conditionallySpecialize(getType.getEffectiveReturnType(), memberClassType);
+                        const accessorType = TypeUtils.getEffectiveTypeOfMember(getMember);
+                        if (accessorType instanceof FunctionType) {
+                            if (usage === EvaluatorUsage.Get) {
+                                type = conditionallySpecialize(accessorType.getEffectiveReturnType(), memberClassType);
+                            } else {
+                                // The type isn't important for set or delete usage.
+                                // We just need to return some defined type.
+                                type = AnyType.create();
+                            }
                         }
                     }
                 }

server/src/analyzer/typeAnalyzer.ts

@@ -1302,6 +1302,22 @@ export class TypeAnalyzer extends ParseTreeWalker {
                 originalFunctionType.setIsAbstractMethod();
                 return inputFunctionType;
             }
+
+            // Handle property setters and deleters.
+            if (decoratorNode.leftExpression instanceof MemberAccessExpressionNode) {
+                const baseType = this._getTypeOfExpression(decoratorNode.leftExpression.leftExpression);
+                if (baseType instanceof PropertyType) {
+                    const memberName = decoratorNode.leftExpression.memberName.nameToken.value;
+                    if (memberName === 'setter') {
+                        baseType.setSetter(originalFunctionType);
+                        return baseType;
+                    } else if (memberName === 'deleter') {
+                        baseType.setDeleter(originalFunctionType);
+                        return baseType;
+                    }
+                }
+            }
+
         } else if (decoratorType instanceof ClassType) {
             if (decoratorType.isBuiltIn()) {
                 switch (decoratorType.getClassName()) {
@@ -1317,7 +1333,16 @@ export class TypeAnalyzer extends ParseTreeWalker {
 
                     case 'property': {
                         if (inputFunctionType instanceof FunctionType) {
-                            return new PropertyType(inputFunctionType);
+                            // Allocate a property only during the first analysis pass.
+                            // Otherwise the analysis won't converge if there are setters
+                            // and deleters applied to the property.
+                            const oldPropertyType = AnalyzerNodeInfo.getExpressionType(decoratorNode);
+                            if (oldPropertyType) {
+                                return oldPropertyType;
+                            }
+                            const newProperty = new PropertyType(inputFunctionType);
+                            AnalyzerNodeInfo.setExpressionType(decoratorNode, newProperty);
+                            return newProperty;
                         }
 
                         break;

server/src/tests/samples/properties1.py

@@ -0,0 +1,52 @@
+# This sample tests the type checker's ability to validate
+# properties.
+
+class ClassA(object):
+    @property
+    def read_only_prop(self):
+        return 1
+
+    @property
+    def read_write_prop(self):
+        return 'hello'
+    
+    @read_write_prop.setter
+    def read_write_prop(self, value: str):
+        return
+
+    @property
+    def deletable_prop(self):
+        return 1
+    
+    @deletable_prop.deleter
+    def deletable_prop(self):
+        return
+
+
+a = ClassA()
+
+val = a.read_only_prop
+
+# This should generate an error because this
+# property has no setter.
+a.read_only_prop = val
+
+# This should generate an error because this
+# property has no deleter. 
+del a.read_only_prop
+
+val = a.read_write_prop
+
+a.read_write_prop = 'hello'
+
+# This should generate an error because this
+# property has no deleter.
+del a.read_write_prop
+
+val = a.deletable_prop
+
+# This should generate an error because this
+# property has no setter.
+a.deletable_prop = val
+
+del a.deletable_prop

server/src/tests/typeAnalyzer.test.ts

@@ -175,6 +175,12 @@ test('Execution1', () => {
     validateResults(analysisResults, 2);
 });
 
+test('Properties1', () => {
+    let analysisResults = TestUtils.typeAnalyzeSampleFiles(['properties1.py']);
+
+    validateResults(analysisResults, 4);
+});
+
 test('Optional1', () => {
     const configOptions = new ConfigOptions('.');