From 3d6bcd32e5d8c859e517fec615c01d7768a873bb Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 25 Sep 2019 20:36:03 -0700 Subject: [PATCH 1/3] Separate reset into init/clear/touch --- .../Impl/Documents/Definitions/IDocument.cs | 5 ++-- .../Ast/Impl/Documents/DocumentBuffer.cs | 30 +++++++++++++++++-- .../Impl/Documents/RunningDocumentTable.cs | 4 +-- src/Analysis/Ast/Impl/Modules/PythonModule.cs | 24 +++++++-------- src/Analysis/Ast/Test/DocumentBufferTests.cs | 23 ++++++-------- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/Analysis/Ast/Impl/Documents/Definitions/IDocument.cs b/src/Analysis/Ast/Impl/Documents/Definitions/IDocument.cs index dc1228ddb..d5b9489ab 100644 --- a/src/Analysis/Ast/Impl/Documents/Definitions/IDocument.cs +++ b/src/Analysis/Ast/Impl/Documents/Definitions/IDocument.cs @@ -63,9 +63,10 @@ public interface IDocument: IPythonModule, IDisposable { void Update(IEnumerable changes); /// - /// Resets document buffer to the provided content or tries to load it if content is null, then parses and analyzes document. + /// Marks document as changed and advances its version. + /// Leads to new parse and analysis of the document. /// - void Reset(string content); + void Invalidate(); /// /// Provides collection of parsing errors, if any. diff --git a/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs b/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs index 21a2bdfb8..0b7998113 100644 --- a/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs +++ b/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs @@ -16,6 +16,7 @@ using System.Collections.Generic; using System.Linq; using System.Text; +using Microsoft.Python.Core.Diagnostics; using Microsoft.Python.Parsing; namespace Microsoft.Python.Analysis.Documents { @@ -23,6 +24,8 @@ internal sealed class DocumentBuffer { private readonly object _lock = new object(); private StringBuilder _sb = new StringBuilder(); private string _content; + private bool _cleared; + private bool _initialized; public int Version { get; private set; } @@ -34,16 +37,38 @@ public string Text { } } - public void Reset(int version, string content) { + public void SetContent(string content) { lock (_lock) { - Version = version; + Check.InvalidOperation(!_initialized, "Buffer is already initialized."); + Check.InvalidOperation(!_cleared, "Buffer cannot be updated since its content was dropped."); + Version = 0; _content = content ?? string.Empty; _sb = null; + _initialized = true; + } + } + + public void Clear() { + lock (_lock) { + _content = string.Empty; + _sb = null; + _cleared = true; + } + } + + public void MarkChanged() { + lock (_lock) { + Check.InvalidOperation(_initialized, "Buffer is not initialized."); + Check.InvalidOperation(!_cleared, "Buffer cannot be updated since its content was dropped."); + Version++; } } public void Update(IEnumerable changes) { lock (_lock) { + Check.InvalidOperation(_initialized, "Buffer is not initialized."); + Check.InvalidOperation(!_cleared, "Buffer cannot be updated since its content was dropped."); + _sb = _sb ?? new StringBuilder(_content); foreach (var change in changes) { @@ -98,7 +123,6 @@ public IEnumerable GetNewLineLocations() { if (i == _sb.Length - 1) { yield return new NewLineLocation(i + 1, NewLineKind.None); } - break; } } diff --git a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs index 810632abc..e85039f00 100644 --- a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs @@ -217,7 +217,7 @@ public void ReloadAll() { } foreach (var (_, entry) in opened) { - entry.Document.Reset(null); + entry.Document.Invalidate(); } } @@ -280,7 +280,7 @@ private bool TryAddModulePath(ModuleCreationOptions mco) { private bool TryOpenDocument(DocumentEntry entry, string content) { if (!entry.Document.IsOpen) { entry.Document.IsOpen = true; - entry.Document.Reset(content); + entry.Document.Invalidate(); entry.LockCount++; return true; } diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index 92de189dd..7a6869402 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -315,14 +315,12 @@ public void Update(IEnumerable changes) { Services.GetService().InvalidateAnalysis(this); } - public void Reset(string content) { + public void Invalidate() { lock (_syncObj) { - if (content != Content) { - ContentState = State.None; - InitializeContent(content, _buffer.Version + 1); - } + ContentState = State.None; + _buffer.MarkChanged(); + Parse(); } - Services.GetService().InvalidateAnalysis(this); } @@ -451,7 +449,7 @@ public void NotifyAnalysisComplete(IDocumentAnalysis analysis) { ContentState = State.Analyzed; if (ModuleType != ModuleType.User) { - _buffer.Reset(_buffer.Version, string.Empty); + _buffer.Clear(); } } @@ -487,7 +485,7 @@ public void AddAstNode(object o, Node n) { public void ClearContent() { lock (_syncObj) { if (ModuleType != ModuleType.User) { - _buffer.Reset(_buffer.Version, string.Empty); + _buffer.Clear(); _astMap.Clear(); } } @@ -517,16 +515,14 @@ protected virtual string LoadContent() { private void InitializeContent(string content, int version) { lock (_syncObj) { - LoadContent(content, version); - - var startParse = ContentState < State.Parsing && (_parsingTask == null || version > 0); - if (startParse) { + SetOrLoadContent(content); + if (ContentState < State.Parsing && _parsingTask == null) { Parse(); } } } - private void LoadContent(string content, int version) { + private void SetOrLoadContent(string content) { if (ContentState < State.Loading) { try { if (IsPersistent) { @@ -534,7 +530,7 @@ private void LoadContent(string content, int version) { } else { content = content ?? LoadContent(); } - _buffer.Reset(version, content); + _buffer.SetContent(content); ContentState = State.Loaded; } catch (IOException) { } catch (UnauthorizedAccessException) { } } diff --git a/src/Analysis/Ast/Test/DocumentBufferTests.cs b/src/Analysis/Ast/Test/DocumentBufferTests.cs index c0230ac18..2743237cc 100644 --- a/src/Analysis/Ast/Test/DocumentBufferTests.cs +++ b/src/Analysis/Ast/Test/DocumentBufferTests.cs @@ -29,7 +29,7 @@ public class DocumentBufferTests { [TestMethod, Priority(0)] public void BasicDocumentBuffer() { var doc = new DocumentBuffer(); - doc.Reset(0, @"def f(x): + doc.SetContent(@"def f(x): return def g(y): @@ -77,7 +77,7 @@ def g(y): public void ResetDocumentBuffer() { var doc = new DocumentBuffer(); - doc.Reset(0, string.Empty); + doc.SetContent(string.Empty); Assert.AreEqual(string.Empty, doc.Text); doc.Update(new[] { @@ -86,18 +86,13 @@ public void ResetDocumentBuffer() { Assert.AreEqual("text", doc.Text); Assert.AreEqual(1, doc.Version); - - doc.Reset(0, @"abcdef"); - - Assert.AreEqual(@"abcdef", doc.Text); - Assert.AreEqual(0, doc.Version); } [TestMethod, Priority(0)] public void ReplaceAllDocumentBuffer() { var doc = new DocumentBuffer(); - doc.Reset(0, string.Empty); + doc.SetContent(string.Empty); Assert.AreEqual(string.Empty, doc.Text); doc.Update(new[] { @@ -134,7 +129,7 @@ public void ReplaceAllDocumentBuffer() { [TestMethod, Priority(0)] public void DeleteMultipleDisjoint() { var doc = new DocumentBuffer(); - doc.Reset(0, @" + doc.SetContent(@" line1 line2 line3 @@ -157,7 +152,7 @@ public void DeleteMultipleDisjoint() { [TestMethod, Priority(0)] public void InsertMultipleDisjoint() { var doc = new DocumentBuffer(); - doc.Reset(0, @" + doc.SetContent(@" line line line @@ -180,7 +175,7 @@ public void InsertMultipleDisjoint() { [TestMethod, Priority(0)] public void DeleteAcrossLines() { var doc = new DocumentBuffer(); - doc.Reset(0, @" + doc.SetContent(@" line1 line2 line3 @@ -199,7 +194,7 @@ public void DeleteAcrossLines() { [TestMethod, Priority(0)] public void SequentialChanges() { var doc = new DocumentBuffer(); - doc.Reset(0, @" + doc.SetContent(@" line1 line2 line3 @@ -218,7 +213,7 @@ public void SequentialChanges() { [TestMethod, Priority(0)] public void InsertTopToBottom() { var doc = new DocumentBuffer(); - doc.Reset(0, @"linelinelineline"); + doc.SetContent(@"linelinelineline"); doc.Update(new[] { DocumentChange.Insert("\n", new SourceLocation(1, 1)), DocumentChange.Insert("1\n", new SourceLocation(2, 5)), @@ -233,7 +228,7 @@ public void InsertTopToBottom() { [DataTestMethod, Priority(0)] public void NewLines(string s, NewLineLocation[] expected) { var doc = new DocumentBuffer(); - doc.Reset(0, s); + doc.SetContent(s); var nls = doc.GetNewLineLocations().ToArray(); for (var i = 0; i < nls.Length; i++) { Assert.AreEqual(nls[i].Kind, expected[i].Kind); From 0a27caca72072c42fe1fd8284553cece7595debb Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 25 Sep 2019 21:14:52 -0700 Subject: [PATCH 2/3] Fix module creation analysis --- src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs | 1 - src/Analysis/Ast/Impl/Modules/PythonModule.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs index e85039f00..323378811 100644 --- a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs @@ -280,7 +280,6 @@ private bool TryAddModulePath(ModuleCreationOptions mco) { private bool TryOpenDocument(DocumentEntry entry, string content) { if (!entry.Document.IsOpen) { entry.Document.IsOpen = true; - entry.Document.Invalidate(); entry.LockCount++; return true; } diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index 7a6869402..442253995 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -311,7 +311,6 @@ public void Update(IEnumerable changes) { Parse(); } - Services.GetService().InvalidateAnalysis(this); } @@ -520,6 +519,7 @@ private void InitializeContent(string content, int version) { Parse(); } } + Services.GetService().InvalidateAnalysis(this); } private void SetOrLoadContent(string content) { From 66d3d459d3cdd907ca8ae5b528da5922edd6732f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 25 Sep 2019 21:48:55 -0700 Subject: [PATCH 3/3] Fix find references --- src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs index 7975f97d2..b972b0023 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs @@ -105,8 +105,11 @@ private void DeclareVariable(PythonVariableModule variableModule, string memberN // First try imports since child modules should win, i.e. in 'from a.b import c' // 'c' should be a submodule if 'b' has one, even if 'b' also declares 'c = 1'. var value = GetValueFromImports(variableModule, imports as IImportChildrenSource, memberName); - // Now try exported + // First try exported or child submodules. value = value ?? variableModule.GetMember(memberName); + // Value may be variable or submodule. If it is variable, we need it in order to add reference. + var variable = variableModule.Analysis?.GlobalScope?.Variables[memberName]; + value = variable?.Value == value ? variable : value; // If nothing is exported, variables are still accessible. value = value ?? variableModule.Analysis?.GlobalScope?.Variables[memberName]?.Value ?? Eval.UnknownType; // Do not allow imported variables to override local declarations