From acde5be5de634aeabe057691af07df536fb1663e Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 Feb 2019 13:45:17 -0800 Subject: [PATCH 1/6] Diagnostics reporting --- .../Ast/Impl/Analyzer/PythonAnalyzer.cs | 9 +- .../Impl/Diagnostics/IDiagnosticsService.cs | 2 +- .../Resolution/MainModuleResolution.cs | 24 ++- .../Resolution/ModuleResolutionBase.cs | 4 +- .../Modules/Resolution/TypeshedResolution.cs | 2 + src/Analysis/Ast/Test/AnalysisTestBase.cs | 2 +- src/Core/Impl/Extensions/IOExtensions.cs | 5 + .../Impl/Diagnostics/DiagnosticsService.cs | 121 ++++++++++--- .../Impl/Implementation/Server.Documents.cs | 3 - .../Impl/Implementation/Server.cs | 2 +- src/LanguageServer/Test/DiagnosticsTests.cs | 162 ++++++++++-------- src/LanguageServer/Test/RdtTests.cs | 67 ++++++++ 12 files changed, 292 insertions(+), 111 deletions(-) create mode 100644 src/LanguageServer/Test/RdtTests.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 5c90c2038..41de248c0 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -31,7 +31,7 @@ public sealed class PythonAnalyzer : IPythonAnalyzer, IDisposable { private readonly CancellationTokenSource _globalCts = new CancellationTokenSource(); private readonly ILogger _log; - public PythonAnalyzer(IServiceManager services, string root) { + public PythonAnalyzer(IServiceManager services) { _services = services; _log = services.GetService(); @@ -40,11 +40,6 @@ public PythonAnalyzer(IServiceManager services, string root) { _dependencyResolver = new DependencyResolver(_services); _services.AddService(_dependencyResolver); } - - //var rdt = services.GetService(); - //if (rdt == null) { - // services.AddService(new RunningDocumentTable(root, services)); - //} } public void Dispose() => _globalCts.Cancel(); @@ -151,7 +146,7 @@ private async Task AnalyzeAsync(IDependencyChainNode node, Ca // Note that we do not set the new analysis here and rather let // Python analyzer to call NotifyAnalysisComplete. await walker.CompleteAsync(cancellationToken); - _log?.Log(TraceEventType.Verbose, $"Analysis of {node.Document.Name}({node.Document.ModuleType}) complete in {(DateTime.Now - startTime).TotalMilliseconds} ms."); + _log?.Log(TraceEventType.Verbose, $"Analysis of {node.Document.Name} ({node.Document.ModuleType}) complete in {(DateTime.Now - startTime).TotalMilliseconds} ms."); return new DocumentAnalysis(node.Document, analysisVersion, walker.GlobalScope, walker.Eval); } } diff --git a/src/Analysis/Ast/Impl/Diagnostics/IDiagnosticsService.cs b/src/Analysis/Ast/Impl/Diagnostics/IDiagnosticsService.cs index 96d7e3674..0a6a83ce9 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/IDiagnosticsService.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/IDiagnosticsService.cs @@ -29,7 +29,7 @@ public interface IDiagnosticsService { void Replace(Uri documentUri, IEnumerable entries); /// - /// Removes document from the diagnostics report. Typically when document closes. + /// Removes document from the diagnostics report. Typically when document is disposed. /// void Remove(Uri documentUri); diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs index 039736b04..7016868a5 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs @@ -26,6 +26,7 @@ using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Core; +using Microsoft.Python.Core.IO; namespace Microsoft.Python.Analysis.Modules.Resolution { internal sealed class MainModuleResolution : ModuleResolutionBase, IModuleManagement { @@ -73,13 +74,21 @@ protected override async Task DoImportAsync(string name, Cancella return null; } + var rdt = _services.GetService(); + IPythonModule module; + if (!string.IsNullOrEmpty(moduleImport.ModulePath) && Uri.TryCreate(moduleImport.ModulePath, UriKind.Absolute, out var uri)) { + module = rdt.GetDocument(uri); + if (module != null) { + return module; + } + } + // If there is a stub, make sure it is loaded and attached // First check stub next to the module. var stub = await GetModuleStubAsync(name, moduleImport.ModulePath, cancellationToken); // If nothing found, try Typeshed. stub = stub ?? await _interpreter.TypeshedResolution.ImportModuleAsync(moduleImport.IsBuiltin ? name : moduleImport.FullName, cancellationToken); - IPythonModule module; if (moduleImport.IsBuiltin) { _log?.Log(TraceEventType.Verbose, "Import built-in compiled (scraped) module: ", name, Configuration.InterpreterPath); module = new CompiledBuiltinPythonModule(name, stub, _services); @@ -88,11 +97,15 @@ protected override async Task DoImportAsync(string name, Cancella module = new CompiledPythonModule(moduleImport.FullName, ModuleType.Compiled, moduleImport.ModulePath, stub, _services); } else { _log?.Log(TraceEventType.Verbose, "Import: ", moduleImport.FullName, moduleImport.ModulePath); - var rdt = _services.GetService(); + // Module inside workspace == user code. // TODO: handle user code and library module separately. + var moduleType = moduleImport.ModulePath.IsUnderRoot(_root, _fs.StringComparison) + ? ModuleType.User + : ModuleType.Library; + var mco = new ModuleCreationOptions { ModuleName = moduleImport.FullName, - ModuleType = ModuleType.Library, + ModuleType = moduleType, FilePath = moduleImport.ModulePath, Stub = stub }; @@ -185,9 +198,12 @@ private async Task GetModuleStubAsync(string name, string moduleP // Try location of stubs that are in a separate folder next to the package. var stubPath = CurrentPathResolver.GetPossibleModuleStubPaths(name).FirstOrDefault(p => _fs.FileExists(p)); if (!string.IsNullOrEmpty(stubPath)) { - return await CreateStubModuleAsync(name, stubPath, cancellationToken); + return await CreateStubModuleAsync(name, stubPath, cancellationToken); } return null; } + + protected override void ReportModuleNotFound(string name) + => _log?.Log(TraceEventType.Information, $"Import not found: {name}"); } } diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/ModuleResolutionBase.cs b/src/Analysis/Ast/Impl/Modules/Resolution/ModuleResolutionBase.cs index b46a3b0ca..ba5ccffa5 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/ModuleResolutionBase.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/ModuleResolutionBase.cs @@ -137,7 +137,7 @@ private async Task DoImportModuleAsync(string name, CancellationT case TryImportModuleResultCode.Success: return result.Module; case TryImportModuleResultCode.ModuleNotFound: - _log?.Log(TraceEventType.Information, $"Import not found: {name}"); + ReportModuleNotFound(name); return null; case TryImportModuleResultCode.NeedRetry: case TryImportModuleResultCode.Timeout: @@ -225,5 +225,7 @@ protected async Task CreateStubModuleAsync(string moduleName, str await module.LoadAndAnalyzeAsync(cancellationToken); return module; } + + protected abstract void ReportModuleNotFound(string name); } } diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/TypeshedResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/TypeshedResolution.cs index 51376d6d6..f4e61a800 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/TypeshedResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/TypeshedResolution.cs @@ -137,5 +137,7 @@ private bool IsPackage(string directory) !string.IsNullOrEmpty(ModulePath.GetPackageInitPy(directory)) : _fs.DirectoryExists(directory); + protected override void ReportModuleNotFound(string name) + => _log?.Log(TraceEventType.Verbose, $"Typeshed stub not found for '{name}'"); } } diff --git a/src/Analysis/Ast/Test/AnalysisTestBase.cs b/src/Analysis/Ast/Test/AnalysisTestBase.cs index 15a2795c7..cb4bc77ff 100644 --- a/src/Analysis/Ast/Test/AnalysisTestBase.cs +++ b/src/Analysis/Ast/Test/AnalysisTestBase.cs @@ -86,7 +86,7 @@ protected async Task CreateServicesAsync(string root, Interpret sm.AddService(dependencyResolver); TestLogger.Log(TraceEventType.Information, "Create PythonAnalyzer"); - var analyzer = new PythonAnalyzer(sm, root); + var analyzer = new PythonAnalyzer(sm); sm.AddService(analyzer); TestLogger.Log(TraceEventType.Information, "Create PythonInterpreter"); diff --git a/src/Core/Impl/Extensions/IOExtensions.cs b/src/Core/Impl/Extensions/IOExtensions.cs index cfabc6cc4..d1a19d9d2 100644 --- a/src/Core/Impl/Extensions/IOExtensions.cs +++ b/src/Core/Impl/Extensions/IOExtensions.cs @@ -124,5 +124,10 @@ public static void WriteTextWithRetry(this IFileSystem fs, string filePath, stri fs.DeleteFile(filePath); } catch (IOException) { } catch (UnauthorizedAccessException) { } } + + public static bool IsUnderRoot(this string path, string root, StringComparison comparison) { + var normalized = PathUtils.NormalizePath(path); + return normalized.StartsWith(root, comparison); + } } } diff --git a/src/LanguageServer/Impl/Diagnostics/DiagnosticsService.cs b/src/LanguageServer/Impl/Diagnostics/DiagnosticsService.cs index b00e10c6a..037cc700b 100644 --- a/src/LanguageServer/Impl/Diagnostics/DiagnosticsService.cs +++ b/src/LanguageServer/Impl/Diagnostics/DiagnosticsService.cs @@ -17,6 +17,7 @@ using System.Collections.Generic; using System.Linq; using Microsoft.Python.Analysis.Diagnostics; +using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core; using Microsoft.Python.Core.Disposables; using Microsoft.Python.Core.Idle; @@ -26,17 +27,40 @@ namespace Microsoft.Python.LanguageServer.Diagnostics { internal sealed class DiagnosticsService : IDiagnosticsService, IDisposable { - private readonly Dictionary> _diagnostics = new Dictionary>(); + private sealed class DocumentDiagnostics { + private DiagnosticsEntry[] _entries; + + public DiagnosticsEntry[] Entries { + get => _entries ?? Array.Empty(); + set { + _entries = value; + Changed = true; + } + } + public bool Changed { get; set; } + + public void Clear() { + Changed = _entries.Length > 0; + _entries = Array.Empty(); + } + } + + private readonly Dictionary _diagnostics = new Dictionary(); private readonly DisposableBag _disposables = DisposableBag.Create(); + private readonly IServiceContainer _services; private readonly IClientApplication _clientApp; private readonly object _lock = new object(); private DiagnosticsSeverityMap _severityMap = new DiagnosticsSeverityMap(); + private IRunningDocumentTable _rdt; private DateTime _lastChangeTime; - private bool _changed; + + private IRunningDocumentTable Rdt => _rdt ?? (_rdt = _services.GetService()); public DiagnosticsService(IServiceContainer services) { - var idleTimeService = services.GetService(); + _services = services; + _clientApp = services.GetService(); + var idleTimeService = services.GetService(); if (idleTimeService != null) { idleTimeService.Idle += OnIdle; idleTimeService.Closing += OnClosing; @@ -45,7 +69,6 @@ public DiagnosticsService(IServiceContainer services) { .Add(() => idleTimeService.Idle -= OnIdle) .Add(() => idleTimeService.Idle -= OnClosing); } - _clientApp = services.GetService(); } #region IDiagnosticsService @@ -53,32 +76,31 @@ public IReadOnlyDictionary> Diagnostics { get { lock (_lock) { return _diagnostics.ToDictionary(kvp => kvp.Key, - kvp => kvp.Value - .Where(e => DiagnosticsSeverityMap.GetEffectiveSeverity(e.ErrorCode, e.Severity) != Severity.Suppressed) - .Select(e => new DiagnosticsEntry( - e.Message, - e.SourceSpan, - e.ErrorCode, - DiagnosticsSeverityMap.GetEffectiveSeverity(e.ErrorCode, e.Severity)) - ).ToList() as IReadOnlyList); + kvp => FilterBySeverityMap(kvp.Value).ToList() as IReadOnlyList); } } } public void Replace(Uri documentUri, IEnumerable entries) { lock (_lock) { - _diagnostics[documentUri] = entries.ToList(); + if (!_diagnostics.TryGetValue(documentUri, out var documentDiagnostics)) { + documentDiagnostics = new DocumentDiagnostics(); + _diagnostics[documentUri] = documentDiagnostics; + } + documentDiagnostics.Entries = entries.ToArray(); + documentDiagnostics.Changed = true; _lastChangeTime = DateTime.Now; - _changed = true; } } public void Remove(Uri documentUri) { lock (_lock) { // Before removing the document, make sure we clear its diagnostics. - _diagnostics[documentUri] = new List(); - PublishDiagnostics(); - _diagnostics.Remove(documentUri); + if (_diagnostics.TryGetValue(documentUri, out var d)) { + d.Clear(); + PublishDiagnostics(); + _diagnostics.Remove(documentUri); + } } } @@ -89,6 +111,10 @@ public DiagnosticsSeverityMap DiagnosticsSeverityMap { set { lock (_lock) { _severityMap = value; + foreach (var d in _diagnostics) { + _diagnostics[d.Key].Changed = true; + _lastChangeTime = DateTime.Now; + } PublishDiagnostics(); } } @@ -103,22 +129,29 @@ public void Dispose() { private void OnClosing(object sender, EventArgs e) => Dispose(); private void OnIdle(object sender, EventArgs e) { - if (_changed && (DateTime.Now - _lastChangeTime).TotalMilliseconds > PublishingDelay) { + if ((DateTime.Now - _lastChangeTime).TotalMilliseconds > PublishingDelay) { + ConnectToRdt(); PublishDiagnostics(); } } private void PublishDiagnostics() { - KeyValuePair>[] diagnostics; + var diagnostics = new List>(); lock (_lock) { - diagnostics = Diagnostics.ToArray(); - _changed = false; + foreach (var d in _diagnostics) { + if (d.Value.Changed) { + diagnostics.Add(d); + _diagnostics[d.Key].Changed = false; + } + } } foreach (var kvp in diagnostics) { var parameters = new PublishDiagnosticsParams { uri = kvp.Key, - diagnostics = kvp.Value.Select(ToDiagnostic).ToArray() + diagnostics = Rdt.GetDocument(kvp.Key)?.IsOpen == true + ? FilterBySeverityMap(kvp.Value).Select(ToDiagnostic).ToArray() + : Array.Empty() }; _clientApp.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", parameters).DoNotWait(); } @@ -127,7 +160,6 @@ private void PublishDiagnostics() { private void ClearAllDiagnostics() { lock (_lock) { _diagnostics.Clear(); - _changed = false; } } @@ -156,5 +188,48 @@ private static Diagnostic ToDiagnostic(DiagnosticsEntry e) { message = e.Message, }; } + + private IEnumerable FilterBySeverityMap(DocumentDiagnostics d) + => d.Entries + .Where(e => DiagnosticsSeverityMap.GetEffectiveSeverity(e.ErrorCode, e.Severity) != Severity.Suppressed) + .Select(e => new DiagnosticsEntry( + e.Message, + e.SourceSpan, + e.ErrorCode, + DiagnosticsSeverityMap.GetEffectiveSeverity(e.ErrorCode, e.Severity)) + ); + + private void ConnectToRdt() { + if (_rdt == null) { + _rdt = _services.GetService(); + if (_rdt != null) { + _rdt.Opened += OnOpenDocument; + _rdt.Closed += OnCloseDocument; + + _disposables + .Add(() => _rdt.Opened -= OnOpenDocument) + .Add(() => _rdt.Closed -= OnCloseDocument); + } + } + } + + private void OnOpenDocument(object sender, DocumentEventArgs e) { + lock (_lock) { + if(_diagnostics.TryGetValue(e.Document.Uri, out var d)) { + d.Changed = d.Entries.Length > 0; + } + } + } + + private void OnCloseDocument(object sender, DocumentEventArgs e) { + lock (_lock) { + // Before removing the document, make sure we clear its diagnostics. + if (_diagnostics.TryGetValue(e.Document.Uri, out var d)) { + d.Clear(); + PublishDiagnostics(); + _diagnostics.Remove(e.Document.Uri); + } + } + } } } diff --git a/src/LanguageServer/Impl/Implementation/Server.Documents.cs b/src/LanguageServer/Impl/Implementation/Server.Documents.cs index ea8de0cba..4d1c6a55a 100644 --- a/src/LanguageServer/Impl/Implementation/Server.Documents.cs +++ b/src/LanguageServer/Impl/Implementation/Server.Documents.cs @@ -16,13 +16,10 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Threading; -using System.Threading.Tasks; using Microsoft.Python.Analysis; using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core; -using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Protocol; namespace Microsoft.Python.LanguageServer.Implementation { diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index cb66d4eb8..a0241f6ce 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -105,7 +105,7 @@ public async Task InitializeAsync(InitializeParams @params, Ca _services.AddService(new DiagnosticsService(_services)); - var analyzer = new PythonAnalyzer(_services, @params.rootPath); + var analyzer = new PythonAnalyzer(_services); _services.AddService(analyzer); _services.AddService(new RunningDocumentTable(@params.rootPath, _services)); diff --git a/src/LanguageServer/Test/DiagnosticsTests.cs b/src/LanguageServer/Test/DiagnosticsTests.cs index 6c1b1bf1c..3d995e5eb 100644 --- a/src/LanguageServer/Test/DiagnosticsTests.cs +++ b/src/LanguageServer/Test/DiagnosticsTests.cs @@ -14,6 +14,7 @@ // permissions and limitations under the License. using System; +using System.Collections.Generic; using System.Threading.Tasks; using FluentAssertions; using Microsoft.Python.Analysis.Diagnostics; @@ -45,7 +46,7 @@ public async Task BasicChange() { const string code = @"x = "; var analysis = await GetAnalysisAsync(code); - var ds = Services.GetService(); + var ds = GetDiagnosticsService(); var doc = analysis.Document; ds.Diagnostics[doc.Uri].Count.Should().Be(1); @@ -74,7 +75,7 @@ public async Task TwoDocuments() { var analysis1 = await GetAnalysisAsync(code1); var analysis2 = await GetNextAnalysisAsync(code2); - var ds = Services.GetService(); + var ds = GetDiagnosticsService(); var doc1 = analysis1.Document; var doc2 = analysis2.Document; @@ -112,26 +113,26 @@ public async Task Publish() { var doc = analysis.Document; var clientApp = Services.GetService(); - var idle = Services.GetService(); - - var expected = 1; + var reported = new List(); clientApp.When(x => x.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", Arg.Any())) - .Do(x => { - var dp = x.Args()[1] as PublishDiagnosticsParams; - dp.Should().NotBeNull(); - dp.diagnostics.Length.Should().Be(expected); - dp.uri.Should().Be(doc.Uri); - }); - idle.Idle += Raise.EventWith(null, EventArgs.Empty); + .Do(x => reported.Add(x.Args()[1] as PublishDiagnosticsParams)); - expected = 0; + PublishDiagnostics(); + reported.Count.Should().Be(1); + reported[0].diagnostics.Length.Should().Be(1); + reported[0].uri.Should().Be(doc.Uri); + + reported.Clear(); doc.Update(new[] {new DocumentChange { InsertedText = "1", ReplacedSpan = new SourceSpan(1, 5, 1, 5) } }); await doc.GetAnalysisAsync(); - idle.Idle += Raise.EventWith(null, EventArgs.Empty); + + PublishDiagnostics(); + reported.Count.Should().Be(1); + reported[0].diagnostics.Length.Should().Be(0); } [TestMethod, Priority(0)] @@ -139,26 +140,21 @@ public async Task CloseDocument() { const string code = @"x = "; var analysis = await GetAnalysisAsync(code); - var ds = Services.GetService(); + var ds = GetDiagnosticsService(); var doc = analysis.Document; ds.Diagnostics[doc.Uri].Count.Should().Be(1); var clientApp = Services.GetService(); - var uri = doc.Uri; - var callReceived = false; + var reported = new List(); clientApp.When(x => x.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", Arg.Any())) - .Do(x => { - var dp = x.Args()[1] as PublishDiagnosticsParams; - dp.Should().NotBeNull(); - dp.diagnostics.Length.Should().Be(0); - dp.uri.Should().Be(uri); - callReceived = true; - }); + .Do(x => reported.Add(x.Args()[1] as PublishDiagnosticsParams)); doc.Dispose(); ds.Diagnostics.TryGetValue(doc.Uri, out _).Should().BeFalse(); - callReceived.Should().BeTrue(); + reported.Count.Should().Be(1); + reported[0].uri.Should().Be(doc.Uri); + reported[0].diagnostics.Length.Should().Be(0); } [TestMethod, Priority(0)] @@ -166,77 +162,103 @@ public async Task SeverityMapping() { const string code = @"import nonexistent"; var analysis = await GetAnalysisAsync(code); - var ds = Services.GetService(); + var ds = GetDiagnosticsService(); var doc = analysis.Document; var diags = ds.Diagnostics[doc.Uri]; diags.Count.Should().Be(1); diags[0].Severity.Should().Be(Severity.Warning); - var uri = doc.Uri; - var callReceived = false; var clientApp = Services.GetService(); - var expectedSeverity = DiagnosticSeverity.Error; + var reported = new List(); clientApp.When(x => x.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", Arg.Any())) - .Do(x => { - var dp = x.Args()[1] as PublishDiagnosticsParams; - dp.Should().NotBeNull(); - dp.uri.Should().Be(uri); - dp.diagnostics.Length.Should().Be(expectedSeverity == DiagnosticSeverity.Unspecified ? 0 : 1); - if (expectedSeverity != DiagnosticSeverity.Unspecified) { - dp.diagnostics[0].severity.Should().Be(expectedSeverity); - } - callReceived = true; - }); + .Do(x => reported.Add(x.Args()[1] as PublishDiagnosticsParams)); ds.DiagnosticsSeverityMap = new DiagnosticsSeverityMap(new[] { ErrorCodes.UnresolvedImport }, null, null, null); - callReceived.Should().BeTrue(); + reported.Count.Should().Be(1); + reported[0].uri.Should().Be(doc.Uri); + reported[0].diagnostics.Length.Should().Be(1); + reported[0].diagnostics[0].severity.Should().Be(DiagnosticSeverity.Error); - expectedSeverity = DiagnosticSeverity.Information; - callReceived = false; + reported.Clear(); ds.DiagnosticsSeverityMap = new DiagnosticsSeverityMap(null, null, new[] { ErrorCodes.UnresolvedImport }, null); - ds.Diagnostics[uri][0].Severity.Should().Be(Severity.Information); - callReceived.Should().BeTrue(); + reported.Count.Should().Be(1); + reported[0].diagnostics[0].severity.Should().Be(DiagnosticSeverity.Information); - expectedSeverity = DiagnosticSeverity.Unspecified; - callReceived = false; + reported.Clear(); ds.DiagnosticsSeverityMap = new DiagnosticsSeverityMap(null, null, null, new[] { ErrorCodes.UnresolvedImport }); - ds.Diagnostics[uri].Count.Should().Be(0); - callReceived.Should().BeTrue(); - - expectedSeverity = DiagnosticSeverity.Unspecified; - callReceived = false; - ds.DiagnosticsSeverityMap = new DiagnosticsSeverityMap(new[] { ErrorCodes.UnresolvedImport }, null, null, new[] { ErrorCodes.UnresolvedImport }); - ds.Diagnostics[uri].Count.Should().Be(0); - callReceived.Should().BeTrue(); + reported.Count.Should().Be(1); + reported[0].diagnostics.Length.Should().Be(0); } [TestMethod, Priority(0)] - public async Task SuppressError() { - const string code = @"import nonexistent"; + public async Task OnlyPublishChangedFile() { + const string code1 = @"x = "; + const string code2 = @"y = "; - var analysis = await GetAnalysisAsync(code); - var ds = Services.GetService(); - var doc = analysis.Document; + var analysis1 = await GetAnalysisAsync(code1); + var analysis2 = await GetNextAnalysisAsync(code2); + var ds = GetDiagnosticsService(); - var diags = ds.Diagnostics[doc.Uri]; - diags.Count.Should().Be(1); - diags[0].Severity.Should().Be(Severity.Warning); + var doc1 = analysis1.Document; + var doc2 = analysis2.Document; + + ds.Diagnostics[doc1.Uri].Count.Should().Be(1); + ds.Diagnostics[doc2.Uri].Count.Should().Be(1); + + // Clear diagnostics 'changed' state by forcing publish. + PublishDiagnostics(); - var uri = doc.Uri; - var callReceived = false; + var reported = new List(); var clientApp = Services.GetService(); clientApp.When(x => x.NotifyWithParameterObjectAsync("textDocument/publishDiagnostics", Arg.Any())) .Do(x => { var dp = x.Args()[1] as PublishDiagnosticsParams; - dp.Should().NotBeNull(); - dp.uri.Should().Be(uri); - dp.diagnostics.Length.Should().Be(0); - callReceived = true; + reported.Add(dp); }); - ds.DiagnosticsSeverityMap = new DiagnosticsSeverityMap(null, null, null, new[] { ErrorCodes.UnresolvedImport }); - callReceived.Should().BeTrue(); + + doc1.Update(new[] {new DocumentChange { + InsertedText = "1", + ReplacedSpan = new SourceSpan(1, 5, 1, 5) + } }); + + await doc1.GetAnalysisAsync(); + ds.Diagnostics[doc1.Uri].Count.Should().Be(0); + ds.Diagnostics[doc2.Uri].Count.Should().Be(1); + + PublishDiagnostics(); + reported.Count.Should().Be(1); + reported[0].uri.Should().Be(doc1.Uri); + reported[0].diagnostics.Length.Should().Be(0); + + doc1.Update(new[] {new DocumentChange { + InsertedText = string.Empty, + ReplacedSpan = new SourceSpan(1, 5, 1, 6) + } }); + + await doc1.GetAnalysisAsync(); + ds.Diagnostics[doc1.Uri].Count.Should().Be(1); + ds.Diagnostics[doc2.Uri].Count.Should().Be(1); + + reported.Clear(); + PublishDiagnostics(); + + reported.Count.Should().Be(1); + reported[0].uri.Should().Be(doc1.Uri); + reported[0].diagnostics.Length.Should().Be(1); + } + + private IDiagnosticsService GetDiagnosticsService() { + var ds = Services.GetService(); + ds.PublishingDelay = 0; + return ds; + } + private void PublishDiagnostics() { + var ds = Services.GetService(); + ds.PublishingDelay = 0; + var idle = Services.GetService(); + idle.Idle += Raise.EventWith(null, EventArgs.Empty); } } } diff --git a/src/LanguageServer/Test/RdtTests.cs b/src/LanguageServer/Test/RdtTests.cs new file mode 100644 index 000000000..61434cfcb --- /dev/null +++ b/src/LanguageServer/Test/RdtTests.cs @@ -0,0 +1,67 @@ +// 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.Diagnostics; +using Microsoft.Python.Analysis.Documents; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.LanguageServer.Tests { + [TestClass] + public class RdtTests : LanguageServerTestBase { + 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 OpenCloseDocuments() { + const string code1 = @"x = "; + const string code2 = @"y = "; + var uri1 = TestData.GetDefaultModuleUri(); + var uri2 = TestData.GetNextModuleUri(); + + await CreateServicesAsync(PythonVersions.LatestAvailable3X, uri1.AbsolutePath); + var ds = Services.GetService(); + var rdt = Services.GetService(); + + rdt.OpenDocument(uri1, code1); + rdt.OpenDocument(uri2, code2); + + var doc1 = rdt.GetDocument(uri1); + var doc2 = rdt.GetDocument(uri2); + + await doc1.GetAnalysisAsync(); + await doc2.GetAnalysisAsync(); + + ds.Diagnostics[doc1.Uri].Count.Should().Be(1); + ds.Diagnostics[doc2.Uri].Count.Should().Be(1); + + rdt.CloseDocument(uri1); + + ds.Diagnostics[uri2].Count.Should().Be(1); + rdt.GetDocument(uri1).Should().BeNull(); + ds.Diagnostics.TryGetValue(uri1, out _).Should().BeFalse(); + } + } +} From d4985463ad71a4035286b016c3ca8229fbb2a237 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 Feb 2019 21:12:50 -0800 Subject: [PATCH 2/6] Handle standole files better --- src/Core/Impl/Extensions/IOExtensions.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Core/Impl/Extensions/IOExtensions.cs b/src/Core/Impl/Extensions/IOExtensions.cs index d1a19d9d2..78ae27ac5 100644 --- a/src/Core/Impl/Extensions/IOExtensions.cs +++ b/src/Core/Impl/Extensions/IOExtensions.cs @@ -126,8 +126,11 @@ public static void WriteTextWithRetry(this IFileSystem fs, string filePath, stri } public static bool IsUnderRoot(this string path, string root, StringComparison comparison) { - var normalized = PathUtils.NormalizePath(path); - return normalized.StartsWith(root, comparison); + if (!string.IsNullOrEmpty(root)) { + var normalized = PathUtils.NormalizePath(path); + return normalized.StartsWith(root, comparison); + } + return false; } } } From fb316946ce8f428cbf22bf5457541c95a872d521 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 19 Feb 2019 12:30:59 -0800 Subject: [PATCH 3/6] Merge issues --- .../Ast/Impl/Analyzer/Handlers/FromImportHandler.cs | 5 +++-- .../Ast/Impl/Modules/Definitions/IModuleResolution.cs | 6 ------ .../Impl/Implementation/Server.Documents.cs | 1 + src/LanguageServer/Test/DiagnosticsTests.cs | 8 +++++--- src/LanguageServer/Test/RdtTests.cs | 4 ++-- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs index 97f503807..e462c9b17 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs @@ -16,6 +16,7 @@ using System.Diagnostics; using System.Linq; using System.Threading; +using System.Threading.Tasks; using Microsoft.Python.Analysis.Core.DependencyResolution; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; @@ -42,7 +43,7 @@ public bool HandleFromImport(FromImportStatement node, CancellationToken cancell var imports = ModuleResolution.CurrentPathResolver.FindImports(Module.FilePath, node); switch (imports) { case ModuleImport moduleImport when moduleImport.FullName == Module.Name: - await ImportMembersFromSelfAsync(node, cancellationToken); + ImportMembersFromSelf(node); break; case ModuleImport moduleImport: ImportMembersFromModule(node, moduleImport.FullName, cancellationToken); @@ -60,7 +61,7 @@ public bool HandleFromImport(FromImportStatement node, CancellationToken cancell return false; } - private async Task ImportMembersFromSelfAsync(FromImportStatement node, CancellationToken cancellationToken = default) { + private void ImportMembersFromSelf(FromImportStatement node) { var names = node.Names; var asNames = node.AsNames; diff --git a/src/Analysis/Ast/Impl/Modules/Definitions/IModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Definitions/IModuleResolution.cs index def49f596..67aa559b4 100644 --- a/src/Analysis/Ast/Impl/Modules/Definitions/IModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Definitions/IModuleResolution.cs @@ -51,12 +51,6 @@ public interface IModuleResolution { /// IPythonModule GetOrLoadModule(string name); - /// - /// Sets user search paths. This changes . - /// - /// Added roots. - IEnumerable SetUserSearchPaths(in IEnumerable searchPaths); - Task ReloadAsync(CancellationToken token = default); } } diff --git a/src/LanguageServer/Impl/Implementation/Server.Documents.cs b/src/LanguageServer/Impl/Implementation/Server.Documents.cs index dabf4e325..2d359eb10 100644 --- a/src/LanguageServer/Impl/Implementation/Server.Documents.cs +++ b/src/LanguageServer/Impl/Implementation/Server.Documents.cs @@ -17,6 +17,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Threading; +using System.Threading.Tasks; using Microsoft.Python.Analysis; using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core; diff --git a/src/LanguageServer/Test/DiagnosticsTests.cs b/src/LanguageServer/Test/DiagnosticsTests.cs index 0d019ea0a..26b773f9f 100644 --- a/src/LanguageServer/Test/DiagnosticsTests.cs +++ b/src/LanguageServer/Test/DiagnosticsTests.cs @@ -132,7 +132,7 @@ public async Task Publish() { ReplacedSpan = new SourceSpan(1, 5, 1, 5) } }); - await doc.GetAnalysisAsync(); + await doc.GetAnalysisAsync(10000); PublishDiagnostics(); reported.Count.Should().Be(1); @@ -227,7 +227,9 @@ public async Task OnlyPublishChangedFile() { ReplacedSpan = new SourceSpan(1, 5, 1, 5) } }); - await doc1.GetAnalysisAsync(); + await doc1.GetAstAsync(); + await doc1.GetAnalysisAsync(10000); + ds.Diagnostics[doc1.Uri].Count.Should().Be(0); ds.Diagnostics[doc2.Uri].Count.Should().Be(1); @@ -241,7 +243,7 @@ public async Task OnlyPublishChangedFile() { ReplacedSpan = new SourceSpan(1, 5, 1, 6) } }); - await doc1.GetAnalysisAsync(); + await doc1.GetAnalysisAsync(1000); ds.Diagnostics[doc1.Uri].Count.Should().Be(1); ds.Diagnostics[doc2.Uri].Count.Should().Be(1); diff --git a/src/LanguageServer/Test/RdtTests.cs b/src/LanguageServer/Test/RdtTests.cs index 61434cfcb..09f25f3df 100644 --- a/src/LanguageServer/Test/RdtTests.cs +++ b/src/LanguageServer/Test/RdtTests.cs @@ -51,8 +51,8 @@ public async Task OpenCloseDocuments() { var doc1 = rdt.GetDocument(uri1); var doc2 = rdt.GetDocument(uri2); - await doc1.GetAnalysisAsync(); - await doc2.GetAnalysisAsync(); + await doc1.GetAnalysisAsync(10000); + await doc2.GetAnalysisAsync(10000); ds.Diagnostics[doc1.Uri].Count.Should().Be(1); ds.Diagnostics[doc2.Uri].Count.Should().Be(1); From c0c5ca0eb0a7b7c6e2a76e5647590118860b7a62 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 19 Feb 2019 14:52:48 -0800 Subject: [PATCH 4/6] Fix test --- src/LanguageServer/Test/DiagnosticsTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/LanguageServer/Test/DiagnosticsTests.cs b/src/LanguageServer/Test/DiagnosticsTests.cs index 26b773f9f..14a1d7e09 100644 --- a/src/LanguageServer/Test/DiagnosticsTests.cs +++ b/src/LanguageServer/Test/DiagnosticsTests.cs @@ -243,7 +243,8 @@ public async Task OnlyPublishChangedFile() { ReplacedSpan = new SourceSpan(1, 5, 1, 6) } }); - await doc1.GetAnalysisAsync(1000); + await doc1.GetAstAsync(); + await doc1.GetAnalysisAsync(10000); ds.Diagnostics[doc1.Uri].Count.Should().Be(1); ds.Diagnostics[doc2.Uri].Count.Should().Be(1); From a4fb42ac9e9a8cc3c49dda88a9df074435f97350 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 19 Feb 2019 14:54:54 -0800 Subject: [PATCH 5/6] Add await doc.GetAstAsync(); --- src/LanguageServer/Test/DiagnosticsTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/LanguageServer/Test/DiagnosticsTests.cs b/src/LanguageServer/Test/DiagnosticsTests.cs index 14a1d7e09..c67d5150d 100644 --- a/src/LanguageServer/Test/DiagnosticsTests.cs +++ b/src/LanguageServer/Test/DiagnosticsTests.cs @@ -57,7 +57,7 @@ public async Task BasicChange() { } }); await doc.GetAstAsync(); - await doc.GetAnalysisAsync(0); + await doc.GetAnalysisAsync(10000); ds.Diagnostics[doc.Uri].Count.Should().Be(0); doc.Update(new[] {new DocumentChange { @@ -132,6 +132,7 @@ public async Task Publish() { ReplacedSpan = new SourceSpan(1, 5, 1, 5) } }); + await doc.GetAstAsync(); await doc.GetAnalysisAsync(10000); PublishDiagnostics(); From 895a7eb484bb2759441b5a895d2d5180edb478d3 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 19 Feb 2019 14:57:34 -0800 Subject: [PATCH 6/6] Adjust time --- src/LanguageServer/Test/DiagnosticsTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/LanguageServer/Test/DiagnosticsTests.cs b/src/LanguageServer/Test/DiagnosticsTests.cs index c67d5150d..e0c3941a5 100644 --- a/src/LanguageServer/Test/DiagnosticsTests.cs +++ b/src/LanguageServer/Test/DiagnosticsTests.cs @@ -57,7 +57,7 @@ public async Task BasicChange() { } }); await doc.GetAstAsync(); - await doc.GetAnalysisAsync(10000); + await doc.GetAnalysisAsync(1000); ds.Diagnostics[doc.Uri].Count.Should().Be(0); doc.Update(new[] {new DocumentChange { @@ -66,7 +66,7 @@ public async Task BasicChange() { } }); await doc.GetAstAsync(); - await doc.GetAnalysisAsync(0); + await doc.GetAnalysisAsync(1000); ds.Diagnostics[doc.Uri].Count.Should().Be(1); } @@ -91,7 +91,7 @@ public async Task TwoDocuments() { } }); await doc2.GetAstAsync(); - await doc2.GetAnalysisAsync(0); + await doc2.GetAnalysisAsync(1000); ds.Diagnostics[doc1.Uri].Count.Should().Be(1); ds.Diagnostics[doc2.Uri].Count.Should().Be(0); @@ -101,7 +101,7 @@ public async Task TwoDocuments() { } }); await doc2.GetAstAsync(); - await doc2.GetAnalysisAsync(0); + await doc2.GetAnalysisAsync(1000); ds.Diagnostics[doc2.Uri].Count.Should().Be(1); doc1.Dispose(); @@ -133,7 +133,7 @@ public async Task Publish() { } }); await doc.GetAstAsync(); - await doc.GetAnalysisAsync(10000); + await doc.GetAnalysisAsync(1000); PublishDiagnostics(); reported.Count.Should().Be(1); @@ -229,7 +229,7 @@ public async Task OnlyPublishChangedFile() { } }); await doc1.GetAstAsync(); - await doc1.GetAnalysisAsync(10000); + await doc1.GetAnalysisAsync(1000); ds.Diagnostics[doc1.Uri].Count.Should().Be(0); ds.Diagnostics[doc2.Uri].Count.Should().Be(1); @@ -245,7 +245,7 @@ public async Task OnlyPublishChangedFile() { } }); await doc1.GetAstAsync(); - await doc1.GetAnalysisAsync(10000); + await doc1.GetAnalysisAsync(1000); ds.Diagnostics[doc1.Uri].Count.Should().Be(1); ds.Diagnostics[doc2.Uri].Count.Should().Be(1);