From 8bca56b1fa4ab8b282ae6491de25525c90aa9efe Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Mar 2019 09:51:05 -0800 Subject: [PATCH 01/27] 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 ba126e70d76fe0be3868757c252046685375b82f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sun, 3 Mar 2019 21:43:40 -0800 Subject: [PATCH 02/27] Undef variables, first cut --- .../Definitions/IExpressionEvaluator.cs | 3 +- .../LookupOptions.cs | 4 +- .../Ast/Impl/Analyzer/ModuleWalker.cs | 1 + .../Ast/Impl/Analyzer/PythonAnalyzer.cs | 5 +- .../Ast/Impl/Diagnostics/ErrorCodes.cs | 1 + .../Ast/Impl/Linting/AnalysisExtensions.cs | 31 ++++ .../Ast/Impl/Linting/ExpressionWalker.cs | 143 ++++++++++++++++++ src/Analysis/Ast/Impl/Linting/Linter.cs | 23 +++ src/Analysis/Ast/Impl/Linting/LinterWalker.cs | 92 +++++++++++ src/Analysis/Ast/Impl/Resources.Designer.cs | 9 ++ src/Analysis/Ast/Impl/Resources.resx | 3 + 11 files changed, 311 insertions(+), 4 deletions(-) rename src/Analysis/Ast/Impl/Analyzer/{Evaluation => Definitions}/LookupOptions.cs (90%) create mode 100644 src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs create mode 100644 src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs create mode 100644 src/Analysis/Ast/Impl/Linting/Linter.cs create mode 100644 src/Analysis/Ast/Impl/Linting/LinterWalker.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/Definitions/IExpressionEvaluator.cs b/src/Analysis/Ast/Impl/Analyzer/Definitions/IExpressionEvaluator.cs index b674b7770..285d9d156 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Definitions/IExpressionEvaluator.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Definitions/IExpressionEvaluator.cs @@ -15,6 +15,7 @@ using System; using System.Collections.Generic; +using Microsoft.Python.Analysis.Analyzer.Evaluation; using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; @@ -57,7 +58,7 @@ public interface IExpressionEvaluator { /// IMember GetValueFromExpression(Expression expr); - IMember LookupNameInScopes(string name, out IScope scope); + IMember LookupNameInScopes(string name, out IScope scope, LookupOptions options = LookupOptions.Normal); IPythonType GetTypeFromString(string typeString); diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/LookupOptions.cs b/src/Analysis/Ast/Impl/Analyzer/Definitions/LookupOptions.cs similarity index 90% rename from src/Analysis/Ast/Impl/Analyzer/Evaluation/LookupOptions.cs rename to src/Analysis/Ast/Impl/Analyzer/Definitions/LookupOptions.cs index 7a5f9a5b0..32956751e 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/LookupOptions.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Definitions/LookupOptions.cs @@ -15,9 +15,9 @@ using System; -namespace Microsoft.Python.Analysis.Analyzer.Evaluation { +namespace Microsoft.Python.Analysis.Analyzer { [Flags] - internal enum LookupOptions { + public enum LookupOptions { None = 0, Local, Nonlocal, diff --git a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs index 3ba82e9aa..1769d5291 100644 --- a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs @@ -15,6 +15,7 @@ using System.Diagnostics; using Microsoft.Python.Analysis.Documents; +using Microsoft.Python.Analysis.Linting; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 205e64d01..fdb1bac6c 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -16,12 +16,12 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.Python.Analysis.Core.DependencyResolution; using Microsoft.Python.Analysis.Dependencies; using Microsoft.Python.Analysis.Documents; +using Microsoft.Python.Analysis.Linting; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Core; @@ -267,6 +267,9 @@ private void Analyze(IDependencyChainNode node, int version cancellationToken.ThrowIfCancellationRequested(); var analysis = new DocumentAnalysis((IDocument)module, version, walker.GlobalScope, walker.Eval); + var linter = new Linter(); + linter.Lint(analysis); + (module as IAnalyzable)?.NotifyAnalysisComplete(analysis); node.Value.TrySetAnalysis(analysis, version, _syncObj); diff --git a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs index 6f7289338..dcebb7356 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs @@ -22,5 +22,6 @@ public static class ErrorCodes { public const string ParameterAlreadySpecified = "parameter-already-specified"; public const string ParameterMissing = "parameter-missing"; public const string UnresolvedImport = "unresolved-import"; + public const string UndefinedVariable = "undefined-variable"; } } diff --git a/src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs b/src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs new file mode 100644 index 000000000..c17664680 --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs @@ -0,0 +1,31 @@ +// 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.Diagnostics; +using Microsoft.Python.Core; +using Microsoft.Python.Parsing; +using Microsoft.Python.Parsing.Ast; +using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; + +namespace Microsoft.Python.Analysis.Linting { + internal static class AnalysisExtensions { + public static void ReportUndefinedVariable(this IDocumentAnalysis analysis, NameExpression node) { + var eval = analysis.ExpressionEvaluator; + eval.ReportDiagnostics(analysis.Document.Uri, new DiagnosticsEntry( + Resources.UndefinedVariable.FormatInvariant(node.Name), + eval.GetLocation(node).Span, ErrorCodes.UndefinedVariable, Severity.Warning)); + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs new file mode 100644 index 000000000..acd3ca1fc --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs @@ -0,0 +1,143 @@ +// 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 System.Collections.Generic; +using System.Linq; +using Microsoft.Python.Parsing.Ast; + +namespace Microsoft.Python.Analysis.Linting { + internal sealed class ExpressionWalker : PythonWalker { + private readonly IDocumentAnalysis _analysis; + private readonly HashSet _additionalNames; + private readonly HashSet _additionalNameNodes; + + public ExpressionWalker(IDocumentAnalysis analysis) + : this(analysis, null, null) { } + + public ExpressionWalker(IDocumentAnalysis analysis, HashSet additionalNames, HashSet additionalNameNodes) { + _analysis = analysis; + _additionalNames = additionalNames; + _additionalNameNodes = additionalNameNodes; + } + + public override bool Walk(CallExpression node) { + foreach (var arg in node.Args) { + arg?.Expression?.Walk(this); + } + return false; + } + + public override bool Walk(ListComprehension node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + + public override bool Walk(SetComprehension node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + public override bool Walk(DictionaryComprehension node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + + public override bool Walk(GeneratorExpression node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + + public override bool Walk(NameExpression node) { + if (_additionalNames?.Contains(node.Name) == true) { + return false; + } + if (_additionalNameNodes?.Contains(node) == true) { + return false; + } + var m = _analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out _); + if (m == null) { + _analysis.ReportUndefinedVariable(node); + } + return false; + } + } + + internal sealed class ComprehensionWalker : PythonWalker { + private readonly IDocumentAnalysis _analysis; + private readonly HashSet _names = new HashSet(); + private readonly HashSet _additionalNameNodes = new HashSet(); + + public ComprehensionWalker(IDocumentAnalysis analysis) { + _analysis = analysis; + } + + public override bool Walk(GeneratorExpression node) { + ProcessComprehension(node, node.Item, node.Iterators); + return false; + } + + public override bool Walk(ListComprehension node) { + ProcessComprehension(node, node.Item, node.Iterators); + return false; + } + + public override bool Walk(SetComprehension node) { + ProcessComprehension(node, node.Item, node.Iterators); + return false; + } + + public override bool Walk(DictionaryComprehension node) { + CollectNames(node); + node.Key?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); + node.Value?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); + foreach (var iter in node.Iterators) { + iter?.Walk(new ExpressionWalker(_analysis, null, _additionalNameNodes)); + } + return true; + } + + private void CollectNames(Comprehension c) { + var nc = new NameCollectorWalker(_names, _additionalNameNodes); + foreach (var cfor in c.Iterators.OfType()) { + cfor.Left?.Walk(nc); + } + } + + private void ProcessComprehension(Comprehension c, Node item, IEnumerable iterators) { + CollectNames(c); + item?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); + foreach (var iter in iterators) { + iter.Walk(new ExpressionWalker(_analysis, null, _additionalNameNodes)); + } + } + + private sealed class NameCollectorWalker : PythonWalker { + private readonly HashSet _names; + private readonly HashSet _additionalNameNodes; + + public NameCollectorWalker(HashSet names, HashSet additionalNameNodes) { + _names = names; + _additionalNameNodes = additionalNameNodes; + } + + public override bool Walk(NameExpression nex) { + if (!string.IsNullOrEmpty(nex.Name)) { + _names.Add(nex.Name); + _additionalNameNodes.Add(nex); + } + return false; + } + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/Linter.cs b/src/Analysis/Ast/Impl/Linting/Linter.cs new file mode 100644 index 000000000..b4bc60d50 --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/Linter.cs @@ -0,0 +1,23 @@ +// 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. + +namespace Microsoft.Python.Analysis.Linting { + internal sealed class Linter { + public void Lint(IDocumentAnalysis analysis) { + var w = new LinterWalker(analysis); + analysis.Ast.Walk(w); + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/LinterWalker.cs b/src/Analysis/Ast/Impl/Linting/LinterWalker.cs new file mode 100644 index 000000000..27a77c7ba --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/LinterWalker.cs @@ -0,0 +1,92 @@ +// 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 System; +using System.Collections.Generic; +using Microsoft.Python.Analysis.Analyzer; +using Microsoft.Python.Parsing.Ast; + +namespace Microsoft.Python.Analysis.Linting { + internal sealed class LinterWalker: PythonWalker { + private readonly IDocumentAnalysis _analysis; + private readonly Stack _scopeStack = new Stack(); + private readonly ExpressionWalker _expressionWalker; + + public LinterWalker(IDocumentAnalysis analysis) { + _analysis = analysis; + _expressionWalker = new ExpressionWalker(_analysis); + } + + public override bool Walk(ClassDefinition cd) { + _scopeStack.Push(_analysis.ExpressionEvaluator.OpenScope(_analysis.Document, cd)); + return true; + } + public override void PostWalk(ClassDefinition cd) => _scopeStack.Pop().Dispose(); + + public override bool Walk(FunctionDefinition fd) { + _scopeStack.Push(_analysis.ExpressionEvaluator.OpenScope(_analysis.Document, fd)); + return true; + } + public override void PostWalk(FunctionDefinition cd) => _scopeStack.Pop().Dispose(); + + public override bool Walk(AssignmentStatement node) { + if (node.Right is ErrorExpression) { + return false; + } + node.Right?.Walk(_expressionWalker); + return false; + } + + public override bool Walk(CallExpression node) { + foreach (var arg in node.Args) { + arg?.Expression?.Walk(_expressionWalker); + } + return false; + } + + public override bool Walk(IfStatement node) { + foreach (var test in node.Tests) { + test.Test.Walk(_expressionWalker); + } + return true; + } + + public override bool Walk(GlobalStatement node) { + foreach (var nex in node.Names) { + var m = _analysis.ExpressionEvaluator.LookupNameInScopes(nex.Name, out _, LookupOptions.Global); + if (m == null) { + _analysis.ReportUndefinedVariable(nex); + } + } + return false; + } + + public override bool Walk(NonlocalStatement node) { + foreach (var nex in node.Names) { + var m = _analysis.ExpressionEvaluator.LookupNameInScopes(nex.Name, out _, LookupOptions.Nonlocal); + if (m == null) { + _analysis.ReportUndefinedVariable(nex); + } + } + return false; + } + public override bool Walk(ComprehensionFor cfor) { + return false; + } + public override bool Walk(ComprehensionIf cif) { + return false; + } + } +} diff --git a/src/Analysis/Ast/Impl/Resources.Designer.cs b/src/Analysis/Ast/Impl/Resources.Designer.cs index 89bef1d20..77dcc4416 100644 --- a/src/Analysis/Ast/Impl/Resources.Designer.cs +++ b/src/Analysis/Ast/Impl/Resources.Designer.cs @@ -645,6 +645,15 @@ internal static string SpaceWithinIndexBracketsShort { } } + /// + /// Looks up a localized string similar to Undefined variable: '{0}'. + /// + internal static string UndefinedVariable { + get { + return ResourceManager.GetString("UndefinedVariable", resourceCulture); + } + } + /// /// Looks up a localized string similar to If checked, comments are wrapped to the specified width. If unchecked, comments are not modified.. /// diff --git a/src/Analysis/Ast/Impl/Resources.resx b/src/Analysis/Ast/Impl/Resources.resx index 10cd30723..687a186ac 100644 --- a/src/Analysis/Ast/Impl/Resources.resx +++ b/src/Analysis/Ast/Impl/Resources.resx @@ -339,4 +339,7 @@ Unknown parameter name. + + Undefined variable: '{0}' + \ No newline at end of file From bde1db1e4ae52f96a1cbec8aee87ed7cfb09a536 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 4 Mar 2019 10:09:38 -0800 Subject: [PATCH 03/27] Reorg --- .../Ast/Impl/Analyzer/PythonAnalyzer.cs | 4 +- .../Impl/Linting/{Linter.cs => ILinter.cs} | 9 +- .../Ast/Impl/Linting/LinterAggregator.cs | 33 ++++ src/Analysis/Ast/Impl/Linting/LinterWalker.cs | 67 ++----- .../Impl/Linting/UndefinedVariablesLinter.cs | 25 +++ .../Impl/Linting/UndefinedVariablesWalker.cs | 71 ++++++++ src/Analysis/Ast/Test/LinterTests.cs | 165 ++++++++++++++++++ 7 files changed, 311 insertions(+), 63 deletions(-) rename src/Analysis/Ast/Impl/Linting/{Linter.cs => ILinter.cs} (80%) create mode 100644 src/Analysis/Ast/Impl/Linting/LinterAggregator.cs create mode 100644 src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs create mode 100644 src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs create mode 100644 src/Analysis/Ast/Test/LinterTests.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index fdb1bac6c..c2cbfa528 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -267,8 +267,8 @@ private void Analyze(IDependencyChainNode node, int version cancellationToken.ThrowIfCancellationRequested(); var analysis = new DocumentAnalysis((IDocument)module, version, walker.GlobalScope, walker.Eval); - var linter = new Linter(); - linter.Lint(analysis); + var linter = new LinterAggregator(); + linter.Lint(analysis, _services); (module as IAnalyzable)?.NotifyAnalysisComplete(analysis); node.Value.TrySetAnalysis(analysis, version, _syncObj); diff --git a/src/Analysis/Ast/Impl/Linting/Linter.cs b/src/Analysis/Ast/Impl/Linting/ILinter.cs similarity index 80% rename from src/Analysis/Ast/Impl/Linting/Linter.cs rename to src/Analysis/Ast/Impl/Linting/ILinter.cs index b4bc60d50..947386dce 100644 --- a/src/Analysis/Ast/Impl/Linting/Linter.cs +++ b/src/Analysis/Ast/Impl/Linting/ILinter.cs @@ -13,11 +13,10 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. +using Microsoft.Python.Core; + namespace Microsoft.Python.Analysis.Linting { - internal sealed class Linter { - public void Lint(IDocumentAnalysis analysis) { - var w = new LinterWalker(analysis); - analysis.Ast.Walk(w); - } + public interface ILinter { + void Lint(IDocumentAnalysis analysis, IServiceContainer services); } } diff --git a/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs b/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs new file mode 100644 index 000000000..5692668bc --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs @@ -0,0 +1,33 @@ +// 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 System.Collections.Generic; +using Microsoft.Python.Core; + +namespace Microsoft.Python.Analysis.Linting { + internal sealed class LinterAggregator { + private readonly List _linters = new List(); + + public LinterAggregator() { + // TODO: develop mechanism for dynamic and external linter discovery. + _linters.Add(new UndefinedVariablesLinter()); + } + public void Lint(IDocumentAnalysis analysis, IServiceContainer services) { + foreach (var l in _linters) { + l.Lint(analysis, services); + } + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/LinterWalker.cs b/src/Analysis/Ast/Impl/Linting/LinterWalker.cs index 27a77c7ba..5fe727f2e 100644 --- a/src/Analysis/Ast/Impl/Linting/LinterWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/LinterWalker.cs @@ -16,77 +16,32 @@ using System; using System.Collections.Generic; using Microsoft.Python.Analysis.Analyzer; +using Microsoft.Python.Core; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Linting { - internal sealed class LinterWalker: PythonWalker { - private readonly IDocumentAnalysis _analysis; + internal abstract class LinterWalker: PythonWalker { private readonly Stack _scopeStack = new Stack(); - private readonly ExpressionWalker _expressionWalker; - public LinterWalker(IDocumentAnalysis analysis) { - _analysis = analysis; - _expressionWalker = new ExpressionWalker(_analysis); + protected IDocumentAnalysis Analysis { get; } + protected IExpressionEvaluator Eval => Analysis.ExpressionEvaluator; + protected IServiceContainer Services { get; } + + protected LinterWalker(IDocumentAnalysis analysis, IServiceContainer services) { + Analysis = analysis; + Services = services; } public override bool Walk(ClassDefinition cd) { - _scopeStack.Push(_analysis.ExpressionEvaluator.OpenScope(_analysis.Document, cd)); + _scopeStack.Push(Eval.OpenScope(Analysis.Document, cd)); return true; } public override void PostWalk(ClassDefinition cd) => _scopeStack.Pop().Dispose(); public override bool Walk(FunctionDefinition fd) { - _scopeStack.Push(_analysis.ExpressionEvaluator.OpenScope(_analysis.Document, fd)); + _scopeStack.Push(Eval.OpenScope(Analysis.Document, fd)); return true; } public override void PostWalk(FunctionDefinition cd) => _scopeStack.Pop().Dispose(); - - public override bool Walk(AssignmentStatement node) { - if (node.Right is ErrorExpression) { - return false; - } - node.Right?.Walk(_expressionWalker); - return false; - } - - public override bool Walk(CallExpression node) { - foreach (var arg in node.Args) { - arg?.Expression?.Walk(_expressionWalker); - } - return false; - } - - public override bool Walk(IfStatement node) { - foreach (var test in node.Tests) { - test.Test.Walk(_expressionWalker); - } - return true; - } - - public override bool Walk(GlobalStatement node) { - foreach (var nex in node.Names) { - var m = _analysis.ExpressionEvaluator.LookupNameInScopes(nex.Name, out _, LookupOptions.Global); - if (m == null) { - _analysis.ReportUndefinedVariable(nex); - } - } - return false; - } - - public override bool Walk(NonlocalStatement node) { - foreach (var nex in node.Names) { - var m = _analysis.ExpressionEvaluator.LookupNameInScopes(nex.Name, out _, LookupOptions.Nonlocal); - if (m == null) { - _analysis.ReportUndefinedVariable(nex); - } - } - return false; - } - public override bool Walk(ComprehensionFor cfor) { - return false; - } - public override bool Walk(ComprehensionIf cif) { - return false; - } } } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs new file mode 100644 index 000000000..70d6ba18d --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs @@ -0,0 +1,25 @@ +// 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.Core; + +namespace Microsoft.Python.Analysis.Linting { + internal sealed class UndefinedVariablesLinter : ILinter { + public void Lint(IDocumentAnalysis analysis, IServiceContainer services) { + var w = new UndefinedVariablesWalker(analysis, services); + analysis.Ast.Walk(w); + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs new file mode 100644 index 000000000..1d5c743ec --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs @@ -0,0 +1,71 @@ +// 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.Analyzer; +using Microsoft.Python.Core; +using Microsoft.Python.Parsing.Ast; + +namespace Microsoft.Python.Analysis.Linting { + internal sealed class UndefinedVariablesWalker : LinterWalker { + private readonly ExpressionWalker _expressionWalker; + + public UndefinedVariablesWalker(IDocumentAnalysis analysis, IServiceContainer services) + : base(analysis, services) { + _expressionWalker = new ExpressionWalker(analysis); + } + + public override bool Walk(AssignmentStatement node) { + if (node.Right is ErrorExpression) { + return false; + } + node.Right?.Walk(_expressionWalker); + return false; + } + + public override bool Walk(CallExpression node) { + foreach (var arg in node.Args) { + arg?.Expression?.Walk(_expressionWalker); + } + return false; + } + + public override bool Walk(IfStatement node) { + foreach (var test in node.Tests) { + test.Test.Walk(_expressionWalker); + } + return true; + } + + public override bool Walk(GlobalStatement node) { + foreach (var nex in node.Names) { + var m = Eval.LookupNameInScopes(nex.Name, out _, LookupOptions.Global); + if (m == null) { + Analysis.ReportUndefinedVariable(nex); + } + } + return false; + } + + public override bool Walk(NonlocalStatement node) { + foreach (var nex in node.Names) { + var m = Eval.LookupNameInScopes(nex.Name, out _, LookupOptions.Nonlocal); + if (m == null) { + Analysis.ReportUndefinedVariable(nex); + } + } + return false; + } + } +} diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs new file mode 100644 index 000000000..334b2bfd0 --- /dev/null +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -0,0 +1,165 @@ +// 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 System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Core; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Parsing; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.Analysis.Tests { + [TestClass] + public class LinterTests : AnalysisTestBase { + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [TestMethod, Priority(0)] + public async Task Variable() { + const string code = @" +import sys +e1, e2, e3 = sys.exc_info() +"; + var analysis = await GetAnalysisAsync(code); + // sys.exc_info() -> (exception_type, exception_value, traceback) + var f = analysis.Should() + .HaveVariable("e1").OfType(BuiltinTypeId.Type) + .And.HaveVariable("e2").OfType("BaseException") + .And.HaveVariable("e3").OfType("TracebackType") + .And.HaveVariable("sys").OfType(BuiltinTypeId.Module) + .Which.Should().HaveMember("exc_info").Which; + + f.Overloads.Should().HaveCount(1); + f.Overloads[0].GetReturnDocumentation(null) + .Should().Be(@"Tuple[ (Optional[Type[BaseException]],Optional[BaseException],Optional[TracebackType])]"); + } + + [TestMethod, Priority(0)] + public async Task TypeShedJsonMakeScanner() { + const string code = @"import _json +scanner = _json.make_scanner()"; + var analysis = await GetAnalysisAsync(code); + + var v0 = analysis.Should().HaveVariable("scanner").Which; + + v0.Should().HaveMember("__call__") + .Which.Should().HaveSingleOverload() + .Which.Should().HaveName("__call__") + .And.HaveParameters("self", "string", "index") + .And.HaveParameterAt(1).WithName("string").WithType("str").WithNoDefaultValue() + .And.HaveParameterAt(2).WithName("index").WithType("int").WithNoDefaultValue() + .And.HaveReturnDocumentation("Tuple[ (Any,int)]"); + } + + [TestMethod, Priority(0)] + public async Task MergeStubs() { + var analysis = await GetAnalysisAsync("import Package.Module\n\nc = Package.Module.Class()"); + + analysis.Should() + .HaveVariable("Package") + .Which.Value.Should().HaveMember("Module"); + + analysis.Should().HaveVariable("c") + .Which.Value.Should().HaveMembers("untyped_method", "inferred_method", "typed_method") + .And.NotHaveMembers("typed_method_2"); + } + + [TestMethod, Priority(0)] + public async Task TypeStubConditionalDefine() { + var seen = new HashSet(); + + const string code = @"import sys + +if sys.version_info < (2, 7): + LT_2_7 : bool = ... +if sys.version_info <= (2, 7): + LE_2_7 : bool = ... +if sys.version_info > (2, 7): + GT_2_7 : bool = ... +if sys.version_info >= (2, 7): + GE_2_7 : bool = ... + +"; + + var fullSet = new[] { "LT_2_7", "LE_2_7", "GT_2_7", "GE_2_7" }; + + foreach (var ver in PythonVersions.Versions) { + if (!seen.Add(ver.Version)) { + continue; + } + + Console.WriteLine(@"Testing with {0}", ver.InterpreterPath); + using (var s = await CreateServicesAsync(TestData.Root, ver)) { + var analysis = await GetAnalysisAsync(code, s, @"testmodule", TestData.GetTestSpecificPath(@"testmodule.pyi")); + + var expected = new List(); + var pythonVersion = ver.Version.ToLanguageVersion(); + if (pythonVersion.Is3x()) { + expected.Add("GT_2_7"); + expected.Add("GE_2_7"); + } else if (pythonVersion == PythonLanguageVersion.V27) { + expected.Add("GE_2_7"); + expected.Add("LE_2_7"); + } else { + expected.Add("LT_2_7"); + expected.Add("LE_2_7"); + } + + analysis.GlobalScope.Variables.Select(m => m.Name).Where(n => n.EndsWithOrdinal("2_7")) + .Should().Contain(expected) + .And.NotContain(fullSet.Except(expected)); + } + } + } + + [TestMethod, Priority(0)] + public async Task TypeShedNoTypeLeaks() { + const string code = @"import json +json.dump() +x = 1"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("json") + .Which.Should().HaveMember("dump"); + + analysis.Should().HaveVariable("x") + .Which.Should().HaveMember("bit_length") + .And.NotHaveMember("SIGABRT"); + } + + [TestMethod, Priority(0)] + public async Task VersionCheck() { + const string code = @" +import asyncio +loop = asyncio.get_event_loop() +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should() + .HaveVariable("loop") + .Which.Value.Should().HaveMembers("add_reader", "add_writer", "call_at", "close"); + } + } +} From 55cc25f279280b71369d4bf1ff0260665b37b692 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 4 Mar 2019 11:13:02 -0800 Subject: [PATCH 04/27] Tests --- .../Impl/Extensions/SourceSpanExtensions.cs | 28 +++ .../Ast/Impl/Linting/LinterAggregator.cs | 1 + .../AnalysisExtensions.cs | 2 +- .../ComprehensionWalker.cs} | 61 +----- .../UndefinedVariables/ExpressionWalker.cs | 93 ++++++++ .../UndefinedVariablesLinter.cs | 2 +- .../UndefinedVariablesWalker.cs | 2 +- src/Analysis/Ast/Test/LinterTests.cs | 206 +++++++++--------- 8 files changed, 238 insertions(+), 157 deletions(-) create mode 100644 src/Analysis/Ast/Impl/Extensions/SourceSpanExtensions.cs rename src/Analysis/Ast/Impl/Linting/{ => UndefinedVariables}/AnalysisExtensions.cs (95%) rename src/Analysis/Ast/Impl/Linting/{ExpressionWalker.cs => UndefinedVariables/ComprehensionWalker.cs} (62%) create mode 100644 src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs rename src/Analysis/Ast/Impl/Linting/{ => UndefinedVariables}/UndefinedVariablesLinter.cs (93%) rename src/Analysis/Ast/Impl/Linting/{ => UndefinedVariables}/UndefinedVariablesWalker.cs (97%) diff --git a/src/Analysis/Ast/Impl/Extensions/SourceSpanExtensions.cs b/src/Analysis/Ast/Impl/Extensions/SourceSpanExtensions.cs new file mode 100644 index 000000000..2bdd9f6b8 --- /dev/null +++ b/src/Analysis/Ast/Impl/Extensions/SourceSpanExtensions.cs @@ -0,0 +1,28 @@ +// Python Tools for Visual Studio +// 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.Core.Text; + +namespace Microsoft.Python.Analysis { + public static class SourceSpanExtensions { + public static bool IsAfter(this SourceSpan span, SourceSpan other) { + if (!span.IsValid || !other.IsValid) { + return false; + } + return span.Start > other.Start; + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs b/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs index 5692668bc..e20266ce1 100644 --- a/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs +++ b/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs @@ -14,6 +14,7 @@ // permissions and limitations under the License. using System.Collections.Generic; +using Microsoft.Python.Analysis.Linting.UndefinedVariables; using Microsoft.Python.Core; namespace Microsoft.Python.Analysis.Linting { diff --git a/src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/AnalysisExtensions.cs similarity index 95% rename from src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs rename to src/Analysis/Ast/Impl/Linting/UndefinedVariables/AnalysisExtensions.cs index c17664680..23450802b 100644 --- a/src/Analysis/Ast/Impl/Linting/AnalysisExtensions.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/AnalysisExtensions.cs @@ -19,7 +19,7 @@ using Microsoft.Python.Parsing.Ast; using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; -namespace Microsoft.Python.Analysis.Linting { +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal static class AnalysisExtensions { public static void ReportUndefinedVariable(this IDocumentAnalysis analysis, NameExpression node) { var eval = analysis.ExpressionEvaluator; diff --git a/src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs similarity index 62% rename from src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs rename to src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs index acd3ca1fc..a118d7658 100644 --- a/src/Analysis/Ast/Impl/Linting/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs @@ -17,62 +17,7 @@ using System.Linq; using Microsoft.Python.Parsing.Ast; -namespace Microsoft.Python.Analysis.Linting { - internal sealed class ExpressionWalker : PythonWalker { - private readonly IDocumentAnalysis _analysis; - private readonly HashSet _additionalNames; - private readonly HashSet _additionalNameNodes; - - public ExpressionWalker(IDocumentAnalysis analysis) - : this(analysis, null, null) { } - - public ExpressionWalker(IDocumentAnalysis analysis, HashSet additionalNames, HashSet additionalNameNodes) { - _analysis = analysis; - _additionalNames = additionalNames; - _additionalNameNodes = additionalNameNodes; - } - - public override bool Walk(CallExpression node) { - foreach (var arg in node.Args) { - arg?.Expression?.Walk(this); - } - return false; - } - - public override bool Walk(ListComprehension node) { - node.Walk(new ComprehensionWalker(_analysis)); - return false; - } - - public override bool Walk(SetComprehension node) { - node.Walk(new ComprehensionWalker(_analysis)); - return false; - } - public override bool Walk(DictionaryComprehension node) { - node.Walk(new ComprehensionWalker(_analysis)); - return false; - } - - public override bool Walk(GeneratorExpression node) { - node.Walk(new ComprehensionWalker(_analysis)); - return false; - } - - public override bool Walk(NameExpression node) { - if (_additionalNames?.Contains(node.Name) == true) { - return false; - } - if (_additionalNameNodes?.Contains(node) == true) { - return false; - } - var m = _analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out _); - if (m == null) { - _analysis.ReportUndefinedVariable(node); - } - return false; - } - } - +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class ComprehensionWalker : PythonWalker { private readonly IDocumentAnalysis _analysis; private readonly HashSet _names = new HashSet(); @@ -104,6 +49,7 @@ public override bool Walk(DictionaryComprehension node) { foreach (var iter in node.Iterators) { iter?.Walk(new ExpressionWalker(_analysis, null, _additionalNameNodes)); } + return true; } @@ -126,7 +72,7 @@ private sealed class NameCollectorWalker : PythonWalker { private readonly HashSet _names; private readonly HashSet _additionalNameNodes; - public NameCollectorWalker(HashSet names, HashSet additionalNameNodes) { + public NameCollectorWalker(HashSet names, HashSet additionalNameNodes) { _names = names; _additionalNameNodes = additionalNameNodes; } @@ -136,6 +82,7 @@ public override bool Walk(NameExpression nex) { _names.Add(nex.Name); _additionalNameNodes.Add(nex); } + return false; } } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs new file mode 100644 index 000000000..0a93dad26 --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -0,0 +1,93 @@ +// 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 System.Collections.Generic; +using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Core.Text; +using Microsoft.Python.Parsing.Ast; + +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { + internal sealed class ExpressionWalker : PythonWalker { + private readonly IDocumentAnalysis _analysis; + private readonly HashSet _additionalNames; + private readonly HashSet _additionalNameNodes; + + public ExpressionWalker(IDocumentAnalysis analysis) + : this(analysis, null, null) { } + + /// + /// Creates walker for detection of undefined variables. + /// + /// Document analysis. + /// Additional defined names. + /// Name nodes for defined names. + public ExpressionWalker(IDocumentAnalysis analysis, HashSet additionalNames, HashSet additionalNameNodes) { + _analysis = analysis; + _additionalNames = additionalNames; + _additionalNameNodes = additionalNameNodes; + } + + public override bool Walk(CallExpression node) { + foreach (var arg in node.Args) { + arg?.Expression?.Walk(this); + } + return false; + } + + public override bool Walk(ListComprehension node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + + public override bool Walk(SetComprehension node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + public override bool Walk(DictionaryComprehension node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + + public override bool Walk(GeneratorExpression node) { + node.Walk(new ComprehensionWalker(_analysis)); + return false; + } + + public override bool Walk(NameExpression node) { + if (_additionalNames?.Contains(node.Name) == true) { + return false; + } + if (_additionalNameNodes?.Contains(node) == true) { + return false; + } + var m = _analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out _); + if (m == null) { + _analysis.ReportUndefinedVariable(node); + } + // Take into account where variable is defined so we do detect + // undefined x in + // y = x + // x = 1 + if(m is ILocatedMember lm) { + var span = lm.Location.Span; + var nodeLoc = node.GetLocation(_analysis.Document); + if (span.IsAfter(nodeLoc.Span)) { + _analysis.ReportUndefinedVariable(node); + } + } + return false; + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesLinter.cs similarity index 93% rename from src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs rename to src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesLinter.cs index 70d6ba18d..ff02b2d34 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariablesLinter.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesLinter.cs @@ -15,7 +15,7 @@ using Microsoft.Python.Core; -namespace Microsoft.Python.Analysis.Linting { +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class UndefinedVariablesLinter : ILinter { public void Lint(IDocumentAnalysis analysis, IServiceContainer services) { var w = new UndefinedVariablesWalker(analysis, services); diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs similarity index 97% rename from src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs rename to src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 1d5c743ec..6fa08fb8c 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -17,7 +17,7 @@ using Microsoft.Python.Core; using Microsoft.Python.Parsing.Ast; -namespace Microsoft.Python.Analysis.Linting { +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class UndefinedVariablesWalker : LinterWalker { private readonly ExpressionWalker _expressionWalker; diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 334b2bfd0..c114d935e 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -13,18 +13,13 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System; -using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using FluentAssertions; -using Microsoft.Python.Core; using Microsoft.Python.Analysis.Tests.FluentAssertions; -using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Parsing; -using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; +using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; namespace Microsoft.Python.Analysis.Tests { [TestClass] @@ -39,127 +34,144 @@ public void TestInitialize() public void Cleanup() => TestEnvironmentImpl.TestCleanup(); [TestMethod, Priority(0)] - public async Task Variable() { + public async Task BasicVariables() { const string code = @" -import sys -e1, e2, e3 = sys.exc_info() +y = x + +class A: + x1: int = 0 + y1: int "; var analysis = await GetAnalysisAsync(code); - // sys.exc_info() -> (exception_type, exception_value, traceback) - var f = analysis.Should() - .HaveVariable("e1").OfType(BuiltinTypeId.Type) - .And.HaveVariable("e2").OfType("BaseException") - .And.HaveVariable("e3").OfType("TracebackType") - .And.HaveVariable("sys").OfType(BuiltinTypeId.Module) - .Which.Should().HaveMember("exc_info").Which; - - f.Overloads.Should().HaveCount(1); - f.Overloads[0].GetReturnDocumentation(null) - .Should().Be(@"Tuple[ (Optional[Type[BaseException]],Optional[BaseException],Optional[TracebackType])]"); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(2, 5, 2, 6); } [TestMethod, Priority(0)] - public async Task TypeShedJsonMakeScanner() { - const string code = @"import _json -scanner = _json.make_scanner()"; + public async Task ClassVariables() { + const string code = @" +class A: + x1: int = 0 + y1: int +"; var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } - var v0 = analysis.Should().HaveVariable("scanner").Which; + [TestMethod, Priority(0)] + public async Task Conditionals() { + const string code = @" +z = 3 +if x > 2 and y == 3 or z < 2: + pass +"; + var analysis = await GetAnalysisAsync(code); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(2); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(3, 4, 3, 5); + d[1].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[1].SourceSpan.Should().Be(3, 14, 3, 15); + } - v0.Should().HaveMember("__call__") - .Which.Should().HaveSingleOverload() - .Which.Should().HaveName("__call__") - .And.HaveParameters("self", "string", "index") - .And.HaveParameterAt(1).WithName("string").WithType("str").WithNoDefaultValue() - .And.HaveParameterAt(2).WithName("index").WithType("int").WithNoDefaultValue() - .And.HaveReturnDocumentation("Tuple[ (Any,int)]"); + [TestMethod, Priority(0)] + public async Task Calls() { + const string code = @" +z = 3 +func(x, 1, y+1, z) +"; + var analysis = await GetAnalysisAsync(code); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(2); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(3, 6, 3, 7); + d[1].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[1].SourceSpan.Should().Be(3, 12, 3, 13); } [TestMethod, Priority(0)] - public async Task MergeStubs() { - var analysis = await GetAnalysisAsync("import Package.Module\n\nc = Package.Module.Class()"); + public async Task TupleAssignment() { + const string code = @" +a, *b, c = range(5) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } - analysis.Should() - .HaveVariable("Package") - .Which.Value.Should().HaveMember("Module"); + [TestMethod, Priority(0)] + public async Task ListComprehension() { + const string code = @" +NAME = ' '.join(str(x) for x in {z, 2, 3}) - analysis.Should().HaveVariable("c") - .Which.Value.Should().HaveMembers("untyped_method", "inferred_method", "typed_method") - .And.NotHaveMembers("typed_method_2"); +class C: + EVENTS = ['x'] + x = [(e, e) for e in EVENTS] + y = EVENTS +"; + var analysis = await GetAnalysisAsync(code); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(2, 34, 2, 35); } [TestMethod, Priority(0)] - public async Task TypeStubConditionalDefine() { - var seen = new HashSet(); - - const string code = @"import sys + public async Task SelfAssignment() { + const string code = @" +def foo(m): + m = m +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } -if sys.version_info < (2, 7): - LT_2_7 : bool = ... -if sys.version_info <= (2, 7): - LE_2_7 : bool = ... -if sys.version_info > (2, 7): - GT_2_7 : bool = ... -if sys.version_info >= (2, 7): - GE_2_7 : bool = ... + [TestMethod, Priority(0)] + public async Task AssignmentAfter() { + const string code = @" +y = x +x = 1 "; - - var fullSet = new[] { "LT_2_7", "LE_2_7", "GT_2_7", "GE_2_7" }; - - foreach (var ver in PythonVersions.Versions) { - if (!seen.Add(ver.Version)) { - continue; - } - - Console.WriteLine(@"Testing with {0}", ver.InterpreterPath); - using (var s = await CreateServicesAsync(TestData.Root, ver)) { - var analysis = await GetAnalysisAsync(code, s, @"testmodule", TestData.GetTestSpecificPath(@"testmodule.pyi")); - - var expected = new List(); - var pythonVersion = ver.Version.ToLanguageVersion(); - if (pythonVersion.Is3x()) { - expected.Add("GT_2_7"); - expected.Add("GE_2_7"); - } else if (pythonVersion == PythonLanguageVersion.V27) { - expected.Add("GE_2_7"); - expected.Add("LE_2_7"); - } else { - expected.Add("LT_2_7"); - expected.Add("LE_2_7"); - } - - analysis.GlobalScope.Variables.Select(m => m.Name).Where(n => n.EndsWithOrdinal("2_7")) - .Should().Contain(expected) - .And.NotContain(fullSet.Except(expected)); - } - } + var analysis = await GetAnalysisAsync(code); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(2, 5, 2, 6); } [TestMethod, Priority(0)] - public async Task TypeShedNoTypeLeaks() { - const string code = @"import json -json.dump() -x = 1"; + public async Task NonLocal() { + const string code = @" +class A: + x: int + def func(): + nonlocal x, y + y = 2 +"; var analysis = await GetAnalysisAsync(code); - analysis.Should().HaveVariable("json") - .Which.Should().HaveMember("dump"); - - analysis.Should().HaveVariable("x") - .Which.Should().HaveMember("bit_length") - .And.NotHaveMember("SIGABRT"); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(5, 21, 5, 22); } [TestMethod, Priority(0)] - public async Task VersionCheck() { + public async Task Global() { const string code = @" -import asyncio -loop = asyncio.get_event_loop() +x = 1 + +class A: + def func(): + global x, y + y = 2 "; var analysis = await GetAnalysisAsync(code); - analysis.Should() - .HaveVariable("loop") - .Which.Value.Should().HaveMembers("add_reader", "add_writer", "call_at", "close"); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(6, 19, 6, 20); } } } From 8b796158fa4872d70a119ae3072161786e943129 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 4 Mar 2019 11:43:49 -0800 Subject: [PATCH 05/27] Tests --- src/Analysis/Ast/Test/LinterTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index c114d935e..6178d681f 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -128,6 +128,16 @@ def foo(m): } + [TestMethod, Priority(0)] + public async Task AssignmentBefore() { + const string code = @" +x = 1 +y = x +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task AssignmentAfter() { const string code = @" From 7ffc9db4f0807314d4d36a8540380b988c26ee2e Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 4 Mar 2019 16:43:30 -0800 Subject: [PATCH 06/27] 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 07/27] 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 6429bfafdbf1d6aff71dbb0845ea18a6b4997746 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 4 Mar 2019 22:31:06 -0800 Subject: [PATCH 08/27] Options and tests --- .../Ast/Impl/Analyzer/PythonAnalyzer.cs | 7 +++-- .../Ast/Impl/Definitions/AnalysisOptions.cs | 20 +++++++++++++ .../Definitions/IAnalysisOptionsProvider.cs | 20 +++++++++++++ .../UndefinedVariables/ExpressionWalker.cs | 1 - .../UndefinedVariablesWalker.cs | 1 + src/Analysis/Ast/Test/AnalysisTestBase.cs | 4 --- src/Analysis/Ast/Test/CollectionsTests.cs | 13 +++++++++ src/Analysis/Ast/Test/LinterTests.cs | 28 +++++++++++++++++-- .../Impl/Definitions/ServerSettings.cs | 1 - src/LanguageServer/Impl/LanguageServer.cs | 12 ++++++-- 10 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 src/Analysis/Ast/Impl/Definitions/AnalysisOptions.cs create mode 100644 src/Analysis/Ast/Impl/Definitions/IAnalysisOptionsProvider.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index c2cbfa528..27348f0f1 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -267,8 +267,11 @@ private void Analyze(IDependencyChainNode node, int version cancellationToken.ThrowIfCancellationRequested(); var analysis = new DocumentAnalysis((IDocument)module, version, walker.GlobalScope, walker.Eval); - var linter = new LinterAggregator(); - linter.Lint(analysis, _services); + var optionsProvider = _services.GetService(); + if (optionsProvider?.Options?.LintingEnabled != false) { + var linter = new LinterAggregator(); + linter.Lint(analysis, _services); + } (module as IAnalyzable)?.NotifyAnalysisComplete(analysis); node.Value.TrySetAnalysis(analysis, version, _syncObj); diff --git a/src/Analysis/Ast/Impl/Definitions/AnalysisOptions.cs b/src/Analysis/Ast/Impl/Definitions/AnalysisOptions.cs new file mode 100644 index 000000000..d34c88998 --- /dev/null +++ b/src/Analysis/Ast/Impl/Definitions/AnalysisOptions.cs @@ -0,0 +1,20 @@ +// 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. + +namespace Microsoft.Python.Analysis { + public class AnalysisOptions { + public bool LintingEnabled { get; set; } + } +} diff --git a/src/Analysis/Ast/Impl/Definitions/IAnalysisOptionsProvider.cs b/src/Analysis/Ast/Impl/Definitions/IAnalysisOptionsProvider.cs new file mode 100644 index 000000000..26a9c7659 --- /dev/null +++ b/src/Analysis/Ast/Impl/Definitions/IAnalysisOptionsProvider.cs @@ -0,0 +1,20 @@ +// 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. + +namespace Microsoft.Python.Analysis { + public interface IAnalysisOptionsProvider { + AnalysisOptions Options { get; } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 0a93dad26..a2ac53637 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -15,7 +15,6 @@ using System.Collections.Generic; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 6fa08fb8c..676385cc2 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -35,6 +35,7 @@ public override bool Walk(AssignmentStatement node) { } public override bool Walk(CallExpression node) { + node.Target?.Walk(_expressionWalker); foreach (var arg in node.Args) { arg?.Expression?.Walk(_expressionWalker); } diff --git a/src/Analysis/Ast/Test/AnalysisTestBase.cs b/src/Analysis/Ast/Test/AnalysisTestBase.cs index 22e055093..261e2744a 100644 --- a/src/Analysis/Ast/Test/AnalysisTestBase.cs +++ b/src/Analysis/Ast/Test/AnalysisTestBase.cs @@ -14,16 +14,13 @@ // permissions and limitations under the License. using System; -using System.Collections.Generic; using System.Diagnostics; using System.IO; -using System.Linq; using System.Threading; using System.Threading.Tasks; using FluentAssertions; using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Core.Interpreter; -using Microsoft.Python.Analysis.Dependencies; using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Analysis.Modules; @@ -32,7 +29,6 @@ using Microsoft.Python.Core.IO; using Microsoft.Python.Core.OS; using Microsoft.Python.Core.Services; -using Microsoft.Python.Core.Shell; using Microsoft.Python.Core.Tests; using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Tests; diff --git a/src/Analysis/Ast/Test/CollectionsTests.cs b/src/Analysis/Ast/Test/CollectionsTests.cs index 8857819f6..41325dd11 100644 --- a/src/Analysis/Ast/Test/CollectionsTests.cs +++ b/src/Analysis/Ast/Test/CollectionsTests.cs @@ -500,5 +500,18 @@ def g(a, b): analysis.Should().HaveVariable("y1").OfType(BuiltinTypeId.Int) .And.HaveVariable("y2").OfType(BuiltinTypeId.Tuple); } + + [TestMethod, Priority(0)] + public async Task NamedTuple() { + const string code = @" +from collections import namedtuple +nt = namedtuple('Point', ['x', 'y']) +pt = nt(1, 2) +"; + var analysis = await GetAnalysisAsync(code); + var nt = analysis.Should().HaveVariable("nt").Which; + nt.Should().HaveType(BuiltinTypeId.Tuple); + nt.Should().HaveMembers("x", "y"); + } } } diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 6178d681f..41b2b9dd7 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -17,6 +17,7 @@ using System.Threading.Tasks; using FluentAssertions; using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; @@ -84,11 +85,13 @@ public async Task Calls() { "; var analysis = await GetAnalysisAsync(code); var d = analysis.Diagnostics.ToArray(); - d.Should().HaveCount(2); + d.Should().HaveCount(3); d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); - d[0].SourceSpan.Should().Be(3, 6, 3, 7); + d[0].SourceSpan.Should().Be(3, 1, 3, 5); d[1].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); - d[1].SourceSpan.Should().Be(3, 12, 3, 13); + d[1].SourceSpan.Should().Be(3, 6, 3, 7); + d[2].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[2].SourceSpan.Should().Be(3, 12, 3, 13); } [TestMethod, Priority(0)] @@ -183,5 +186,24 @@ def func(): d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); d[0].SourceSpan.Should().Be(6, 19, 6, 20); } + + [DataRow(false)] + [DataRow(true)] + [DataTestMethod, Priority(0)] + public async Task OptionsSwitch(bool enabled) { + const string code = @"x = y"; + + var sm = CreateServiceManager(); + var op = new AnalysisOptionsProvider(); + sm.AddService(op); + + op.Options.LintingEnabled = enabled; + var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X, sm); + analysis.Diagnostics.Should().HaveCount(enabled ? 1 : 0); + } + + private class AnalysisOptionsProvider: IAnalysisOptionsProvider { + public AnalysisOptions Options { get; } = new AnalysisOptions(); + } } } diff --git a/src/LanguageServer/Impl/Definitions/ServerSettings.cs b/src/LanguageServer/Impl/Definitions/ServerSettings.cs index 7bf28c3ac..32428d0bf 100644 --- a/src/LanguageServer/Impl/Definitions/ServerSettings.cs +++ b/src/LanguageServer/Impl/Definitions/ServerSettings.cs @@ -37,7 +37,6 @@ public class PythonCompletionOptions { public bool showAdvancedMembers = true; public bool addBrackets = false; } - public readonly PythonCompletionOptions completion = new PythonCompletionOptions(); } } diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index dfa2a4f37..e0a6fff6f 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -21,7 +21,6 @@ using System.Threading.Tasks; using Microsoft.Python.Analysis; using Microsoft.Python.Analysis.Diagnostics; -using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core; using Microsoft.Python.Core.Disposables; using Microsoft.Python.Core.Idle; @@ -46,6 +45,7 @@ public sealed partial class LanguageServer : IDisposable { private readonly CancellationTokenSource _sessionTokenSource = new CancellationTokenSource(); private readonly Prioritizer _prioritizer = new Prioritizer(); private readonly CancellationTokenSource _shutdownCts = new CancellationTokenSource(); + private readonly AnalysisOptionsProvider _optionsProvider = new AnalysisOptionsProvider(); private IServiceContainer _services; private Server _server; @@ -79,6 +79,7 @@ public CancellationToken Start(IServiceManager services, JsonRpc rpc) { .Add(() => _pathsWatcher?.Dispose()) .Add(() => _rpc.TraceSource.Listeners.Remove(rpcTraceListener)); + services.AddService(_optionsProvider); return _sessionTokenSource.Token; } @@ -111,6 +112,9 @@ public async Task DidChangeConfiguration(JToken token, CancellationToken cancell settings.symbolsHierarchyDepthLimit = GetSetting(analysis, "symbolsHierarchyDepthLimit", 10); settings.symbolsHierarchyMaxSymbols = GetSetting(analysis, "symbolsHierarchyMaxSymbols", 1000); + var linting = pythonSection["linting"]; + _optionsProvider.Options.LintingEnabled = GetSetting(linting, "enabled", true); + _logger.LogLevel = GetLogLevel(analysis).ToTraceEventType(); HandlePathWatchChange(token, cancellationToken); @@ -377,8 +381,6 @@ private void HandlePathWatchChange(JToken section, CancellationToken cancellatio if (!_watchSearchPaths || (_watchSearchPaths && _searchPaths.SetEquals(_initParams.initializationOptions.searchPaths))) { // Were not watching OR were watching but paths have changed. Recreate the watcher. _pathsWatcher?.Dispose(); - var interpreter = _services.GetService(); - var logger = _services.GetService(); _pathsWatcher = new PathsWatcher( _initParams.initializationOptions.searchPaths, () =>_server.NotifyPackagesChanged(cancellationToken), @@ -468,5 +470,9 @@ public PrioritizerDisposable(CancellationToken cancellationToken) { public void Dispose() => _ppc.Dispose(); } + + private class AnalysisOptionsProvider : IAnalysisOptionsProvider { + public AnalysisOptions Options { get; } = new AnalysisOptions(); + } } } From 42dc0a0054d71566639e4d5298eaddb1fbce09aa Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 5 Mar 2019 10:46:37 -0800 Subject: [PATCH 09/27] Don't squiggle builtin-ins --- src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs | 10 ++++++---- .../Linting/UndefinedVariables/ExpressionWalker.cs | 2 +- src/Analysis/Ast/Test/LinterTests.cs | 10 ++++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 27348f0f1..a394abf76 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -267,10 +267,12 @@ private void Analyze(IDependencyChainNode node, int version cancellationToken.ThrowIfCancellationRequested(); var analysis = new DocumentAnalysis((IDocument)module, version, walker.GlobalScope, walker.Eval); - var optionsProvider = _services.GetService(); - if (optionsProvider?.Options?.LintingEnabled != false) { - var linter = new LinterAggregator(); - linter.Lint(analysis, _services); + if (module.ModuleType == ModuleType.User) { + var optionsProvider = _services.GetService(); + if (optionsProvider?.Options?.LintingEnabled != false) { + var linter = new LinterAggregator(); + linter.Lint(analysis, _services); + } } (module as IAnalyzable)?.NotifyAnalysisComplete(analysis); diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index a2ac53637..2650b5c37 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -79,7 +79,7 @@ public override bool Walk(NameExpression node) { // undefined x in // y = x // x = 1 - if(m is ILocatedMember lm) { + if(m is ILocatedMember lm && lm.Location.DocumentUri == _analysis.Document.Uri) { var span = lm.Location.Span; var nodeLoc = node.GetLocation(_analysis.Document); if (span.IsAfter(nodeLoc.Span)) { diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 41b2b9dd7..916354786 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -202,6 +202,16 @@ public async Task OptionsSwitch(bool enabled) { analysis.Diagnostics.Should().HaveCount(enabled ? 1 : 0); } + [TestMethod, Priority(0)] + public async Task Builtins() { + const string code = @" +print(1) +abs(3) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + private class AnalysisOptionsProvider: IAnalysisOptionsProvider { public AnalysisOptions Options { get; } = new AnalysisOptions(); } From 9b130fd3f40b3c77386fded5649eb1f34d68f47a Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 5 Mar 2019 11:00:16 -0800 Subject: [PATCH 10/27] Test for function arguments --- src/Analysis/Ast/Test/LinterTests.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 916354786..463255f4f 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -154,6 +154,21 @@ public async Task AssignmentAfter() { d[0].SourceSpan.Should().Be(2, 5, 2, 6); } + [TestMethod, Priority(0)] + public async Task FunctionArguments() { + const string code = @" +def z(x): + return x + +def func(a, b, c): + a = b + x = c + z(c * 3) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task NonLocal() { const string code = @" From 2490a0a9eb87d5490d340d75f523047501f1e629 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 5 Mar 2019 11:51:00 -0800 Subject: [PATCH 11/27] Fix tuple assignment in analysis --- .../Analyzer/Handlers/AssignmentHandler.cs | 1 - .../Handlers/TupleExpressionHandler.cs | 38 +++++++++++++------ src/Analysis/Ast/Test/AssignmentTests.cs | 10 +++++ src/Analysis/Ast/Test/LinterTests.cs | 33 ++++++++++++++++ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs index 27b3de368..eff2e3f0c 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs @@ -15,7 +15,6 @@ using System.Diagnostics; using System.Linq; -using Microsoft.Python.Analysis.Analyzer.Evaluation; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs index ffb839918..b7490d4ca 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs @@ -13,10 +13,10 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System; using System.Linq; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; +using Microsoft.Python.Core; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Analyzer.Handlers { @@ -24,30 +24,46 @@ internal sealed class TupleExpressionHandler : StatementHandler { public TupleExpressionHandler(AnalysisWalker walker) : base(walker) { } public void HandleTupleAssignment(TupleExpression lhs, Expression rhs, IMember value) { + string[] names; if (rhs is TupleExpression tex) { var returnedExpressions = tex.Items.ToArray(); - var names = lhs.Items.OfType().Select(x => x.Name).ToArray(); - for (var i = 0; i < Math.Min(names.Length, returnedExpressions.Length); i++) { - if (returnedExpressions[i] != null && !string.IsNullOrEmpty(names[i])) { - var v = Eval.GetValueFromExpression(returnedExpressions[i]); - Eval.DeclareVariable(names[i], v ?? Eval.UnknownType, VariableSource.Declaration, returnedExpressions[i]); + names = lhs.Items.OfType().Select(x => x.Name).ToArray(); + for (var i = 0; i < names.Length; i++) { + Expression e = null; + if (returnedExpressions.Length > 0) { + e = i < returnedExpressions.Length ? returnedExpressions[i] : returnedExpressions[returnedExpressions.Length - 1]; + } + + if (e != null && !string.IsNullOrEmpty(names[i])) { + var v = Eval.GetValueFromExpression(e); + Eval.DeclareVariable(names[i], v ?? Eval.UnknownType, VariableSource.Declaration, e); } } + return; } // Tuple = 'tuple value' (such as from callable). Transfer values. + var expressions = lhs.Items.OfType().ToArray(); + names = expressions.Select(x => x.Name).ToArray(); if (value is IPythonCollection seq) { var types = seq.Contents.Select(c => c.GetPythonType()).ToArray(); - var expressions = lhs.Items.OfType().ToArray(); - var names = expressions.Select(x => x.Name).ToArray(); - for (var i = 0; i < Math.Min(names.Length, types.Length); i++) { - if (names[i] != null && types[i] != null) { - var instance = types[i].CreateInstance(null, Eval.GetLoc(expressions[i]), ArgumentSet.Empty); + for (var i = 0; i < names.Length; i++) { + IPythonType t = null; + if (types.Length > 0) { + t = i < types.Length ? types[i] : types[types.Length - 1]; + } + + if (names[i] != null && t != null) { + var instance = t.CreateInstance(null, Eval.GetLoc(expressions[i]), ArgumentSet.Empty); Eval.DeclareVariable(names[i], instance, VariableSource.Declaration, expressions[i]); } } + } else { + foreach (var n in names.ExcludeDefault()) { + Eval.DeclareVariable(n, value, VariableSource.Declaration, rhs); + } } } } diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index 14d7b6153..366d20be0 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -280,5 +280,15 @@ public async Task StrIndex() { var analysis = await GetAnalysisAsync(code); analysis.Should().HaveVariable("x").OfType(BuiltinTypeId.Str); } + + [TestMethod, Priority(0)] + public async Task IncompleteTuple() { + const string code = @" +a, b = 1 +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("a").OfType(BuiltinTypeId.Int) + .And.HaveVariable("b").OfType(BuiltinTypeId.Int); + } } } diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 463255f4f..add460070 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -103,6 +103,39 @@ public async Task TupleAssignment() { analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task TupleUsage() { + const string code = @" +a, b = 1 +x = a +y = b +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task FunctionNoneArgument() { + const string code = @" +def func(a=None, b=True): + x = a + y = b +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task ForVariables() { + const string code = @" +c = {} +for i in c: + x = i +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task ListComprehension() { const string code = @" From f9216e3e3bacd18485c1745a14bfcaaf284795ec Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 5 Mar 2019 12:19:57 -0800 Subject: [PATCH 12/27] Don't false positive on functions and classes forward references --- .../UndefinedVariables/ExpressionWalker.cs | 10 ++++--- src/Analysis/Ast/Test/AssignmentTests.cs | 9 +++++++ src/Analysis/Ast/Test/LinterTests.cs | 27 +++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 2650b5c37..71bf0adb0 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -80,10 +80,12 @@ public override bool Walk(NameExpression node) { // y = x // x = 1 if(m is ILocatedMember lm && lm.Location.DocumentUri == _analysis.Document.Uri) { - var span = lm.Location.Span; - var nodeLoc = node.GetLocation(_analysis.Document); - if (span.IsAfter(nodeLoc.Span)) { - _analysis.ReportUndefinedVariable(node); + if (!(m is IPythonFunctionType || m is IPythonClassType)) { + var span = lm.Location.Span; + var nodeLoc = node.GetLocation(_analysis.Document); + if (span.IsAfter(nodeLoc.Span)) { + _analysis.ReportUndefinedVariable(node); + } } } return false; diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index 366d20be0..183a72084 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -155,6 +155,15 @@ public async Task Tuple() { .And.HaveVariable("z").OfType(BuiltinTypeId.Float); } + [TestMethod, Priority(0)] + public async Task TupleUnknownReturn() { + const string code = @" +x, y, z = func() +"; + var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X); + analysis.Should().HaveVariable("x").And.HaveVariable("y").And.HaveVariable("z"); + } + [TestMethod, Priority(0)] public async Task AnnotatedAssign() { const string code = @" diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index add460070..346d0718d 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -125,6 +125,33 @@ def func(a=None, b=True): analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task FunctionReference() { + const string code = @" +def func1(): + func2() + return + +def func2(a=None, b=True): ... +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task ClassReference() { + const string code = @" +class A(B): + def __init__(self): + x = B() + return + +class B(): ... +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task ForVariables() { const string code = @" From 5cea68d370b53d70ec00438c456103ff185844e5 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 6 Mar 2019 06:10:21 -0800 Subject: [PATCH 13/27] Disable tracking assignment location Fix declaration of variables in for loop --- .../Ast/Impl/Analyzer/Handlers/LoopHandler.cs | 13 +------ .../UndefinedVariables/ComprehensionWalker.cs | 25 +++--------- .../UndefinedVariables/ExpressionWalker.cs | 20 ++++++---- .../UndefinedVariables/NameCollectorWalker.cs | 38 +++++++++++++++++++ src/Analysis/Ast/Test/LinterTests.cs | 15 ++++++++ 5 files changed, 74 insertions(+), 37 deletions(-) create mode 100644 src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs index f4539ef16..3d0b42e8b 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs @@ -24,11 +24,7 @@ public LoopHandler(AnalysisWalker walker) : base(walker) { } public void HandleFor(ForStatement node) { var iterable = Eval.GetValueFromExpression(node.List); var iterator = (iterable as IPythonIterable)?.GetIterator(); - if (iterator == null) { - // TODO: report that expression does not evaluate to iterable. - } var value = iterator?.Next ?? Eval.UnknownType; - switch (node.Left) { case NameExpression nex: // for x in y: @@ -38,13 +34,8 @@ public void HandleFor(ForStatement node) { // x = [('abc', 42, True), ('abc', 23, False)] // for some_str, some_int, some_bool in x: var names = tex.Items.OfType().Select(x => x.Name).ToArray(); - if (value is IPythonIterable valueIterable) { - var valueIterator = valueIterable.GetIterator(); - foreach (var n in names) { - Eval.DeclareVariable(n, valueIterator?.Next ?? Eval.UnknownType, VariableSource.Declaration, Eval.GetLoc(node.Left)); - } - } else { - // TODO: report that expression yields value that does not evaluate to iterable. + foreach (var n in names) { + Eval.DeclareVariable(n, value, VariableSource.Declaration, Eval.GetLoc(node.Left)); } break; } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs index a118d7658..3715bd015 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs @@ -42,6 +42,12 @@ public override bool Walk(SetComprehension node) { return false; } + public override bool Walk(ForStatement node) { + var nc = new NameCollectorWalker(_names, _additionalNameNodes); + node.Left?.Walk(nc); + return true; + } + public override bool Walk(DictionaryComprehension node) { CollectNames(node); node.Key?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); @@ -67,24 +73,5 @@ private void ProcessComprehension(Comprehension c, Node item, IEnumerable _names; - private readonly HashSet _additionalNameNodes; - - public NameCollectorWalker(HashSet names, HashSet additionalNameNodes) { - _names = names; - _additionalNameNodes = additionalNameNodes; - } - - public override bool Walk(NameExpression nex) { - if (!string.IsNullOrEmpty(nex.Name)) { - _names.Add(nex.Name); - _additionalNameNodes.Add(nex); - } - - return false; - } - } } } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 71bf0adb0..962097d95 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -79,14 +79,20 @@ public override bool Walk(NameExpression node) { // undefined x in // y = x // x = 1 - if(m is ILocatedMember lm && lm.Location.DocumentUri == _analysis.Document.Uri) { - if (!(m is IPythonFunctionType || m is IPythonClassType)) { - var span = lm.Location.Span; - var nodeLoc = node.GetLocation(_analysis.Document); - if (span.IsAfter(nodeLoc.Span)) { - _analysis.ReportUndefinedVariable(node); - } + if (m is ILocatedMember lm && lm.Location.DocumentUri == _analysis.Document.Uri) { + // Do not complain about functions and classes that appear later in the file + if (m is IPythonFunctionType || m is IPythonClassType) { + return false; } + // Since analyzer remembers the last assignment, it may look that the variable + // is not defined yet, but we also should look up, perhaps it was defined + // earlier and redefined later. + // https://github.com/Microsoft/python-language-server/issues/682 + //var span = lm.Location.Span; + //var nodeLoc = node.GetLocation(_analysis.Document); + //if (span.IsAfter(nodeLoc.Span)) { + // _analysis.ReportUndefinedVariable(node); + //} } return false; } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs new file mode 100644 index 000000000..23cb0c78e --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs @@ -0,0 +1,38 @@ +// 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 System.Collections.Generic; +using Microsoft.Python.Parsing.Ast; + +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { + internal sealed class NameCollectorWalker : PythonWalker { + private readonly HashSet _names; + private readonly HashSet _additionalNameNodes; + + public NameCollectorWalker(HashSet names, HashSet additionalNameNodes) { + _names = names; + _additionalNameNodes = additionalNameNodes; + } + + public override bool Walk(NameExpression nex) { + if (!string.IsNullOrEmpty(nex.Name)) { + _names.Add(nex.Name); + _additionalNameNodes.Add(nex); + } + + return false; + } + } +} diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 346d0718d..4ee2db444 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -202,6 +202,7 @@ public async Task AssignmentBefore() { } [TestMethod, Priority(0)] + [Ignore("https://github.com/Microsoft/python-language-server/issues/682")] public async Task AssignmentAfter() { const string code = @" y = x @@ -287,6 +288,20 @@ public async Task Builtins() { analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task Enumeration() { + const string code = @" +x = {} +for a, b in enumerate(x): + if a: + pass + if b: + pass +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + private class AnalysisOptionsProvider: IAnalysisOptionsProvider { public AnalysisOptions Options { get; } = new AnalysisOptions(); } From 8dfc390cfddcf4b3f84e9fe26d6b50d961f65fcc Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 6 Mar 2019 07:44:24 -0800 Subject: [PATCH 14/27] Track multiple locations --- .../Ast/Impl/Analyzer/AnalysisWalker.cs | 6 +-- .../Evaluation/ExpressionEval.Scopes.cs | 3 ++ .../Analyzer/Handlers/AssignmentHandler.cs | 4 +- .../Ast/Impl/Analyzer/Handlers/LoopHandler.cs | 20 +++---- .../Ast/Impl/Values/Definitions/IVariable.cs | 15 +++++- src/Analysis/Ast/Impl/Values/Variable.cs | 19 ++++++- .../Ast/Impl/Values/VariableCollection.cs | 19 +++++-- src/Analysis/Ast/Test/AssignmentTests.cs | 53 +++++++++++++++++++ 8 files changed, 114 insertions(+), 25 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs b/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs index 5c6ce1867..71331f319 100644 --- a/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs @@ -66,11 +66,7 @@ public override bool Walk(ExpressionStatement node) { return false; } - public override bool Walk(ForStatement node) { - LoopHandler.HandleFor(node); - return base.Walk(node); - } - + public override bool Walk(ForStatement node) => LoopHandler.HandleFor(node); public override bool Walk(FromImportStatement node) => ImportHandler.HandleFromImport(node); public override bool Walk(GlobalStatement node) => NonLocalHandler.HandleGlobal(node); public override bool Walk(IfStatement node) => ConditionalHandler.HandleIf(node); diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs index 3928ee517..b6954f0b7 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs @@ -61,16 +61,19 @@ public IMember LookupNameInScopes(string name, out IScope scope, LookupOptions o } } else if (scopes.Count >= 2) { if (!options.HasFlag(LookupOptions.Nonlocal)) { + // Remove all scopes except global and current scopes. while (scopes.Count > 2) { scopes.RemoveAt(1); } } if (!options.HasFlag(LookupOptions.Local)) { + // Remove local scope. scopes.RemoveAt(0); } if (!options.HasFlag(LookupOptions.Global)) { + // Remove global scope. scopes.RemoveAt(scopes.Count - 1); } } diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs index 27b3de368..8eb2dae01 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/AssignmentHandler.cs @@ -62,7 +62,7 @@ public void HandleAssignment(AssignmentStatement node) { if (Eval.CurrentScope.NonLocals[ne.Name] != null) { Eval.LookupNameInScopes(ne.Name, out var scope, LookupOptions.Nonlocal); if (scope != null) { - scope.Variables[ne.Name].Value = value; + scope.Variables[ne.Name].Assign(value, Eval.GetLoc(ne)); } else { // TODO: report variable is not declared in outer scopes. } @@ -72,7 +72,7 @@ public void HandleAssignment(AssignmentStatement node) { if (Eval.CurrentScope.Globals[ne.Name] != null) { Eval.LookupNameInScopes(ne.Name, out var scope, LookupOptions.Global); if (scope != null) { - scope.Variables[ne.Name].Value = value; + scope.Variables[ne.Name].Assign(value, Eval.GetLoc(ne)); } else { // TODO: report variable is not declared in global scope. } diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs index f4539ef16..b9aefc3c5 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs @@ -21,35 +21,31 @@ namespace Microsoft.Python.Analysis.Analyzer.Handlers { internal sealed class LoopHandler : StatementHandler { public LoopHandler(AnalysisWalker walker) : base(walker) { } - public void HandleFor(ForStatement node) { + public bool HandleFor(ForStatement node) { var iterable = Eval.GetValueFromExpression(node.List); var iterator = (iterable as IPythonIterable)?.GetIterator(); - if (iterator == null) { - // TODO: report that expression does not evaluate to iterable. - } var value = iterator?.Next ?? Eval.UnknownType; switch (node.Left) { case NameExpression nex: // for x in y: - Eval.DeclareVariable(nex.Name, value, VariableSource.Declaration, Eval.GetLoc(node.Left)); + if (!string.IsNullOrEmpty(nex.Name)) { + Eval.DeclareVariable(nex.Name, value, VariableSource.Declaration, Eval.GetLoc(nex)); + } break; case TupleExpression tex: // x = [('abc', 42, True), ('abc', 23, False)] // for some_str, some_int, some_bool in x: - var names = tex.Items.OfType().Select(x => x.Name).ToArray(); - if (value is IPythonIterable valueIterable) { - var valueIterator = valueIterable.GetIterator(); - foreach (var n in names) { - Eval.DeclareVariable(n, valueIterator?.Next ?? Eval.UnknownType, VariableSource.Declaration, Eval.GetLoc(node.Left)); + foreach (var nex in tex.Items.OfType()) { + if (!string.IsNullOrEmpty(nex.Name)) { + Eval.DeclareVariable(nex.Name, value, VariableSource.Declaration, Eval.GetLoc(nex)); } - } else { - // TODO: report that expression yields value that does not evaluate to iterable. } break; } node.Body?.Walk(Walker); + return false; } public void HandleWhile(WhileStatement node) { diff --git a/src/Analysis/Ast/Impl/Values/Definitions/IVariable.cs b/src/Analysis/Ast/Impl/Values/Definitions/IVariable.cs index b8e4976c8..b47d7f61a 100644 --- a/src/Analysis/Ast/Impl/Values/Definitions/IVariable.cs +++ b/src/Analysis/Ast/Impl/Values/Definitions/IVariable.cs @@ -13,6 +13,7 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. +using System.Collections.Generic; using Microsoft.Python.Analysis.Types; namespace Microsoft.Python.Analysis.Values { @@ -24,13 +25,25 @@ public interface IVariable: ILocatedMember { /// Variable name. /// string Name { get; } + /// /// Variable source. /// VariableSource Source { get; } + /// /// Variable value. /// - IMember Value { get; set; } + IMember Value { get; } + + /// + /// Assigns value to the variable. + /// + void Assign(IMember value, LocationInfo location); + + /// + /// Provides list of all known assignment locations along the path of analysis. + /// + IReadOnlyList Locations { get; } } } diff --git a/src/Analysis/Ast/Impl/Values/Variable.cs b/src/Analysis/Ast/Impl/Values/Variable.cs index e18beb614..1780fc9b7 100644 --- a/src/Analysis/Ast/Impl/Values/Variable.cs +++ b/src/Analysis/Ast/Impl/Values/Variable.cs @@ -13,12 +13,16 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. +using System; +using System.Collections.Generic; using System.Diagnostics; using Microsoft.Python.Analysis.Types; namespace Microsoft.Python.Analysis.Values { [DebuggerDisplay("{DebuggerDisplay}")] internal sealed class Variable : IVariable { + private List _locations; + public Variable(string name, IMember value, VariableSource source, LocationInfo location = null) { Name = name; Value = value; @@ -28,9 +32,22 @@ public Variable(string name, IMember value, VariableSource source, LocationInfo public string Name { get; } public VariableSource Source { get; } - public IMember Value { get; set; } + public IMember Value { get; private set; } public LocationInfo Location { get; } public PythonMemberType MemberType => PythonMemberType.Variable; + public IReadOnlyList Locations => _locations as IReadOnlyList ?? new[] { Location }; + + public void Assign(IMember value, LocationInfo location) { + if (Value == null || Value.GetPythonType().IsUnknown() || value?.GetPythonType().IsUnknown() == false) { + Value = value; + } + AddLocation(location); + } + + private void AddLocation(LocationInfo location) { + _locations = _locations ?? new List { Location }; + _locations.Add(location); + } private string DebuggerDisplay { get { diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index 7c1b48a88..6845e6ad0 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -20,13 +20,12 @@ using System.Diagnostics; using System.Linq; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Core.Diagnostics; namespace Microsoft.Python.Analysis.Values { [DebuggerDisplay("Count: {Count}")] internal sealed class VariableCollection : IVariableCollection { public static readonly IVariableCollection Empty = new VariableCollection(); - private readonly ConcurrentDictionary _variables = new ConcurrentDictionary(); + private readonly ConcurrentDictionary _variables = new ConcurrentDictionary(); #region ICollection public int Count => _variables.Count; @@ -37,8 +36,16 @@ internal sealed class VariableCollection : IVariableCollection { #region IVariableCollection public IVariable this[string name] => _variables.TryGetValue(name, out var v) ? v : null; public bool Contains(string name) => _variables.ContainsKey(name); - public bool TryGetVariable(string key, out IVariable value) => _variables.TryGetValue(key, out value); public IReadOnlyList Names => _variables.Keys.ToArray(); + + public bool TryGetVariable(string key, out IVariable value) { + value = null; + if (_variables.TryGetValue(key, out var v)) { + value = v; + return true; + } + return false; + } #endregion #region IMemberContainer @@ -48,7 +55,11 @@ internal sealed class VariableCollection : IVariableCollection { internal void DeclareVariable(string name, IMember value, VariableSource source, LocationInfo location) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); - _variables[name] = new Variable(name, value, source, location); + if (_variables.TryGetValue(name, out var existing)) { + existing.Assign(value, location); + } else { + _variables[name] = new Variable(name, value, source, location); + } } } } diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index 14d7b6153..bc6277f06 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -280,5 +280,58 @@ public async Task StrIndex() { var analysis = await GetAnalysisAsync(code); analysis.Should().HaveVariable("x").OfType(BuiltinTypeId.Str); } + + [TestMethod, Priority(0)] + public async Task Locations() { + const string code = @" +a = 1 +a = 2 + +for x in y: + a = 3 + +if True: + a = 4 +elif False: + a = 5 +else: + a = 6 + +def func(): + a = 0 + +def func(a): + a = 0 + +def func(): + global a + a = 7 + +class A: + a: int + +def outer(): + b = 1 + def inner(): + nonlocal b + b = 2 +"; + var analysis = await GetAnalysisAsync(code); + var a = analysis.Should().HaveVariable("a").Which; + a.Locations.Should().HaveCount(7); + a.Locations[0].Span.Should().Be(2, 1, 2, 2); + a.Locations[1].Span.Should().Be(3, 1, 3, 2); + a.Locations[2].Span.Should().Be(6, 5, 6, 6); + a.Locations[3].Span.Should().Be(9, 5, 9, 6); + a.Locations[4].Span.Should().Be(11, 5, 11, 6); + a.Locations[5].Span.Should().Be(13, 5, 13, 6); + a.Locations[6].Span.Should().Be(23, 5, 23, 6); + + var outer = analysis.Should().HaveFunction("outer").Which; + var b = outer.Should().HaveVariable("b").Which; + b.Locations.Should().HaveCount(2); + b.Locations[0].Span.Should().Be(29, 5, 29, 6); + b.Locations[1].Span.Should().Be(32, 9, 32, 10); + } } } From a30eea36797611401288437ed24938979397fad6 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 6 Mar 2019 09:21:38 -0800 Subject: [PATCH 15/27] Tests --- src/Analysis/Ast/Test/AssignmentTests.cs | 53 ----------- src/Analysis/Ast/Test/LocationTests.cs | 112 +++++++++++++++++++++++ 2 files changed, 112 insertions(+), 53 deletions(-) create mode 100644 src/Analysis/Ast/Test/LocationTests.cs diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index bc6277f06..14d7b6153 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -280,58 +280,5 @@ public async Task StrIndex() { var analysis = await GetAnalysisAsync(code); analysis.Should().HaveVariable("x").OfType(BuiltinTypeId.Str); } - - [TestMethod, Priority(0)] - public async Task Locations() { - const string code = @" -a = 1 -a = 2 - -for x in y: - a = 3 - -if True: - a = 4 -elif False: - a = 5 -else: - a = 6 - -def func(): - a = 0 - -def func(a): - a = 0 - -def func(): - global a - a = 7 - -class A: - a: int - -def outer(): - b = 1 - def inner(): - nonlocal b - b = 2 -"; - var analysis = await GetAnalysisAsync(code); - var a = analysis.Should().HaveVariable("a").Which; - a.Locations.Should().HaveCount(7); - a.Locations[0].Span.Should().Be(2, 1, 2, 2); - a.Locations[1].Span.Should().Be(3, 1, 3, 2); - a.Locations[2].Span.Should().Be(6, 5, 6, 6); - a.Locations[3].Span.Should().Be(9, 5, 9, 6); - a.Locations[4].Span.Should().Be(11, 5, 11, 6); - a.Locations[5].Span.Should().Be(13, 5, 13, 6); - a.Locations[6].Span.Should().Be(23, 5, 23, 6); - - var outer = analysis.Should().HaveFunction("outer").Which; - var b = outer.Should().HaveVariable("b").Which; - b.Locations.Should().HaveCount(2); - b.Locations[0].Span.Should().Be(29, 5, 29, 6); - b.Locations[1].Span.Should().Be(32, 9, 32, 10); - } } } diff --git a/src/Analysis/Ast/Test/LocationTests.cs b/src/Analysis/Ast/Test/LocationTests.cs new file mode 100644 index 000000000..c8cf488ea --- /dev/null +++ b/src/Analysis/Ast/Test/LocationTests.cs @@ -0,0 +1,112 @@ +// 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 System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Analysis.Values; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.Analysis.Tests { + [TestClass] + public class LocationTests : AnalysisTestBase { + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [TestMethod, Priority(0)] + public async Task Assignments() { + const string code = @" +a = 1 +a = 2 + +for x in y: + a = 3 + +if True: + a = 4 +elif False: + a = 5 +else: + a = 6 + +def func(): + a = 0 + +def func(a): + a = 0 + +def func(): + global a + a = 7 + +class A: + a: int +"; + var analysis = await GetAnalysisAsync(code); + var a = analysis.Should().HaveVariable("a").Which; + a.Locations.Should().HaveCount(7); + a.Locations[0].Span.Should().Be(2, 1, 2, 2); + a.Locations[1].Span.Should().Be(3, 1, 3, 2); + a.Locations[2].Span.Should().Be(6, 5, 6, 6); + a.Locations[3].Span.Should().Be(9, 5, 9, 6); + a.Locations[4].Span.Should().Be(11, 5, 11, 6); + a.Locations[5].Span.Should().Be(13, 5, 13, 6); + a.Locations[6].Span.Should().Be(23, 5, 23, 6); + } + + [TestMethod, Priority(0)] + public async Task NonLocal() { + const string code = @" +def outer(): + b = 1 + def inner(): + nonlocal b + b = 2 +"; + var analysis = await GetAnalysisAsync(code); + var outer = analysis.Should().HaveFunction("outer").Which; + var b = outer.Should().HaveVariable("b").Which; + b.Locations.Should().HaveCount(2); + b.Locations[0].Span.Should().Be(3, 5, 3, 6); + b.Locations[1].Span.Should().Be(6, 9, 6, 10); + } + + [TestMethod, Priority(0)] + public async Task FunctionParameter() { + const string code = @" +def func(a): + a = 1 + if True: + a = 2 +"; + var analysis = await GetAnalysisAsync(code); + var outer = analysis.Should().HaveFunction("func").Which; + var a = outer.Should().HaveVariable("a").Which; + a.Locations.Should().HaveCount(3); + a.Locations[0].Span.Should().Be(2, 10, 2, 11); + a.Locations[1].Span.Should().Be(3, 5, 3, 6); + a.Locations[2].Span.Should().Be(5, 9, 5, 10); + } + } +} From 8a1c83ac8e3138e1fa9874273928bc9291433874 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 6 Mar 2019 09:22:11 -0800 Subject: [PATCH 16/27] using --- src/Analysis/Ast/Test/LocationTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Analysis/Ast/Test/LocationTests.cs b/src/Analysis/Ast/Test/LocationTests.cs index c8cf488ea..46f8d496c 100644 --- a/src/Analysis/Ast/Test/LocationTests.cs +++ b/src/Analysis/Ast/Test/LocationTests.cs @@ -16,9 +16,6 @@ using System.Threading.Tasks; using FluentAssertions; using Microsoft.Python.Analysis.Tests.FluentAssertions; -using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Analysis.Values; -using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; From 4404e5a9b5d14520edb260a7f4828d6cfd41a469 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 6 Mar 2019 10:05:22 -0800 Subject: [PATCH 17/27] Properly look at locations --- .../UndefinedVariables/ExpressionWalker.cs | 24 ++++++++---------- src/Analysis/Ast/Test/LinterTests.cs | 25 ++++++++++++++++++- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 962097d95..6ab84498f 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -14,7 +14,9 @@ // permissions and limitations under the License. using System.Collections.Generic; +using System.Linq; using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Analysis.Values; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { @@ -71,7 +73,7 @@ public override bool Walk(NameExpression node) { if (_additionalNameNodes?.Contains(node) == true) { return false; } - var m = _analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out _); + var m = _analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out var scope); if (m == null) { _analysis.ReportUndefinedVariable(node); } @@ -79,20 +81,16 @@ public override bool Walk(NameExpression node) { // undefined x in // y = x // x = 1 - if (m is ILocatedMember lm && lm.Location.DocumentUri == _analysis.Document.Uri) { + var v = scope?.Variables[node.Name]; + if (v != null && v.Location.DocumentUri == _analysis.Document.Uri) { // Do not complain about functions and classes that appear later in the file - if (m is IPythonFunctionType || m is IPythonClassType) { - return false; + if (!(v.Value is IPythonFunctionType || v.Value is IPythonClassType)) { + var span = v.Locations.First().Span; + var nodeLoc = node.GetLocation(_analysis.Document); + if (span.IsAfter(nodeLoc.Span)) { + _analysis.ReportUndefinedVariable(node); + } } - // Since analyzer remembers the last assignment, it may look that the variable - // is not defined yet, but we also should look up, perhaps it was defined - // earlier and redefined later. - // https://github.com/Microsoft/python-language-server/issues/682 - //var span = lm.Location.Span; - //var nodeLoc = node.GetLocation(_analysis.Document); - //if (span.IsAfter(nodeLoc.Span)) { - // _analysis.ReportUndefinedVariable(node); - //} } return false; } diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 4ee2db444..711cb858c 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -202,7 +202,17 @@ public async Task AssignmentBefore() { } [TestMethod, Priority(0)] - [Ignore("https://github.com/Microsoft/python-language-server/issues/682")] + public async Task AssignmentBeforeAndAfter() { + const string code = @" +x = 1 +y = x +x = 's' +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] public async Task AssignmentAfter() { const string code = @" y = x @@ -302,6 +312,19 @@ public async Task Enumeration() { analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task ReassignInLoop() { + const string code = @" +x = {} +for a, b in enumerate(x): + y = a + a = b + b = 1 +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + private class AnalysisOptionsProvider: IAnalysisOptionsProvider { public AnalysisOptions Options { get; } = new AnalysisOptions(); } From 13e6e99c040706b5ead960cff3df5604611b6e82 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 11 Mar 2019 23:28:56 -0700 Subject: [PATCH 18/27] Comprehensions etc --- .../Ast/Impl/Analyzer/AnalysisWalker.cs | 13 ++- .../Evaluation/ExpressionEval.Collections.cs | 60 +++++++++++ .../Analyzer/Evaluation/ExpressionEval.cs | 25 ++--- .../Analyzer/Handlers/ConditionalHandler.cs | 2 - .../Ast/Impl/Analyzer/Handlers/LoopHandler.cs | 9 +- .../Analyzer/Handlers/StatementHandler.cs | 2 - .../Handlers/TupleExpressionHandler.cs | 99 +++++++++++++------ .../Ast/Impl/Extensions/NodeExtensions.cs | 7 ++ .../UndefinedVariables/ExpressionWalker.cs | 16 ++- .../UndefinedVariablesWalker.cs | 14 +-- src/Analysis/Ast/Impl/Types/ArgumentSet.cs | 2 +- src/Analysis/Ast/Impl/Values/GlobalScope.cs | 2 + src/Analysis/Ast/Impl/Values/Scope.cs | 2 +- .../Ast/Impl/Values/VariableCollection.cs | 3 + src/Analysis/Ast/Test/CollectionsTests.cs | 14 +++ src/Analysis/Ast/Test/ComprehensionTests.cs | 82 +++++++++++++++ src/Analysis/Ast/Test/LinterTests.cs | 25 ++++- src/LanguageServer/Test/CompletionTests.cs | 11 +++ 18 files changed, 318 insertions(+), 70 deletions(-) create mode 100644 src/Analysis/Ast/Test/ComprehensionTests.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs b/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs index 71331f319..c99d4cbc7 100644 --- a/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/AnalysisWalker.cs @@ -62,7 +62,14 @@ public override bool Walk(AssignmentStatement node) { } public override bool Walk(ExpressionStatement node) { - AssignmentHandler.HandleAnnotatedExpression(node.Expression as ExpressionWithAnnotation, null); + switch (node.Expression) { + case ExpressionWithAnnotation ea: + AssignmentHandler.HandleAnnotatedExpression(ea, null); + break; + case Comprehension comp: + Eval.ProcessComprehension(comp); + break; + } return false; } @@ -72,9 +79,7 @@ public override bool Walk(ExpressionStatement node) { public override bool Walk(IfStatement node) => ConditionalHandler.HandleIf(node); public override bool Walk(ImportStatement node) => ImportHandler.HandleImport(node); - - public override bool Walk(NonlocalStatement node) - => NonLocalHandler.HandleNonLocal(node); + public override bool Walk(NonlocalStatement node) => NonLocalHandler.HandleNonLocal(node); public override bool Walk(TryStatement node) { TryExceptHandler.HandleTryExcept(node); diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Collections.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Collections.cs index 859e91e2b..c549c0dfa 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Collections.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Collections.cs @@ -98,5 +98,65 @@ public IMember GetValueFromGenerator(GeneratorExpression expression) { } return UnknownType; } + + public IMember GetValueFromComprehension(Comprehension node) { + var oldVariables = CurrentScope.Variables.OfType().ToDictionary(k => k.Name, v => v); + try { + ProcessComprehension(node); + switch (node) { + case ListComprehension lc: + var v1 = GetValueFromExpression(lc.Item) ?? UnknownType; + return PythonCollectionType.CreateList(Interpreter, GetLoc(lc), new[] { v1 }); + case SetComprehension sc: + var v2 = GetValueFromExpression(sc.Item) ?? UnknownType; + return PythonCollectionType.CreateSet(Interpreter, GetLoc(sc), new[] { v2 }); + case DictionaryComprehension dc: + var k = GetValueFromExpression(dc.Key) ?? UnknownType; + var v = GetValueFromExpression(dc.Value) ?? UnknownType; + return new PythonDictionary(new PythonDictionaryType(Interpreter), GetLoc(dc), new Dictionary { { k, v } }); + } + + return UnknownType; + } finally { + // Remove temporary variables since this is assignment and the right hand + // side comprehension does not leak internal variables into the scope. + var newVariables = CurrentScope.Variables.ToDictionary(k => k.Name, v => v); + var variables = (VariableCollection)CurrentScope.Variables; + foreach (var kvp in newVariables) { + if (!oldVariables.ContainsKey(kvp.Key)) { + variables.RemoveVariable(kvp.Key); + } else { + variables.DeclareVariable(oldVariables[kvp.Key]); + } + } + } + } + + internal void ProcessComprehension(Comprehension node) { + foreach (var cfor in node.Iterators.OfType().Where(c => c.Left != null)) { + var value = GetValueFromExpression(cfor.List); + if (value != null) { + switch (cfor.Left) { + case NameExpression nex when value is IPythonCollection coll: + DeclareVariable(nex.Name, coll.GetIterator().Next, VariableSource.Declaration, GetLoc(nex)); + break; + case NameExpression nex: + DeclareVariable(nex.Name, UnknownType, VariableSource.Declaration, GetLoc(nex)); + break; + case TupleExpression tex when value is IPythonDictionary dict && tex.Items.Count > 0: + if (tex.Items[0] is NameExpression nx0 && !string.IsNullOrEmpty(nx0.Name)) { + DeclareVariable(nx0.Name, dict.Keys.FirstOrDefault() ?? UnknownType, VariableSource.Declaration, GetLoc(nx0)); + } + if (tex.Items.Count > 1 && tex.Items[1] is NameExpression nx1 && !string.IsNullOrEmpty(nx1.Name)) { + DeclareVariable(nx1.Name, dict.Values.FirstOrDefault() ?? UnknownType, VariableSource.Declaration, GetLoc(nx1)); + } + foreach (var item in tex.Items.Skip(2).OfType().Where(x => !string.IsNullOrEmpty(x.Name))) { + DeclareVariable(item.Name, UnknownType, VariableSource.Declaration, GetLoc(item)); + } + break; + } + } + } + } } } diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs index 14465c9f5..b8d1b8942 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs @@ -69,6 +69,15 @@ public ExpressionEval(IServiceContainer services, IPythonModule module, PythonAs public LocationInfo GetLocation(Node node) => node?.GetLocation(Module, Ast) ?? LocationInfo.Empty; public IEnumerable Diagnostics => _diagnostics; + public void ReportDiagnostics(Uri documentUri, DiagnosticsEntry entry) { + // Do not add if module is library, etc. Only handle user code. + if (Module.ModuleType == ModuleType.User) { + lock (_lock) { + _diagnostics.Add(entry); + } + } + } + public IMember GetValueFromExpression(Expression expr) => GetValueFromExpression(expr, DefaultLookupOptions); @@ -89,9 +98,7 @@ public IMember GetValueFromExpression(Expression expr, LookupOptions options) { return null; } - while (expr is ParenthesisExpression parExpr) { - expr = parExpr.Expression; - } + expr = expr.RemoveParenthesis(); IMember m; switch (expr) { @@ -131,6 +138,9 @@ public IMember GetValueFromExpression(Expression expr, LookupOptions options) { case GeneratorExpression genex: m = GetValueFromGenerator(genex); break; + case Comprehension comp: + m = GetValueFromComprehension(comp); + break; default: m = GetValueFromBinaryOp(expr) ?? GetConstantFromLiteral(expr, options); break; @@ -228,14 +238,5 @@ private IMember GetValueFromConditional(ConditionalExpression expr) { return trueValue ?? falseValue ?? UnknownType; } - - public void ReportDiagnostics(Uri documentUri, DiagnosticsEntry entry) { - // Do not add if module is library, etc. Only handle user code. - if (Module.ModuleType == ModuleType.User) { - lock (_lock) { - _diagnostics.Add(entry); - } - } - } } } diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs index b104acb94..8720bc8e3 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs @@ -15,8 +15,6 @@ using System; using System.Linq; -using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core.OS; using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Ast; diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs index 985956a9f..2e359fd4b 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/LoopHandler.cs @@ -34,12 +34,9 @@ public bool HandleFor(ForStatement node) { break; case TupleExpression tex: // x = [('abc', 42, True), ('abc', 23, False)] - // for some_str, some_int, some_bool in x: - foreach (var nex in tex.Items.OfType()) { - if (!string.IsNullOrEmpty(nex.Name)) { - Eval.DeclareVariable(nex.Name, value, VariableSource.Declaration, Eval.GetLoc(nex)); - } - } + // for some_str, (some_int, some_bool) in x: + var h = new TupleExpressionHandler(Walker); + h.HandleTupleAssignment(tex, node.List, value); break; } diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/StatementHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/StatementHandler.cs index 3104db964..843fb41ed 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/StatementHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/StatementHandler.cs @@ -14,10 +14,8 @@ // permissions and limitations under the License. using Microsoft.Python.Analysis.Analyzer.Evaluation; -using Microsoft.Python.Analysis.Analyzer.Symbols; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core.Logging; using Microsoft.Python.Parsing.Ast; diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs index b7490d4ca..71a565e61 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs @@ -14,6 +14,7 @@ // permissions and limitations under the License. using System.Linq; +using Microsoft.Python.Analysis.Analyzer.Evaluation; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; @@ -24,45 +25,83 @@ internal sealed class TupleExpressionHandler : StatementHandler { public TupleExpressionHandler(AnalysisWalker walker) : base(walker) { } public void HandleTupleAssignment(TupleExpression lhs, Expression rhs, IMember value) { - string[] names; - if (rhs is TupleExpression tex) { - var returnedExpressions = tex.Items.ToArray(); - names = lhs.Items.OfType().Select(x => x.Name).ToArray(); - for (var i = 0; i < names.Length; i++) { - Expression e = null; - if (returnedExpressions.Length > 0) { - e = i < returnedExpressions.Length ? returnedExpressions[i] : returnedExpressions[returnedExpressions.Length - 1]; - } + AssignTuple(lhs, tex, Eval); + return; + } - if (e != null && !string.IsNullOrEmpty(names[i])) { - var v = Eval.GetValueFromExpression(e); - Eval.DeclareVariable(names[i], v ?? Eval.UnknownType, VariableSource.Declaration, e); - } + if (AssignTuple(lhs, value, Eval)) { + return; + } + + foreach (var n in lhs.Items.OfType().Select(x => x.Name).ExcludeDefault()) { + Eval.DeclareVariable(n, value, VariableSource.Declaration, rhs); + } + } + + internal static void AssignTuple(TupleExpression lhs, TupleExpression rhs, ExpressionEval eval) { + var returnedExpressions = rhs.Items.ToArray(); + var names = lhs.Items.OfType().Select(x => x.Name).ToArray(); + for (var i = 0; i < names.Length; i++) { + Expression e = null; + if (returnedExpressions.Length > 0) { + e = i < returnedExpressions.Length ? returnedExpressions[i] : returnedExpressions[returnedExpressions.Length - 1]; } - return; + if (e != null && !string.IsNullOrEmpty(names[i])) { + var v = eval.GetValueFromExpression(e); + eval.DeclareVariable(names[i], v ?? eval.UnknownType, VariableSource.Declaration, e); + } } + } + internal static bool AssignTuple(TupleExpression lhs, IMember value, ExpressionEval eval) { // Tuple = 'tuple value' (such as from callable). Transfer values. - var expressions = lhs.Items.OfType().ToArray(); - names = expressions.Select(x => x.Name).ToArray(); - if (value is IPythonCollection seq) { - var types = seq.Contents.Select(c => c.GetPythonType()).ToArray(); - for (var i = 0; i < names.Length; i++) { - IPythonType t = null; - if (types.Length > 0) { - t = i < types.Length ? types[i] : types[types.Length - 1]; - } + if (!(value is IPythonCollection seq)) { + return false; + } - if (names[i] != null && t != null) { - var instance = t.CreateInstance(null, Eval.GetLoc(expressions[i]), ArgumentSet.Empty); - Eval.DeclareVariable(names[i], instance, VariableSource.Declaration, expressions[i]); - } + var types = seq.Contents.Select(c => c.GetPythonType()).ToArray(); + var typeEnum = new TypeEnumerator(types, eval.UnknownType); + AssignTuple(lhs, typeEnum, eval); + return true; + } + + private static void AssignTuple(TupleExpression tex, TypeEnumerator typeEnum, ExpressionEval eval) { + foreach (var item in tex.Items) { + switch (item) { + case NameExpression nex when !string.IsNullOrEmpty(nex.Name): + var instance = typeEnum.Next.CreateInstance(null, LocationInfo.Empty, ArgumentSet.Empty); + eval.DeclareVariable(nex.Name, instance, VariableSource.Declaration, nex); + break; + case TupleExpression te: + AssignTuple(te, typeEnum, eval); + break; } - } else { - foreach (var n in names.ExcludeDefault()) { - Eval.DeclareVariable(n, value, VariableSource.Declaration, rhs); + } + } + + private class TypeEnumerator { + private readonly IPythonType[] _types; + private readonly IPythonType _filler; + private int _index; + + public TypeEnumerator(IPythonType[] types, IPythonType filler) { + _types = types; + _filler = filler; + } + + public IPythonType Next { + get { + IPythonType t; + if (_types.Length > 0) { + t = _index < _types.Length ? _types[_index] : _types[_types.Length - 1]; + } else { + t = _filler; + } + + _index++; + return t; } } } diff --git a/src/Analysis/Ast/Impl/Extensions/NodeExtensions.cs b/src/Analysis/Ast/Impl/Extensions/NodeExtensions.cs index 82ea2f5b0..281c38744 100644 --- a/src/Analysis/Ast/Impl/Extensions/NodeExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/NodeExtensions.cs @@ -61,5 +61,12 @@ public static bool IsInAst(this ScopeStatement node, PythonAst ast) { } return ast == node; } + + public static Expression RemoveParenthesis(this Expression e) { + while (e is ParenthesisExpression parExpr) { + e = parExpr.Expression; + } + return e; + } } } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 6ab84498f..2d42b419b 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -16,7 +16,8 @@ using System.Collections.Generic; using System.Linq; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Analysis.Values; +using Microsoft.Python.Core; +using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { @@ -87,12 +88,23 @@ public override bool Walk(NameExpression node) { if (!(v.Value is IPythonFunctionType || v.Value is IPythonClassType)) { var span = v.Locations.First().Span; var nodeLoc = node.GetLocation(_analysis.Document); - if (span.IsAfter(nodeLoc.Span)) { + // Exclude same-name variables declared within the same statement + // like 'e' that appears before its declaration in '[e in for e in {}]' + if (span.IsAfter(nodeLoc.Span) && !IsSpanInComprehension(nodeLoc.Span)) { _analysis.ReportUndefinedVariable(node); } } } return false; } + + private bool IsSpanInComprehension(SourceSpan span) { + var start = span.Start.ToIndex(_analysis.Ast); + var end = span.End.ToIndex(_analysis.Ast); + return ((Node)_analysis.ExpressionEvaluator.CurrentScope.Node) + .TraverseDepthFirst(n => n.GetChildNodes()) + .OfType() + .Any(n => n.StartIndex <= start && end < n.EndIndex); + } } } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs index 676385cc2..c57ad7b56 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/UndefinedVariablesWalker.cs @@ -19,32 +19,28 @@ namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class UndefinedVariablesWalker : LinterWalker { - private readonly ExpressionWalker _expressionWalker; - public UndefinedVariablesWalker(IDocumentAnalysis analysis, IServiceContainer services) - : base(analysis, services) { - _expressionWalker = new ExpressionWalker(analysis); - } + : base(analysis, services) { } public override bool Walk(AssignmentStatement node) { if (node.Right is ErrorExpression) { return false; } - node.Right?.Walk(_expressionWalker); + node.Right?.Walk(new ExpressionWalker(Analysis)); return false; } public override bool Walk(CallExpression node) { - node.Target?.Walk(_expressionWalker); + node.Target?.Walk(new ExpressionWalker(Analysis)); foreach (var arg in node.Args) { - arg?.Expression?.Walk(_expressionWalker); + arg?.Expression?.Walk(new ExpressionWalker(Analysis)); } return false; } public override bool Walk(IfStatement node) { foreach (var test in node.Tests) { - test.Test.Walk(_expressionWalker); + test.Test.Walk(new ExpressionWalker(Analysis)); } return true; } diff --git a/src/Analysis/Ast/Impl/Types/ArgumentSet.cs b/src/Analysis/Ast/Impl/Types/ArgumentSet.cs index 65599495f..8efcd414c 100644 --- a/src/Analysis/Ast/Impl/Types/ArgumentSet.cs +++ b/src/Analysis/Ast/Impl/Types/ArgumentSet.cs @@ -66,7 +66,7 @@ public ArgumentSet(IPythonFunctionType fn, int overloadIndex, IPythonInstance in /// /// Creates set of arguments for a function call based on the call expression /// and the function signature. The result contains expressions - /// for arguments, but not actual values. on how to + /// for arguments, but not actual values. on how to /// get values for actual parameters. /// /// Function type. diff --git a/src/Analysis/Ast/Impl/Values/GlobalScope.cs b/src/Analysis/Ast/Impl/Values/GlobalScope.cs index 80d55ec47..a3a746f04 100644 --- a/src/Analysis/Ast/Impl/Values/GlobalScope.cs +++ b/src/Analysis/Ast/Impl/Values/GlobalScope.cs @@ -14,6 +14,7 @@ // permissions and limitations under the License. using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Values { internal sealed class GlobalScope: Scope, IGlobalScope { @@ -23,5 +24,6 @@ public GlobalScope(IPythonModule module): } public IPythonModule Module { get; } + public override ScopeStatement Node => Module.Analysis?.Ast; } } diff --git a/src/Analysis/Ast/Impl/Values/Scope.cs b/src/Analysis/Ast/Impl/Values/Scope.cs index e9173df95..3c8370354 100644 --- a/src/Analysis/Ast/Impl/Values/Scope.cs +++ b/src/Analysis/Ast/Impl/Values/Scope.cs @@ -38,7 +38,7 @@ public Scope(ScopeStatement node, IScope outerScope, bool visibleToChildren = tr #region IScope public string Name => Node?.Name ?? ""; - public ScopeStatement Node { get; } + public virtual ScopeStatement Node { get; } public IScope OuterScope { get; } public bool VisibleToChildren { get; } diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index 6845e6ad0..73dea055f 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -61,5 +61,8 @@ internal void DeclareVariable(string name, IMember value, VariableSource source, _variables[name] = new Variable(name, value, source, location); } } + + internal void DeclareVariable(Variable variable) =>_variables[variable.Name] = variable; + internal void RemoveVariable(string name) => _variables.TryRemove(name, out _); } } diff --git a/src/Analysis/Ast/Test/CollectionsTests.cs b/src/Analysis/Ast/Test/CollectionsTests.cs index 41325dd11..fbd0eb304 100644 --- a/src/Analysis/Ast/Test/CollectionsTests.cs +++ b/src/Analysis/Ast/Test/CollectionsTests.cs @@ -317,6 +317,19 @@ print some_bool .And.HaveVariable("some_bool").OfType(BuiltinTypeId.Bool); } + [TestMethod, Priority(0)] + public async Task ForTuple() { + const string code = @" +x = [('abc', 42, True), ('abc', 23, False)] +for a, (b, c) in x: + pass +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("a").OfType(BuiltinTypeId.Str) + .And.HaveVariable("b").OfType(BuiltinTypeId.Int) + .And.HaveVariable("c").OfType(BuiltinTypeId.Bool); + } + [TestMethod, Priority(0)] public async Task Generator2X() { const string code = @" @@ -502,6 +515,7 @@ def g(a, b): } [TestMethod, Priority(0)] + [Ignore] public async Task NamedTuple() { const string code = @" from collections import namedtuple diff --git a/src/Analysis/Ast/Test/ComprehensionTests.cs b/src/Analysis/Ast/Test/ComprehensionTests.cs new file mode 100644 index 000000000..bab03e13f --- /dev/null +++ b/src/Analysis/Ast/Test/ComprehensionTests.cs @@ -0,0 +1,82 @@ +// 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 System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Analysis.Types; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.Analysis.Tests { + [TestClass] + public class ComprehensionTests : AnalysisTestBase { + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [TestMethod, Priority(0)] + public async Task ListComprehension() { + const string code = @" +x = [e for e in {1, 2, 3}] +y = x[0] +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("x").OfType(BuiltinTypeId.List) + .And.HaveVariable("y").OfType(BuiltinTypeId.Int) + .And.NotHaveVariable("e"); + } + + [TestMethod, Priority(0)] + public async Task ListComprehensionExpression() { + const string code = @" +x = [e > 0 for e in {1, 2, 3}] +y = x[0] +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("y").OfType(BuiltinTypeId.Bool) + .And.NotHaveVariable("e"); + } + + [TestMethod, Priority(0)] + public async Task DictionaryComprehension() { + const string code = @" +x = {str(k): e > 0 for e in {1, 2, 3}} + +keys = x.keys() +k = keys[0] +values = x.values() +v = values[0] +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("x").OfType(BuiltinTypeId.Dict) + .And.HaveVariable("k").OfType(BuiltinTypeId.Str) + .And.HaveVariable("v").OfType(BuiltinTypeId.Bool) + .And.NotHaveVariable("e"); + } + + [TestMethod, Priority(0)] + public async Task ListComprehensionStatement() { + const string code = @"[e > 0 for e in {1, 2, 3}]"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("e").OfType(BuiltinTypeId.Int); + } + } +} diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LinterTests.cs index 711cb858c..38018edd8 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LinterTests.cs @@ -325,7 +325,30 @@ public async Task ReassignInLoop() { analysis.Diagnostics.Should().BeEmpty(); } - private class AnalysisOptionsProvider: IAnalysisOptionsProvider { + [TestMethod, Priority(0)] + public async Task ListComprehensionStatement() { + const string code = @" +[a == 1 for a in {}] +x = a +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task DictionaryComprehension() { + const string code = @" +b = {str(a): a == 1 for a in {}} +x = a +"; + var analysis = await GetAnalysisAsync(code); + var d = analysis.Diagnostics.ToArray(); + d.Should().HaveCount(1); + d[0].ErrorCode.Should().Be(ErrorCodes.UndefinedVariable); + d[0].SourceSpan.Should().Be(3, 5, 3, 6); + } + + private class AnalysisOptionsProvider : IAnalysisOptionsProvider { public AnalysisOptions Options { get; } = new AnalysisOptions(); } } diff --git a/src/LanguageServer/Test/CompletionTests.cs b/src/LanguageServer/Test/CompletionTests.cs index 172d695c6..b149422d0 100644 --- a/src/LanguageServer/Test/CompletionTests.cs +++ b/src/LanguageServer/Test/CompletionTests.cs @@ -1044,5 +1044,16 @@ def main(req: func.HttpRequest) -> func.HttpResponse: result.Should().HaveLabels("get"); result.Completions.First(x => x.label == "get").Should().HaveDocumentation("dict.get*"); } + + [TestMethod, Priority(0)] + public async Task InForEnumeration() { + var analysis = await GetAnalysisAsync(@" +for a, b in x: + +"); + var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion); + var result = cs.GetCompletions(analysis, new SourceLocation(3, 4)); + result.Should().HaveLabels("a", "b"); + } } } From 4adf172ba6584ad693b4296c771e7c72a4ff4349 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Mar 2019 10:28:09 -0700 Subject: [PATCH 19/27] Test update --- src/Analysis/Ast/Impl/Dependencies/DependencyResolver.cs | 1 - src/LanguageServer/Test/GoToDefinitionTests.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Dependencies/DependencyResolver.cs b/src/Analysis/Ast/Impl/Dependencies/DependencyResolver.cs index 41d324f02..21b26ff1d 100644 --- a/src/Analysis/Ast/Impl/Dependencies/DependencyResolver.cs +++ b/src/Analysis/Ast/Impl/Dependencies/DependencyResolver.cs @@ -21,7 +21,6 @@ namespace Microsoft.Python.Analysis.Dependencies { internal sealed class DependencyResolver : IDependencyResolver { - private readonly IDependencyFinder _dependencyFinder; private readonly DependencyGraph _vertices = new DependencyGraph(); private readonly Dictionary> _changedVertices = new Dictionary>(); private readonly object _syncObj = new object(); diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index 299ad4651..25f02508c 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -82,7 +82,7 @@ def func(a, b): reference.range.Should().Be(11, 0, 14, 12); reference = ds.FindDefinition(analysis, new SourceLocation(18, 1)); - reference.range.Should().Be(17, 0, 17, 1); // TODO: store all locations + reference.range.Should().Be(3, 0, 3, 1); // TODO: store all locations reference = ds.FindDefinition(analysis, new SourceLocation(19, 5)); reference.range.Should().Be(5, 6, 9, 18); From 1aaa445d0883550ae2662b1dfe2478e7930b211f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Mar 2019 11:35:04 -0700 Subject: [PATCH 20/27] Add support for lambdas --- .../Analyzer/Evaluation/ExpressionEval.Callables.cs | 12 ++++++++++++ .../Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs | 3 +++ src/Analysis/Ast/Test/AssignmentTests.cs | 2 -- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs index 3373a64ec..77e3eb020 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Callables.cs @@ -69,6 +69,18 @@ public IMember GetValueFromCallable(CallExpression expr) { return value; } + public IMember GetValueFromLambda(LambdaExpression expr) { + if (expr == null) { + return null; + } + + var loc = GetLoc(expr); + var ft = new PythonFunctionType(expr.Function, Module, null, loc); + var overload = new PythonFunctionOverload(expr.Function, ft, Module, GetLoc(expr)); + ft.AddOverload(overload); + return ft; + } + public IMember GetValueFromClassCtor(IPythonClassType cls, CallExpression expr) { SymbolTable.Evaluate(cls.ClassDefinition); // Determine argument types diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs index b8d1b8942..8de5d64d0 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs @@ -141,6 +141,9 @@ public IMember GetValueFromExpression(Expression expr, LookupOptions options) { case Comprehension comp: m = GetValueFromComprehension(comp); break; + case LambdaExpression lambda: + m = GetValueFromLambda(lambda); + break; default: m = GetValueFromBinaryOp(expr) ?? GetConstantFromLiteral(expr, options); break; diff --git a/src/Analysis/Ast/Test/AssignmentTests.cs b/src/Analysis/Ast/Test/AssignmentTests.cs index 183a72084..439d6dea5 100644 --- a/src/Analysis/Ast/Test/AssignmentTests.cs +++ b/src/Analysis/Ast/Test/AssignmentTests.cs @@ -214,7 +214,6 @@ def __init__(self): } [TestMethod, Priority(0)] - [Ignore] public async Task LambdaExpression1() { const string code = @" x = lambda a: a @@ -225,7 +224,6 @@ public async Task LambdaExpression1() { } [TestMethod, Priority(0)] - [Ignore] public async Task LambdaExpression2() { const string code = @" def f(a): From 88611714ed0975c489263ff5307d58464546b40c Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Mar 2019 11:55:17 -0700 Subject: [PATCH 21/27] Add lambda linting --- .../UndefinedVariables/ExpressionWalker.cs | 5 +++ .../UndefinedVariables/LambdaWalker.cs | 44 +++++++++++++++++++ .../UndefinedVariables/NameCollectorWalker.cs | 1 - ...nterTests.cs => LintUndefinedVarsTests.cs} | 12 ++++- src/Analysis/Ast/Test/ScrapeTests.cs | 2 +- 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 src/Analysis/Ast/Impl/Linting/UndefinedVariables/LambdaWalker.cs rename src/Analysis/Ast/Test/{LinterTests.cs => LintUndefinedVarsTests.cs} (97%) diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 2d42b419b..628d1cec6 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -48,6 +48,11 @@ public override bool Walk(CallExpression node) { return false; } + public override bool Walk(LambdaExpression node) { + node.Walk(new LambdaWalker(_analysis)); + return false; + } + public override bool Walk(ListComprehension node) { node.Walk(new ComprehensionWalker(_analysis)); return false; diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/LambdaWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/LambdaWalker.cs new file mode 100644 index 000000000..38c404ea5 --- /dev/null +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/LambdaWalker.cs @@ -0,0 +1,44 @@ +// 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 System.Collections.Generic; +using System.Linq; +using Microsoft.Python.Core; +using Microsoft.Python.Parsing.Ast; + +namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { + internal sealed class LambdaWalker : PythonWalker { + private readonly IDocumentAnalysis _analysis; + private readonly HashSet _names = new HashSet(); + private readonly HashSet _additionalNameNodes = new HashSet(); + + public LambdaWalker(IDocumentAnalysis analysis) { + _analysis = analysis; + } + + public override bool Walk(FunctionDefinition node) { + CollectNames(node); + node.Body?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); + return false; + } + + private void CollectNames(FunctionDefinition fd) { + var nc = new NameCollectorWalker(_names, _additionalNameNodes); + foreach (var nex in fd.Parameters.Select(p => p.NameExpression).ExcludeDefault()) { + nex.Walk(nc); + } + } + } +} diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs index 23cb0c78e..01fc065c8 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/NameCollectorWalker.cs @@ -31,7 +31,6 @@ public override bool Walk(NameExpression nex) { _names.Add(nex.Name); _additionalNameNodes.Add(nex); } - return false; } } diff --git a/src/Analysis/Ast/Test/LinterTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs similarity index 97% rename from src/Analysis/Ast/Test/LinterTests.cs rename to src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index 38018edd8..66b78f2af 100644 --- a/src/Analysis/Ast/Test/LinterTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -24,7 +24,7 @@ namespace Microsoft.Python.Analysis.Tests { [TestClass] - public class LinterTests : AnalysisTestBase { + public class LintUndefinedVarsTests : AnalysisTestBase { public TestContext TestContext { get; set; } [TestInitialize] @@ -348,6 +348,16 @@ public async Task DictionaryComprehension() { d[0].SourceSpan.Should().Be(3, 5, 3, 6); } + [TestMethod, Priority(0)] + public async Task Lambda() { + const string code = @" +x = lambda a: a +x(1) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + private class AnalysisOptionsProvider : IAnalysisOptionsProvider { public AnalysisOptions Options { get; } = new AnalysisOptions(); } diff --git a/src/Analysis/Ast/Test/ScrapeTests.cs b/src/Analysis/Ast/Test/ScrapeTests.cs index 34a5a4f39..0a222de49 100644 --- a/src/Analysis/Ast/Test/ScrapeTests.cs +++ b/src/Analysis/Ast/Test/ScrapeTests.cs @@ -226,7 +226,7 @@ await FullStdLibTest(v, } [TestMethod, TestCategory("60s"), Priority(1)] - [Timeout(10 * 60 * 1000)] + [Timeout(10 * 120 * 1000)] public async Task FullStdLibAnaconda2() { var v = PythonVersions.Anaconda27_x64 ?? PythonVersions.Anaconda27; await FullStdLibTest(v, From e76540c74aaac48a5edfbb293cae68c0d5705a7c Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Mar 2019 12:57:03 -0700 Subject: [PATCH 22/27] Handle tuple assignment better --- .../Handlers/TupleExpressionHandler.cs | 23 +++++++------------ src/Analysis/Ast/Test/CollectionsTests.cs | 12 +++++++++- .../Ast/Test/LintUndefinedVarsTests.cs | 11 +++++++++ 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs index 71a565e61..aad08089f 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs @@ -17,7 +17,6 @@ using Microsoft.Python.Analysis.Analyzer.Evaluation; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; -using Microsoft.Python.Core; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis.Analyzer.Handlers { @@ -27,15 +26,8 @@ public TupleExpressionHandler(AnalysisWalker walker) : base(walker) { } public void HandleTupleAssignment(TupleExpression lhs, Expression rhs, IMember value) { if (rhs is TupleExpression tex) { AssignTuple(lhs, tex, Eval); - return; - } - - if (AssignTuple(lhs, value, Eval)) { - return; - } - - foreach (var n in lhs.Items.OfType().Select(x => x.Name).ExcludeDefault()) { - Eval.DeclareVariable(n, value, VariableSource.Declaration, rhs); + } else { + AssignTuple(lhs, value, Eval); } } @@ -55,16 +47,17 @@ internal static void AssignTuple(TupleExpression lhs, TupleExpression rhs, Expre } } - internal static bool AssignTuple(TupleExpression lhs, IMember value, ExpressionEval eval) { + internal static void AssignTuple(TupleExpression lhs, IMember value, ExpressionEval eval) { // Tuple = 'tuple value' (such as from callable). Transfer values. - if (!(value is IPythonCollection seq)) { - return false; + IPythonType[] types; + if (value is IPythonCollection seq) { + types = seq.Contents.Select(c => c.GetPythonType()).ToArray(); + } else { + types = new[] {eval.UnknownType}; } - var types = seq.Contents.Select(c => c.GetPythonType()).ToArray(); var typeEnum = new TypeEnumerator(types, eval.UnknownType); AssignTuple(lhs, typeEnum, eval); - return true; } private static void AssignTuple(TupleExpression tex, TypeEnumerator typeEnum, ExpressionEval eval) { diff --git a/src/Analysis/Ast/Test/CollectionsTests.cs b/src/Analysis/Ast/Test/CollectionsTests.cs index fbd0eb304..be9f31a1c 100644 --- a/src/Analysis/Ast/Test/CollectionsTests.cs +++ b/src/Analysis/Ast/Test/CollectionsTests.cs @@ -318,7 +318,7 @@ print some_bool } [TestMethod, Priority(0)] - public async Task ForTuple() { + public async Task ForSequenceWithList1() { const string code = @" x = [('abc', 42, True), ('abc', 23, False)] for a, (b, c) in x: @@ -330,6 +330,16 @@ public async Task ForTuple() { .And.HaveVariable("c").OfType(BuiltinTypeId.Bool); } + [TestMethod, Priority(0)] + public async Task ForSequenceWithList2() { + const string code = @" +for a, (b, c) in x: + pass +"; + var analysis = await GetAnalysisAsync(code); + analysis.Should().HaveVariable("a").And.HaveVariable("b").And.HaveVariable("c"); + } + [TestMethod, Priority(0)] public async Task Generator2X() { const string code = @" diff --git a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index 66b78f2af..0139c5b21 100644 --- a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -163,6 +163,17 @@ public async Task ForVariables() { analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task ForVariablesList() { + const string code = @" +for i, (j, k) in {}: + x = j + y = k +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task ListComprehension() { const string code = @" From fe099b50923d8ac4f920b8afbe740c6f41a63f6d Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 12 Mar 2019 17:09:23 -0700 Subject: [PATCH 23/27] Test update --- .../Handlers/TupleExpressionHandler.cs | 33 +++++++++---------- src/Analysis/Ast/Test/ScrapeTests.cs | 10 +++--- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs index aad08089f..124cac1bb 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/TupleExpressionHandler.cs @@ -49,46 +49,45 @@ internal static void AssignTuple(TupleExpression lhs, TupleExpression rhs, Expre internal static void AssignTuple(TupleExpression lhs, IMember value, ExpressionEval eval) { // Tuple = 'tuple value' (such as from callable). Transfer values. - IPythonType[] types; + IMember[] values; if (value is IPythonCollection seq) { - types = seq.Contents.Select(c => c.GetPythonType()).ToArray(); + values = seq.Contents.ToArray(); } else { - types = new[] {eval.UnknownType}; + values = new[] { value }; } - var typeEnum = new TypeEnumerator(types, eval.UnknownType); + var typeEnum = new ValueEnumerator(values, eval.UnknownType); AssignTuple(lhs, typeEnum, eval); } - private static void AssignTuple(TupleExpression tex, TypeEnumerator typeEnum, ExpressionEval eval) { + private static void AssignTuple(TupleExpression tex, ValueEnumerator valueEnum, ExpressionEval eval) { foreach (var item in tex.Items) { switch (item) { case NameExpression nex when !string.IsNullOrEmpty(nex.Name): - var instance = typeEnum.Next.CreateInstance(null, LocationInfo.Empty, ArgumentSet.Empty); - eval.DeclareVariable(nex.Name, instance, VariableSource.Declaration, nex); + eval.DeclareVariable(nex.Name, valueEnum.Next, VariableSource.Declaration, nex); break; case TupleExpression te: - AssignTuple(te, typeEnum, eval); + AssignTuple(te, valueEnum, eval); break; } } } - private class TypeEnumerator { - private readonly IPythonType[] _types; - private readonly IPythonType _filler; + private class ValueEnumerator { + private readonly IMember[] _values; + private readonly IMember _filler; private int _index; - public TypeEnumerator(IPythonType[] types, IPythonType filler) { - _types = types; + public ValueEnumerator(IMember[] values, IMember filler) { + _values = values; _filler = filler; } - public IPythonType Next { + public IMember Next { get { - IPythonType t; - if (_types.Length > 0) { - t = _index < _types.Length ? _types[_index] : _types[_types.Length - 1]; + IMember t; + if (_values.Length > 0) { + t = _index < _values.Length ? _values[_index] : _values[_values.Length - 1]; } else { t = _filler; } diff --git a/src/Analysis/Ast/Test/ScrapeTests.cs b/src/Analysis/Ast/Test/ScrapeTests.cs index 0a222de49..66156b636 100644 --- a/src/Analysis/Ast/Test/ScrapeTests.cs +++ b/src/Analysis/Ast/Test/ScrapeTests.cs @@ -213,8 +213,9 @@ public async Task FullStdLibV27() { await FullStdLibTest(v); } - [TestMethod, TestCategory("60s"), Priority(1)] - [Timeout(10 * 60 * 1000)] + [TestMethod, Priority(1)] + [Timeout(10 * 180 * 1000)] + [Ignore] public async Task FullStdLibAnaconda3() { var v = PythonVersions.Anaconda36_x64 ?? PythonVersions.Anaconda36; await FullStdLibTest(v, @@ -225,8 +226,9 @@ await FullStdLibTest(v, ); } - [TestMethod, TestCategory("60s"), Priority(1)] - [Timeout(10 * 120 * 1000)] + [TestMethod, Priority(1)] + [Timeout(10 * 180 * 1000)] + [Ignore] public async Task FullStdLibAnaconda2() { var v = PythonVersions.Anaconda27_x64 ?? PythonVersions.Anaconda27; await FullStdLibTest(v, From 29e3dd15c37d8d0eb98e5439b6c10315536d6da9 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 13 Mar 2019 15:19:26 -0700 Subject: [PATCH 24/27] Correct comprehension iterator handlint --- .../Evaluation/ExpressionEval.Scopes.cs | 22 +++++++-------- .../Ast/Impl/Analyzer/PythonInterpreter.cs | 6 ++--- .../UndefinedVariables/ComprehensionWalker.cs | 21 ++++++++------- .../UndefinedVariables/ExpressionWalker.cs | 18 ++++++------- .../Ast/Test/LintUndefinedVarsTests.cs | 27 +++++++++++++++++++ 5 files changed, 61 insertions(+), 33 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs index ca27a311e..4dc334008 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs @@ -141,8 +141,8 @@ public IPythonType GetTypeFromAnnotation(Expression expr, out bool isGeneric, Lo /// as a child of the specified scope. Scope is pushed on the stack /// and will be removed when returned the disposable is disposed. /// - public IDisposable OpenScope(IPythonModule module, ScopeStatement node, out Scope fromScope) { - fromScope = null; + public IDisposable OpenScope(IPythonModule module, ScopeStatement node, out Scope outerScope) { + outerScope = null; if (node == null) { return Disposable.Empty; } @@ -156,26 +156,26 @@ public IDisposable OpenScope(IPythonModule module, ScopeStatement node, out Scop } if (node.Parent != null) { - if (!_scopeLookupCache.TryGetValue(node.Parent, out fromScope)) { - fromScope = gs + if (!_scopeLookupCache.TryGetValue(node.Parent, out outerScope)) { + outerScope = gs .TraverseDepthFirst(s => s.Children.OfType()) .FirstOrDefault(s => s.Node == node.Parent); - _scopeLookupCache[node.Parent] = fromScope; + _scopeLookupCache[node.Parent] = outerScope; } } - fromScope = fromScope ?? gs; - if (fromScope != null) { + outerScope = outerScope ?? gs; + if (outerScope != null) { Scope scope; if (node is PythonAst) { // node points to global scope, it is not a function or a class. scope = gs; } else { - scope = fromScope.Children.OfType().FirstOrDefault(s => s.Node == node); + scope = outerScope.Children.OfType().FirstOrDefault(s => s.Node == node); if (scope == null) { - scope = new Scope(node, fromScope, true); - fromScope.AddChildScope(scope); - _scopeLookupCache[node] = fromScope; + scope = new Scope(node, outerScope, true); + outerScope.AddChildScope(scope); + _scopeLookupCache[node] = scope; } } diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs b/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs index 3745ce3b5..40002bdd2 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs @@ -49,9 +49,6 @@ private async Task LoadBuiltinTypesAsync(string root, IServiceManager sm, Cancel _moduleResolution = new MainModuleResolution(root, sm); await _moduleResolution.InitializeAsync(cancellationToken); - _stubResolution = new TypeshedResolution(sm); - await _stubResolution.InitializeAsync(cancellationToken); - var builtinModule = _moduleResolution.BuiltinsModule; lock (_lock) { _builtinTypes[BuiltinTypeId.NoneType] @@ -60,6 +57,9 @@ private async Task LoadBuiltinTypesAsync(string root, IServiceManager sm, Cancel = UnknownType = new PythonType("Unknown", builtinModule, string.Empty, LocationInfo.Empty); } await _moduleResolution.LoadBuiltinTypesAsync(cancellationToken); + + _stubResolution = new TypeshedResolution(sm); + await _stubResolution.InitializeAsync(cancellationToken); } public static async Task CreateAsync(InterpreterConfiguration configuration, string root, IServiceManager sm, CancellationToken cancellationToken = default) { diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs index 3715bd015..9f6ca794a 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ComprehensionWalker.cs @@ -20,8 +20,8 @@ namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class ComprehensionWalker : PythonWalker { private readonly IDocumentAnalysis _analysis; - private readonly HashSet _names = new HashSet(); - private readonly HashSet _additionalNameNodes = new HashSet(); + private readonly HashSet _localNames = new HashSet(); + private readonly HashSet _localNameNodes = new HashSet(); public ComprehensionWalker(IDocumentAnalysis analysis) { _analysis = analysis; @@ -43,24 +43,24 @@ public override bool Walk(SetComprehension node) { } public override bool Walk(ForStatement node) { - var nc = new NameCollectorWalker(_names, _additionalNameNodes); + var nc = new NameCollectorWalker(_localNames, _localNameNodes); node.Left?.Walk(nc); return true; } public override bool Walk(DictionaryComprehension node) { CollectNames(node); - node.Key?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); - node.Value?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); + var ew = new ExpressionWalker(_analysis, _localNames, _localNameNodes); + node.Key?.Walk(ew); + node.Value?.Walk(ew); foreach (var iter in node.Iterators) { - iter?.Walk(new ExpressionWalker(_analysis, null, _additionalNameNodes)); + iter?.Walk(ew); } - return true; } private void CollectNames(Comprehension c) { - var nc = new NameCollectorWalker(_names, _additionalNameNodes); + var nc = new NameCollectorWalker(_localNames, _localNameNodes); foreach (var cfor in c.Iterators.OfType()) { cfor.Left?.Walk(nc); } @@ -68,9 +68,10 @@ private void CollectNames(Comprehension c) { private void ProcessComprehension(Comprehension c, Node item, IEnumerable iterators) { CollectNames(c); - item?.Walk(new ExpressionWalker(_analysis, _names, _additionalNameNodes)); + var ew = new ExpressionWalker(_analysis, _localNames, _localNameNodes); + item?.Walk(ew); foreach (var iter in iterators) { - iter.Walk(new ExpressionWalker(_analysis, null, _additionalNameNodes)); + iter.Walk(ew); } } } diff --git a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs index 628d1cec6..c8e663625 100644 --- a/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs +++ b/src/Analysis/Ast/Impl/Linting/UndefinedVariables/ExpressionWalker.cs @@ -23,8 +23,8 @@ namespace Microsoft.Python.Analysis.Linting.UndefinedVariables { internal sealed class ExpressionWalker : PythonWalker { private readonly IDocumentAnalysis _analysis; - private readonly HashSet _additionalNames; - private readonly HashSet _additionalNameNodes; + private readonly HashSet _localNames; + private readonly HashSet _localNameNodes; public ExpressionWalker(IDocumentAnalysis analysis) : this(analysis, null, null) { } @@ -33,12 +33,12 @@ public ExpressionWalker(IDocumentAnalysis analysis) /// Creates walker for detection of undefined variables. /// /// Document analysis. - /// Additional defined names. - /// Name nodes for defined names. - public ExpressionWalker(IDocumentAnalysis analysis, HashSet additionalNames, HashSet additionalNameNodes) { + /// Locally defined names, such as variables in a comprehension. + /// Name nodes for local names. + public ExpressionWalker(IDocumentAnalysis analysis, HashSet localNames, HashSet localNameNodes) { _analysis = analysis; - _additionalNames = additionalNames; - _additionalNameNodes = additionalNameNodes; + _localNames = localNames; + _localNameNodes = localNameNodes; } public override bool Walk(CallExpression node) { @@ -73,10 +73,10 @@ public override bool Walk(GeneratorExpression node) { } public override bool Walk(NameExpression node) { - if (_additionalNames?.Contains(node.Name) == true) { + if (_localNames?.Contains(node.Name) == true) { return false; } - if (_additionalNameNodes?.Contains(node) == true) { + if (_localNameNodes?.Contains(node) == true) { return false; } var m = _analysis.ExpressionEvaluator.LookupNameInScopes(node.Name, out var scope); diff --git a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs index 0139c5b21..09377f449 100644 --- a/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs +++ b/src/Analysis/Ast/Test/LintUndefinedVarsTests.cs @@ -163,6 +163,17 @@ public async Task ForVariables() { analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task ForVariablesCondition() { + const string code = @" +c = {} +for a, b in c if a < 0: + x = b +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task ForVariablesList() { const string code = @" @@ -174,6 +185,22 @@ public async Task ForVariablesList() { analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task ForExpression() { + const string code = @" +def func1(a): + return a + +def func2(a, b): + return a + b + +func1(func2(a) for a, b in {} if a < 0) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] public async Task ListComprehension() { const string code = @" From aa2a415fc5d743a702bb0d304f024c25247c0c0c Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 13 Mar 2019 16:00:36 -0700 Subject: [PATCH 25/27] Fix race condition at builtins load --- .../Ast/Impl/Analyzer/PythonInterpreter.cs | 8 ++--- .../Impl/Modules/Definitions/IModuleCache.cs | 5 ---- src/Analysis/Ast/Impl/Modules/ModuleCache.cs | 30 ------------------- .../Resolution/MainModuleResolution.cs | 26 +++++++++------- 4 files changed, 19 insertions(+), 50 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs b/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs index 40002bdd2..4864de8ec 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonInterpreter.cs @@ -47,19 +47,19 @@ private async Task LoadBuiltinTypesAsync(string root, IServiceManager sm, Cancel sm.AddService(this); _moduleResolution = new MainModuleResolution(root, sm); - await _moduleResolution.InitializeAsync(cancellationToken); - - var builtinModule = _moduleResolution.BuiltinsModule; lock (_lock) { + var builtinModule = _moduleResolution.CreateBuiltinsModule(); _builtinTypes[BuiltinTypeId.NoneType] = new PythonType("NoneType", builtinModule, string.Empty, LocationInfo.Empty, BuiltinTypeId.NoneType); _builtinTypes[BuiltinTypeId.Unknown] = UnknownType = new PythonType("Unknown", builtinModule, string.Empty, LocationInfo.Empty); } - await _moduleResolution.LoadBuiltinTypesAsync(cancellationToken); + await _moduleResolution.InitializeAsync(cancellationToken); _stubResolution = new TypeshedResolution(sm); await _stubResolution.InitializeAsync(cancellationToken); + + await _moduleResolution.LoadBuiltinTypesAsync(cancellationToken); } public static async Task CreateAsync(InterpreterConfiguration configuration, string root, IServiceManager sm, CancellationToken cancellationToken = default) { diff --git a/src/Analysis/Ast/Impl/Modules/Definitions/IModuleCache.cs b/src/Analysis/Ast/Impl/Modules/Definitions/IModuleCache.cs index 1965ef402..40ea4c0a6 100644 --- a/src/Analysis/Ast/Impl/Modules/Definitions/IModuleCache.cs +++ b/src/Analysis/Ast/Impl/Modules/Definitions/IModuleCache.cs @@ -13,13 +13,8 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Python.Analysis.Documents; - namespace Microsoft.Python.Analysis.Modules { public interface IModuleCache { - Task ImportFromCacheAsync(string name, CancellationToken cancellationToken); string GetCacheFilePath(string filePath); string ReadCachedModule(string filePath); void WriteCachedModule(string filePath, string code); diff --git a/src/Analysis/Ast/Impl/Modules/ModuleCache.cs b/src/Analysis/Ast/Impl/Modules/ModuleCache.cs index 685eea09b..4052283c7 100644 --- a/src/Analysis/Ast/Impl/Modules/ModuleCache.cs +++ b/src/Analysis/Ast/Impl/Modules/ModuleCache.cs @@ -18,9 +18,7 @@ using System.IO; using System.Security.Cryptography; using System.Text; -using System.Threading; using System.Threading.Tasks; -using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core; using Microsoft.Python.Core.IO; using Microsoft.Python.Core.Logging; @@ -44,34 +42,6 @@ public ModuleCache(IPythonInterpreter interpreter, IServiceContainer services) { _skipCache = string.IsNullOrEmpty(_interpreter.Configuration.DatabasePath); } - public async Task ImportFromCacheAsync(string name, CancellationToken cancellationToken) { - if (string.IsNullOrEmpty(ModuleCachePath)) { - return null; - } - - var cache = GetCacheFilePath("python.{0}.pyi".FormatInvariant(name)); - if (!_fs.FileExists(cache)) { - cache = GetCacheFilePath("python._{0}.pyi".FormatInvariant(name)); - if (!_fs.FileExists(cache)) { - cache = GetCacheFilePath("{0}.pyi".FormatInvariant(name)); - if (!_fs.FileExists(cache)) { - return null; - } - } - } - - var rdt = _services.GetService(); - var mco = new ModuleCreationOptions { - ModuleName = name, - ModuleType = ModuleType.Compiled, - FilePath = cache - }; - var module = rdt.AddModule(mco); - - await module.LoadAndAnalyzeAsync(cancellationToken); - return module; - } - public string GetCacheFilePath(string filePath) { if (string.IsNullOrEmpty(filePath) || !PathEqualityComparer.IsValidPath(ModuleCachePath)) { if (!_loggedBadDbPath) { diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs index 9a951daa2..fd13ffda2 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs @@ -39,18 +39,24 @@ internal sealed class MainModuleResolution : ModuleResolutionBase, IModuleManage public MainModuleResolution(string root, IServiceContainer services) : base(root, services) { } + internal IBuiltinsPythonModule CreateBuiltinsModule() { + if (BuiltinsModule == null) { + // Initialize built-in + var moduleName = BuiltinTypeId.Unknown.GetModuleName(_interpreter.LanguageVersion); + + ModuleCache = new ModuleCache(_interpreter, _services); + var modulePath = ModuleCache.GetCacheFilePath(_interpreter.Configuration.InterpreterPath); + + var b = new BuiltinsPythonModule(moduleName, modulePath, _services); + BuiltinsModule = b; + Modules[BuiltinModuleName] = new ModuleRef(b); + } + return BuiltinsModule; + } + internal async Task InitializeAsync(CancellationToken cancellationToken = default) { - // Add names from search paths await ReloadAsync(cancellationToken); cancellationToken.ThrowIfCancellationRequested(); - - // Initialize built-in - var moduleName = BuiltinTypeId.Unknown.GetModuleName(_interpreter.LanguageVersion); - var modulePath = ModuleCache.GetCacheFilePath(_interpreter.Configuration.InterpreterPath); - - var b = new BuiltinsPythonModule(moduleName, modulePath, _services); - BuiltinsModule = b; - Modules[BuiltinModuleName] = new ModuleRef(b); } public async Task> GetSearchPathsAsync(CancellationToken cancellationToken = default) { @@ -177,8 +183,6 @@ internal async Task LoadBuiltinTypesAsync(CancellationToken cancellationToken = public async Task ReloadAsync(CancellationToken cancellationToken = default) { Modules.Clear(); - - ModuleCache = new ModuleCache(_interpreter, _services); PathResolver = new PathResolver(_interpreter.LanguageVersion); var addedRoots = new HashSet(); From add83584ec9f4dc8d8923bf88fb44915edf917a3 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Mar 2019 10:38:46 -0700 Subject: [PATCH 26/27] Merge master --- .../Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs | 6 +++--- .../Ast/Impl/Modules/Resolution/MainModuleResolution.cs | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs index 3d0f54d3b..4dc334008 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Scopes.cs @@ -173,9 +173,9 @@ public IDisposable OpenScope(IPythonModule module, ScopeStatement node, out Scop } else { scope = outerScope.Children.OfType().FirstOrDefault(s => s.Node == node); if (scope == null) { - scope = new Scope(node, fromScope, true); - fromScope.AddChildScope(scope); - _scopeLookupCache[node] = fromScope; + scope = new Scope(node, outerScope, true); + outerScope.AddChildScope(scope); + _scopeLookupCache[node] = scope; } } diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs index e13d613d6..29e91c87f 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs @@ -189,7 +189,12 @@ public async Task ReloadAsync(CancellationToken cancellationToken = default) { foreach (var m in Modules) { GetRdt()?.UnlockDocument(m.Value.Value.Uri); } + + // Preserve builtins, they don't need to be reloaded since interpreter does not change. + var builtins = Modules[BuiltinModuleName]; Modules.Clear(); + Modules[BuiltinModuleName] = builtins; + PathResolver = new PathResolver(_interpreter.LanguageVersion); var addedRoots = new HashSet(); From 91a7215cb6c8677429ab1c589e41180a09c0950c Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Mar 2019 10:50:57 -0700 Subject: [PATCH 27/27] Restore linter --- src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 1edbf7f92..330a5d940 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -359,12 +359,18 @@ private void AnalyzeEntry(PythonAnalyzerEntry entry, IPythonModule module, Pytho ast.Walk(walker); cancellationToken.ThrowIfCancellationRequested(); - // Note that we do not set the new analysis here and rather let - // Python analyzer to call NotifyAnalysisComplete. walker.Complete(); cancellationToken.ThrowIfCancellationRequested(); var analysis = new DocumentAnalysis((IDocument)module, version, walker.GlobalScope, walker.Eval); + if (module.ModuleType == ModuleType.User) { + var optionsProvider = _services.GetService(); + if (optionsProvider?.Options?.LintingEnabled != false) { + var linter = new LinterAggregator(); + linter.Lint(analysis, _services); + } + } + (module as IAnalyzable)?.NotifyAnalysisComplete(analysis); entry.TrySetAnalysis(analysis, version); }