- Fix #1455: AF: Library module of type Stub has been analyzed already! (#1470)

- Restore task handling in unit tests
- Throw exception from Debug.Fail instead of FailFast

Author: AlexanderSher

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;
             }
         }

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) {

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;
                 }
 

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) {

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(

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 {

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();
     }
 }

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;
     }
 }

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;
+    }
 }

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})";
             }