From 8bca56b1fa4ab8b282ae6491de25525c90aa9efe Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Mar 2019 09:51:05 -0800 Subject: [PATCH 1/4] Fix #668 (partial) --- .../Analyzer/Handlers/ConditionalHandler.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs index cb41801b9..b104acb94 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs @@ -66,24 +66,6 @@ public bool HandleIf(IfStatement node) { someRecognized = true; } } - - // Handle basic check such as - // if isinstance(value, type): - // return value - // by assigning type to the value unless clause is raising exception. - var ce = node.Tests.FirstOrDefault()?.Test as CallExpression; - if (ce?.Target is NameExpression ne && ne.Name == @"isinstance" && ce.Args.Count == 2) { - var nex = ce.Args[0].Expression as NameExpression; - var name = nex?.Name; - var typeName = (ce.Args[1].Expression as NameExpression)?.Name; - if (name != null && typeName != null) { - var typeId = typeName.GetTypeId(); - if (typeId != BuiltinTypeId.Unknown) { - var t = new PythonType(typeName, Module, string.Empty, LocationInfo.Empty, typeId); - Eval.DeclareVariable(name, t, VariableSource.Declaration, nex); - } - } - } return !someRecognized; } From 7ffc9db4f0807314d4d36a8540380b988c26ee2e Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 4 Mar 2019 16:43:30 -0800 Subject: [PATCH 2/4] Tests --- src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs | 10 ++++++++-- src/Analysis/Ast/Impl/Modules/PythonModule.cs | 12 ++++-------- src/Analysis/Ast/Test/BasicTests.cs | 14 ++++++++------ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 205e64d01..a30c5a97b 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -227,8 +227,14 @@ private async Task AnalyzeAffectedEntriesAsync(IDependencyChainWalker !k.IsTypeshed).Count == 0) { + + var count = walker.MissingKeys.Where(k => !k.IsTypeshed).Count; + _log?.Log(TraceEventType.Verbose, $"Walker count is {count}, missing keys:"); + foreach (var key in walker.MissingKeys) { + _log?.Log(TraceEventType.Verbose, $" Name: {key.Name}, Path: {key.FilePath}"); + } + + if (count == 0) { Interlocked.Exchange(ref _runningTasks, 0); _analysisCompleteEvent.Set(); } diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index 7be31275e..ce6949fc5 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -111,6 +111,7 @@ internal PythonModule(ModuleCreationOptions creationOptions, IServiceContainer s ContentState = State.Analyzed; } InitializeContent(creationOptions.Content); + Log?.Log(TraceEventType.Verbose, $"Created module: {Name} {FilePath}"); } #region IPythonType @@ -385,21 +386,16 @@ private void Parse(CancellationToken cancellationToken) { _diagnosticsService?.Replace(Uri, _parseErrors); } - ContentState = State.Parsed; - } - - NewAst?.Invoke(this, EventArgs.Empty); - - if (ContentState < State.Analyzing) { ContentState = State.Analyzing; + Log?.Log(TraceEventType.Verbose, $"Enqueue: {Name} {FilePath}"); var analyzer = Services.GetService(); analyzer.EnqueueDocumentForAnalysis(this, ast, version, _allProcessingCts.Token); - } - lock (AnalysisLock) { _parsingTask = null; } + + NewAst?.Invoke(this, EventArgs.Empty); } private class CollectingErrorSink : ErrorSink { diff --git a/src/Analysis/Ast/Test/BasicTests.cs b/src/Analysis/Ast/Test/BasicTests.cs index 47d47b8cd..c8b64e2f2 100644 --- a/src/Analysis/Ast/Test/BasicTests.cs +++ b/src/Analysis/Ast/Test/BasicTests.cs @@ -75,12 +75,14 @@ public async Task ImportTest() { import sys x = sys.path "; - var analysis = await GetAnalysisAsync(code); - analysis.GlobalScope.Variables.Count.Should().Be(2); - - analysis.Should() - .HaveVariable("sys").OfType(BuiltinTypeId.Module) - .And.HaveVariable("x").OfType(BuiltinTypeId.List); + for (var i = 0; i < 20; i++) { + var analysis = await GetAnalysisAsync(code); + analysis.GlobalScope.Variables.Count.Should().Be(2); + + analysis.Should() + .HaveVariable("sys").OfType(BuiltinTypeId.Module) + .And.HaveVariable("x").OfType(BuiltinTypeId.List); + } } [TestMethod, Priority(0)] From 6303c45f234ba1c40058520c8700f6815c303035 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 4 Mar 2019 18:11:48 -0800 Subject: [PATCH 3/4] Revert "Tests" This reverts commit 7ffc9db4f0807314d4d36a8540380b988c26ee2e. --- src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs | 10 ++-------- src/Analysis/Ast/Impl/Modules/PythonModule.cs | 12 ++++++++---- src/Analysis/Ast/Test/BasicTests.cs | 14 ++++++-------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index a30c5a97b..205e64d01 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -227,14 +227,8 @@ private async Task AnalyzeAffectedEntriesAsync(IDependencyChainWalker !k.IsTypeshed).Count; - _log?.Log(TraceEventType.Verbose, $"Walker count is {count}, missing keys:"); - foreach (var key in walker.MissingKeys) { - _log?.Log(TraceEventType.Verbose, $" Name: {key.Name}, Path: {key.FilePath}"); - } - - if (count == 0) { + + if (walker.MissingKeys.Where(k => !k.IsTypeshed).Count == 0) { Interlocked.Exchange(ref _runningTasks, 0); _analysisCompleteEvent.Set(); } diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index ce6949fc5..7be31275e 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -111,7 +111,6 @@ internal PythonModule(ModuleCreationOptions creationOptions, IServiceContainer s ContentState = State.Analyzed; } InitializeContent(creationOptions.Content); - Log?.Log(TraceEventType.Verbose, $"Created module: {Name} {FilePath}"); } #region IPythonType @@ -386,16 +385,21 @@ private void Parse(CancellationToken cancellationToken) { _diagnosticsService?.Replace(Uri, _parseErrors); } + ContentState = State.Parsed; + } + + NewAst?.Invoke(this, EventArgs.Empty); + + if (ContentState < State.Analyzing) { ContentState = State.Analyzing; - Log?.Log(TraceEventType.Verbose, $"Enqueue: {Name} {FilePath}"); var analyzer = Services.GetService(); analyzer.EnqueueDocumentForAnalysis(this, ast, version, _allProcessingCts.Token); + } + lock (AnalysisLock) { _parsingTask = null; } - - NewAst?.Invoke(this, EventArgs.Empty); } private class CollectingErrorSink : ErrorSink { diff --git a/src/Analysis/Ast/Test/BasicTests.cs b/src/Analysis/Ast/Test/BasicTests.cs index c8b64e2f2..47d47b8cd 100644 --- a/src/Analysis/Ast/Test/BasicTests.cs +++ b/src/Analysis/Ast/Test/BasicTests.cs @@ -75,14 +75,12 @@ public async Task ImportTest() { import sys x = sys.path "; - for (var i = 0; i < 20; i++) { - var analysis = await GetAnalysisAsync(code); - analysis.GlobalScope.Variables.Count.Should().Be(2); - - analysis.Should() - .HaveVariable("sys").OfType(BuiltinTypeId.Module) - .And.HaveVariable("x").OfType(BuiltinTypeId.List); - } + var analysis = await GetAnalysisAsync(code); + analysis.GlobalScope.Variables.Count.Should().Be(2); + + analysis.Should() + .HaveVariable("sys").OfType(BuiltinTypeId.Module) + .And.HaveVariable("x").OfType(BuiltinTypeId.List); } [TestMethod, Priority(0)] From 82a33b1ae243e1bdd1c3c4646d2c8af8a776a8c6 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 16 Mar 2019 15:23:54 -0700 Subject: [PATCH 4/4] Handle from A import B as C better --- .../Ast/Impl/Modules/BuiltinsPythonModule.cs | 7 ++ src/Analysis/Ast/Impl/Types/PythonType.cs | 2 +- src/Analysis/Ast/Test/BasicTests.cs | 17 +++++ .../Impl/Sources/DefinitionSource.cs | 64 ++++++++++++++----- .../Test/GoToDefinitionTests.cs | 13 ++++ 5 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/Analysis/Ast/Impl/Modules/BuiltinsPythonModule.cs b/src/Analysis/Ast/Impl/Modules/BuiltinsPythonModule.cs index 28de09602..ee31598bc 100644 --- a/src/Analysis/Ast/Impl/Modules/BuiltinsPythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/BuiltinsPythonModule.cs @@ -65,6 +65,11 @@ private void SpecializeTypes() { continue; } + if (biType.IsUnknown()) { + // Under no circumstances we modify the unknown type. + continue; + } + if (biType.IsHidden) { _hiddenNames.Add(biType.Name); } @@ -98,6 +103,8 @@ private void SpecializeTypes() { biType.AddMember(@"__iter__", BuiltinsSpecializations.__iter__(Interpreter, typeId), true); } break; + case BuiltinTypeId.Unknown: + break; default: biType.TrySetTypeId(typeId); switch (typeId) { diff --git a/src/Analysis/Ast/Impl/Types/PythonType.cs b/src/Analysis/Ast/Impl/Types/PythonType.cs index 3c23c95fa..a18cc7d9e 100644 --- a/src/Analysis/Ast/Impl/Types/PythonType.cs +++ b/src/Analysis/Ast/Impl/Types/PythonType.cs @@ -62,7 +62,7 @@ public PythonType( private PythonType(string name, IPythonModule declaringModule, BuiltinTypeId typeId = BuiltinTypeId.Unknown) { _name = name ?? throw new ArgumentNullException(nameof(name)); DeclaringModule = declaringModule; - _typeId = typeId; _typeId = typeId; + _typeId = typeId; } #region IPythonType diff --git a/src/Analysis/Ast/Test/BasicTests.cs b/src/Analysis/Ast/Test/BasicTests.cs index 2be4d75df..5b451c084 100644 --- a/src/Analysis/Ast/Test/BasicTests.cs +++ b/src/Analysis/Ast/Test/BasicTests.cs @@ -85,6 +85,23 @@ import sys .And.HaveVariable("x").OfType(BuiltinTypeId.List); } + [DataRow(true, true)] + [DataRow(false, true)] + [DataRow(true, false)] + [DataRow(false, false)] + [DataTestMethod, Priority(0)] + public async Task UnknownType(bool isPython3X, bool isAnaconda) { + const string code = @"x = 1"; + + var configuration = isPython3X + ? isAnaconda ? PythonVersions.LatestAnaconda3X : PythonVersions.LatestAvailable3X + : isAnaconda ? PythonVersions.LatestAnaconda2X : PythonVersions.LatestAvailable2X; + var analysis = await GetAnalysisAsync(code, configuration); + + var unkType = analysis.Document.Interpreter.UnknownType; + unkType.TypeId.Should().Be(BuiltinTypeId.Unknown); + } + [DataRow(true, true)] [DataRow(false, true)] [DataRow(true, false)] diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index 9e3e15429..4df412c85 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -20,6 +20,7 @@ using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; +using Microsoft.Python.Core; using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Completion; using Microsoft.Python.LanguageServer.Protocol; @@ -55,23 +56,10 @@ public Reference FindDefinition(IDocumentAnalysis analysis, SourceLocation locat } var value = eval.GetValueFromExpression(expr); - if (value.IsUnknown()) { - // If this is 'import A as B' A is not declared as a variable, so try modules. - string moduleName = null; - if (!string.IsNullOrEmpty(name)) { - switch (statement) { - case ImportStatement imp when imp.Names.Any(x => x?.MakeString() == name): - case FromImportStatement fimp when fimp.Root.Names.Any(x => x?.Name == name): - moduleName = name; - break; - } - - if (moduleName != null) { - var module = analysis.Document.Interpreter.ModuleResolution.GetImportedModule(moduleName); - if (module != null && CanNavigateToModule(module, analysis)) { - return new Reference {range = default, uri = module.Uri}; - } - } + if (value.IsUnknown() && !string.IsNullOrEmpty(name)) { + var reference = FromImport(statement, name, analysis, out value); + if (reference != null) { + return reference; } } @@ -82,6 +70,48 @@ public Reference FindDefinition(IDocumentAnalysis analysis, SourceLocation locat } } + private Reference FromImport(Node statement, string name, IDocumentAnalysis analysis, out IMember value) { + value = null; + string moduleName = null; + switch (statement) { + // In 'import A as B' A is not declared as a variable, so try locating B. + case ImportStatement imp when imp.Names.Any(x => x?.MakeString() == name): + case FromImportStatement fimp when fimp.Root.Names.Any(x => x?.Name == name): + moduleName = name; + break; + } + + if (moduleName != null) { + var module = analysis.Document.Interpreter.ModuleResolution.GetImportedModule(moduleName); + if (module != null && CanNavigateToModule(module, analysis)) { + return new Reference { range = default, uri = module.Uri }; + } + } + + // Perhaps it is a member such as A in 'from X import A as B' + switch (statement) { + case ImportStatement imp: { + // Import A as B + var index = imp.Names.IndexOf(x => x?.MakeString() == name); + if (index >= 0 && index < imp.AsNames.Count) { + value = analysis.ExpressionEvaluator.GetValueFromExpression(imp.AsNames[index]); + return null; + } + break; + } + case FromImportStatement fimp: { + // From X import A as B + var index = fimp.Names.IndexOf(x => x?.Name == name); + if (index >= 0 && index < fimp.AsNames.Count) { + value = analysis.ExpressionEvaluator.GetValueFromExpression(fimp.AsNames[index]); + return null; + } + break; + } + } + return null; + } + private Reference FromMember(IMember value, Expression expr, Node statement, IDocumentAnalysis analysis) { Node node = null; IPythonModule module = null; diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index 25f02508c..21f6ace79 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -152,6 +152,19 @@ public async Task GotoModuleSourceFromImport() { reference.uri.AbsolutePath.Should().NotContain("pyi"); } + [TestMethod, Priority(0)] + public async Task GotoModuleSourceFromImportAs() { + const string code = @"from logging import RootLogger as rl"; + var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X); + var ds = new DefinitionSource(); + + var reference = ds.FindDefinition(analysis, new SourceLocation(1, 23)); + reference.Should().NotBeNull(); + reference.range.start.line.Should().BeGreaterThan(500); + reference.uri.AbsolutePath.Should().Contain("logging"); + reference.uri.AbsolutePath.Should().NotContain("pyi"); + } + [TestMethod, Priority(0)] public async Task GotoBuiltinObject() { const string code = @"