Prefer type from parameter annotation to the type of default value (#1159)

* Treat Any return as unknown

* Reverse condition

* Prefer annotation to default value

* Restore some code

* Merge issue

Author: Mikhail Arkhipov

src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs

@@ -51,35 +51,14 @@ public override void Evaluate() {
                        || Module.ModuleType == ModuleType.Specialized;
 
             using (Eval.OpenScope(_function.DeclaringModule, FunctionDefinition, out _)) {
-                // Process annotations.
-                var annotationType = Eval.GetTypeFromAnnotation(FunctionDefinition.ReturnAnnotation);
-                if (!annotationType.IsUnknown()) {
-                    // Annotations are typically types while actually functions return
-                    // instances unless specifically annotated to a type such as Type[T].
-                    var t = annotationType.CreateInstance(annotationType.Name, ArgumentSet.Empty);
-                    // If instance could not be create, such as when return type is List[T] and 
-                    // type of T is not yet known, just use the type.
-                    var instance = t.IsUnknown() ? annotationType : t;
-                    _overload.SetReturnValue(instance, true);
-                } else {
-                    // Check if function is a generator
-                    var suite = FunctionDefinition.Body as SuiteStatement;
-                    var yieldExpr = suite?.Statements.OfType<ExpressionStatement>().Select(s => s.Expression as YieldExpression).ExcludeDefault().FirstOrDefault();
-                    if (yieldExpr != null) {
-                        // Function return is an iterator
-                        var yieldValue = Eval.GetValueFromExpression(yieldExpr.Expression) ?? Eval.UnknownType;
-                        var returnValue = new PythonGenerator(Eval.Interpreter, yieldValue);
-                        _overload.SetReturnValue(returnValue, true);
-                    }
-                }
-
+                var returnType = TryDetermineReturnValue();
                 DeclareParameters(!stub);
 
                 // Do process body of constructors since they may be declaring
                 // variables that are later used to determine return type of other
                 // methods and properties.
                 var ctor = _function.Name.EqualsOrdinal("__init__") || _function.Name.EqualsOrdinal("__new__");
-                if (ctor || annotationType.IsUnknown() || Module.ModuleType == ModuleType.User) {
+                if (ctor || returnType.IsUnknown() || Module.ModuleType == ModuleType.User) {
                     // Return type from the annotation is sufficient for libraries and stubs, no need to walk the body.
                     FunctionDefinition.Body?.Walk(this);
                     // For libraries remove declared local function variables to free up some memory.
@@ -129,6 +108,30 @@ public override bool Walk(FunctionDefinition node) {
             return false;
         }
 
+        private IPythonType TryDetermineReturnValue() {
+            var annotationType = Eval.GetTypeFromAnnotation(FunctionDefinition.ReturnAnnotation);
+            if (!annotationType.IsUnknown()) {
+                // Annotations are typically types while actually functions return
+                // instances unless specifically annotated to a type such as Type[T].
+                var t = annotationType.CreateInstance(annotationType.Name, ArgumentSet.Empty);
+                // If instance could not be created, such as when return type is List[T] and
+                // type of T is not yet known, just use the type.
+                 var instance = t.IsUnknown() ? annotationType : t;
+                _overload.SetReturnValue(instance, true); _overload.SetReturnValue(instance, true);
+            } else {
+                // Check if function is a generator
+                var suite = FunctionDefinition.Body as SuiteStatement;
+                var yieldExpr = suite?.Statements.OfType<ExpressionStatement>().Select(s => s.Expression as YieldExpression).ExcludeDefault().FirstOrDefault();
+                if (yieldExpr != null) {
+                    // Function return is an iterator
+                    var yieldValue = Eval.GetValueFromExpression(yieldExpr.Expression) ?? Eval.UnknownType;
+                    var returnValue = new PythonGenerator(Eval.Interpreter, yieldValue);
+                    _overload.SetReturnValue(returnValue, true);
+                }
+            }
+            return annotationType;
+        }
+
         private void DeclareParameters(bool declareVariables) {
             // For class method no need to add extra parameters, but first parameter type should be the class.
             // For static and unbound methods do not add or set anything.
@@ -157,8 +160,8 @@ private void DeclareParameters(bool declareVariables) {
             for (var i = skip; i < FunctionDefinition.Parameters.Length; i++) {
                 var p = FunctionDefinition.Parameters[i];
                 if (!string.IsNullOrEmpty(p.Name)) {
-                    IPythonType paramType = null;
-                    if (p.DefaultValue != null) {
+                    var paramType = Eval.GetTypeFromAnnotation(p.Annotation);
+                    if (paramType.IsUnknown() && p.DefaultValue != null) {
                         defaultValue = Eval.GetValueFromExpression(p.DefaultValue);
                         // If parameter has default value, look for the annotation locally first
                         // since outer type may be getting redefined. Consider 's = None; def f(s: s = 123): ...

src/Analysis/Ast/Test/FunctionTests.cs

@@ -13,7 +13,6 @@
 // See the Apache Version 2.0 License for specific language governing
 // permissions and limitations under the License.
 
-using System;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
@@ -589,5 +588,24 @@ def b(self): pass
             analysis.Should().NotHaveVariable("func");
             analysis.Should().HaveVariable("B").Which.Should().NotHaveMember("b");
         }
+
+        [TestMethod, Priority(0)]
+        public async Task AnnotatedParameterPriority() {
+            const string code = @"
+class Foo:
+    def __init__(self, name: str):
+        self.name = name
+
+def func() -> int:
+    return Foo
+
+def test(foo: Foo = func()):
+    return foo
+
+x = test()
+";
+            var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X);
+            analysis.Should().HaveVariable("x").OfType("Foo");
+        }
     }
 }

src/LanguageServer/Test/CompletionTests.cs

@@ -1172,5 +1172,27 @@ def func(self):
             result = cs.GetCompletions(analysis, new SourceLocation(9, 5));
             result.Completions.Select(c => c.label).Should().NotContain("_A__x").And.NotContain("__x");
         }
+
+        [TestMethod, Priority(0)]
+        public async Task ParameterAnnotatedDefault() {
+            const string code = @"
+from typing import Any
+
+class Foo:
+    z: int
+    def __init__(self, name: str):
+        self.name = name
+
+def func() -> Any:
+    return 123
+
+def test(x: Foo = func()):
+    x.
+";
+            var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X);
+            var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion);
+            var comps = cs.GetCompletions(analysis, new SourceLocation(13, 7));
+            comps.Should().HaveLabels("name", "z");
+        }
     }
 }