Multiple analysis fixes (#1297)

Multiple analysis fixes. - Fixes #1151: Reliable way to determine final analysis of a module - Fixes #1201: NRE in SymbolCollector.AddProperty - Fixes #1228: NRE in TryFindMissingDependencies - Fixes #1294: Handle valid case for reloading AST - Fixes #1295: FindReferences doesn't work on package init members - Fixes #1296: DependencyResolver graph misses intermediate imports - Part of the fix for #1174: All references are not listed when using ms language server (fixes in analysis itself are also required for this one)
microsoft · Jul 9, 2019 · 5f322f0 · 5f322f0
1 parent b826df7
commit 5f322f0
Show file tree

Hide file tree

Showing 49 changed files with 729 additions and 376 deletions.
diff --git a/src/Analysis/Ast/Impl/Analyzer/ActivityTracker.cs b/src/Analysis/Ast/Impl/Analyzer/ActivityTracker.cs
@@ -19,48 +19,21 @@
 
 namespace Microsoft.Python.Analysis.Analyzer {
     internal static class ActivityTracker {
-        private static readonly Dictionary<string, AnalysisState> _modules = new Dictionary<string, AnalysisState>();
+        private static readonly HashSet<string> _modules = new HashSet<string>();
         private static readonly object _lock = new object();
         private static bool _tracking;
         private static Stopwatch _sw;
 
-        private struct AnalysisState {
-            public int Count;
-            public bool IsComplete;
-        }
-
         public static void OnEnqueueModule(string path) {
             if (string.IsNullOrEmpty(path)) {
                 return;
             }
 
             lock (_lock) {
-                if (!_modules.TryGetValue(path, out var st)) {
-                    _modules[path] = default;
-                } else {
-                    st.IsComplete = false;
-                }
-            }
-        }
-
-        public static void OnModuleAnalysisComplete(string path) {
-            lock (_lock) {
-                if (_modules.TryGetValue(path, out var st)) {
-                    st.Count++;
-                    st.IsComplete = true;
-                }
+                _modules.Add(path);
             }
         }
 
-        public static bool IsAnalysisComplete {
-            get {
-                lock (_lock) {
-                    return _modules.All(m => m.Value.IsComplete);
-                }
-            }
-        }
-
-
         public static void StartTracking() {
             lock (_lock) {
                 if (!_tracking) {
@@ -71,22 +44,15 @@ public static void StartTracking() {
             }
         }
 
-        public static void EndTracking() {
+        public static (int modulesCount, double totalMilliseconds) EndTracking() {
             lock (_lock) {
                 if (_tracking) {
                     _sw?.Stop();
                     _tracking = false;
                 }
-            }
-        }
 
-        public static int ModuleCount {
-            get {
-                lock (_lock) {
-                    return _modules.Count;
-                }
+                return (_modules.Count, _sw?.Elapsed.TotalMilliseconds ?? 0);
             }
         }
-        public static double MillisecondsElapsed => _sw?.Elapsed.TotalMilliseconds ?? 0;
     }
 }
diff --git a/src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs b/src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs
@@ -15,39 +15,56 @@
 
 using System;
 using System.Diagnostics;
+using Microsoft.Python.Analysis.Documents;
 using Microsoft.Python.Analysis.Modules;
 using Microsoft.Python.Analysis.Types;
 using Microsoft.Python.Core;
 
 namespace Microsoft.Python.Analysis.Analyzer {
     [DebuggerDisplay("{Name} : {FilePath}")]
-    internal struct AnalysisModuleKey : IEquatable<AnalysisModuleKey> {
+    internal readonly struct AnalysisModuleKey : IEquatable<AnalysisModuleKey> {
+        private enum KeyType { Default, Typeshed, LibraryAsDocument }
+
+        private readonly KeyType _type;
         public string Name { get; }
         public string FilePath { get; }
-        public bool IsTypeshed { get; }
+        public bool IsTypeshed => _type == KeyType.Typeshed;
+        public bool IsLibraryAsDocument => _type == KeyType.LibraryAsDocument;
 
         public AnalysisModuleKey(IPythonModule module) {
             Name = module.Name;
             FilePath = module.ModuleType == ModuleType.CompiledBuiltin ? null : module.FilePath;
-            IsTypeshed = module is StubPythonModule stub && stub.IsTypeshed;
+            _type = module is StubPythonModule stub && stub.IsTypeshed
+                ? KeyType.Typeshed
+                : module.ModuleType == ModuleType.Library && module is IDocument document && document.IsOpen
+                    ? KeyType.LibraryAsDocument
+                    : KeyType.Default;
         }
 
         public AnalysisModuleKey(string name, string filePath, bool isTypeshed) {
             Name = name;
             FilePath = filePath;
-            IsTypeshed = isTypeshed;
+            _type = isTypeshed ? KeyType.Typeshed : KeyType.Default;
+        }
+
+        private AnalysisModuleKey(string name, string filePath, KeyType type) {
+            Name = name;
+            FilePath = filePath;
+            _type = type;
         }
 
+        public AnalysisModuleKey GetLibraryAsDocumentKey() => new AnalysisModuleKey(Name, FilePath, KeyType.LibraryAsDocument);
+
         public bool Equals(AnalysisModuleKey other)
-            => Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && IsTypeshed == other.IsTypeshed;
+            => Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && _type == other._type;
 
         public override bool Equals(object obj) => obj is AnalysisModuleKey other && Equals(other);
 
         public override int GetHashCode() {
             unchecked {
                 var hashCode = (Name != null ? Name.GetHashCode() : 0);
                 hashCode = (hashCode * 397) ^ (FilePath != null ? FilePath.GetPathHashCode() : 0);
-                hashCode = (hashCode * 397) ^ IsTypeshed.GetHashCode();
+                hashCode = (hashCode * 397) ^ _type.GetHashCode();
                 return hashCode;
             }
         }

diff --git a/src/Analysis/Ast/Impl/Analyzer/Definitions/IAnalyzable.cs b/src/Analysis/Ast/Impl/Analyzer/Definitions/IAnalyzable.cs
@@ -26,6 +26,7 @@ internal interface IAnalyzable {
         /// <summary>
         /// Notifies document that its analysis is now complete.
         /// </summary>
-        void NotifyAnalysisComplete(int version, ModuleWalker walker, bool isFinalPass);
+        /// <param name="analysis">Document analysis</param>
+        void NotifyAnalysisComplete(IDocumentAnalysis analysis);
     }
 }
diff --git a/src/Analysis/Ast/Impl/Analyzer/Definitions/IPythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/Definitions/IPythonAnalyzer.cs
@@ -20,14 +20,15 @@
 using Microsoft.Python.Analysis.Diagnostics;
 using Microsoft.Python.Analysis.Types;
 using Microsoft.Python.Core.Collections;
+using Microsoft.Python.Parsing.Ast;
 
 namespace Microsoft.Python.Analysis.Analyzer {
     public interface IPythonAnalyzer {
         Task WaitForCompleteAnalysisAsync(CancellationToken cancellationToken = default);
         /// <summary>
         /// Schedules module for analysis. Module will be scheduled if version of AST is greater than the one used to get previous analysis
         /// </summary>
-        void EnqueueDocumentForAnalysis(IPythonModule module, int version);
+        void EnqueueDocumentForAnalysis(IPythonModule module, PythonAst ast, int version);
 
         /// <summary>
         /// Schedules module for analysis for its existing AST, but with new dependencies.

diff --git a/src/Analysis/Ast/Impl/Analyzer/EmptyAnalysis.cs b/src/Analysis/Ast/Impl/Analyzer/EmptyAnalysis.cs
@@ -30,7 +30,7 @@ public EmptyAnalysis(IServiceContainer services, IDocument document) {
             Document = document ?? throw new ArgumentNullException(nameof(document));
             GlobalScope = new EmptyGlobalScope(document);
             Ast = AstUtilities.MakeEmptyAst(document.Uri);
-            ExpressionEvaluator = new ExpressionEval(services, document);
+            ExpressionEvaluator = new ExpressionEval(services, document, Ast);
         }
 
         public IDocument Document { get; }

diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs
@@ -94,7 +94,7 @@ public IMember GetValueFromClassCtor(IPythonClassType cls, CallExpression expr)
             var init = cls.GetMember<IPythonFunctionType>(@"__init__");
             if (init != null) {
                 using (OpenScope(cls.DeclaringModule, cls.ClassDefinition, out _)) {
-                    var a = new ArgumentSet(init, 0, new PythonInstance(cls), expr, Module, this);
+                    var a = new ArgumentSet(init, 0, new PythonInstance(cls), expr, this);
                     if (a.Errors.Count > 0) {
                         // AddDiagnostics(Module.Uri, a.Errors);
                     }

diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs
@@ -41,7 +41,7 @@ public void DeclareVariable(string name, IMember value, VariableSource source)
             => DeclareVariable(name, value, source, default(Location));
 
         public void DeclareVariable(string name, IMember value, VariableSource source, IPythonModule module)
-            => DeclareVariable(name, value, source, new Location(module, default));
+            => DeclareVariable(name, value, source, new Location(module));
 
         public void DeclareVariable(string name, IMember value, VariableSource source, Node location, bool overwrite = false)
             => DeclareVariable(name, value, source, GetLocationOfName(location), overwrite);

diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs
@@ -38,15 +38,12 @@ internal sealed partial class ExpressionEval : IExpressionEvaluator {
         private readonly object _lock = new object();
         private readonly List<DiagnosticsEntry> _diagnostics = new List<DiagnosticsEntry>();
 
-        public ExpressionEval(IServiceContainer services, IPythonModule module)
-            : this(services, module, new GlobalScope(module)) { }
-
-        public ExpressionEval(IServiceContainer services, IPythonModule module, GlobalScope gs) {
+        public ExpressionEval(IServiceContainer services, IPythonModule module, PythonAst ast) {
             Services = services ?? throw new ArgumentNullException(nameof(services));
             Module = module ?? throw new ArgumentNullException(nameof(module));
-            Ast = module.GetAst();
+            Ast = ast ?? throw new ArgumentNullException(nameof(ast));
 
-            GlobalScope = gs;
+            GlobalScope = new GlobalScope(module, ast);
             CurrentScope = GlobalScope;
             DefaultLocation = new Location(module);
             //Log = services.GetService<ILogger>();
@@ -60,7 +57,7 @@ public ExpressionEval(IServiceContainer services, IPythonModule module, GlobalSc
         public IPythonType UnknownType => Interpreter.UnknownType;
         public Location DefaultLocation { get; }
 
-        public LocationInfo GetLocationInfo(Node node) => node?.GetLocation(Module) ?? LocationInfo.Empty;
+        public LocationInfo GetLocationInfo(Node node) => node?.GetLocation(this) ?? LocationInfo.Empty;
 
         public Location GetLocationOfName(Node node) {
             if (node == null || (Module.ModuleType != ModuleType.User && Module.ModuleType != ModuleType.Library)) {
@@ -103,7 +100,7 @@ public Location GetLocationOfName(Node node) {
         public IServiceContainer Services { get; }
         IScope IExpressionEvaluator.CurrentScope => CurrentScope;
         IGlobalScope IExpressionEvaluator.GlobalScope => GlobalScope;
-        public LocationInfo GetLocation(Node node) => node?.GetLocation(Module) ?? LocationInfo.Empty;
+        public LocationInfo GetLocation(Node node) => node?.GetLocation(this) ?? LocationInfo.Empty;
         public IEnumerable<DiagnosticsEntry> Diagnostics => _diagnostics;
 
         public void ReportDiagnostics(Uri documentUri, DiagnosticsEntry entry) {

diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs
@@ -125,8 +125,8 @@ private void SpecializeFuture(FromImportStatement node) {
 
             var printNameExpression = node.Names.FirstOrDefault(n => n?.Name == "print_function");
             if (printNameExpression != null) {
-                var fn = new PythonFunctionType("print", new Location(Module, default), null, string.Empty);
-                var o = new PythonFunctionOverload(fn.Name, new Location(Module, default));
+                var fn = new PythonFunctionType("print", new Location(Module), null, string.Empty);
+                var o = new PythonFunctionOverload(fn.Name, new Location(Module));
                 var parameters = new List<ParameterInfo> {
                     new ParameterInfo("*values", Interpreter.GetBuiltinType(BuiltinTypeId.Object), ParameterKind.List, null),
                     new ParameterInfo("sep", Interpreter.GetBuiltinType(BuiltinTypeId.Str), ParameterKind.KeywordOnly, null),

diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs
@@ -56,19 +56,22 @@ private void HandleImport(ModuleName moduleImportExpression, NameExpression asNa
             // import_module('fob.oar.baz')
             var importNames = ImmutableArray<string>.Empty;
             var variableModule = default(PythonVariableModule);
+            var importedNamesCount = 0;
             foreach (var nameExpression in moduleImportExpression.Names) {
                 importNames = importNames.Add(nameExpression.Name);
                 var imports = ModuleResolution.CurrentPathResolver.GetImportsFromAbsoluteName(Module.FilePath, importNames, forceAbsolute);
                 if (!HandleImportSearchResult(imports, variableModule, asNameExpression, moduleImportExpression, out variableModule)) {
-                    return;
+                    break;
                 }
+
+                importedNamesCount++;
             }
 
             // "import fob.oar.baz as baz" is handled as baz = import_module('fob.oar.baz')
             // "import fob.oar.baz" is handled as fob = import_module('fob')
-            if (!string.IsNullOrEmpty(asNameExpression?.Name)) {
+            if (!string.IsNullOrEmpty(asNameExpression?.Name) && importedNamesCount == moduleImportExpression.Names.Count) {
                 Eval.DeclareVariable(asNameExpression.Name, variableModule, VariableSource.Import, asNameExpression);
-            } else if (importNames.Count > 0 && !string.IsNullOrEmpty(importNames[0]) && _variableModules.TryGetValue(importNames[0], out variableModule)) {
+            } else if (importedNamesCount > 0 && !string.IsNullOrEmpty(importNames[0]) && _variableModules.TryGetValue(importNames[0], out variableModule)) {
                 var firstName = moduleImportExpression.Names[0];
                 Eval.DeclareVariable(importNames[0], variableModule, VariableSource.Import, firstName);
             }
@@ -87,7 +90,7 @@ private bool HandleImportSearchResult(in IImportSearchResult imports, in PythonV
                 case RelativeImportBeyondTopLevel importBeyondTopLevel:
                     var message = Resources.ErrorRelativeImportBeyondTopLevel.FormatInvariant(importBeyondTopLevel.RelativeImportName);
                     Eval.ReportDiagnostics(Eval.Module.Uri, 
-                        new DiagnosticsEntry(message, location.GetLocation(Eval.Module).Span, ErrorCodes.UnresolvedImport, Severity.Warning, DiagnosticSource.Analysis));
+                        new DiagnosticsEntry(message, location.GetLocation(Eval).Span, ErrorCodes.UnresolvedImport, Severity.Warning, DiagnosticSource.Analysis));
                     variableModule = default;
                     return false;
                 case ImportNotFound importNotFound:

diff --git a/src/Analysis/Ast/Impl/Analyzer/LibraryAnalysis.cs b/src/Analysis/Ast/Impl/Analyzer/LibraryAnalysis.cs
@@ -29,22 +29,15 @@ namespace Microsoft.Python.Analysis.Analyzer {
     /// Analysis of a library code.
     /// </summary>
     internal sealed class LibraryAnalysis : IDocumentAnalysis {
-        public LibraryAnalysis(IDocument document, int version, IServiceContainer services, GlobalScope globalScope, IReadOnlyList<string> starImportMemberNames) {
+        public LibraryAnalysis(IDocument document, int version, IGlobalScope globalScope, IExpressionEvaluator eval, IReadOnlyList<string> starImportMemberNames) {
             Check.ArgumentNotNull(nameof(document), document);
             Check.ArgumentNotNull(nameof(globalScope), globalScope);
 
             Document = document;
             Version = version;
             GlobalScope = globalScope;
 
-            var ast = Document.GetAst();
-            ast.Reduce(x => x is ImportStatement || x is FromImportStatement);
-            var c = (IAstNodeContainer)Document;
-            c.ClearContent();
-            c.ClearAst();
-            c.AddAstNode(document, ast);
-
-            ExpressionEvaluator = new ExpressionEval(services, document, globalScope);
+            ExpressionEvaluator = eval;
             StarImportMemberNames = starImportMemberNames;
         }
 

diff --git a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs
@@ -37,8 +37,8 @@ internal class ModuleWalker : AnalysisWalker {
         private int _allReferencesCount;
         private bool _allIsUsable = true;
 
-        public ModuleWalker(IServiceContainer services, IPythonModule module)
-            : base(new ExpressionEval(services, module)) {
+        public ModuleWalker(IServiceContainer services, IPythonModule module, PythonAst ast)
+            : base(new ExpressionEval(services, module, ast)) {
             _stubAnalysis = Module.Stub is IDocument doc ? doc.GetAnyAnalysis() : null;
         }