Scopes

Author: Mikhail Arkhipov

src/Analysis/Engine/Impl/AnalysisVariable.cs

@@ -16,20 +16,18 @@
 
 namespace Microsoft.PythonTools.Analysis {
     class AnalysisVariable : IAnalysisVariable {
-        public AnalysisVariable(VariableType type, ILocationInfo location, int? version = null) {
+        public AnalysisVariable(IVariableDefinition variable, VariableType type, ILocationInfo location, int? version = null) {
+            Variable = variable;
             Location = location;
             Type = type;
             Version = version;
         }
 
         #region IAnalysisVariable Members
-
         public ILocationInfo Location { get; }
-
         public VariableType Type { get; }
-
         public int? Version { get; }
-
+        public IVariableDefinition Variable { get; }
         #endregion
 
         public override bool Equals(object obj) {

src/Analysis/Engine/Impl/Definitions/IAnalysisVariable.cs

@@ -22,5 +22,7 @@ public interface IAnalysisVariable {
         ILocationInfo Location { get; }
 
         VariableType Type { get; }
+
+        IVariableDefinition Variable { get; }
     }
 }

src/Analysis/Engine/Impl/Infrastructure/Extensions/HashSetExtensions.cs

@@ -0,0 +1,27 @@
+// Python Tools for Visual Studio
+// Copyright(c) Microsoft Corporation
+// All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the License); you may not use
+// this file except in compliance with the License. You may obtain a copy of the
+// License at http://www.apache.org/licenses/LICENSE-2.0
+//
+// THIS CODE IS PROVIDED ON AN  *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
+// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
+// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
+// MERCHANTABLITY OR NON-INFRINGEMENT.
+//
+// See the Apache Version 2.0 License for specific language governing
+// permissions and limitations under the License.
+
+using System.Collections.Generic;
+
+namespace Microsoft.PythonTools.Analysis.Infrastructure {
+    static class HashSetExtensions {
+         public static void Add<T>(this HashSet<T> set, IEnumerable<T> source) {
+            foreach(var e in source) {
+                set.Add(e);
+            }
+        }
+    }
+}

src/Analysis/Engine/Impl/ModuleAnalysis.cs

@@ -114,7 +114,9 @@ private static IAnalysisSet ResolveAndAdd(AnalysisUnit unit, IAnalysisSet set, A
         }
 
         internal IEnumerable<AnalysisVariable> ToVariables(IReferenceable referenceable) {
-            if (referenceable is VariableDef def) {
+            var def = referenceable as VariableDef;
+
+            if (def != null) {
                 foreach (var type in def.Types.WhereNotNull()) {
                     var varType = VariableType.Value;
                     if (type.DeclaringModule == null) {
@@ -123,22 +125,22 @@ internal IEnumerable<AnalysisVariable> ToVariables(IReferenceable referenceable)
                     }
 
                     foreach (var loc in type.Locations.WhereNotNull()) {
-                        yield return new AnalysisVariable(varType, loc);
+                        yield return new AnalysisVariable(def, varType, loc);
                     }
                 }
             }
 
             foreach (var reference in referenceable.Definitions) {
                 var loc = reference.GetLocationInfo();
                 if (loc != null) {
-                    yield return new AnalysisVariable(VariableType.Definition, loc);
+                    yield return new AnalysisVariable(def, VariableType.Definition, loc);
                 }
             }
 
             foreach (var reference in referenceable.References) {
                 var loc = reference.GetLocationInfo();
                 if (loc != null) {
-                    yield return new AnalysisVariable(VariableType.Reference, loc);
+                    yield return new AnalysisVariable(def, VariableType.Reference, loc);
                 }
             }
         }
@@ -234,23 +236,33 @@ public VariablesResult GetVariables(Expression expr, SourceLocation location, st
                 if (!scope.EnumerateTowardsGlobal.Any()) {
                     variables = _unit.State.BuiltinModule.GetDefinitions(name.Name).SelectMany(ToVariables);
                 } else {
-                    foreach (var defScope in scope.EnumerateTowardsGlobal
-                        .Where(s => s.ContainsVariable(name.Name) && (s == scope || s.VisibleToChildren || IsFirstLineOfFunction(scope, s, location)))) {
-                        var scopeVariables = GetVariablesInScope(name, defScope).Distinct();
-                        // Filter our definitions that are below the requested location (such as reassignments)
-                        var above = scopeVariables.Where(v => new SourceLocation(v.Location.StartLine, v.Location.StartColumn) <= location);
-                        variables = variables.Union(above);
-                        
-                        // Break at the first definition so we don't spill into global scope
-                        // for similarly named function parameters.
-                        if (scopeVariables.Any(v => v.Type == VariableType.Definition)) {
+                    foreach (var s in scope.EnumerateTowardsGlobal.Where(s => s.VisibleToChildren)) {
+                        var scopeVariables = GetVariablesInScope(name, s).Distinct();
+                        var args = scopeVariables.Where(v => IsFunctionArgument(v.Variable));
+                        if(args.Any()) {
+                            variables = variables.Union(args);
                             break;
                         }
+                        variables = variables.Union(scopeVariables);
                     }
+
+                    // Now take outermost definition and treat inner ones (such as reassignments) as references
+                    var others = variables.Where(v => v.Type == VariableType.Reference || v.Type == VariableType.Value);
+                    var definitions = variables
+                        .Where(v => v.Type == VariableType.Definition)
+                        .OrderBy(v => v.Location.Span.Start)
+                        .ToArray();
+
+                    if (definitions.Length > 0) {
+                        var defsToRefs = definitions.Skip(1).Select(v => new AnalysisVariable(v.Variable, VariableType.Reference, v.Location));
+                        variables = definitions.Take(1).Concat(others.Concat(defsToRefs));
+                     }
                 }
-            } else if (expr is MemberExpression member && !string.IsNullOrEmpty(member.Name)) {
-                var objects = eval.Evaluate(member.Target);
+                return new VariablesResult(variables, unit.Tree);
+            }
 
+            if (expr is MemberExpression member && !string.IsNullOrEmpty(member.Name)) {
+                var objects = eval.Evaluate(member.Target);
                 foreach (var v in objects.OfType<IReferenceableContainer>()) {
                     variables = variables.Union(v.GetDefinitions(member.Name).SelectMany(ToVariables));
                 }
@@ -259,6 +271,9 @@ public VariablesResult GetVariables(Expression expr, SourceLocation location, st
             return new VariablesResult(variables, unit.Tree);
         }
 
+        private bool IsFunctionArgument(IVariableDefinition v)
+            => v?.Types?.MaybeEnumerate().FirstOrDefault() is ParameterInfo;
+
         private IEnumerable<IAnalysisVariable> GetVariablesInScope(NameExpression name, IScope scope) {
             var result = new List<IAnalysisVariable>();
 
@@ -297,19 +312,6 @@ private IEnumerable<IAnalysisVariable> GetVariablesInScope(NameExpression name,
             return result;
         }
 
-        private static bool IsFirstLineOfFunction(IScope innerScope, IScope outerScope, SourceLocation location) {
-            if (innerScope.OuterScope == outerScope && innerScope is FunctionScope) {
-                var funcScope = (FunctionScope)innerScope;
-                var def = funcScope.Function.FunctionDefinition;
-
-                // TODO: Use indexes rather than lines to check location
-                if (location.Line == def.GetStart(def.GlobalParent).Line) {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         private class ErrorWalker : PythonWalker {
             public bool HasError { get; private set; }