- Fix microsoft#1455: AF: Library module of type Stub has been analyz…

…ed already! - Restore task handling in unit tests - Throw exception from Debug.Fail instead of FailFast
AlexanderSher · Aug 22, 2019 · 220c7a2 · 220c7a2
1 parent 86c8426
commit 220c7a2
Show file tree

Hide file tree

Showing 19 changed files with 138 additions and 1,148 deletions.
diff --git a/src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs b/src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs
@@ -23,48 +23,41 @@
 namespace Microsoft.Python.Analysis.Analyzer {
     [DebuggerDisplay("{Name} : {FilePath}")]
     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 => _type == KeyType.Typeshed;
-        public bool IsLibraryAsDocument => _type == KeyType.LibraryAsDocument;
+        public bool IsTypeshed { get; }
+        public bool IsNonUserAsDocument { get; }
 
         public AnalysisModuleKey(IPythonModule module) {
             Name = module.Name;
             FilePath = module.ModuleType == ModuleType.CompiledBuiltin ? null : module.FilePath;
-            _type = module is StubPythonModule stub && stub.IsTypeshed              
-                ? KeyType.Typeshed
-                : module.ModuleType == ModuleType.Library && module is IDocument document && document.IsOpen
-                    ? KeyType.LibraryAsDocument
-                    : KeyType.Default;
+            IsTypeshed = module is StubPythonModule stub && stub.IsTypeshed;
+            IsNonUserAsDocument = (module.IsNonUserFile() || module.IsCompiled()) && module is IDocument document && document.IsOpen;
         }
 
-        public AnalysisModuleKey(string name, string filePath, bool isTypeshed) {
-            Name = name;
-            FilePath = filePath;
-            _type = isTypeshed ? KeyType.Typeshed : KeyType.Default;
-        }
+        public AnalysisModuleKey(string name, string filePath, bool isTypeshed) 
+            : this(name, filePath, isTypeshed, false) { }
 
-        private AnalysisModuleKey(string name, string filePath, KeyType type) {
+        private AnalysisModuleKey(string name, string filePath, bool isTypeshed, bool isNonUserAsDocument) {
             Name = name;
             FilePath = filePath;
-            _type = type;
+            IsTypeshed = isTypeshed;
+            IsNonUserAsDocument = isNonUserAsDocument;
         }
 
-        public AnalysisModuleKey GetLibraryAsDocumentKey() => new AnalysisModuleKey(Name, FilePath, KeyType.LibraryAsDocument);
+        public AnalysisModuleKey GetNonUserAsDocumentKey() => new AnalysisModuleKey(Name, FilePath, IsTypeshed, true);
 
         public bool Equals(AnalysisModuleKey other)
-            => Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && _type == other._type;
+            => Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && IsTypeshed == other.IsTypeshed && IsNonUserAsDocument == other.IsNonUserAsDocument;
 
         public override bool Equals(object obj) => obj is AnalysisModuleKey other && Equals(other);
 
         public override int GetHashCode() {
             unchecked {
-                var hashCode = (Name != null ? Name.GetHashCode() : 0);
+                var hashCode = Name != null ? Name.GetHashCode() : 0;
                 hashCode = (hashCode * 397) ^ (FilePath != null ? FilePath.GetPathHashCode() : 0);
-                hashCode = (hashCode * 397) ^ _type.GetHashCode();
+                hashCode = (hashCode * 397) ^ IsTypeshed.GetHashCode();
+                hashCode = (hashCode * 397) ^ IsNonUserAsDocument.GetHashCode();
                 return hashCode;
             }
         }

diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs
@@ -17,6 +17,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Runtime.ExceptionServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Python.Analysis.Caching;
@@ -166,9 +167,9 @@ public void EnqueueDocumentForAnalysis(IPythonModule module, PythonAst ast, int
                     // It is possible that parsing request for the library has been started when document is open,
                     // but it is closed at the moment of analysis and then become open again.
                     // In this case, we still need to analyze the document, but using correct entry.
-                    var libraryAsDocumentKey = key.GetLibraryAsDocumentKey();
-                    if (entry.PreviousAnalysis is LibraryAnalysis && _analysisEntries.TryGetValue(libraryAsDocumentKey, out var documentEntry)) {
-                        key = libraryAsDocumentKey;
+                    var nonUserAsDocumentKey = key.GetNonUserAsDocumentKey();
+                    if (entry.PreviousAnalysis is LibraryAnalysis && _analysisEntries.TryGetValue(nonUserAsDocumentKey, out var documentEntry)) {
+                        key = nonUserAsDocumentKey;
                         entry = documentEntry;
                     }
 
@@ -239,7 +240,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");
 
-            var graphVersion = _dependencyResolver.ChangeValue(key, entry, entry.IsUserOrBuiltin || key.IsLibraryAsDocument, dependencies);
+            var graphVersion = _dependencyResolver.ChangeValue(key, entry, entry.IsUserOrBuiltin || key.IsNonUserAsDocument, dependencies);
 
             lock (_syncObj) {
                 if (_version > graphVersion) {
@@ -301,6 +302,11 @@ private bool TryCreateSession(in int graphVersion, in PythonAnalyzerEntry entry,
         }
 
         private void StartNextSession(Task task) {
+            if (task.IsFaulted && task.Exception != null) {
+                var exception = task.Exception.InnerException;
+                ExceptionDispatchInfo.Capture(exception).Throw();
+            }
+
             PythonAnalyzerSession session;
             lock (_syncObj) {
                 if (_nextSession == null) {

diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerEntry.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerEntry.cs
@@ -113,7 +113,7 @@ public bool IsValidVersion(int version, out IPythonModule module, out PythonAst
                 }
 
                 if (ast == null) {
-                    Debug.Assert(!(_previousAnalysis is LibraryAnalysis), $"Library module {module.Name} of type {module.ModuleType} has been analyzed already!");
+                    Debug.Assert(!(_analysisVersion <= version && _previousAnalysis is LibraryAnalysis), $"Library module {module.Name} of type {module.ModuleType} has been analyzed already!");
                     return false;
                 }
 

diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs
@@ -17,6 +17,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Runtime;
+using System.Runtime.ExceptionServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Python.Analysis.Analyzer.Evaluation;
@@ -29,6 +30,7 @@
 using Microsoft.Python.Core;
 using Microsoft.Python.Core.Logging;
 using Microsoft.Python.Core.Services;
+using Microsoft.Python.Core.Testing;
 using Microsoft.Python.Parsing.Ast;
 
 namespace Microsoft.Python.Analysis.Analyzer {
@@ -210,6 +212,7 @@ private async Task<int> AnalyzeAffectedEntriesAsync(Stopwatch stopWatch) {
                 if (Interlocked.Increment(ref _runningTasks) >= _maxTaskRunning || _walker.Remaining == 1) {
                     RunAnalysis(node, stopWatch);
                 } else {
+                    ace.AddOne();
                     StartAnalysis(node, ace, stopWatch).DoNotWait();
                 }
             }
@@ -250,7 +253,6 @@ private Task StartAnalysis(IDependencyChainNode<PythonAnalyzerEntry> node, Async
         /// </summary>
         private void Analyze(IDependencyChainNode<PythonAnalyzerEntry> node, AsyncCountdownEvent ace, Stopwatch stopWatch) {
             try {
-                ace?.AddOne();
                 var entry = node.Value;
 
                 if (!entry.IsValidVersion(_walker.Version, out var module, out var ast)) {
@@ -372,6 +374,10 @@ private void LogException(IPythonModule module, Exception exception) {
             if (_log != null) {
                 _log.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) failed. {exception}");
             }
+
+            if (TestEnvironment.Current != null) {
+                ExceptionDispatchInfo.Capture(exception).Throw();
+            }
         }
 
         private IDocumentAnalysis CreateAnalysis(IDependencyChainNode<PythonAnalyzerEntry> node, IDocument document, PythonAst ast, int version, ModuleWalker walker, bool isCanceled) {

diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs
@@ -125,6 +125,7 @@ public override bool Walk(AssignmentStatement node) {
         public override bool Walk(ReturnStatement node) {
             var value = Eval.GetValueFromExpression(node.Expression);
             if (value != null) {
+
                 // although technically legal, __init__ in a constructor should not have a not-none return value
                 if (_function.IsDunderInit() && !value.IsOfType(BuiltinTypeId.NoneType)) {
                     Eval.ReportDiagnostics(Module.Uri, new DiagnosticsEntry(

diff --git a/src/Analysis/Ast/Impl/Extensions/FunctionDefinitionExtensions.cs b/src/Analysis/Ast/Impl/Extensions/FunctionDefinitionExtensions.cs
@@ -1,4 +1,19 @@
-using Microsoft.Python.Analysis.Types;
+// 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,
+// MERCHANTABILITY OR NON-INFRINGEMENT.
+//
+// See the Apache Version 2.0 License for specific language governing
+// permissions and limitations under the License.
+
+using Microsoft.Python.Analysis.Types;
 
 namespace Microsoft.Python.Analysis {
     public static class ClassMemberExtensions {

diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs
@@ -67,5 +67,8 @@ internal static string GetComment(this IPythonModule module, int lineNum) {
 
             return line.Substring(commentPos + 1).Trim('\t', ' ');
         }
+
+        internal static bool IsNonUserFile(this IPythonModule module) => module.ModuleType.IsNonUserFile();
+        internal static bool IsCompiled(this IPythonModule module) => module.ModuleType.IsCompiled();
     }
 }
diff --git a/src/Analysis/Ast/Impl/Extensions/PythonTypeExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonTypeExtensions.cs
@@ -27,5 +27,9 @@ public static bool IsGenericParameter(this IPythonType value)
 
         public static bool IsGeneric(this IPythonType value)
             => value is IGenericTypeParameter || (value is IGenericType gt && gt.IsGeneric);
+
+        public static string GetQualifiedName(this IPythonType t) => $"{t.DeclaringModule.Name}:{t.Name}";
+
+        internal static IPythonType ToBound(this IPythonType t) => t is PythonFunctionType.PythonUnboundMethod m ? m.Function : t;
     }
 }
diff --git a/src/Analysis/Ast/Impl/Modules/Definitions/ModuleType.cs b/src/Analysis/Ast/Impl/Modules/Definitions/ModuleType.cs
@@ -60,4 +60,9 @@ public enum ModuleType {
         /// </summary>
         Specialized
     }
+
+    public static class ModuleTypeExtensions {
+        public static bool IsNonUserFile(this ModuleType type) => type == ModuleType.Library || type == ModuleType.Stub;
+        public static bool IsCompiled(this ModuleType type) => type == ModuleType.Compiled || type == ModuleType.CompiledBuiltin;
+    }
 }
diff --git a/src/Caching/Impl/ModuleUniqueId.cs b/src/Caching/Impl/ModuleUniqueId.cs
@@ -78,8 +78,7 @@ public static string GetUniqueId(string moduleName, string filePath, ModuleType
             }
 
             var config = interpreter.Configuration;
-            if (moduleType == ModuleType.Builtins || moduleType == ModuleType.CompiledBuiltin ||
-                string.IsNullOrEmpty(filePath) || modulePathType == PythonLibraryPathType.StdLib) {
+            if (moduleType.IsCompiled() || string.IsNullOrEmpty(filePath) || modulePathType == PythonLibraryPathType.StdLib) {
                 // If module is a standard library, unique id is its name + interpreter version.
                 return $"{moduleName}({config.Version.Major}.{config.Version.Minor})";
             }

diff --git a/src/Core/Impl/Extensions/TaskExtensions.cs b/src/Core/Impl/Extensions/TaskExtensions.cs
@@ -17,7 +17,7 @@
 using System.Runtime.ExceptionServices;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.Python.Core.Threading;
+using Microsoft.Python.Core.Testing;
 
 namespace Microsoft.Python.Core {
     public static class TaskExtensions {
@@ -52,6 +52,12 @@ private static void SetCompletionResultToContinuation<T>(Task<T> task, object st
         /// <see cref="OperationCanceledException"/> is always ignored.
         /// </remarks>
         public static void DoNotWait(this Task task) {
+            if (TestEnvironment.Current != null && TestEnvironment.Current.TryAddTaskToWait(task)) {
+                if (!task.IsCompleted) {
+                    return;
+                }
+            }
+
             if (task.IsCompleted) {
                 ReThrowTaskException(task);
                 return;

diff --git a/src/Core/Test/TestLogger.cs b/src/Core/Test/TestLogger.cs
@@ -37,7 +37,6 @@ public void Dispose() {
         public TraceEventType LogLevel { get; set; } = TraceEventType.Verbose;
         public void Log(TraceEventType eventType, IFormattable message) => Log(eventType, message.ToString());
         public void Log(TraceEventType eventType, string message) {
-
             var m = $"[{TestEnvironmentImpl.Elapsed()}]: {message}";
             switch (eventType) {
                 case TraceEventType.Error:

diff --git a/src/UnitTests/Core/Impl/Ben.Demystifier/ILReader.cs b/src/UnitTests/Core/Impl/Ben.Demystifier/ILReader.cs