From 8bca56b1fa4ab8b282ae6491de25525c90aa9efe Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Mar 2019 09:51:05 -0800 Subject: [PATCH 1/5] 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/5] 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/5] 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 faae4cdabaed0d5458cfde20c777406bf0510e2e Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Mar 2019 12:53:19 -0700 Subject: [PATCH 4/5] Suppress completion after error tokens --- .../Impl/Completion/CompletionSource.cs | 11 ++++++----- .../Impl/Completion/ErrorExpressionCompletion.cs | 3 +++ src/LanguageServer/Test/CompletionTests.cs | 9 +++++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/LanguageServer/Impl/Completion/CompletionSource.cs b/src/LanguageServer/Impl/Completion/CompletionSource.cs index 992af07d0..b1fde28c9 100644 --- a/src/LanguageServer/Impl/Completion/CompletionSource.cs +++ b/src/LanguageServer/Impl/Completion/CompletionSource.cs @@ -40,7 +40,7 @@ public CompletionResult GetCompletions(IDocumentAnalysis analysis, SourceLocatio // no completions on integer ., the user is typing a float return CompletionResult.Empty; case ConstantExpression ce2 when ce2.Value is string: - // no completions in strings + // no completions in strings case null when context.Ast.IsInsideComment(context.Location): case null when context.Ast.IsInsideString(context.Location): return CompletionResult.Empty; @@ -82,10 +82,11 @@ public CompletionResult GetCompletions(IDocumentAnalysis analysis, SourceLocatio return result; default: { var result = ErrorExpressionCompletion.GetCompletions(scope, statement, expression, context); - return result == CompletionResult.Empty - ? TopLevelCompletion.GetCompletions(statement, scope, context) - : result; - } + if (result == null) { + return CompletionResult.Empty; + } + return result == CompletionResult.Empty ? TopLevelCompletion.GetCompletions(statement, scope, context) : result; + } } } } diff --git a/src/LanguageServer/Impl/Completion/ErrorExpressionCompletion.cs b/src/LanguageServer/Impl/Completion/ErrorExpressionCompletion.cs index 662f060e2..9508c2ba2 100644 --- a/src/LanguageServer/Impl/Completion/ErrorExpressionCompletion.cs +++ b/src/LanguageServer/Impl/Completion/ErrorExpressionCompletion.cs @@ -83,6 +83,9 @@ public static CompletionResult GetCompletions(ScopeStatement scope, Node stateme case TokenKind.KeywordAs: return lastToken.Key.Start <= context.Position && context.Position <= lastToken.Key.End ? null : CompletionResult.Empty; + case TokenKind.Error: + return null; + default: return CompletionResult.Empty; } diff --git a/src/LanguageServer/Test/CompletionTests.cs b/src/LanguageServer/Test/CompletionTests.cs index d832e4ccf..f0c0770d6 100644 --- a/src/LanguageServer/Test/CompletionTests.cs +++ b/src/LanguageServer/Test/CompletionTests.cs @@ -854,6 +854,15 @@ public async Task NoCompletionInString() { result.Should().HaveNoCompletion(); } + [TestMethod, Priority(0)] + public async Task NoCompletionInOpenString() { + + var analysis = await GetAnalysisAsync("'''."); + var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion); + var result = cs.GetCompletions(analysis, new SourceLocation(1, 5)); + result.Should().HaveNoCompletion(); + } + [TestMethod, Priority(0)] public async Task NoCompletionInComment() { From 9e83e82a34bca0b32e04650d2997d81aa03ada1f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Mar 2019 12:59:49 -0700 Subject: [PATCH 5/5] Fix OOB exception --- src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs | 2 +- src/LanguageServer/Test/CompletionTests.cs | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs index d06f48b6e..adb4d3183 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs @@ -70,7 +70,7 @@ private void HandleImport(ModuleName moduleImportExpression, NameExpression asNa // "import fob.oar.baz" is handled as fob = import_module('fob') if (asNameExpression != null) { Eval.DeclareVariable(asNameExpression.Name, variableModule, VariableSource.Import, asNameExpression); - } else if (_variableModules.TryGetValue(importNames[0], out variableModule)) { + } else if (importNames.Count > 0 && _variableModules.TryGetValue(importNames[0], out variableModule)) { var firstName = moduleImportExpression.Names[0]; Eval.DeclareVariable(importNames[0], variableModule, VariableSource.Import, firstName); } diff --git a/src/LanguageServer/Test/CompletionTests.cs b/src/LanguageServer/Test/CompletionTests.cs index f0c0770d6..ad372f8ca 100644 --- a/src/LanguageServer/Test/CompletionTests.cs +++ b/src/LanguageServer/Test/CompletionTests.cs @@ -863,6 +863,13 @@ public async Task NoCompletionInOpenString() { result.Should().HaveNoCompletion(); } + [TestMethod, Priority(0)] + public async Task NoCompletionBadImportExpression() { + var analysis = await GetAnalysisAsync("import os,."); + var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion); + cs.GetCompletions(analysis, new SourceLocation(1, 12)); // Should not crash. + } + [TestMethod, Priority(0)] public async Task NoCompletionInComment() {