Support goto definition in library files (#1861)

* Remove stale reference * Don't suppress LHS diagnostics on augmented assign * Revert "Don't suppress LHS diagnostics on augmented assign" This reverts commit 6109ac7. * Escape [ and ] * PR feedback * Fix navigation to local unknown parameters * Enable option for library code navigation * Eval both sides of binary of for references * Simplify condition Co-authored-by: Jake Bailey <[email protected]>
microsoft · Mar 18, 2020 · 8d42298 · 8d42298
1 parent 1a9d6e5
commit 8d42298
Show file tree

Hide file tree

Showing 10 changed files with 86 additions and 22 deletions.
diff --git a/src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs b/src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs
@@ -26,7 +26,6 @@ namespace Microsoft.Python.Analysis.Analyzer {
         public string Name { get; }
         public string FilePath { get; }
         public bool IsTypeshed { get; }
-        public bool IsNonUserAsDocument { get; }
 
         public AnalysisModuleKey(IPythonModule module) : this(
             module.Name,
@@ -41,13 +40,12 @@ private AnalysisModuleKey(string name, string filePath, bool isTypeshed, bool is
             Name = name;
             FilePath = filePath;
             IsTypeshed = isTypeshed;
-            IsNonUserAsDocument = isNonUserAsDocument;
         }
 
         public AnalysisModuleKey GetNonUserAsDocumentKey() => new AnalysisModuleKey(Name, FilePath, IsTypeshed, true);
 
         public bool Equals(AnalysisModuleKey other)
-            => Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && IsTypeshed == other.IsTypeshed && IsNonUserAsDocument == other.IsNonUserAsDocument;
+            => Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && IsTypeshed == other.IsTypeshed;
 
         public override bool Equals(object obj) => obj is AnalysisModuleKey other && Equals(other);
 
@@ -56,7 +54,6 @@ public override int GetHashCode() {
                 var hashCode = Name != null ? Name.GetHashCode() : 0;
                 hashCode = (hashCode * 397) ^ (FilePath != null ? FilePath.GetPathHashCode() : 0);
                 hashCode = (hashCode * 397) ^ IsTypeshed.GetHashCode();
-                hashCode = (hashCode * 397) ^ IsNonUserAsDocument.GetHashCode();
                 return hashCode;
             }
         }

diff --git a/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs b/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs
@@ -18,6 +18,7 @@
 using Microsoft.Python.Analysis.Analyzer.Evaluation;
 using Microsoft.Python.Analysis.Analyzer.Handlers;
 using Microsoft.Python.Analysis.Analyzer.Symbols;
+using Microsoft.Python.Analysis.Modules;
 using Microsoft.Python.Analysis.Types;
 using Microsoft.Python.Parsing.Ast;
 
@@ -61,6 +62,26 @@ public override bool Walk(NamedExpression node) {
             return base.Walk(node);
         }
 
+        public override bool Walk(CallExpression node) {
+            Eval.ProcessCallForReferences(node);
+            return base.Walk(node);
+        }
+
+        public override void PostWalk(DelStatement node) {
+            if (Module.ModuleType != ModuleType.User &&
+                Eval.Services.GetService<IAnalysisOptionsProvider>()?.Options.KeepLibraryAst != true) {
+                return;
+            }
+
+            var names = node.Expressions.OfType<NameExpression>()
+                .Concat(node.Expressions.OfType<TupleExpression>().SelectMany(t => t.Items.OfType<NameExpression>()))
+                .Where(x => !string.IsNullOrEmpty(x.Name));
+
+            foreach (var nex in names) {
+                Eval.LookupNameInScopes(nex.Name)?.AddReference(Eval.GetLocationOfName(nex));
+            }
+        }
+
         public override bool Walk(ExpressionStatement node) {
             switch (node.Expression) {
                 case ExpressionWithAnnotation ea:
@@ -69,9 +90,6 @@ public override bool Walk(ExpressionStatement node) {
                 case Comprehension comp:
                     Eval.ProcessComprehension(comp);
                     return false;
-                case CallExpression callex:
-                    Eval.ProcessCallForReferences(callex);
-                    return true;
                 default:
                     return base.Walk(node);
             }

diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs
@@ -389,7 +389,8 @@ private void DeclareParameter(Parameter p, ParameterInfo pi) {
         }
 
         internal void ProcessCallForReferences(CallExpression callExpr, LookupOptions lookupOptions = LookupOptions.Normal) {
-            if (Module.ModuleType != ModuleType.User) {
+            if (Module.ModuleType != ModuleType.User && 
+                Services.GetService<IAnalysisOptionsProvider>()?.Options.KeepLibraryAst != true) {
                 return;
             }
 

diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs
@@ -252,7 +252,7 @@ private void AnalyzeDocument(in AnalysisModuleKey key, in PythonAnalyzerEntry en
             ActivityTracker.StartTracking();
             _log?.Log(TraceEventType.Verbose, $"Analysis of {entry.Module.Name} ({entry.Module.ModuleType}) queued. Dependencies: {string.Join(", ", dependencies.Select(d => d.IsTypeshed ? $"{d.Name} (stub)" : d.Name))}");
 
-            var graphVersion = _dependencyResolver.ChangeValue(key, entry, entry.IsUserOrBuiltin || key.IsNonUserAsDocument, dependencies);
+            var graphVersion = _dependencyResolver.ChangeValue(key, entry, entry.IsUserOrBuiltin, dependencies);
             lock (_syncObj) {
                 if (_version >= graphVersion) {
                     return;

diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs
@@ -512,6 +512,11 @@ private IDocumentAnalysis CreateAnalysis(IDependencyChainSingleNode<PythonAnalyz
                                              node.IsValidVersion;
                 }
 
+                var optionsProvider = _services.GetService<IAnalysisOptionsProvider>();
+                if (optionsProvider?.Options.KeepLibraryAst == true) {
+                    createLibraryAnalysis = false;
+                }
+
                 if (!createLibraryAnalysis) {
                     return new DocumentAnalysis(document, version, walker.GlobalScope, walker.Eval, walker.StarImportMemberNames);
                 }

diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs
@@ -60,15 +60,15 @@ public override void Evaluate() {
                 // 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 optionsProvider = Eval.Services.GetService<IAnalysisOptionsProvider>();
+                var keepAst = optionsProvider?.Options.KeepLibraryAst == true;
                 var ctor = _function.IsDunderInit() || _function.IsDunderNew();
-                if (ctor || returnType.IsUnknown() || Module.ModuleType == ModuleType.User) {
+                if (ctor || returnType.IsUnknown() || Module.ModuleType == ModuleType.User || keepAst) {
                     // 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
                     // unless function has inner classes or functions.
-                    var optionsProvider = Eval.Services.GetService<IAnalysisOptionsProvider>();
-                    if (Module.ModuleType != ModuleType.User &&
-                        optionsProvider?.Options.KeepLibraryLocalVariables != true &&
+                    if (Module.ModuleType != ModuleType.User && !keepAst &&
                         Eval.CurrentScope.Variables.All(
                             v => v.GetPythonType<IPythonClassType>() == null &&
                                  v.GetPythonType<IPythonFunctionType>() == null)

diff --git a/src/Analysis/Ast/Impl/Definitions/AnalysisOptions.cs b/src/Analysis/Ast/Impl/Definitions/AnalysisOptions.cs
@@ -36,13 +36,6 @@ public enum AnalysisCachingLevel {
     public class AnalysisOptions {
         public bool LintingEnabled { get; set; }
 
-        /// <summary>
-        /// Keep in memory information on local variables declared in
-        /// functions in libraries. Provides ability to navigate to
-        /// symbols used in function bodies in packages and libraries.
-        /// </summary>
-        public bool KeepLibraryLocalVariables { get; set; }
-
         /// <summary>
         /// Keep in memory AST of library source code. May somewhat
         /// improve performance when library code has to be re-analyzed.

diff --git a/src/Analysis/Ast/Impl/Types/LocatedMember.cs b/src/Analysis/Ast/Impl/Types/LocatedMember.cs
@@ -65,8 +65,10 @@ public virtual void AddReference(Location location) {
                     (this.DeclaringModule?.ModuleType == ModuleType.Builtins && MemberType != PythonMemberType.Function)) {
                     return;
                 }
+                var keepReferencesInLibraries = 
+                    location.Module.Analysis.ExpressionEvaluator.Services.GetService<IAnalysisOptionsProvider>()?.Options.KeepLibraryAst == true;
                 // Don't add references to library code.
-                if (location.Module?.ModuleType == ModuleType.User && !location.Equals(Location)) {
+                if ((location.Module?.ModuleType == ModuleType.User || keepReferencesInLibraries) && !location.Equals(Location)) {
                     _references = _references ?? new HashSet<Location>();
                     _references.Add(location);
                 }

diff --git a/src/LanguageServer/Impl/LanguageServer.Configuration.cs b/src/LanguageServer/Impl/LanguageServer.Configuration.cs
@@ -130,7 +130,6 @@ private void HandleDiagnosticsChanges(JToken pythonSection, LanguageServerSettin
 
             var memory = analysis["memory"];
             var optionsProvider = _services.GetService<IAnalysisOptionsProvider>();
-            optionsProvider.Options.KeepLibraryLocalVariables = GetSetting(memory, "keepLibraryLocalVariables", false);
             optionsProvider.Options.KeepLibraryAst = GetSetting(memory, "keepLibraryAst", false);
             optionsProvider.Options.AnalysisCachingLevel = GetAnalysisCachingLevel(analysis);
 

diff --git a/src/LanguageServer/Test/ReferencesTests.cs b/src/LanguageServer/Test/ReferencesTests.cs
@@ -435,5 +435,54 @@ public async Task EmptyAnalysis() {
             var references = await rs.FindAllReferencesAsync(null, new SourceLocation(1, 1), ReferenceSearchOptions.All);
             references.Should().BeEmpty();
         }
+
+        [TestMethod, Priority(0)]
+        public async Task DelStatement1() {
+            const string code = @"
+def func(a, b):
+    return a+b
+
+del func
+";
+            var analysis = await GetAnalysisAsync(code);
+            var rs = new ReferenceSource(Services);
+            var refs = await rs.FindAllReferencesAsync(analysis.Document.Uri, new SourceLocation(5, 6), ReferenceSearchOptions.All);
+
+            refs.Should().HaveCount(2);
+            refs[0].range.Should().Be(1, 4, 1, 8);
+            refs[0].uri.Should().Be(analysis.Document.Uri);
+            refs[1].range.Should().Be(4, 4, 4, 8);
+            refs[1].uri.Should().Be(analysis.Document.Uri);
+        }
+
+        [TestMethod, Priority(0)]
+        public async Task DelStatement2() {
+            const string code = @"
+def func1(a, b):
+    return a+b
+
+def func2(a, b):
+    return a+b
+
+del (func1, func2)
+";
+            var analysis = await GetAnalysisAsync(code);
+            var rs = new ReferenceSource(Services);
+            var refs = await rs.FindAllReferencesAsync(analysis.Document.Uri, new SourceLocation(8, 6), ReferenceSearchOptions.All);
+
+            refs.Should().HaveCount(2);
+            refs[0].range.Should().Be(1, 4, 1, 9);
+            refs[0].uri.Should().Be(analysis.Document.Uri);
+            refs[1].range.Should().Be(7, 5, 7, 10);
+            refs[1].uri.Should().Be(analysis.Document.Uri);
+
+            refs = await rs.FindAllReferencesAsync(analysis.Document.Uri, new SourceLocation(8, 14), ReferenceSearchOptions.All);
+
+            refs.Should().HaveCount(2);
+            refs[0].range.Should().Be(4, 4, 4, 9);
+            refs[0].uri.Should().Be(analysis.Document.Uri);
+            refs[1].range.Should().Be(7, 12, 7, 17);
+            refs[1].uri.Should().Be(analysis.Document.Uri);
+        }
     }
 }