Take into account scopes when checking definition order (#1077)

Author: Mikhail Arkhipov

src/Analysis/Ast/Impl/Extensions/ScopeExtensions.cs

@@ -13,6 +13,7 @@
 // See the Apache Version 2.0 License for specific language governing
 // permissions and limitations under the License.
 
+using System.Linq;
 using Microsoft.Python.Analysis.Documents;
 using Microsoft.Python.Analysis.Values;
 using Microsoft.Python.Core.Text;
@@ -31,6 +32,9 @@ public static int GetBodyStartIndex(this IScope scope, PythonAst ast) {
             }
         }
 
+        public static bool IsNestedInScope(this IScope s, IScope outer) 
+            => s.OuterScope != null && s.OuterScope.EnumerateTowardsGlobal.Any(x => x == outer);
+
         public static IScope FindScope(this IScope parent, IDocument document, SourceLocation location) {
             var children = parent.Children;
             var ast = document.Analysis.Ast;

src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs

@@ -15,6 +15,7 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using Microsoft.Python.Analysis.Analyzer;
 using Microsoft.Python.Analysis.Types;
 using Microsoft.Python.Core;
 using Microsoft.Python.Core.Text;
@@ -77,27 +78,42 @@ public override bool Walk(NameExpression node) {
             }
 
             var analysis = _walker.Analysis;
-            var m = analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out _, out var v);
+            var m = analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out var variableDefinitionScope, out var v);
             if (m == null) {
                 _walker.ReportUndefinedVariable(node);
             }
             v?.AddReference(new Location(analysis.Document, node.IndexSpan));
 
+            // Make sure we have definition and the document matches
+            if (v?.Definition == null || v.Definition.DocumentUri != analysis.Document.Uri) {
+                return false;
+            }
+            // Do not complain about functions and classes that appear later in the file
+            if (v.Value is IPythonFunctionType || v.Value is IPythonClassType) {
+                return false;
+            }
+
             // Take into account where variable is defined so we do detect
             // undefined x in 
             //    y = x
             //    x = 1
-            if (v?.Definition != null && v.Definition.DocumentUri == analysis.Document.Uri) {
-                // Do not complain about functions and classes that appear later in the file
-                if (!(v.Value is IPythonFunctionType || v.Value is IPythonClassType)) {
-                    var span = v.Definition.Span;
-                    var nodeLoc = node.GetLocation(analysis.Document);
-                    // Exclude same-name variables declared within the same statement
-                    // like 'e' that appears before its declaration in '[e in for e in {}]'
-                    if (span.IsAfter(nodeLoc.Span) && !IsSpanInComprehension(nodeLoc.Span)) {
-                        _walker.ReportUndefinedVariable(node);
-                    }
-                }
+            var variableDefinitionSpan = v.Definition.Span;
+            var nameUseLocation = node.GetLocation(analysis.Document);
+
+            // Make sure we are in the same scope in order to avoid
+            //    def func():
+            //      y = x
+            //
+            //    x = 1
+            var nameUseScope = analysis.ExpressionEvaluator.GlobalScope.FindScope(analysis.Document, nameUseLocation.Span.Start);
+            if (nameUseScope.IsNestedInScope(variableDefinitionScope)) {
+                return false;
+            }
+
+            // Exclude same-name variables declared within the same statement
+            // like 'e' that appears before its declaration in '[e in for e in {}]'
+            if (variableDefinitionSpan.IsAfter(nameUseLocation.Span) && !IsSpanInComprehension(nameUseLocation.Span)) {
+                _walker.ReportUndefinedVariable(node);
             }
             return false;
         }

src/Analysis/Ast/Test/LintUndefinedVarsTests.cs

@@ -587,6 +587,30 @@ def bar(x, y, z):
             d.Should().BeEmpty();
         }
 
+        [TestMethod, Priority(0)]
+        public async Task DifferentScopes() {
+            const string code = @"
+def func():
+    var = _CONSTANT
+
+_CONSTANT = 1
+";
+            var d = await LintAsync(code);
+            d.Should().BeEmpty();
+        }
+
+        [TestMethod, Priority(0)]
+        public async Task GlobalScope() {
+            const string code = @"
+var = _CONSTANT
+_CONSTANT = 1
+";
+            var d = await LintAsync(code);
+            d.Should().HaveCount(1);
+            d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable);
+            d[0].SourceSpan.Should().Be(2, 7, 2, 16);
+        }
+
         private async Task<IReadOnlyList<DiagnosticsEntry>> LintAsync(string code, InterpreterConfiguration configuration = null) {
             var analysis = await GetAnalysisAsync(code, configuration ?? PythonVersions.LatestAvailable3X);
             var a = Services.GetService<IPythonAnalyzer>();