From c6a9c223be9265f72b042dfcf41aad7006e9f9ba Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 30 Sep 2019 12:03:05 -0700 Subject: [PATCH 1/5] Remove stale reference --- .../Impl/Microsoft.Python.LanguageServer.csproj | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj b/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj index ca455737f..a33882e0d 100644 --- a/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj +++ b/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj @@ -33,14 +33,6 @@ - - - $(AnalysisReference)/Microsoft.Python.Analysis.Engine.dll - - - PreserveNewest - - From 6109ac761f1c2398c5ca6b4f38826cfb28051454 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Nov 2019 13:55:07 -0800 Subject: [PATCH 2/5] Don't suppress LHS diagnostics on augmented assign --- .../UndefinedVariables/UndefinedVariablesWalker.cs | 2 -- src/Analysis/Ast/Test/AssignmentTests.cs | 9 +++++++++ src/Analysis/Ast/Test/LintUndefinedVarsTests.cs | 12 ++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 7365bf08d..42ac73bf0 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -49,9 +49,7 @@ public override bool Walk(SuiteStatement node) { HandleNonLocal(nls); break; case AugmentedAssignStatement augs: - _suppressDiagnostics = true; augs.Left?.Walk(new ExpressionWalker(this)); - _suppressDiagnostics = false; augs.Right?.Walk(new ExpressionWalker(this)); break; case AssignmentStatement asst: diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index cf3a54344..298454515 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -219,6 +219,15 @@ def __init__(self): .Which.Should().HaveMembers("abc", "y", "__class__"); } + [TestMethod, Priority(0)] + public async Task AugmentedAssignToUndefined() { + const string code = @" +x += 1 +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().NotHaveVariable("x"); + } + [TestMethod, Priority(0)] public async Task BaseInstanceVariable() { const string code = @" diff --git a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index cdee6b70f..dec8c849a 100644 --- a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -821,6 +821,18 @@ class Subclass(MyClass): d.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task AugmentedAssignToUndefined() { + const string code = @" +x += 1 +"; + var d = await LintAsync(code); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(2, 1, 2, 2); + } + + private async Task> LintAsync(string code, InterpreterConfiguration configuration = null) { var analysis = await GetAnalysisAsync(code, configuration ?? PythonVersions.LatestAvailable3X); var a = Services.GetService(); From bcfc3b7246947674cbc18d7a76d2b90c105acf89 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Nov 2019 15:30:27 -0800 Subject: [PATCH 3/5] Revert "Don't suppress LHS diagnostics on augmented assign" This reverts commit 6109ac761f1c2398c5ca6b4f38826cfb28051454. --- .../UndefinedVariables/UndefinedVariablesWalker.cs | 2 ++ src/Analysis/Ast/Test/AssignmentTests.cs | 9 --------- src/Analysis/Ast/Test/LintUndefinedVarsTests.cs | 12 ------------ 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 42ac73bf0..7365bf08d 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -49,7 +49,9 @@ public override bool Walk(SuiteStatement node) { HandleNonLocal(nls); break; case AugmentedAssignStatement augs: + _suppressDiagnostics = true; augs.Left?.Walk(new ExpressionWalker(this)); + _suppressDiagnostics = false; augs.Right?.Walk(new ExpressionWalker(this)); break; case AssignmentStatement asst: diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index 298454515..cf3a54344 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -219,15 +219,6 @@ def __init__(self): .Which.Should().HaveMembers("abc", "y", "__class__"); } - [TestMethod, Priority(0)] - public async Task AugmentedAssignToUndefined() { - const string code = @" -x += 1 -"; - var analysis = await GetAnalysisAsync(code); - analysis.Should().NotHaveVariable("x"); - } - [TestMethod, Priority(0)] public async Task BaseInstanceVariable() { const string code = @" diff --git a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index dec8c849a..cdee6b70f 100644 --- a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -821,18 +821,6 @@ class Subclass(MyClass): d.Should().BeEmpty(); } - [TestMethod, Priority(0)] - public async Task AugmentedAssignToUndefined() { - const string code = @" -x += 1 -"; - var d = await LintAsync(code); - d.Should().HaveCount(1); - d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); - d[0].SourceSpan.Should().Be(2, 1, 2, 2); - } - - private async Task> LintAsync(string code, InterpreterConfiguration configuration = null) { var analysis = await GetAnalysisAsync(code, configuration ?? PythonVersions.LatestAvailable3X); var a = Services.GetService(); From 86c20c62df29111c31da785cccffcb08ec481258 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 10 Dec 2019 10:23:13 -0800 Subject: [PATCH 4/5] Handle gotodef on unknown types --- .../Impl/Sources/DefinitionSource.cs | 5 ++++- .../Test/GoToDefinitionTests.cs | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index 5bbebe062..f6c14532c 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -123,6 +123,8 @@ private Reference HandleFromImport(IDocumentAnalysis analysis, SourceLocation lo case ImplicitPackageImport packageImport: module = mres.GetImportedModule(packageImport.FullName); break; + case ImportNotFound _: + return null; } // Are we in the module name (i.e. A in 'from A import B')? @@ -262,7 +264,8 @@ private Reference TryFromVariable(string name, IDocumentAnalysis analysis, Sourc definingMember = null; var m = analysis.ExpressionEvaluator.LookupNameInScopes(name, out var scope, LookupOptions.All); - if (m == null || scope.Module.ModuleType == ModuleType.Builtins || !(scope.Variables[name] is IVariable v)) { + var v = scope?.Variables[name]; + if (m == null || scope == null || scope.Module.ModuleType == ModuleType.Builtins || scope.Variables[name].IsUnknown()) { return null; } diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index 6f98ec046..37bcb933b 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -688,5 +688,27 @@ def c(): ... reference.uri.AbsolutePath.Should().Contain("other.py"); reference.range.Should().Be(3, 4, 3, 5); } + + [TestMethod, Priority(0)] + public async Task UnknownType() { + const string code = @" +A +"; + var analysis = await GetAnalysisAsync(code); + var ds = new DefinitionSource(Services); + var reference = ds.FindDefinition(analysis, new SourceLocation(2, 1), out _); + reference.Should().BeNull(); + } + + [TestMethod, Priority(0)] + public async Task UnknownImportedType() { + const string code = @" +from nonexistent import some +"; + var analysis = await GetAnalysisAsync(code); + var ds = new DefinitionSource(Services); + var reference = ds.FindDefinition(analysis, new SourceLocation(2, 26), out _); + reference.Should().BeNull(); + } } } From cf3c0dd1463027bf3db56cbd1813a41f97abf4a3 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 10 Dec 2019 10:54:31 -0800 Subject: [PATCH 5/5] PR feedback --- src/LanguageServer/Impl/Sources/DefinitionSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index f6c14532c..a91cad1fe 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -265,7 +265,7 @@ private Reference TryFromVariable(string name, IDocumentAnalysis analysis, Sourc var m = analysis.ExpressionEvaluator.LookupNameInScopes(name, out var scope, LookupOptions.All); var v = scope?.Variables[name]; - if (m == null || scope == null || scope.Module.ModuleType == ModuleType.Builtins || scope.Variables[name].IsUnknown()) { + if (m == null || scope == null || scope.Module.ModuleType == ModuleType.Builtins || v.IsUnknown()) { return null; }