From 8bca56b1fa4ab8b282ae6491de25525c90aa9efe Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Mar 2019 09:51:05 -0800 Subject: [PATCH 01/15] Fix #668 (partial) --- .../Analyzer/Handlers/ConditionalHandler.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs index cb41801b9..b104acb94 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/ConditionalHandler.cs @@ -66,24 +66,6 @@ public bool HandleIf(IfStatement node) { someRecognized = true; } } - - // Handle basic check such as - // if isinstance(value, type): - // return value - // by assigning type to the value unless clause is raising exception. - var ce = node.Tests.FirstOrDefault()?.Test as CallExpression; - if (ce?.Target is NameExpression ne && ne.Name == @"isinstance" && ce.Args.Count == 2) { - var nex = ce.Args[0].Expression as NameExpression; - var name = nex?.Name; - var typeName = (ce.Args[1].Expression as NameExpression)?.Name; - if (name != null && typeName != null) { - var typeId = typeName.GetTypeId(); - if (typeId != BuiltinTypeId.Unknown) { - var t = new PythonType(typeName, Module, string.Empty, LocationInfo.Empty, typeId); - Eval.DeclareVariable(name, t, VariableSource.Declaration, nex); - } - } - } return !someRecognized; } From 7ffc9db4f0807314d4d36a8540380b988c26ee2e Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 4 Mar 2019 16:43:30 -0800 Subject: [PATCH 02/15] 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 03/15] 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 01781c0417b836768708eebaad825330d0b8c987 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 27 Mar 2019 16:03:02 -0700 Subject: [PATCH 04/15] Exp --- .../Ast/Impl/Analyzer/PythonAnalyzer.cs | 48 +++++++++++++++++-- .../Ast/Impl/Documents/DocumentBuffer.cs | 16 +++---- src/LanguageServer/Impl/Program.cs | 2 +- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index b627aa4f7..674dbe877 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -14,6 +14,7 @@ // permissions and limitations under the License. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -55,6 +56,7 @@ public PythonAnalyzer(IServiceManager services) { _analysisRunningEvent.Set(); _progress = new ProgressReporter(services.GetService()); + Task.Run(() => Worker()).DoNotWait(); } public void Dispose() { @@ -137,7 +139,7 @@ public void EnqueueDocumentForAnalysis(IPythonModule module, ImmutableArray LintModule(IPythonModule module) { return linter.Lint(module.Analysis, _services); } + private readonly ConcurrentQueue _queue = new ConcurrentQueue(); + private readonly ManualResetEventSlim _workAvailable = new ManualResetEventSlim(false); + private readonly ManualResetEventSlim _workerAvailable = new ManualResetEventSlim(true); + private readonly object _lock = new object(); + private int _workerCount; - private async Task AnalyzeDocumentAsync(AnalysisModuleKey key, PythonAnalyzerEntry entry, ImmutableArray dependencies, CancellationToken cancellationToken) { + private void AnalyzeDocument(AnalysisModuleKey key, PythonAnalyzerEntry entry, ImmutableArray dependencies, CancellationToken cancellationToken) { + lock (_lock) { + _queue.Enqueue(() => DoAnalyzeDocument(key, entry, dependencies, cancellationToken)); + _workAvailable.Set(); + } + } + + private void Worker() { + while(true) { + _workAvailable.Wait(); + if (_queue.TryDequeue(out var action)) { + _workerAvailable.Wait(); + lock (_lock) { + _workerCount++; + if (_workerCount > 4) { + _workerAvailable.Reset(); + } + } + + Task.Run(action).ContinueWith(t => { + lock (_lock) { + _workerCount--; + _workerAvailable.Set(); + } + }); + } else { + _workAvailable.Reset(); + } + } + } + + private void DoAnalyzeDocument(AnalysisModuleKey key, PythonAnalyzerEntry entry, ImmutableArray dependencies, CancellationToken cancellationToken) { _analysisCompleteEvent.Reset(); _log?.Log(TraceEventType.Verbose, $"Analysis of {entry.Module.Name}({entry.Module.ModuleType}) queued"); @@ -213,7 +251,7 @@ private async Task AnalyzeDocumentAsync(AnalysisModuleKey key, PythonAnalyzerEnt StartAnalysis(entry, walker.Version, cancellationToken); } - await waitForAnalysisTask; + waitForAnalysisTask.Wait(cancellationToken); } int version; @@ -239,7 +277,7 @@ private async Task AnalyzeDocumentAsync(AnalysisModuleKey key, PythonAnalyzerEnt var remaining = originalRemaining; try { _log?.Log(TraceEventType.Verbose, $"Analysis version {walker.Version} of {originalRemaining} entries has started."); - remaining = await AnalyzeAffectedEntriesAsync(walker, stopWatch, analysisToken); + remaining = AnalyzeAffectedEntriesAsync(walker, stopWatch, analysisToken).GetAwaiter().GetResult(); } finally { _analysisRunningEvent.Set(); stopWatch.Stop(); diff --git a/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs b/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs index 7e042c1a3..4a386d6ba 100644 --- a/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs +++ b/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs @@ -21,22 +21,21 @@ namespace Microsoft.Python.Analysis.Documents { internal sealed class DocumentBuffer { - private readonly StringBuilder _sb = new StringBuilder(); - private string _cachedText; + private StringBuilder _sb; + private string _content; public int Version { get; private set; } - public string Text => _cachedText ?? (_cachedText = _sb.ToString()); + public string Text => _content ?? (_content = _sb.ToString()); public void Reset(int version, string content) { Version = version; - _sb.Clear(); - if (!string.IsNullOrEmpty(content)) { - _sb.Append(content); - } - _cachedText = null; + _sb = null; + _content = content; } public void Update(IEnumerable changes) { + _sb = _sb ?? new StringBuilder(_content); + _content = null; var lastStart = int.MaxValue; var lineLoc = SplitLines(_sb).ToArray(); @@ -57,7 +56,6 @@ public void Update(IEnumerable changes) { } } Version++; - _cachedText = null; } private static IEnumerable SplitLines(StringBuilder text) { diff --git a/src/LanguageServer/Impl/Program.cs b/src/LanguageServer/Impl/Program.cs index 11348f518..2fefa03ba 100644 --- a/src/LanguageServer/Impl/Program.cs +++ b/src/LanguageServer/Impl/Program.cs @@ -13,7 +13,7 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -// #define WAIT_FOR_DEBUGGER +#define WAIT_FOR_DEBUGGER using System; using System.Diagnostics; From f544f8a19eeee582bd13c46e90f9b5f3985f88b8 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 27 Mar 2019 19:30:18 -0700 Subject: [PATCH 05/15] Limit concurrency --- src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs | 5 +++-- src/LanguageServer/Impl/Program.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 674dbe877..14d4c9a4d 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -193,13 +193,14 @@ private void AnalyzeDocument(AnalysisModuleKey key, PythonAnalyzerEntry entry, I } private void Worker() { - while(true) { + ThreadPool.GetMaxThreads(out var max, out _); + while (true) { _workAvailable.Wait(); if (_queue.TryDequeue(out var action)) { _workerAvailable.Wait(); lock (_lock) { _workerCount++; - if (_workerCount > 4) { + if (_workerCount >= Environment.ProcessorCount) { _workerAvailable.Reset(); } } diff --git a/src/LanguageServer/Impl/Program.cs b/src/LanguageServer/Impl/Program.cs index 2fefa03ba..11348f518 100644 --- a/src/LanguageServer/Impl/Program.cs +++ b/src/LanguageServer/Impl/Program.cs @@ -13,7 +13,7 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -#define WAIT_FOR_DEBUGGER +// #define WAIT_FOR_DEBUGGER using System; using System.Diagnostics; From 01bd7196838a7a614bf59cbec0709c8cf300bdb5 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 27 Mar 2019 19:56:57 -0700 Subject: [PATCH 06/15] Concurrency limit --- src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 14d4c9a4d..0671135ca 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -193,14 +193,14 @@ private void AnalyzeDocument(AnalysisModuleKey key, PythonAnalyzerEntry entry, I } private void Worker() { - ThreadPool.GetMaxThreads(out var max, out _); + var maxConcurrent = Math.Max(Environment.ProcessorCount / 2, 4); while (true) { _workAvailable.Wait(); if (_queue.TryDequeue(out var action)) { _workerAvailable.Wait(); lock (_lock) { _workerCount++; - if (_workerCount >= Environment.ProcessorCount) { + if (_workerCount >= maxConcurrent) { _workerAvailable.Reset(); } } @@ -377,7 +377,7 @@ private void Analyze(IDependencyChainNode node, IDependency node.Commit(); _log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) failed."); } finally { - if(_version == walker.Version) { + if (_version == walker.Version) { _progress.ReportRemaining(walker.Remaining); } Interlocked.Decrement(ref _runningTasks); From 2f8e564e3bb1529c0349f88e37483e5e25e3f189 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 28 Mar 2019 07:20:20 -0700 Subject: [PATCH 07/15] Drop cache after analysis --- src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs | 2 ++ src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs index 123afc63f..c1e7a455f 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs @@ -164,6 +164,8 @@ public IMember GetValueFromExpression(Expression expr, LookupOptions options) { return m; } + internal void ClearCache() => _scopeLookupCache.Clear(); + private IMember GetValueFromFormatSpecifier(FormatSpecifier formatSpecifier) { return new PythonFString(formatSpecifier.Unparsed, Interpreter, GetLoc(formatSpecifier)); } diff --git a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs index b8aa5144c..83b91e9eb 100644 --- a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs @@ -105,6 +105,8 @@ public void Complete() { .Where(s => !string.IsNullOrEmpty(s)) .ToImmutableArray(); } + + Eval.ClearCache(); } public IGlobalScope GlobalScope => Eval.GlobalScope; From 4ad56bbdf1cf3cbe78a8b0b888b5cf2a976cbe03 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 28 Mar 2019 16:47:16 -0700 Subject: [PATCH 08/15] Fix regression --- src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs index 536075e0d..34d7b85c3 100644 --- a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs @@ -212,6 +212,9 @@ private DocumentEntry CreateDocument(ModuleCreationOptions mco) { document = new CompiledBuiltinPythonModule(mco.ModuleName, mco.Stub, _services); break; case ModuleType.User: + TryAddModulePath(mco); + document = new PythonModule(mco, _services); + break; case ModuleType.Library when TryAddModulePath(mco): document = new PythonModule(mco, _services); break; From 2a50eb9c158342810cca7a20755d3bc297130106 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 10 Apr 2019 11:00:35 -0700 Subject: [PATCH 09/15] Fix test --- src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs index cda525e89..e04c4192f 100644 --- a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs @@ -120,7 +120,7 @@ private void HandleAllAppendExtend(CallExpression node) { return; } - ExtendAll(node, values); + ExtendAll(me.Target, values); } private void ExtendAll(Node location, IPythonCollection values) { From b341d5aa5ef903e1171d49c1d58bab79ba074fad Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sun, 5 May 2019 13:56:22 -0700 Subject: [PATCH 10/15] Optimize variable collection --- .../Ast/Impl/Values/VariableCollection.cs | 82 ++++++++++++++----- 1 file changed, 63 insertions(+), 19 deletions(-) diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index e6645c09a..60fabf8df 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -15,7 +15,6 @@ using System; using System.Collections; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -25,48 +24,93 @@ 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 Lazy> _variables + = new Lazy>(() => new Dictionary()); + private readonly object _lock = new object(); #region ICollection - public int Count => _variables.Count; - public IEnumerator GetEnumerator() => _variables.Values.ToList().GetEnumerator(); + public int Count { + get { + lock (_lock) { + return _variables.IsValueCreated ? _variables.Value.Count : 0; + } + } + } + + public IEnumerator GetEnumerator() { + lock (_lock) { + return _variables.IsValueCreated + ? _variables.Value.Values.ToList().GetEnumerator() + : Enumerable.Empty().GetEnumerator(); + } + } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); #endregion #region IVariableCollection - public IVariable this[string name] => _variables.TryGetValue(name, out var v) ? v : null; - public bool Contains(string name) => _variables.ContainsKey(name); - public IReadOnlyList Names => _variables.Keys.ToArray(); + + public IVariable this[string name] { + get { + lock (_lock) { + return _variables.IsValueCreated + ? _variables.Value.TryGetValue(name, out var v) ? v : null + : null; + } + } + } + + public bool Contains(string name) { + lock (_lock) { + return _variables.IsValueCreated && _variables.Value.ContainsKey(name); + } + } + + public IReadOnlyList Names { + get { + lock (_lock) { + return _variables.IsValueCreated ? _variables.Value.Keys.ToArray() : Array.Empty(); + } + } + } public bool TryGetVariable(string key, out IVariable value) { value = null; - if (_variables.TryGetValue(key, out var v)) { - value = v; - return true; + lock (_lock) { + if (_variables.IsValueCreated && _variables.Value.TryGetValue(key, out var v)) { + value = v; + } } - return false; + return value != null; } #endregion #region IMemberContainer - public IMember GetMember(string name) => _variables.TryGetValue(name, out var v) ? v : null; - public IEnumerable GetMemberNames() => _variables.Keys.ToArray(); + public IMember GetMember(string name) => TryGetVariable(name, out var v) ? v : null; + public IEnumerable GetMemberNames() => Names; #endregion internal void DeclareVariable(string name, IMember value, VariableSource source, Location location = default) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); - if (_variables.TryGetValue(name, out var existing)) { - existing.Assign(value, location); - } else { - _variables[name] = new Variable(name, value, source, location); + lock (_lock) { + if (_variables.Value.TryGetValue(name, out var existing)) { + existing.Assign(value, location); + } else { + _variables.Value[name] = new Variable(name, value, source, location); + } } } internal void LinkVariable(string name, IVariable v, Location location) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); - _variables[name] = new Variable(name, v, location); + lock (_lock) { + _variables.Value[name] = new Variable(name, v, location); + } } - internal void RemoveVariable(string name) => _variables.TryRemove(name, out _); + internal void RemoveVariable(string name) { + lock (_lock) { + _variables.Value.Remove(name); + } + } } } From 1ca3b08a83bee4197b0fdf325a75aea55aa053a0 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sun, 5 May 2019 15:40:07 -0700 Subject: [PATCH 11/15] Clean --- src/Analysis/Ast/Impl/Values/GlobalScope.cs | 14 +++---- src/Analysis/Ast/Impl/Values/Scope.cs | 7 ++-- .../Ast/Impl/Values/VariableCollection.cs | 37 +++++++++++-------- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/Analysis/Ast/Impl/Values/GlobalScope.cs b/src/Analysis/Ast/Impl/Values/GlobalScope.cs index df375b923..42e42e41d 100644 --- a/src/Analysis/Ast/Impl/Values/GlobalScope.cs +++ b/src/Analysis/Ast/Impl/Values/GlobalScope.cs @@ -35,13 +35,13 @@ private void DeclareBuiltinVariables() { var listType = Module.Interpreter.GetBuiltinType(BuiltinTypeId.List); var dictType = Module.Interpreter.GetBuiltinType(BuiltinTypeId.Dict); - VariableCollection.DeclareVariable("__debug__", boolType, VariableSource.Builtin); - VariableCollection.DeclareVariable("__doc__", strType, VariableSource.Builtin); - VariableCollection.DeclareVariable("__file__", strType, VariableSource.Builtin); - VariableCollection.DeclareVariable("__name__", strType, VariableSource.Builtin); - VariableCollection.DeclareVariable("__package__", strType, VariableSource.Builtin); - VariableCollection.DeclareVariable("__path__", listType, VariableSource.Builtin); - VariableCollection.DeclareVariable("__dict__", dictType, VariableSource.Builtin); + DeclareVariable("__debug__", boolType, VariableSource.Builtin); + DeclareVariable("__doc__", strType, VariableSource.Builtin); + DeclareVariable("__file__", strType, VariableSource.Builtin); + DeclareVariable("__name__", strType, VariableSource.Builtin); + DeclareVariable("__package__", strType, VariableSource.Builtin); + DeclareVariable("__path__", listType, VariableSource.Builtin); + DeclareVariable("__dict__", dictType, VariableSource.Builtin); } } } diff --git a/src/Analysis/Ast/Impl/Values/Scope.cs b/src/Analysis/Ast/Impl/Values/Scope.cs index fc3933ede..c62e93d58 100644 --- a/src/Analysis/Ast/Impl/Values/Scope.cs +++ b/src/Analysis/Ast/Impl/Values/Scope.cs @@ -26,12 +26,11 @@ namespace Microsoft.Python.Analysis.Values { /// Represents scope where variables can be declared. /// internal class Scope : IScope { + private VariableCollection _variables; private VariableCollection _nonLocals; private VariableCollection _globals; private List _childScopes; - protected VariableCollection VariableCollection { get; } = new VariableCollection(); - public Scope(ScopeStatement node, IScope outerScope, IPythonModule module) { Node = node; OuterScope = outerScope; @@ -47,7 +46,7 @@ public Scope(ScopeStatement node, IScope outerScope, IPythonModule module) { public IPythonModule Module { get; } public IReadOnlyList Children => _childScopes?.ToArray() ?? Array.Empty(); - public IVariableCollection Variables => VariableCollection; + public IVariableCollection Variables => _variables ?? VariableCollection.Empty; public IVariableCollection NonLocals => _nonLocals ?? VariableCollection.Empty; public IVariableCollection Globals => _globals ?? VariableCollection.Empty; @@ -89,6 +88,8 @@ public void DeclareGlobal(string name, Location location) internal void AddChildScope(Scope s) => (_childScopes ?? (_childScopes = new List())).Add(s); + private VariableCollection VariableCollection => _variables ?? (_variables = new VariableCollection()); + private void DeclareBuiltinVariables() { if (Node == null || Module.ModuleType != ModuleType.User || this is IGlobalScope) { return; diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index 60fabf8df..fc8159edf 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -23,24 +23,31 @@ namespace Microsoft.Python.Analysis.Values { [DebuggerDisplay("Count: {Count}")] internal sealed class VariableCollection : IVariableCollection { - public static readonly IVariableCollection Empty = new VariableCollection(); - private readonly Lazy> _variables - = new Lazy>(() => new Dictionary()); + public static readonly VariableCollection Empty = new VariableCollection(); + private Dictionary _variables; private readonly object _lock = new object(); + private Dictionary Variables { + get { + lock (_lock) { + return _variables ?? (_variables = new Dictionary()); + } + } + } + #region ICollection public int Count { get { lock (_lock) { - return _variables.IsValueCreated ? _variables.Value.Count : 0; + return _variables?.Count ?? 0; } } } public IEnumerator GetEnumerator() { lock (_lock) { - return _variables.IsValueCreated - ? _variables.Value.Values.ToList().GetEnumerator() + return _variables != null + ? _variables.Values.ToList().GetEnumerator() : Enumerable.Empty().GetEnumerator(); } } @@ -52,8 +59,8 @@ public IEnumerator GetEnumerator() { public IVariable this[string name] { get { lock (_lock) { - return _variables.IsValueCreated - ? _variables.Value.TryGetValue(name, out var v) ? v : null + return _variables != null + ? _variables.TryGetValue(name, out var v) ? v : null : null; } } @@ -61,14 +68,14 @@ public IVariable this[string name] { public bool Contains(string name) { lock (_lock) { - return _variables.IsValueCreated && _variables.Value.ContainsKey(name); + return _variables != null && _variables.ContainsKey(name); } } public IReadOnlyList Names { get { lock (_lock) { - return _variables.IsValueCreated ? _variables.Value.Keys.ToArray() : Array.Empty(); + return _variables != null ? _variables.Keys.ToArray() : Array.Empty(); } } } @@ -76,7 +83,7 @@ public IReadOnlyList Names { public bool TryGetVariable(string key, out IVariable value) { value = null; lock (_lock) { - if (_variables.IsValueCreated && _variables.Value.TryGetValue(key, out var v)) { + if (_variables != null && _variables.TryGetValue(key, out var v)) { value = v; } } @@ -92,10 +99,10 @@ public bool TryGetVariable(string key, out IVariable value) { internal void DeclareVariable(string name, IMember value, VariableSource source, Location location = default) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); lock (_lock) { - if (_variables.Value.TryGetValue(name, out var existing)) { + if (Variables.TryGetValue(name, out var existing)) { existing.Assign(value, location); } else { - _variables.Value[name] = new Variable(name, value, source, location); + Variables[name] = new Variable(name, value, source, location); } } } @@ -103,13 +110,13 @@ internal void DeclareVariable(string name, IMember value, VariableSource source, internal void LinkVariable(string name, IVariable v, Location location) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); lock (_lock) { - _variables.Value[name] = new Variable(name, v, location); + Variables[name] = new Variable(name, v, location); } } internal void RemoveVariable(string name) { lock (_lock) { - _variables.Value.Remove(name); + _variables?.Remove(name); } } } From 81dbd3c4e73fe3045999556332b43b9cc8653398 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sun, 5 May 2019 16:39:59 -0700 Subject: [PATCH 12/15] Special type of imported variable --- src/Analysis/Ast/Impl/Modules/PythonModule.cs | 3 +- .../Ast/Impl/Modules/PythonVariableModule.cs | 6 +- .../Specializations/Typing/Types/AnyType.cs | 4 +- .../Typing/Types/GenericType.cs | 5 +- .../Typing/Values/TypingType.cs | 4 +- .../Impl/Types/Definitions/IImportedMember.cs | 26 +++++ .../Impl/Types/Definitions/ILocatedMember.cs | 5 - src/Analysis/Ast/Impl/Types/LocatedMember.cs | 20 +--- .../Ast/Impl/Types/PythonFunctionOverload.cs | 8 +- src/Analysis/Ast/Impl/Types/PythonType.cs | 7 +- .../Ast/Impl/Types/PythonTypeWrapper.cs | 1 - .../Ast/Impl/Types/PythonUnionType.cs | 8 +- .../Ast/Impl/Values/ImportedVariable.cs | 102 ++++++++++++++++++ src/Analysis/Ast/Impl/Values/Variable.cs | 37 +------ .../Ast/Impl/Values/VariableCollection.cs | 2 +- src/Analysis/Ast/Test/ReferencesTests.cs | 3 +- .../Impl/Sources/DefinitionSource.cs | 4 +- .../Impl/Sources/ReferenceSource.cs | 18 +++- 18 files changed, 180 insertions(+), 83 deletions(-) create mode 100644 src/Analysis/Ast/Impl/Types/Definitions/IImportedMember.cs create mode 100644 src/Analysis/Ast/Impl/Values/ImportedVariable.cs diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index 4e4744c94..44ebadba4 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -71,8 +71,7 @@ private enum State { private object AnalysisLock { get; } = new object(); private State ContentState { get; set; } = State.None; - protected PythonModule(string name, ModuleType moduleType, IServiceContainer services) - : base(PythonMemberType.Module) { + protected PythonModule(string name, ModuleType moduleType, IServiceContainer services) : base(null) { Name = name ?? throw new ArgumentNullException(nameof(name)); Services = services ?? throw new ArgumentNullException(nameof(services)); ModuleType = moduleType; diff --git a/src/Analysis/Ast/Impl/Modules/PythonVariableModule.cs b/src/Analysis/Ast/Impl/Modules/PythonVariableModule.cs index d28233904..471c25e45 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonVariableModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonVariableModule.cs @@ -47,15 +47,15 @@ internal sealed class PythonVariableModule : LocatedMember, IPythonModule, IEqua public IGlobalScope GlobalScope => Module?.GlobalScope; public BuiltinTypeId TypeId => BuiltinTypeId.Module; public Uri Uri => Module?.Uri; + public override PythonMemberType MemberType => PythonMemberType.Module; - public PythonVariableModule(string name, IPythonInterpreter interpreter) - : base(PythonMemberType.Module) { + public PythonVariableModule(string name, IPythonInterpreter interpreter): base(null) { Name = name; Interpreter = interpreter; SetDeclaringModule(this); } - public PythonVariableModule(IPythonModule module): base(PythonMemberType.Module, module) { + public PythonVariableModule(IPythonModule module): base(module) { Name = module.Name; Interpreter = module.Interpreter; Module = module; diff --git a/src/Analysis/Ast/Impl/Specializations/Typing/Types/AnyType.cs b/src/Analysis/Ast/Impl/Specializations/Typing/Types/AnyType.cs index 091fa6659..d1e5f9e0e 100644 --- a/src/Analysis/Ast/Impl/Specializations/Typing/Types/AnyType.cs +++ b/src/Analysis/Ast/Impl/Specializations/Typing/Types/AnyType.cs @@ -20,9 +20,9 @@ namespace Microsoft.Python.Analysis.Specializations.Typing.Types { internal sealed class AnyType : LocatedMember, IPythonType { - public AnyType(IPythonModule declaringModule) - : base(PythonMemberType.Class, declaringModule) { } + public AnyType(IPythonModule declaringModule) : base(declaringModule) { } + public override PythonMemberType MemberType => PythonMemberType.Class; public string Name => "Any"; public BuiltinTypeId TypeId => BuiltinTypeId.Type; public string Documentation => Name; diff --git a/src/Analysis/Ast/Impl/Specializations/Typing/Types/GenericType.cs b/src/Analysis/Ast/Impl/Specializations/Typing/Types/GenericType.cs index f211139f8..df9d14a97 100644 --- a/src/Analysis/Ast/Impl/Specializations/Typing/Types/GenericType.cs +++ b/src/Analysis/Ast/Impl/Specializations/Typing/Types/GenericType.cs @@ -59,11 +59,12 @@ public GenericType( Parameters = parameters ?? Array.Empty(); } - private GenericType(string name, IPythonModule declaringModule) - : base(PythonMemberType.Generic, declaringModule) { + private GenericType(string name, IPythonModule declaringModule) : base(declaringModule) { Name = name ?? throw new ArgumentNullException(nameof(name)); } + public override PythonMemberType MemberType => PythonMemberType.Generic; + /// /// Type parameters such as in Tuple[T1, T2. ...] or /// Generic[_T1, _T2, ...] as returned by TypeVar. diff --git a/src/Analysis/Ast/Impl/Specializations/Typing/Values/TypingType.cs b/src/Analysis/Ast/Impl/Specializations/Typing/Values/TypingType.cs index 572a17ca5..387dcdbd2 100644 --- a/src/Analysis/Ast/Impl/Specializations/Typing/Values/TypingType.cs +++ b/src/Analysis/Ast/Impl/Specializations/Typing/Values/TypingType.cs @@ -25,12 +25,12 @@ namespace Microsoft.Python.Analysis.Specializations.Typing.Values { internal sealed class TypingType : LocatedMember, IPythonType { private readonly IPythonType _type; - public TypingType(IPythonModule declaringModule, IPythonType type) - : base(PythonMemberType.Class, declaringModule) { + public TypingType(IPythonModule declaringModule, IPythonType type): base(declaringModule) { _type = type ?? throw new ArgumentNullException(nameof(type)); Name = $"Type[{_type.Name}]"; } + public override PythonMemberType MemberType => PythonMemberType.Class; public string Name { get; } public BuiltinTypeId TypeId => BuiltinTypeId.Type; public string Documentation => Name; diff --git a/src/Analysis/Ast/Impl/Types/Definitions/IImportedMember.cs b/src/Analysis/Ast/Impl/Types/Definitions/IImportedMember.cs new file mode 100644 index 000000000..962859f78 --- /dev/null +++ b/src/Analysis/Ast/Impl/Types/Definitions/IImportedMember.cs @@ -0,0 +1,26 @@ +// 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.Types { + /// + /// Imported member that has link to its parent in another module. + /// + public interface IImportedMember : ILocatedMember { + /// + /// Link to the primary definition such as when variable is imported from another file. + /// + ILocatedMember Parent { get; } + } +} diff --git a/src/Analysis/Ast/Impl/Types/Definitions/ILocatedMember.cs b/src/Analysis/Ast/Impl/Types/Definitions/ILocatedMember.cs index e585c4092..c1cceb88c 100644 --- a/src/Analysis/Ast/Impl/Types/Definitions/ILocatedMember.cs +++ b/src/Analysis/Ast/Impl/Types/Definitions/ILocatedMember.cs @@ -37,11 +37,6 @@ public interface ILocatedMember: IMember { /// LocationInfo Definition { get; } - /// - /// Link to the primary definition such as when variable is imported from another file. - /// - ILocatedMember Parent { get; } - /// /// List of references to the member. /// diff --git a/src/Analysis/Ast/Impl/Types/LocatedMember.cs b/src/Analysis/Ast/Impl/Types/LocatedMember.cs index 1d319be96..d752e3f4f 100644 --- a/src/Analysis/Ast/Impl/Types/LocatedMember.cs +++ b/src/Analysis/Ast/Impl/Types/LocatedMember.cs @@ -23,31 +23,18 @@ namespace Microsoft.Python.Analysis.Types { internal abstract class LocatedMember : ILocatedMember { private HashSet _references; - protected LocatedMember(PythonMemberType memberType) { - MemberType = memberType; - } - - protected LocatedMember(PythonMemberType memberType, IPythonModule module) - : this(memberType, new Location(module, default)) { } - - protected LocatedMember(PythonMemberType memberType, Location location, ILocatedMember parent = null) - : this(location, parent) { - MemberType = memberType; - } + protected LocatedMember(IPythonModule module) : this(new Location(module, default)) { } - private LocatedMember(Location location, ILocatedMember parent = null) { - Parent = parent; - Parent?.AddReference(location); + protected LocatedMember(Location location) { Location = location; } - public virtual PythonMemberType MemberType { get; } = PythonMemberType.Unknown; + public abstract PythonMemberType MemberType { get; } public virtual IPythonModule DeclaringModule => Location.Module; public virtual LocationInfo Definition => Location.LocationInfo; - public ILocatedMember Parent { get; } public virtual IReadOnlyList References { get { @@ -98,7 +85,6 @@ protected EmptyLocatedMember(PythonMemberType memberType) { public PythonMemberType MemberType { get; } public IPythonModule DeclaringModule => null; public LocationInfo Definition => LocationInfo.Empty; - public ILocatedMember Parent => null; public IReadOnlyList References => Array.Empty(); public void AddReference(Location location) { } public void RemoveReferences(IPythonModule module) { } diff --git a/src/Analysis/Ast/Impl/Types/PythonFunctionOverload.cs b/src/Analysis/Ast/Impl/Types/PythonFunctionOverload.cs index 94773c43b..b2d84b8f8 100644 --- a/src/Analysis/Ast/Impl/Types/PythonFunctionOverload.cs +++ b/src/Analysis/Ast/Impl/Types/PythonFunctionOverload.cs @@ -58,11 +58,14 @@ public PythonFunctionOverload(FunctionDefinition fd, IPythonClassMember classMem _returnDocumentation = ast != null ? fd.ReturnAnnotation?.ToCodeString(ast) : null; } - public PythonFunctionOverload(string name, Location location) - : base(PythonMemberType.Function, location) { + public PythonFunctionOverload(string name, Location location) : base(location) { Name = name ?? throw new ArgumentNullException(nameof(name)); } + #region ILocatedMember + public override PythonMemberType MemberType => PythonMemberType.Function; + #endregion + internal void SetParameters(IReadOnlyList parameters) => Parameters = parameters; internal void SetDocumentationProvider(Func documentationProvider) @@ -147,7 +150,6 @@ public string GetReturnDocumentation(IPythonType self = null) { } public IReadOnlyList Parameters { get; private set; } = Array.Empty(); - public override PythonMemberType MemberType => PythonMemberType.Function; public IMember StaticReturnValue { get; private set; } public IMember Call(IArgumentSet args, IPythonType self, Node callLocation = null) { diff --git a/src/Analysis/Ast/Impl/Types/PythonType.cs b/src/Analysis/Ast/Impl/Types/PythonType.cs index 252165952..41e9bda1c 100644 --- a/src/Analysis/Ast/Impl/Types/PythonType.cs +++ b/src/Analysis/Ast/Impl/Types/PythonType.cs @@ -54,13 +54,16 @@ public PythonType( _documentationProvider = documentationProvider; } - private PythonType(string name, Location location, BuiltinTypeId typeId) - : base(typeId.GetMemberId(), location) { + private PythonType(string name, Location location, BuiltinTypeId typeId) : base(location) { Check.ArgumentNotNull(nameof(location), location.Module); _name = name ?? throw new ArgumentNullException(nameof(name)); _typeId = typeId; } + #region ILocatedMember + public override PythonMemberType MemberType => _typeId.GetMemberId(); + #endregion + #region IPythonType public virtual string Name => TypeId == BuiltinTypeId.Ellipsis ? "..." : _name; diff --git a/src/Analysis/Ast/Impl/Types/PythonTypeWrapper.cs b/src/Analysis/Ast/Impl/Types/PythonTypeWrapper.cs index 9ba61a893..5201297b1 100644 --- a/src/Analysis/Ast/Impl/Types/PythonTypeWrapper.cs +++ b/src/Analysis/Ast/Impl/Types/PythonTypeWrapper.cs @@ -76,7 +76,6 @@ public virtual IMember Index(IPythonInstance instance, object index) #region ILocatedMember public Location Location => InnerType?.Location ?? default; public LocationInfo Definition => InnerType?.Definition ?? LocationInfo.Empty; - public ILocatedMember Parent => InnerType?.Parent; public IReadOnlyList References => InnerType?.References ?? Array.Empty(); public void AddReference(Location location) => InnerType?.AddReference(location); public void RemoveReferences(IPythonModule module) => InnerType?.RemoveReferences(module); diff --git a/src/Analysis/Ast/Impl/Types/PythonUnionType.cs b/src/Analysis/Ast/Impl/Types/PythonUnionType.cs index 2e7c69865..c5f83c75f 100644 --- a/src/Analysis/Ast/Impl/Types/PythonUnionType.cs +++ b/src/Analysis/Ast/Impl/Types/PythonUnionType.cs @@ -26,19 +26,19 @@ internal sealed class PythonUnionType : LocatedMember, IPythonUnionType { private readonly HashSet _types = new HashSet(PythonTypeComparer.Instance); private readonly object _lock = new object(); - public PythonUnionType(IEnumerable types, IPythonModule declaringModule) - : base(PythonMemberType.Union, declaringModule) { + public PythonUnionType(IEnumerable types, IPythonModule declaringModule) : base(declaringModule) { _types.UnionWith(types); } - private PythonUnionType(IPythonType x, IPythonType y) - : base(PythonMemberType.Union, x.DeclaringModule) { + private PythonUnionType(IPythonType x, IPythonType y) : base(x.DeclaringModule) { Check.Argument(nameof(x), () => !(x is IPythonUnionType)); Check.Argument(nameof(y), () => !(y is IPythonUnionType)); _types.Add(x); _types.Add(y); } + public override PythonMemberType MemberType => PythonMemberType.Union; + #region IPythonType public string Name { diff --git a/src/Analysis/Ast/Impl/Values/ImportedVariable.cs b/src/Analysis/Ast/Impl/Values/ImportedVariable.cs new file mode 100644 index 000000000..46a22fefe --- /dev/null +++ b/src/Analysis/Ast/Impl/Values/ImportedVariable.cs @@ -0,0 +1,102 @@ +// 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.Diagnostics; +using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Core; + +namespace Microsoft.Python.Analysis.Values { + [DebuggerDisplay("{DebuggerDisplay}")] + internal sealed class ImportedVariable : Variable, IImportedMember { + public ImportedVariable(string name, IVariable parent, Location location) + : base(name, parent.Value, VariableSource.Import, location) { + Parent = parent; + Parent?.AddReference(location); + } + + #region IImportedMember + public ILocatedMember Parent { get; } + #endregion + + #region ILocatedMember + public override LocationInfo Definition { + get { + if (!Location.IsValid && Parent != null) { + return Parent?.Definition ?? LocationInfo.Empty; + } + return base.Definition; + } + } + + public override IReadOnlyList References { + get { + if (!Location.IsValid && Parent != null) { + return Parent?.References ?? Array.Empty(); + } + return base.References; + } + } + + protected override bool AddOrRemoveReference(Action action) { + // Variable can be + // a) Declared locally in the module. In this case it has non-default + // definition location and no parent (link). + // b) Imported from another module via 'from module import X'. + // In this case it has non-default definition location and non-null parent (link). + // c) Imported from another module via 'from module import *'. + // In this case it has default location (which means it is not explicitly declared) + // and the non-null parent (link). + var explicitlyDeclared = Location.IsValid; + if (!explicitlyDeclared && Parent != null) { + action(Parent); + return true; + } + // Explicitly declared. + // Values: + // a) If value is not a located member, then add reference to the variable. + // b) If variable name is the same as the value member name, then the variable + // is implicit declaration (like declared function or a class) and we need + // to add reference to the actual type instead. + var lmv = GetImplicitlyDeclaredValue(); + if (lmv != null) { + // Variable is not user-declared and rather is holder of a function or class definition. + action(lmv); + return true; + } + action(Parent); + return false; + } + + #endregion + + private ILocatedMember GetImplicitlyDeclaredValue() + => Value is ILocatedMember lm && Name.EqualsOrdinal(lm.GetPythonType()?.Name) && !Location.IsValid ? lm : null; + + private string DebuggerDisplay { + get { + switch (Value) { + case IPythonInstance pi: + return $"{Name} : instance of {pi.Type.Name}"; + case IPythonType pt: + return $"{Name} : typeInfo of {pt.Name}"; + default: + return $"{Name} : member {Value?.MemberType}"; + } + } + } + } +} diff --git a/src/Analysis/Ast/Impl/Values/Variable.cs b/src/Analysis/Ast/Impl/Values/Variable.cs index cc01e5a9d..41e3d9d57 100644 --- a/src/Analysis/Ast/Impl/Values/Variable.cs +++ b/src/Analysis/Ast/Impl/Values/Variable.cs @@ -21,21 +21,13 @@ namespace Microsoft.Python.Analysis.Values { [DebuggerDisplay("{DebuggerDisplay}")] - internal sealed class Variable : LocatedMember, IVariable { - public Variable(string name, IMember value, VariableSource source, Location location) - : base(PythonMemberType.Variable, location) { + internal class Variable : LocatedMember, IVariable { + public Variable(string name, IMember value, VariableSource source, Location location) : base(location) { Name = name ?? throw new ArgumentNullException(nameof(name)); Value = value is IVariable v ? v.Value : value; Source = source; } - public Variable(string name, IVariable parent, Location location) - : base(PythonMemberType.Variable, location, parent) { - Name = name ?? throw new ArgumentNullException(nameof(name)); - Value = parent.Value; - Source = VariableSource.Import; - } - #region IVariable public string Name { get; } public VariableSource Source { get; } @@ -54,12 +46,11 @@ public void Assign(IMember value, Location location) { #endregion #region ILocatedMember + public override PythonMemberType MemberType => PythonMemberType.Variable; + public override LocationInfo Definition { get { if (!Location.IsValid) { - if (Parent != null) { - return Parent?.Definition ?? LocationInfo.Empty; - } var lmv = GetImplicitlyDeclaredValue(); if (lmv != null) { return lmv.Definition; @@ -72,9 +63,6 @@ public override LocationInfo Definition { public override IReadOnlyList References { get { if (!Location.IsValid) { - if (Parent != null) { - return Parent?.References ?? Array.Empty(); - } var lmv = GetImplicitlyDeclaredValue(); if (lmv != null) { return lmv.References; @@ -101,21 +89,7 @@ public override void RemoveReferences(IPythonModule module) { } } - private bool AddOrRemoveReference(Action action) { - // Variable can be - // a) Declared locally in the module. In this case it has non-default - // definition location and no parent (link). - // b) Imported from another module via 'from module import X'. - // In this case it has non-default definition location and non-null parent (link). - // c) Imported from another module via 'from module import *'. - // In this case it has default location (which means it is not explicitly declared) - // and the non-null parent (link). - var explicitlyDeclared = Location.IsValid; - if (!explicitlyDeclared && Parent != null) { - action(Parent); - return true; - } - // Explicitly declared. + protected virtual bool AddOrRemoveReference(Action action) { // Values: // a) If value is not a located member, then add reference to the variable. // b) If variable name is the same as the value member name, then the variable @@ -127,7 +101,6 @@ private bool AddOrRemoveReference(Action action) { action(lmv); return true; } - action(Parent); return false; } diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index fc8159edf..78e502762 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -110,7 +110,7 @@ internal void DeclareVariable(string name, IMember value, VariableSource source, internal void LinkVariable(string name, IVariable v, Location location) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); lock (_lock) { - Variables[name] = new Variable(name, v, location); + Variables[name] = new ImportedVariable(name, v, location); } } diff --git a/src/Analysis/Ast/Test/ReferencesTests.cs b/src/Analysis/Ast/Test/ReferencesTests.cs index 936f9dc7b..3301048ec 100644 --- a/src/Analysis/Ast/Test/ReferencesTests.cs +++ b/src/Analysis/Ast/Test/ReferencesTests.cs @@ -318,7 +318,8 @@ from MultiValues import t x = t "; var analysis = await GetAnalysisAsync(code); - var t = analysis.Should().HaveVariable("t").Which; + var t = analysis.Should().HaveVariable("t").Which as IImportedMember; + t.Should().NotBeNull(); t.Definition.Span.Should().Be(2, 25, 2, 26); t.Definition.DocumentUri.AbsolutePath.Should().Contain("module.py"); t.References.Should().HaveCount(2); diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index a44248797..06c0c9ac8 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -95,10 +95,10 @@ private Reference TryFromVariable(string name, IDocumentAnalysis analysis, Sourc var indexSpan = v.Definition.Span.ToIndexSpan(analysis.Ast); var index = location.ToIndex(analysis.Ast); if (indexSpan.Start <= index && index < indexSpan.End) { - if (v.Parent == null) { + if (!(v is IImportedMember im)) { return null; } - definition = v.Parent.Definition; + definition = im.Parent.Definition; } } diff --git a/src/LanguageServer/Impl/Sources/ReferenceSource.cs b/src/LanguageServer/Impl/Sources/ReferenceSource.cs index 95510305a..2056d27e5 100644 --- a/src/LanguageServer/Impl/Sources/ReferenceSource.cs +++ b/src/LanguageServer/Impl/Sources/ReferenceSource.cs @@ -120,16 +120,26 @@ private static async Task AnalyzeFiles(IModuleManagement moduleManagement, IEnum var module = moduleManagement.GetOrLoadModule(fullName); if (module is IDocument document) { analysisTasks.Add(document.GetAnalysisAsync(cancellationToken: cancellationToken)); - } + } } } await Task.WhenAll(analysisTasks); } - + private ILocatedMember GetRootDefinition(ILocatedMember lm) { - for (; lm.Parent != null; lm = lm.Parent) { } - return lm; + if (!(lm is IImportedMember im) || im.Parent == null) { + return lm; + } + + var parent = im.Parent; + for (; parent != null;) { + if (!(parent is IImportedMember im1) || im1.Parent == null) { + break; + } + parent = im1.Parent; + } + return parent; } } } From b9da9a5de2545466203fe4bb0cd2028aba1d3f1a Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 6 May 2019 12:44:03 -0700 Subject: [PATCH 13/15] Merge issues --- src/Analysis/Ast/Impl/Values/VariableCollection.cs | 2 +- src/LanguageServer/Impl/Sources/DefinitionSource.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index 308b3493d..c4e4b007e 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -111,7 +111,7 @@ internal void DeclareVariable(string name, IMember value, VariableSource source, internal void LinkVariable(string name, IVariable v, Location location) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); - var variable = new Variable(name, v, location); + var variable = new Variable(name, v, VariableSource.Import, location); lock (_syncObj) { _variables[name] = variable; } diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index d2223b8d4..5be1b774d 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -165,7 +165,7 @@ private Reference TryFromVariable(string name, IDocumentAnalysis analysis, Sourc var m = analysis.ExpressionEvaluator.LookupNameInScopes(name, out var scope); if (m != null && scope.Variables[name] is IVariable v) { - member = v; + definingMember = v; var definition = v.Definition; if (statement is ImportStatement || statement is FromImportStatement) { // If we are on the variable definition in this module, From a71d0e83fc5e2666df0dde7f60bab57bb8370241 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 6 May 2019 13:45:53 -0700 Subject: [PATCH 14/15] Merge fixes --- .../Ast/Impl/Values/VariableCollection.cs | 2 +- .../Impl/Sources/DefinitionSource.cs | 38 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Analysis/Ast/Impl/Values/VariableCollection.cs b/src/Analysis/Ast/Impl/Values/VariableCollection.cs index c4e4b007e..893358d73 100644 --- a/src/Analysis/Ast/Impl/Values/VariableCollection.cs +++ b/src/Analysis/Ast/Impl/Values/VariableCollection.cs @@ -111,7 +111,7 @@ internal void DeclareVariable(string name, IMember value, VariableSource source, internal void LinkVariable(string name, IVariable v, Location location) { name = !string.IsNullOrWhiteSpace(name) ? name : throw new ArgumentException(nameof(name)); - var variable = new Variable(name, v, VariableSource.Import, location); + var variable = new ImportedVariable(name, v, location); lock (_syncObj) { _variables[name] = variable; } diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index 5be1b774d..054505ed0 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -82,7 +82,7 @@ public Reference FindDefinition(IDocumentAnalysis analysis, SourceLocation locat private Reference HandleFromImport(IDocumentAnalysis analysis, SourceLocation location, FromImportStatement statement, Node expr, out ILocatedMember definingMember) { definingMember = null; - + // Are in the dotted name? var locationIndex = location.ToIndex(analysis.Ast); if (statement.Root.StartIndex <= locationIndex && locationIndex <= statement.Root.EndIndex) { @@ -164,27 +164,27 @@ private Reference TryFromVariable(string name, IDocumentAnalysis analysis, Sourc definingMember = null; var m = analysis.ExpressionEvaluator.LookupNameInScopes(name, out var scope); - if (m != null && scope.Variables[name] is IVariable v) { - definingMember = v; - var definition = v.Definition; - if (statement is ImportStatement || statement is FromImportStatement) { - // If we are on the variable definition in this module, - // then goto definition should go to the parent, if any. - var indexSpan = v.Definition.Span.ToIndexSpan(analysis.Ast); - var index = location.ToIndex(analysis.Ast); - if (indexSpan.Start <= index && index < indexSpan.End) { - if (!(v is IImportedMember im)) { - return null; - } - definition = im.Parent.Definition; - } - } + if (m == null || !(scope.Variables[name] is IVariable v)) { + return null; + } - if (CanNavigateToModule(definition.DocumentUri)) { - return new Reference { range = definition.Span, uri = definition.DocumentUri }; + definingMember = v; + if (statement is ImportStatement || statement is FromImportStatement) { + // If we are on the variable definition in this module, + // then goto definition should go to the parent, if any. + var indexSpan = v.Definition.Span.ToIndexSpan(analysis.Ast); + var index = location.ToIndex(analysis.Ast); + if (indexSpan.Start <= index && index < indexSpan.End) { + if (!(v is IImportedMember im)) { + return null; + } + var definition = im.Parent != null ? im.Parent.Definition : (v.Value as ILocatedMember)?.Definition; + if (definition != null && CanNavigateToModule(definition.DocumentUri)) { + return new Reference { range = definition.Span, uri = definition.DocumentUri }; + } } } - return null; + return FromMember(v); } private Reference FromMemberExpression(MemberExpression mex, IDocumentAnalysis analysis, out ILocatedMember definingMember) { From 561f0ebbc5eaadc447aa0f0bd2c09578fae5901a Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 6 May 2019 14:48:34 -0700 Subject: [PATCH 15/15] Fix test --- src/LanguageServer/Impl/Sources/DefinitionSource.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index 054505ed0..27e82157a 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -175,10 +175,8 @@ private Reference TryFromVariable(string name, IDocumentAnalysis analysis, Sourc var indexSpan = v.Definition.Span.ToIndexSpan(analysis.Ast); var index = location.ToIndex(analysis.Ast); if (indexSpan.Start <= index && index < indexSpan.End) { - if (!(v is IImportedMember im)) { - return null; - } - var definition = im.Parent != null ? im.Parent.Definition : (v.Value as ILocatedMember)?.Definition; + var parent = (v as IImportedMember)?.Parent; + var definition = parent?.Definition ?? (v.Value as ILocatedMember)?.Definition; if (definition != null && CanNavigateToModule(definition.DocumentUri)) { return new Reference { range = definition.Span, uri = definition.DocumentUri }; }