From c6a9c223be9265f72b042dfcf41aad7006e9f9ba Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 30 Sep 2019 12:03:05 -0700 Subject: [PATCH 1/7] 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/7] 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/7] 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 c16646d97345765a93cac68179e467e9fd8ce697 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 12 Dec 2019 23:04:51 -0800 Subject: [PATCH 4/7] Escape [ and ] --- .../Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs | 5 ++--- .../Impl/Documentation/DocstringConverter.cs | 2 +- src/LanguageServer/Test/DocstringConverterTests.cs | 7 +++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs index 5348dbc2d..d4b3b79d1 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs @@ -38,10 +38,9 @@ internal sealed class FunctionEvaluator : MemberEvaluator { public FunctionEvaluator(ExpressionEval eval, PythonFunctionOverload overload) : base(eval, overload.FunctionDefinition) { _overload = overload; - _function = overload.ClassMember ?? throw new NullReferenceException(nameof(overload.ClassMember)); + _function = overload.ClassMember ?? throw new ArgumentNullException(nameof(overload)); _self = _function.DeclaringType as PythonClassType; - - FunctionDefinition = overload.FunctionDefinition; + FunctionDefinition = overload.FunctionDefinition ?? throw new ArgumentNullException(nameof(overload)); } private FunctionDefinition FunctionDefinition { get; } diff --git a/src/LanguageServer/Impl/Documentation/DocstringConverter.cs b/src/LanguageServer/Impl/Documentation/DocstringConverter.cs index 046488540..3f345a11a 100644 --- a/src/LanguageServer/Impl/Documentation/DocstringConverter.cs +++ b/src/LanguageServer/Impl/Documentation/DocstringConverter.cs @@ -176,7 +176,7 @@ private static readonly (Regex, string)[] PotentialHeaders = new[] { private static readonly Regex TildaHeaderRegex = new Regex(@"^\s*~~~+$", RegexOptions.Singleline | RegexOptions.Compiled); private static readonly Regex PlusHeaderRegex = new Regex(@"^\s*\+\+\++$", RegexOptions.Singleline | RegexOptions.Compiled); private static readonly Regex LeadingAsteriskRegex = new Regex(@"^(\s+\* )(.*)$", RegexOptions.Singleline | RegexOptions.Compiled); - private static readonly Regex UnescapedMarkdownCharsRegex = new Regex(@"(? Date: Fri, 13 Dec 2019 11:09:45 -0800 Subject: [PATCH 5/7] PR feedback --- src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs index d4b3b79d1..cba3040ca 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs @@ -38,9 +38,9 @@ internal sealed class FunctionEvaluator : MemberEvaluator { public FunctionEvaluator(ExpressionEval eval, PythonFunctionOverload overload) : base(eval, overload.FunctionDefinition) { _overload = overload; - _function = overload.ClassMember ?? throw new ArgumentNullException(nameof(overload)); + _function = overload.ClassMember ?? throw new ArgumentNullException(nameof(overload.ClassMember)); _self = _function.DeclaringType as PythonClassType; - FunctionDefinition = overload.FunctionDefinition ?? throw new ArgumentNullException(nameof(overload)); + FunctionDefinition = overload.FunctionDefinition ?? throw new ArgumentNullException(nameof(overload.FunctionDefinition)); } private FunctionDefinition FunctionDefinition { get; } From 94db074a358169a66a690d0ea8cf15f044bb3ac0 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 9 Jun 2020 07:56:01 -0700 Subject: [PATCH 6/7] Improve linting of LHS expressions (cherry picked from commit a4e26084a227b5a9ac1603cfe2f839e5b89e0730) --- .../UndefinedVariables/ExpressionWalker.cs | 1 - .../UndefinedVariablesWalker.cs | 15 +++++---------- src/Analysis/Ast/Test/LintNoSelfArgumentTests.cs | 1 - src/Analysis/Ast/Test/LintReturnInInitTests.cs | 1 - src/Analysis/Ast/Test/LintUndefinedVarsTests.cs | 12 ++++++++++++ 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 011fe16f2..acbc8e35f 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -17,7 +17,6 @@ using System.Linq; using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Core; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing.Ast; using Microsoft.Python.Parsing.Extensions; diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 42ac73bf0..8e13f750f 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -26,7 +26,6 @@ namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class UndefinedVariablesWalker : LinterWalker { private readonly List _diagnostics = new List(); - private bool _suppressDiagnostics; public UndefinedVariablesWalker(IDocumentAnalysis analysis, IServiceContainer services) : base(analysis, services) { } @@ -53,11 +52,9 @@ public override bool Walk(SuiteStatement node) { augs.Right?.Walk(new ExpressionWalker(this)); break; case AssignmentStatement asst: - _suppressDiagnostics = true; - foreach (var lhs in asst.Left ?? Enumerable.Empty()) { + foreach (var lhs in asst.Left.MaybeEnumerate().Where(x => !(x is NameExpression))) { lhs?.Walk(new ExpressionWalker(this)); } - _suppressDiagnostics = false; asst.Right?.Walk(new ExpressionWalker(this)); break; default: @@ -73,12 +70,10 @@ public void ReportUndefinedVariable(NameExpression node) { ReportUndefinedVariable(node.Name, eval.GetLocation(node).Span); } - public void ReportUndefinedVariable(string name, SourceSpan span) { - if (!_suppressDiagnostics) { - _diagnostics.Add(new DiagnosticsEntry( - Resources.UndefinedVariable.FormatInvariant(name), - span, ErrorCodes.UndefinedVariable, Severity.Warning, DiagnosticSource.Linter)); - } + private void ReportUndefinedVariable(string name, SourceSpan span) { + _diagnostics.Add(new DiagnosticsEntry( + Resources.UndefinedVariable.FormatInvariant(name), + span, ErrorCodes.UndefinedVariable, Severity.Warning, DiagnosticSource.Linter)); } private void HandleGlobal(GlobalStatement node) { diff --git a/src/Analysis/Ast/Test/LintNoSelfArgumentTests.cs b/src/Analysis/Ast/Test/LintNoSelfArgumentTests.cs index af7297e43..f3cd45970 100644 --- a/src/Analysis/Ast/Test/LintNoSelfArgumentTests.cs +++ b/src/Analysis/Ast/Test/LintNoSelfArgumentTests.cs @@ -19,7 +19,6 @@ using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Tests.FluentAssertions; using Microsoft.Python.Core; -using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; diff --git a/src/Analysis/Ast/Test/LintReturnInInitTests.cs b/src/Analysis/Ast/Test/LintReturnInInitTests.cs index 4ae7626a2..8e7bee0ee 100644 --- a/src/Analysis/Ast/Test/LintReturnInInitTests.cs +++ b/src/Analysis/Ast/Test/LintReturnInInitTests.cs @@ -18,7 +18,6 @@ using FluentAssertions; using Microsoft.Python.Analysis.Tests.FluentAssertions; using Microsoft.Python.Parsing; -using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; diff --git a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index dec8c849a..00988c53a 100644 --- a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -832,6 +832,18 @@ public async Task AugmentedAssignToUndefined() { d[0].SourceSpan.Should().Be(2, 1, 2, 2); } + [TestMethod, Priority(0)] + public async Task UndefinedInInit() { + const string code = @" +class TestIt(): + def __init__(self): + slf.y = 6 +"; + var d = await LintAsync(code); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(4, 9, 4, 12); + } private async Task> LintAsync(string code, InterpreterConfiguration configuration = null) { var analysis = await GetAnalysisAsync(code, configuration ?? PythonVersions.LatestAvailable3X); From 574c56579e0dfe5bf0e3a69bc24be298e5e42371 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 10 Jun 2020 11:07:53 -0700 Subject: [PATCH 7/7] PR feedback --- .../UndefinedVariablesWalker.cs | 19 ++++++++++++++----- .../Ast/Test/LintUndefinedVarsTests.cs | 12 ++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 8e13f750f..b35fc280c 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -26,6 +26,7 @@ namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class UndefinedVariablesWalker : LinterWalker { private readonly List _diagnostics = new List(); + private bool _suppressDiagnostics; public UndefinedVariablesWalker(IDocumentAnalysis analysis, IServiceContainer services) : base(analysis, services) { } @@ -52,9 +53,15 @@ public override bool Walk(SuiteStatement node) { augs.Right?.Walk(new ExpressionWalker(this)); break; case AssignmentStatement asst: - foreach (var lhs in asst.Left.MaybeEnumerate().Where(x => !(x is NameExpression))) { - lhs?.Walk(new ExpressionWalker(this)); + var lhs = asst.Left.MaybeEnumerate().ToArray(); + if (lhs.Length > 0) { + lhs[0]?.Walk(new ExpressionWalker(this)); } + _suppressDiagnostics = true; + foreach (var l in lhs.Skip(1)) { + l?.Walk(new ExpressionWalker(this)); + } + _suppressDiagnostics = false; asst.Right?.Walk(new ExpressionWalker(this)); break; default: @@ -71,9 +78,11 @@ public void ReportUndefinedVariable(NameExpression node) { } private void ReportUndefinedVariable(string name, SourceSpan span) { - _diagnostics.Add(new DiagnosticsEntry( - Resources.UndefinedVariable.FormatInvariant(name), - span, ErrorCodes.UndefinedVariable, Severity.Warning, DiagnosticSource.Linter)); + if (!_suppressDiagnostics) { + _diagnostics.Add(new DiagnosticsEntry( + Resources.UndefinedVariable.FormatInvariant(name), + span, ErrorCodes.UndefinedVariable, Severity.Warning, DiagnosticSource.Linter)); + } } private void HandleGlobal(GlobalStatement node) { diff --git a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index 00988c53a..a53ff5abb 100644 --- a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -845,6 +845,18 @@ def __init__(self): d[0].SourceSpan.Should().Be(4, 9, 4, 12); } + [TestMethod, Priority(0)] + public async Task UndefinedIndex() { + const string code = @" +x = [1, 2] +x[y] = 1 +"; + var d = await LintAsync(code); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(3, 3, 3, 4); + } + private async Task> LintAsync(string code, InterpreterConfiguration configuration = null) { var analysis = await GetAnalysisAsync(code, configuration ?? PythonVersions.LatestAvailable3X); var a = Services.GetService();