From aec8d9bc8f034381a9d09c5204f497e25f66adc3 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 2 Apr 2018 16:53:39 -0700 Subject: [PATCH 01/14] Tooltip display improvements --- .../Interpreter/Ast/AstPythonModule.cs | 16 +- .../Interpreter/Ast/AstScrapedPythonModule.cs | 13 +- .../LanguageServer/DisplayTextBuilder.cs | 161 ++++++++++++++++++ .../LanguageServer/RestTextConverter.cs | 2 +- .../Product/Analysis/LanguageServer/Server.cs | 148 +++------------- .../Analysis/LanguageServer/Structures.cs | 15 +- Python/Product/Analysis/MemberResult.cs | 146 ++++++++-------- Python/Product/Analysis/ModuleReference.cs | 5 +- Python/Product/Analysis/Values/IModule.cs | 2 + 9 files changed, 290 insertions(+), 218 deletions(-) create mode 100644 Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs diff --git a/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs b/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs index c65c7c54d3..bd4c0d0620 100644 --- a/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs +++ b/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs @@ -29,8 +29,9 @@ sealed class AstPythonModule : IPythonModule, IProjectEntry, ILocatedMember { private readonly IPythonInterpreter _interpreter; private readonly Dictionary _properties; private readonly List _childModules; - private bool _foundChildModules; private readonly Dictionary _members; + private bool _foundChildModules; + private string _documentation = string.Empty; public static IPythonModule FromFile( IPythonInterpreter interpreter, @@ -86,7 +87,6 @@ string moduleFullName internal AstPythonModule() { Name = string.Empty; - Documentation = string.Empty; FilePath = string.Empty; _properties = new Dictionary(); _childModules = new List(); @@ -96,7 +96,7 @@ internal AstPythonModule() { internal AstPythonModule(string moduleName, IPythonInterpreter interpreter, PythonAst ast, string filePath) { Name = moduleName; - Documentation = ast.Documentation; + _documentation = ast.Documentation; FilePath = filePath; DocumentUri = ProjectEntry.MakeDocumentUri(FilePath); Locations = new[] { new LocationInfo(filePath, DocumentUri, 1, 1) }; @@ -126,7 +126,15 @@ internal void AddChildModule(string name, IPythonModule module) { } public string Name { get; } - public string Documentation { get; } + public string Documentation { + get { + if(_documentation == null) { + _members.TryGetValue("__doc__", out var m); + _documentation = (m as AstPythonStringLiteral)?.Value ?? string.Empty; + } + return _documentation; + } + } public string FilePath { get; } public Uri DocumentUri { get; } public PythonMemberType MemberType => PythonMemberType.Module; diff --git a/Python/Product/Analysis/Interpreter/Ast/AstScrapedPythonModule.cs b/Python/Product/Analysis/Interpreter/Ast/AstScrapedPythonModule.cs index a217cfcfcc..7c5cee7ed4 100644 --- a/Python/Product/Analysis/Interpreter/Ast/AstScrapedPythonModule.cs +++ b/Python/Product/Analysis/Interpreter/Ast/AstScrapedPythonModule.cs @@ -40,14 +40,21 @@ class AstScrapedPythonModule : IPythonModule public AstScrapedPythonModule(string name, string filePath) { Name = name ?? throw new ArgumentNullException(nameof(name)); - _documentation = string.Empty; _filePath = filePath; _members = new Dictionary(); } public string Name { get; } - public string Documentation => _documentation; + public string Documentation { + get { + if (_documentation == null) { + var m = GetMember(null, "__doc__") as AstPythonStringLiteral; + _documentation = m != null ? m.Value : string.Empty; + } + return _documentation; + } + } public PythonMemberType MemberType => PythonMemberType.Module; @@ -158,7 +165,7 @@ public void Imported(IModuleContext context) { proc.Start(); var exitCode = proc.Wait(60000); - + if (exitCode == null) { proc.Kill(); fact.Log(TraceLevel.Error, "ScrapeTimeout", proc.FileName, proc.Arguments); diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs new file mode 100644 index 0000000000..dabebaaa3a --- /dev/null +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -0,0 +1,161 @@ +// Python Tools for Visual Studio +// Copyright(c) Microsoft Corporation +// All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the License); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS +// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY +// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABLITY 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.IO; +using System.Text; +using Microsoft.PythonTools.Analysis.Infrastructure; + +namespace Microsoft.PythonTools.Analysis.LanguageServer { + sealed class DisplayTextBuilder { + private readonly RestTextConverter _textConverter = new RestTextConverter(); + public void BuildMarkdownSignature(SignatureHelp signatureHelp) { + foreach (var s in signatureHelp.signatures) { + // Recostruct full signature so editor can display current parameter + var sb = new StringBuilder(); + + if (s.documentation != null) { + s.documentation.value = _textConverter.ToMarkdown(s.documentation.value); + } + sb.Append(s.label); + sb.Append('('); + if (s.parameters != null) { + foreach (var p in s.parameters) { + if (sb[sb.Length - 1] != '(') { + sb.Append(", "); + } + sb.Append(p.label); + if (p.documentation != null) { + p.documentation.value = _textConverter.ToMarkdown(p.documentation.value); + } + } + } + sb.Append(')'); + s.label = sb.ToString(); + } + } + + public string MakeHoverText(IEnumerable values, string originalExpression, InformationDisplayOptions displayOptions) { + string firstLongDescription = null; + var multiline = false; + var result = new StringBuilder(); + var documentations = new HashSet(); + string doc; + + foreach (var v in values) { + doc = !string.IsNullOrEmpty(v.Documentation) ? v.Documentation : v.Description; + firstLongDescription = firstLongDescription ?? doc; + + doc = displayOptions.trimDocumentationLines ? LimitLines(doc) : doc; + if (string.IsNullOrEmpty(doc)) { + continue; + } + + if (documentations.Add(doc)) { + if (documentations.Count > 1) { + if (result.Length == 0) { + // Nop + } else if (result[result.Length - 1] != '\n') { + result.Append(", "); + } else { + multiline = true; + } + } + result.Append(doc); + } + } + + if (documentations.Count == 1 && !string.IsNullOrEmpty(firstLongDescription)) { + result.Clear(); + result.Append(firstLongDescription); + } + + doc = result.ToString(); + if (displayOptions.trimDocumentationText && doc.Length > displayOptions.maxDocumentationTextLength) { + doc = doc.Substring(0, + Math.Max(3, displayOptions.maxDocumentationTextLength) - 3) + "..."; + + result.Clear(); + result.Append(doc); + } + + if (!string.IsNullOrEmpty(originalExpression)) { + if (multiline) { + result.Insert(0, originalExpression + ": " + Environment.NewLine); + } else if (result.Length > 0) { + result.Insert(0, originalExpression + ": "); + } else { + result.Append(originalExpression); + result.Append(": "); + result.Append(""); + } + } + + return _textConverter.ToMarkdown(result.ToString()); + } + + public string MakeModuleHoverText(ModuleReference modRef) { + // Return module information + var contents = "{0} : module".FormatUI(modRef.Name); + if (!string.IsNullOrEmpty(modRef.Module?.Documentation)) { + contents += $"{Environment.NewLine}{Environment.NewLine}{modRef.Module.Documentation}"; + } + return contents; + } + + private static string LimitLines( + string str, + int maxLines = 30, + int charsPerLine = 200, + bool ellipsisAtEnd = true, + bool stopAtFirstBlankLine = false + ) { + if (string.IsNullOrEmpty(str)) { + return str; + } + + var lineCount = 0; + var prettyPrinted = new StringBuilder(); + var wasEmpty = true; + + using (var reader = new StringReader(str)) { + for (var line = reader.ReadLine(); line != null && lineCount < maxLines; line = reader.ReadLine()) { + if (string.IsNullOrWhiteSpace(line)) { + if (wasEmpty) { + continue; + } + wasEmpty = true; + if (stopAtFirstBlankLine) { + lineCount = maxLines; + break; + } + lineCount += 1; + prettyPrinted.AppendLine(); + } else { + wasEmpty = false; + lineCount += (line.Length / charsPerLine) + 1; + prettyPrinted.AppendLine(line); + } + } + } + if (ellipsisAtEnd && lineCount >= maxLines) { + prettyPrinted.AppendLine("..."); + } + return prettyPrinted.ToString().Trim(); + } + } +} diff --git a/Python/Product/Analysis/LanguageServer/RestTextConverter.cs b/Python/Product/Analysis/LanguageServer/RestTextConverter.cs index 2eac49adbb..c11867f85a 100644 --- a/Python/Product/Analysis/LanguageServer/RestTextConverter.cs +++ b/Python/Product/Analysis/LanguageServer/RestTextConverter.cs @@ -86,7 +86,7 @@ private string TransformLines(string docstring) { var sb = new StringBuilder(); foreach(var s in _md) { - sb.AppendLine(s + " "); // Keep hard line breaks + sb.AppendLine(s + " "); // Keep hard line breaks } return sb.ToString().Trim(); } diff --git a/Python/Product/Analysis/LanguageServer/Server.cs b/Python/Product/Analysis/LanguageServer/Server.cs index 0f5d0f40a3..846e11a88e 100644 --- a/Python/Product/Analysis/LanguageServer/Server.cs +++ b/Python/Product/Analysis/LanguageServer/Server.cs @@ -22,7 +22,6 @@ using System.IO; using System.Linq; using System.Reflection; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.PythonTools.Analysis.Infrastructure; @@ -38,7 +37,7 @@ public sealed class Server : ServerBase, IDisposable { internal readonly ParseQueue _parseQueue; private readonly Dictionary _pendingParse; private readonly VolatileCounter _pendingAnalysisEnqueue; - private readonly RestTextConverter _textConverter = new RestTextConverter(); + private readonly DisplayTextBuilder _displayTextBuilder = new DisplayTextBuilder(); // Uri does not consider #fragment for equality private readonly ConcurrentDictionary _projectFiles; @@ -52,6 +51,7 @@ public sealed class Server : ServerBase, IDisposable { internal PythonAnalyzer _analyzer; internal ClientCapabilities _clientCaps; + private InformationDisplayOptions _displayOptions; private bool _traceLogging; private bool _testEnvironment; @@ -96,6 +96,13 @@ public async override Task Initialize(InitializeParams @params _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); } + _displayOptions = @params.initializationOptions.displayOptions ?? new InformationDisplayOptions { + trimDocumentationLines = true, + maxDocumentationLineLength = 200, + trimDocumentationText = true, + maxDocumentationTextLength = 4096 + }; + if (string.IsNullOrEmpty(_analyzer.InterpreterFactory?.Configuration?.InterpreterPath)) { LogMessage(MessageType.Log, "Initializing for generic interpreter"); } else { @@ -455,7 +462,7 @@ public override Task SignatureHelp(TextDocumentPositionParams @pa activeSignature = activeSignature, activeParameter = activeParameter }; - BuildMarkdownSignature(sh); + _displayTextBuilder.BuildMarkdownSignature(sh); return Task.FromResult(sh); } @@ -566,12 +573,12 @@ public override Task Hover(TextDocumentPositionParams @params) { var index = tree.LocationToIndex(@params.position); var w = new ImportedModuleNameWalker(entry.ModuleName, index); tree.Walk(w); - ModuleReference modRef; if (!string.IsNullOrEmpty(w.ImportedName) && - _analyzer.Modules.TryImport(w.ImportedName, out modRef)) { - - // Return module information - return Task.FromResult(new Hover { contents = "{0} : module".FormatUI(w.ImportedName) }); + _analyzer.Modules.TryImport(w.ImportedName, out var modRef)) { + var contents = _displayTextBuilder.MakeModuleHoverText(modRef); + if (contents != null) { + return Task.FromResult(new Hover { contents = contents }); + } } Expression expr; @@ -611,7 +618,10 @@ public override Task Hover(TextDocumentPositionParams @params) { var names = values.Select(GetFullTypeName).Where(n => !string.IsNullOrEmpty(n)).Distinct().ToArray(); var res = new Hover { - contents = MakeHoverText(values, originalExpr), + contents = new MarkupContent { + kind = MarkupKind.Markdown, + value = _displayTextBuilder.MakeHoverText(values, originalExpr, _displayOptions) + }, range = exprSpan, _version = version, _typeNames = names @@ -746,100 +756,6 @@ private IEnumerable GetImportNames(Uri document) { } } - private string MakeHoverText(IEnumerable values, string originalExpression) { - string firstLongDescription = null; - var multiline = false; - var result = new StringBuilder(); - var descriptions = new HashSet(); - - foreach (var v in values) { - if (string.IsNullOrEmpty(firstLongDescription)) { - firstLongDescription = v.Description; - } - - var description = LimitLines(v.ShortDescription ?? ""); - if (string.IsNullOrEmpty(description)) { - continue; - } - - if (descriptions.Add(description)) { - if (descriptions.Count > 1) { - if (result.Length == 0) { - // Nop - } else if (result[result.Length - 1] != '\n') { - result.Append(", "); - } else { - multiline = true; - } - } - result.Append(description); - } - } - - if (descriptions.Count == 1 && !string.IsNullOrEmpty(firstLongDescription)) { - result.Clear(); - result.Append(firstLongDescription); - } - - if (!string.IsNullOrEmpty(originalExpression)) { - if (originalExpression.Length > 4096) { - originalExpression = originalExpression.Substring(0, 4093) + "..."; - } - if (multiline) { - result.Insert(0, originalExpression + ": " + Environment.NewLine); - } else if (result.Length > 0) { - result.Insert(0, originalExpression + ": "); - } else { - result.Append(originalExpression); - result.Append(": "); - result.Append(""); - } - } - - return result.ToString(); - } - - internal static string LimitLines( - string str, - int maxLines = 30, - int charsPerLine = 200, - bool ellipsisAtEnd = true, - bool stopAtFirstBlankLine = false - ) { - if (string.IsNullOrEmpty(str)) { - return str; - } - - var lineCount = 0; - var prettyPrinted = new StringBuilder(); - var wasEmpty = true; - - using (var reader = new StringReader(str)) { - for (var line = reader.ReadLine(); line != null && lineCount < maxLines; line = reader.ReadLine()) { - if (string.IsNullOrWhiteSpace(line)) { - if (wasEmpty) { - continue; - } - wasEmpty = true; - if (stopAtFirstBlankLine) { - lineCount = maxLines; - break; - } - lineCount += 1; - prettyPrinted.AppendLine(); - } else { - wasEmpty = false; - lineCount += (line.Length / charsPerLine) + 1; - prettyPrinted.AppendLine(line); - } - } - } - if (ellipsisAtEnd && lineCount >= maxLines) { - prettyPrinted.AppendLine("..."); - } - return prettyPrinted.ToString().Trim(); - } - private static string GetFullTypeName(AnalysisValue value) { if (value is IHasQualifiedName qualName) { return qualName.FullyQualifiedName; @@ -1309,32 +1225,6 @@ string prefix } } - private void BuildMarkdownSignature(SignatureHelp signatureHelp) { - foreach (var s in signatureHelp.signatures) { - // Recostruct full signature so editor can display current parameter - var sb = new StringBuilder(); - - if (s.documentation != null) { - s.documentation.value = _textConverter.ToMarkdown(s.documentation.value); - } - sb.Append(s.label); - sb.Append('('); - if (s.parameters != null) { - foreach (var p in s.parameters) { - if (sb[sb.Length - 1] != '(') { - sb.Append(", "); - } - sb.Append(p.label); - if (p.documentation != null) { - p.documentation.value = _textConverter.ToMarkdown(p.documentation.value); - } - } - } - sb.Append(')'); - s.label = sb.ToString(); - } - } - private void IfTestWaitForAnalysisComplete() { if (_testEnvironment) { WaitForDirectoryScanAsync().Wait(); diff --git a/Python/Product/Analysis/LanguageServer/Structures.cs b/Python/Product/Analysis/LanguageServer/Structures.cs index 2f7267bb99..3a5ca605dc 100644 --- a/Python/Product/Analysis/LanguageServer/Structures.cs +++ b/Python/Product/Analysis/LanguageServer/Structures.cs @@ -206,6 +206,12 @@ public class MarkupContent { public static implicit operator MarkupContent(string text) => new MarkupContent { kind = MarkupKind.PlainText, value = text }; } + public class InformationDisplayOptions { + public bool trimDocumentationLines; + public int maxDocumentationLineLength; + public bool trimDocumentationText; + public int maxDocumentationTextLength; + } /// /// Required layout for the initializationOptions member of initializeParams @@ -229,6 +235,7 @@ public struct Interpreter { } public Interpreter interpreter; public string[] searchPaths; + public InformationDisplayOptions displayOptions; } @@ -508,12 +515,12 @@ public struct ExecuteCommandOptions { } [Serializable] - public struct SaveOptions { + public class SaveOptions { public bool includeText; } [Serializable] - public struct TextDocumentSyncOptions { + public class TextDocumentSyncOptions { /// /// Open and close notifications are sent to the server. /// @@ -521,12 +528,12 @@ public struct TextDocumentSyncOptions { public TextDocumentSyncKind change; public bool willSave; public bool willSaveWaitUntil; - public SaveOptions? save; + public SaveOptions save; } [Serializable] public struct ServerCapabilities { - public TextDocumentSyncOptions? textDocumentSync; + public TextDocumentSyncOptions textDocumentSync; public bool hoverProvider; public CompletionOptions? completionProvider; public SignatureHelpOptions? signatureHelpProvider; diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index ca16c33e4a..d186692afc 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -25,8 +25,6 @@ namespace Microsoft.PythonTools.Analysis { public struct MemberResult { - private readonly string _name; - private string _completion; private readonly Lazy> _vars; private readonly Lazy _type; @@ -36,22 +34,22 @@ public struct MemberResult { new Lazy>(Enumerable.Empty); internal MemberResult(string name, IEnumerable vars) { - _name = _completion = name; + Name = Completion = name; _vars = new Lazy>(() => vars.MaybeEnumerate()); _type = UnknownType; _type = new Lazy(GetMemberType); } public MemberResult(string name, PythonMemberType type) { - _name = _completion = name; + Name = Completion = name; _type = new Lazy(() => type); _vars = EmptyValues; } public MemberResult(string name, string completion, IEnumerable vars, PythonMemberType? type) { - _name = name; + Name = name; _vars = new Lazy>(() => vars.MaybeEnumerate()); - _completion = completion; + Completion = completion; _type = UnknownType; if (type != null) { _type = new Lazy(() => type.Value); @@ -61,7 +59,7 @@ public MemberResult(string name, string completion, IEnumerable v } internal MemberResult(string name, Func> vars, Func type) { - _name = _completion = name; + Name = Completion = name; _vars = vars == null ? EmptyValues : new Lazy>(vars); _type = type == null ? UnknownType : new Lazy(type); } @@ -70,13 +68,8 @@ public MemberResult FilterCompletion(string completion) { return new MemberResult(Name, completion, Values, MemberType); } - public string Name { - get { return _name; } - } - - public string Completion { - get { return _completion; } - } + public string Name { get; } + public string Completion { get; } private static string GetDescription(AnalysisValue ns) { var d = ns?.ShortDescription; @@ -106,71 +99,83 @@ private static IEnumerable SeparateMultipleMembers(IEnumerable>(); - var allTypes = new HashSet(); - - foreach (var ns in SeparateMultipleMembers(Values)) { - var docString = ns.Documentation?.TrimDocumentation(); - var typeString = GetDescription(ns); - if (string.IsNullOrEmpty(docString)) { - docString = ""; - } - if (!docs.TryGetValue(docString, out var docTypes)) { - docs[docString] = docTypes = new HashSet(); - } - if (!string.IsNullOrEmpty(typeString)) { - docTypes.Add(typeString); - allTypes.Add(typeString); - } + var value = Values.FirstOrDefault(); + if (value == null) { + return string.Empty; } + switch (value.MemberType) { + case PythonMemberType.Module: + return value.Documentation ?? string.Empty; + } + return GetDocumentation(); + } + } - var doc = new StringBuilder(); + private string GetDocumentation() { + var docs = new Dictionary>(); + var allTypes = new HashSet(); - if (allTypes.Count == 0) { - return "unknown type"; - } else if (allTypes.Count == 1) { - doc.AppendLine(allTypes.First()); - doc.AppendLine(); - } else { - var types = allTypes.OrderBy(s => s).ToList(); - var orStr = types.Count == 2 ? " or " : ", or "; - doc.AppendLine(string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last()); - doc.AppendLine(); + foreach (var ns in SeparateMultipleMembers(Values)) { + var docString = ns.Documentation?.TrimDocumentation(); + var typeString = GetDescription(ns); + if (string.IsNullOrEmpty(docString)) { + docString = ""; } - var typeToDoc = new Dictionary(); - foreach (var docType in docs) { - if (string.IsNullOrEmpty(docType.Key)) { - continue; - } + // If first line of doc is already in the type string, then filter it out. + // This is because some functions have signature as a first doc line and + // some do not have one. We are already showing signature as part of the type. + var lines = docString.Split(new char[] { '\n' }).Where(x => x != "\r").ToArray(); + if(lines.Length > 0 && typeString.IndexOf(lines[0].Trim()) >= 0) { + docString = string.Join(Environment.NewLine, lines.Skip(1).ToArray()); + } - string typeDisplay = "unknown type"; - var types = docType.Value.OrderBy(s => s).ToList(); - if (types.Count == 0) { - continue; - } else if (types.Count == 1) { - typeDisplay = types[0]; - } else { - var orStr = types.Count == 2 ? " or " : ", or "; - typeDisplay = string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last(); - } - typeToDoc[string.Join(",", types)] = typeDisplay + ": " + docType.Key; + if (!docs.TryGetValue(docString, out var docTypes)) { + docs[docString] = docTypes = new HashSet(); + } + if (!string.IsNullOrEmpty(typeString)) { + docTypes.Add(typeString); + allTypes.Add(typeString); + } + } + + var doc = new StringBuilder(); + if (allTypes.Count == 0) { + return "unknown type"; + } + + var typeToDoc = new Dictionary(); + foreach (var docType in docs) { + if (string.IsNullOrEmpty(docType.Key)) { + continue; } - foreach (var typeDoc in typeToDoc.OrderBy(kv => kv.Key)) { - doc.AppendLine(typeDoc.Value); - doc.AppendLine(); + string typeDisplay = "unknown type"; + var types = docType.Value.OrderBy(s => s).ToList(); + if (types.Count == 0) { + continue; + } else if (types.Count == 1) { + typeDisplay = types[0]; + } else { + var orStr = types.Count == 2 ? " or " : ", or "; + typeDisplay = string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last(); } + typeToDoc[string.Join(",", types)] = typeDisplay + ":\n" + docType.Key; + } - return Utils.CleanDocumentation(doc.ToString()); + foreach (var typeDoc in typeToDoc.OrderBy(kv => kv.Key)) { + doc.AppendLine(typeDoc.Value); + doc.AppendLine(); } + + return Utils.CleanDocumentation(doc.ToString()); } public PythonMemberType MemberType => _type.Value; private PythonMemberType GetMemberType() { - bool includesNone = false; - PythonMemberType result = PythonMemberType.Unknown; + var includesNone = false; + var result = PythonMemberType.Unknown; var allVars = Values.SelectMany(ns => { var mmi = ns as MultipleMemberInfo; @@ -234,20 +239,11 @@ public override bool Equals(object obj) { if (!(obj is MemberResult)) { return false; } - return Name == ((MemberResult)obj).Name; } - public static bool operator ==(MemberResult x, MemberResult y) { - return x.Name == y.Name; - } - - public static bool operator !=(MemberResult x, MemberResult y) { - return x.Name != y.Name; - } - - public override int GetHashCode() { - return Name.GetHashCode(); - } + public static bool operator ==(MemberResult x, MemberResult y) => x.Name == y.Name; + public static bool operator !=(MemberResult x, MemberResult y) => x.Name != y.Name; + public override int GetHashCode() => Name.GetHashCode(); } } diff --git a/Python/Product/Analysis/ModuleReference.cs b/Python/Product/Analysis/ModuleReference.cs index 20d0ac1ddf..fc94e72c17 100644 --- a/Python/Product/Analysis/ModuleReference.cs +++ b/Python/Product/Analysis/ModuleReference.cs @@ -24,13 +24,14 @@ class ModuleReference { public IModule Module; private readonly Lazy> _references = new Lazy>(); + private string _name; public ModuleReference(IModule module = null, string name = null) { Module = module; - name = name ?? ""; + _name = name; } - public string Name { get; } + public string Name => (_name ?? AnalysisModule?.Name) ?? string.Empty; public AnalysisValue AnalysisModule { get { diff --git a/Python/Product/Analysis/Values/IModule.cs b/Python/Product/Analysis/Values/IModule.cs index ef607033c7..31f49385fd 100644 --- a/Python/Product/Analysis/Values/IModule.cs +++ b/Python/Product/Analysis/Values/IModule.cs @@ -30,5 +30,7 @@ interface IModule { IEnumerable GetModuleMemberNames(IModuleContext context); IAnalysisSet GetModuleMember(Node node, AnalysisUnit unit, string name, bool addRef = true, InterpreterScope linkedScope = null, string linkedName = null); void Imported(AnalysisUnit unit); + string Description { get; } + string Documentation { get; } } } From e28931b1304dfd7c6e4944ee6bcc9ad34d7d38ba Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 3 Apr 2018 13:23:48 -0700 Subject: [PATCH 02/14] Tooltip and completion doc display --- .../Interpreter/Ast/AstPythonModule.cs | 4 + .../LanguageServer/DisplayTextBuilder.cs | 26 ++-- Python/Product/Analysis/MemberResult.cs | 115 ++++++++---------- .../Product/Analysis/Values/BuiltinModule.cs | 34 ++---- 4 files changed, 69 insertions(+), 110 deletions(-) diff --git a/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs b/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs index bd4c0d0620..72245c5df6 100644 --- a/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs +++ b/Python/Product/Analysis/Interpreter/Ast/AstPythonModule.cs @@ -131,6 +131,10 @@ public string Documentation { if(_documentation == null) { _members.TryGetValue("__doc__", out var m); _documentation = (m as AstPythonStringLiteral)?.Value ?? string.Empty; + if(string.IsNullOrEmpty(_documentation)) { + _members.TryGetValue($"_{Name}", out m); + _documentation = (m as AstNestedPythonModule)?.Documentation ?? string.Empty; + } } return _documentation; } diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index dabebaaa3a..c2aeb88778 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -51,13 +51,13 @@ public void BuildMarkdownSignature(SignatureHelp signatureHelp) { public string MakeHoverText(IEnumerable values, string originalExpression, InformationDisplayOptions displayOptions) { string firstLongDescription = null; - var multiline = false; var result = new StringBuilder(); var documentations = new HashSet(); - string doc; foreach (var v in values) { - doc = !string.IsNullOrEmpty(v.Documentation) ? v.Documentation : v.Description; + var doc = !string.IsNullOrEmpty(v.Documentation) ? v.Documentation : string.Empty; + var desc = !string.IsNullOrEmpty(v.Description) ? v.Description : string.Empty; + doc = doc.Length > desc.Length ? doc : desc; firstLongDescription = firstLongDescription ?? doc; doc = displayOptions.trimDocumentationLines ? LimitLines(doc) : doc; @@ -71,8 +71,6 @@ public string MakeHoverText(IEnumerable values, string originalEx // Nop } else if (result[result.Length - 1] != '\n') { result.Append(", "); - } else { - multiline = true; } } result.Append(doc); @@ -84,24 +82,18 @@ public string MakeHoverText(IEnumerable values, string originalEx result.Append(firstLongDescription); } - doc = result.ToString(); - if (displayOptions.trimDocumentationText && doc.Length > displayOptions.maxDocumentationTextLength) { - doc = doc.Substring(0, + var displayText = result.ToString(); + if (displayOptions.trimDocumentationText && displayText.Length > displayOptions.maxDocumentationTextLength) { + displayText = displayText.Substring(0, Math.Max(3, displayOptions.maxDocumentationTextLength) - 3) + "..."; result.Clear(); - result.Append(doc); + result.Append(displayText); } if (!string.IsNullOrEmpty(originalExpression)) { - if (multiline) { - result.Insert(0, originalExpression + ": " + Environment.NewLine); - } else if (result.Length > 0) { - result.Insert(0, originalExpression + ": "); - } else { - result.Append(originalExpression); - result.Append(": "); - result.Append(""); + if (result.Length == 0) { + result.Append($"{originalExpression}: "); } } diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index d186692afc..b42840f050 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -33,6 +33,7 @@ public struct MemberResult { private static readonly Lazy> EmptyValues = new Lazy>(Enumerable.Empty); + #region Constructors internal MemberResult(string name, IEnumerable vars) { Name = Completion = name; _vars = new Lazy>(() => vars.MaybeEnumerate()); @@ -63,6 +64,7 @@ internal MemberResult(string name, Func> vars, Func

>(vars); _type = type == null ? UnknownType : new Lazy(type); } + #endregion public MemberResult FilterCompletion(string completion) { return new MemberResult(Name, completion, Values, MemberType); @@ -70,63 +72,31 @@ public MemberResult FilterCompletion(string completion) { public string Name { get; } public string Completion { get; } + public PythonMemberType MemberType => _type.Value; - private static string GetDescription(AnalysisValue ns) { - var d = ns?.ShortDescription; - if (string.IsNullOrEmpty(d)) { - return null; - } - switch (ns.MemberType) { - case PythonMemberType.Instance: - return "instance of " + d; - case PythonMemberType.Constant: - return "constant " + d; - } - return d; - } + ///

+ /// Gets the location(s) for the member(s) if they are available. + /// + /// New in 1.5. + /// + public IEnumerable Locations => Values.SelectMany(ns => ns.Locations); - private static IEnumerable SeparateMultipleMembers(IEnumerable values) { - foreach (var v in values) { - if (v is MultipleMemberInfo mm) { - foreach (var m in mm.Members) { - yield return m; - } - } else { - yield return v; - } - } - } + public string Documentation => GetDocumentation(); - public string Documentation { - get { - var value = Values.FirstOrDefault(); - if (value == null) { - return string.Empty; - } - switch (value.MemberType) { - case PythonMemberType.Module: - return value.Documentation ?? string.Empty; - } - return GetDocumentation(); - } - } + internal IEnumerable Values => _vars.Value; private string GetDocumentation() { var docs = new Dictionary>(); - var allTypes = new HashSet(); foreach (var ns in SeparateMultipleMembers(Values)) { - var docString = ns.Documentation?.TrimDocumentation(); + var docString = ns.Documentation?.TrimDocumentation() ?? string.Empty; var typeString = GetDescription(ns); - if (string.IsNullOrEmpty(docString)) { - docString = ""; - } // If first line of doc is already in the type string, then filter it out. // This is because some functions have signature as a first doc line and // some do not have one. We are already showing signature as part of the type. var lines = docString.Split(new char[] { '\n' }).Where(x => x != "\r").ToArray(); - if(lines.Length > 0 && typeString.IndexOf(lines[0].Trim()) >= 0) { + if(typeString != null && lines.Length > 0 && typeString.IndexOf(lines[0].Trim()) >= 0) { docString = string.Join(Environment.NewLine, lines.Skip(1).ToArray()); } @@ -135,43 +105,65 @@ private string GetDocumentation() { } if (!string.IsNullOrEmpty(typeString)) { docTypes.Add(typeString); - allTypes.Add(typeString); } } var doc = new StringBuilder(); - if (allTypes.Count == 0) { - return "unknown type"; - } - - var typeToDoc = new Dictionary(); + var typeToDoc = new Dictionary>(); foreach (var docType in docs) { if (string.IsNullOrEmpty(docType.Key)) { continue; } + if (!docType.Value.Any()) { + continue; + } - string typeDisplay = "unknown type"; + var typeDisplay = "unknown type"; var types = docType.Value.OrderBy(s => s).ToList(); - if (types.Count == 0) { - continue; - } else if (types.Count == 1) { + if (types.Count == 1) { typeDisplay = types[0]; } else { var orStr = types.Count == 2 ? " or " : ", or "; typeDisplay = string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last(); } - typeToDoc[string.Join(",", types)] = typeDisplay + ":\n" + docType.Key; + typeToDoc[string.Join(",", types)] = new Tuple(typeDisplay, docType.Key); } foreach (var typeDoc in typeToDoc.OrderBy(kv => kv.Key)) { - doc.AppendLine(typeDoc.Value); + doc.AppendLine(typeDoc.Value.Item1 + ":"); + doc.Append(Utils.CleanDocumentation(typeDoc.Value.Item2)); + doc.AppendLine(); doc.AppendLine(); } - return Utils.CleanDocumentation(doc.ToString()); + return doc.ToString().Trim(); } - public PythonMemberType MemberType => _type.Value; + private static string GetDescription(AnalysisValue ns) { + var d = ns?.ShortDescription; + if (string.IsNullOrEmpty(d)) { + return null; + } + switch (ns.MemberType) { + case PythonMemberType.Instance: + return "instance of " + d; + case PythonMemberType.Constant: + return "constant " + d; + } + return d; + } + + private static IEnumerable SeparateMultipleMembers(IEnumerable values) { + foreach (var v in values) { + if (v is MultipleMemberInfo mm) { + foreach (var m in mm.Members) { + yield return m; + } + } else { + yield return v; + } + } + } private PythonMemberType GetMemberType() { var includesNone = false; @@ -226,15 +218,6 @@ private PythonMemberType GetMemberType() { return result; } - internal IEnumerable Values => _vars.Value; - - /// - /// Gets the location(s) for the member(s) if they are available. - /// - /// New in 1.5. - /// - public IEnumerable Locations => Values.SelectMany(ns => ns.Locations); - public override bool Equals(object obj) { if (!(obj is MemberResult)) { return false; diff --git a/Python/Product/Analysis/Values/BuiltinModule.cs b/Python/Product/Analysis/Values/BuiltinModule.cs index 6a4ce1c6a8..022c447e5d 100644 --- a/Python/Product/Analysis/Values/BuiltinModule.cs +++ b/Python/Product/Analysis/Values/BuiltinModule.cs @@ -14,6 +14,7 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. +using System; using System.Collections.Generic; using System.Linq; using Microsoft.PythonTools.Analysis.Analyzer; @@ -50,7 +51,7 @@ public override IDictionary GetAllMembers(IModuleContext m if (_specializedValues != null) { foreach (var value in _specializedValues) { IAnalysisSet existing; - if(!res.TryGetValue(value.Key, out existing)) { + if (!res.TryGetValue(value.Key, out existing)) { res[value.Key] = value.Value; } else { var newSet = existing.Union(value.Value, canMutate: false); @@ -61,33 +62,12 @@ public override IDictionary GetAllMembers(IModuleContext m return res; } - public override string Documentation { - get { - return _type.Documentation; - } - } - - public override string Description { - get { - return "built-in module " + _interpreterModule.Name; - } - } - - public override string Name { - get { - return _interpreterModule.Name; - } - } - - public override IPythonType PythonType { - get { - return this.ProjectState.Types[BuiltinTypeId.Module]; - } - } + public override string Documentation => $"{Description}{Environment.NewLine}{_type.Documentation}"; + public override string Description => $"built-in module {_interpreterModule.Name}"; + public override string Name => _interpreterModule.Name; - public override PythonMemberType MemberType { - get { return _interpreterModule.MemberType; } - } + public override IPythonType PythonType => ProjectState.Types[BuiltinTypeId.Module]; + public override PythonMemberType MemberType => _interpreterModule.MemberType; internal override BuiltinTypeId TypeId => BuiltinTypeId.Module; From 2539d870523331d08885f87532ddf904fe86c645 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 3 Apr 2018 16:52:35 -0700 Subject: [PATCH 03/14] Test fixes --- Python/Product/Analysis/Analysis.csproj | 1 + .../LanguageServer/DisplayTextBuilder.cs | 34 ++++++++++++++++--- Python/Product/Analysis/MemberResult.cs | 19 +++++++---- Python/Tests/Analysis/LanguageServerTests.cs | 6 ++-- 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/Python/Product/Analysis/Analysis.csproj b/Python/Product/Analysis/Analysis.csproj index 24dd6375b0..76f7d7fe28 100644 --- a/Python/Product/Analysis/Analysis.csproj +++ b/Python/Product/Analysis/Analysis.csproj @@ -103,6 +103,7 @@ + diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index c2aeb88778..8a4bb93ec9 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -19,6 +19,7 @@ using System.IO; using System.Text; using Microsoft.PythonTools.Analysis.Infrastructure; +using Microsoft.PythonTools.Interpreter; namespace Microsoft.PythonTools.Analysis.LanguageServer { sealed class DisplayTextBuilder { @@ -39,6 +40,14 @@ public void BuildMarkdownSignature(SignatureHelp signatureHelp) { sb.Append(", "); } sb.Append(p.label); + if (!string.IsNullOrEmpty(p._type)) { + sb.Append(':'); + sb.Append(p._type); + } + if(!string.IsNullOrEmpty(p._defaultValue)) { + sb.Append('='); + sb.Append(p._defaultValue); + } if (p.documentation != null) { p.documentation.value = _textConverter.ToMarkdown(p.documentation.value); } @@ -51,13 +60,12 @@ public void BuildMarkdownSignature(SignatureHelp signatureHelp) { public string MakeHoverText(IEnumerable values, string originalExpression, InformationDisplayOptions displayOptions) { string firstLongDescription = null; + var multiline = false; var result = new StringBuilder(); var documentations = new HashSet(); foreach (var v in values) { - var doc = !string.IsNullOrEmpty(v.Documentation) ? v.Documentation : string.Empty; - var desc = !string.IsNullOrEmpty(v.Description) ? v.Description : string.Empty; - doc = doc.Length > desc.Length ? doc : desc; + var doc = GetDocString(v); firstLongDescription = firstLongDescription ?? doc; doc = displayOptions.trimDocumentationLines ? LimitLines(doc) : doc; @@ -71,6 +79,8 @@ public string MakeHoverText(IEnumerable values, string originalEx // Nop } else if (result[result.Length - 1] != '\n') { result.Append(", "); + } else { + multiline = true; } } result.Append(doc); @@ -92,7 +102,14 @@ public string MakeHoverText(IEnumerable values, string originalEx } if (!string.IsNullOrEmpty(originalExpression)) { - if (result.Length == 0) { + if (originalExpression.Length > 4096) { + originalExpression = originalExpression.Substring(0, 4093) + "..."; + } + if (multiline) { + result.Insert(0, $"{originalExpression}:{Environment.NewLine}"); + } else if (result.Length > 0) { + result.Insert(0, $"{originalExpression}: "); + } else { result.Append($"{originalExpression}: "); } } @@ -109,6 +126,15 @@ public string MakeModuleHoverText(ModuleReference modRef) { return contents; } + private static string GetDocString(AnalysisValue v) { + var doc = !string.IsNullOrEmpty(v.Documentation) ? v.Documentation : string.Empty; + var desc = !string.IsNullOrEmpty(v.Description) ? v.Description : string.Empty; + if (v.MemberType == PythonMemberType.Instance || v.MemberType == PythonMemberType.Constant) { + return !string.IsNullOrEmpty(desc) ? desc : doc; + } + return doc.Length > desc.Length ? doc : desc; + } + private static string LimitLines( string str, int maxLines = 30, diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index b42840f050..19ce88b8ee 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -96,7 +96,7 @@ private string GetDocumentation() { // This is because some functions have signature as a first doc line and // some do not have one. We are already showing signature as part of the type. var lines = docString.Split(new char[] { '\n' }).Where(x => x != "\r").ToArray(); - if(typeString != null && lines.Length > 0 && typeString.IndexOf(lines[0].Trim()) >= 0) { + if(!string.IsNullOrEmpty(docString) && typeString != null && lines.Length > 1 && typeString.IndexOf(lines[0].Trim()) >= 0) { docString = string.Join(Environment.NewLine, lines.Skip(1).ToArray()); } @@ -111,9 +111,6 @@ private string GetDocumentation() { var doc = new StringBuilder(); var typeToDoc = new Dictionary>(); foreach (var docType in docs) { - if (string.IsNullOrEmpty(docType.Key)) { - continue; - } if (!docType.Value.Any()) { continue; } @@ -130,8 +127,18 @@ private string GetDocumentation() { } foreach (var typeDoc in typeToDoc.OrderBy(kv => kv.Key)) { - doc.AppendLine(typeDoc.Value.Item1 + ":"); - doc.Append(Utils.CleanDocumentation(typeDoc.Value.Item2)); + doc.Append(typeDoc.Value.Item1); + if (!string.IsNullOrEmpty(typeDoc.Value.Item2)) { + var cleaned = Utils.CleanDocumentation(typeDoc.Value.Item2); + if (!string.IsNullOrEmpty(cleaned)) { + if (cleaned.IndexOf('\n') >= 0) { + doc.AppendLine(":"); + } else { + doc.Append(": "); + } + doc.Append(cleaned); + } + } doc.AppendLine(); doc.AppendLine(); } diff --git a/Python/Tests/Analysis/LanguageServerTests.cs b/Python/Tests/Analysis/LanguageServerTests.cs index 8665366676..92f780549a 100644 --- a/Python/Tests/Analysis/LanguageServerTests.cs +++ b/Python/Tests/Analysis/LanguageServerTests.cs @@ -336,7 +336,7 @@ def f(a, *b, **c): pass "); await AssertSignature(s, mod, new SourceLocation(1, 3), - new string[] { "f()", "f(a:=)", "f(a:=,b:=)", "f(a:=,*b:tuple=)", "f(a:=,**b:dict=)", "f(a:=,*b:tuple=,**c:dict=)" }, + new string[] { "f()", "f(a)", "f(a, b)", "f(a, *b:tuple)", "f(a, **b:dict)", "f(a, *b:tuple, **c:dict)" }, new string[0] ); @@ -501,7 +501,7 @@ def g(self): await AssertHover(s, mod, new SourceLocation(13, 1), "c: C", new[] { "test-module.C" }, new SourceSpan(13, 1, 13, 2)); await AssertHover(s, mod, new SourceLocation(14, 7), "c: C", new[] { "test-module.C" }, new SourceSpan(14, 7, 14, 8)); await AssertHover(s, mod, new SourceLocation(14, 9), "c.f: method f of test-module.C objects*", new[] { "test-module.C.f" }, new SourceSpan(14, 7, 14, 10)); - await AssertHover(s, mod, new SourceLocation(14, 1), "c_g: def test-module.C.f.g(self)\r\ndeclared in C.f", new[] { "test-module.C.f.g" }, new SourceSpan(14, 1, 14, 4)); + await AssertHover(s, mod, new SourceLocation(14, 1), "c_g: def test-module.C.f.g(self) \r\ndeclared in C.f", new[] { "test-module.C.f.g" }, new SourceSpan(14, 1, 14, 4)); await AssertHover(s, mod, new SourceLocation(16, 1), "x: int, float", new[] { "int", "float" }, new SourceSpan(16, 1, 16, 2)); } @@ -644,7 +644,7 @@ public static async Task AssertSignature(Server s, TextDocumentIdentifier docume })).signatures; AssertUtil.CheckCollection( - sigs.Select(sig => $"{sig.label}({string.Join(",", sig.parameters.Select(p => $"{p.label}:{p._type}={p._defaultValue}"))})"), + sigs.Select(sig => sig.label), contains, excludes ); From 550356ba248b07ec76c19f13a99218ef3837c14b Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 4 Apr 2018 16:26:00 -0700 Subject: [PATCH 04/14] Async analyzer creation mode --- .../Product/Analysis/LanguageServer/Server.cs | 107 +++++++++++------- .../Analysis/LanguageServer/Structures.cs | 7 ++ 2 files changed, 76 insertions(+), 38 deletions(-) diff --git a/Python/Product/Analysis/LanguageServer/Server.cs b/Python/Product/Analysis/LanguageServer/Server.cs index 846e11a88e..a11f489732 100644 --- a/Python/Product/Analysis/LanguageServer/Server.cs +++ b/Python/Product/Analysis/LanguageServer/Server.cs @@ -46,6 +46,7 @@ public sealed class Server : ServerBase, IDisposable { // For pending changes, we use alternate comparer that checks #fragment private readonly ConcurrentDictionary> _pendingChanges; private readonly ManualResetEventSlim _documentChangeProcessingComplete = new ManualResetEventSlim(true); + private readonly TaskCompletionSource _analyzerCreationTcs = new TaskCompletionSource(); internal Task _loadingFromDirectory; @@ -86,16 +87,49 @@ private void TraceMessage(IFormattable message) { #region Client message handling - public async override Task Initialize(InitializeParams @params) { + public override async Task Initialize(InitializeParams @params) { _testEnvironment = @params.initializationOptions.interpreter.properties.ContainsKey("TestEnvironment"); if (_testEnvironment) { // Test environment needs predictable initialization. // Tests can only proceed when analysis is fully done. _analyzer = CreateAnalyzer(@params.initializationOptions.interpreter).Result; } else { - _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); + if (@params.initializationOptions.asyncStartup) { + CreateAnalyzer(@params.initializationOptions.interpreter).ContinueWith(t => { + if (t.IsFaulted) { + _analyzerCreationTcs.TrySetException(t.Exception); + } else { + try { + _analyzer = t.Result; + OnAnalyzerCreated(@params); + _analyzerCreationTcs.TrySetResult(true); + } catch (Exception ex) { + _analyzerCreationTcs.TrySetException(ex); + } + } + }).DoNotWait(); + } else { + _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); + _analyzerCreationTcs.TrySetResult(true); + } } + return new InitializeResult { + capabilities = new ServerCapabilities { + textDocumentSync = new TextDocumentSyncOptions { openClose = true, change = TextDocumentSyncKind.Incremental }, + completionProvider = new CompletionOptions { + triggerCharacters = new[] { "." }, + resolveProvider = true + }, + hoverProvider = true, + signatureHelpProvider = new SignatureHelpOptions { triggerCharacters = new[] { "(,)" } }, + // https://github.com/Microsoft/PTVS/issues/3803 + // definitionProvider = true, + referencesProvider = true + } + }; + } + private void OnAnalyzerCreated(InitializeParams @params) { _displayOptions = @params.initializationOptions.displayOptions ?? new InformationDisplayOptions { trimDocumentationLines = true, maxDocumentationLineLength = 200, @@ -126,21 +160,6 @@ public async override Task Initialize(InitializeParams @params LogMessage(MessageType.Log, $"Loading files from {_rootDir}"); _loadingFromDirectory = LoadFromDirectoryAsync(_rootDir); } - - return new InitializeResult { - capabilities = new ServerCapabilities { - textDocumentSync = new TextDocumentSyncOptions { openClose = true, change = TextDocumentSyncKind.Incremental }, - completionProvider = new CompletionOptions { - triggerCharacters = new[] { "." }, - resolveProvider = true - }, - hoverProvider = true, - signatureHelpProvider = new SignatureHelpOptions { triggerCharacters = new[] { "(,)" } }, - // https://github.com/Microsoft/PTVS/issues/3803 - // definitionProvider = true, - referencesProvider = true - } - }; } public override Task Shutdown() { @@ -151,6 +170,7 @@ public override Task Shutdown() { public override async Task DidOpenTextDocument(DidOpenTextDocumentParams @params) { TraceMessage($"Opening document {@params.textDocument.uri}"); + await _analyzerCreationTcs.Task; var entry = GetEntry(@params.textDocument.uri, throwIfMissing: false); var doc = entry as IDocument; @@ -175,6 +195,8 @@ public override async Task DidOpenTextDocument(DidOpenTextDocumentParams @params } public override void DidChangeTextDocument(DidChangeTextDocumentParams @params) { + _analyzerCreationTcs.Task.Wait(); + var changes = @params.contentChanges; if (changes == null) { return; @@ -243,6 +265,8 @@ public override void DidChangeTextDocument(DidChangeTextDocumentParams @params) } public override async Task DidChangeWatchedFiles(DidChangeWatchedFilesParams @params) { + await _analyzerCreationTcs.Task; + IProjectEntry entry; foreach (var c in @params.changes.MaybeEnumerate()) { switch (c.type) { @@ -269,7 +293,8 @@ public override async Task DidChangeWatchedFiles(DidChangeWatchedFilesParams @pa } } - public override Task DidCloseTextDocument(DidCloseTextDocumentParams @params) { + public override async Task DidCloseTextDocument(DidCloseTextDocumentParams @params) { + await _analyzerCreationTcs.Task; var doc = GetEntry(@params.textDocument.uri) as IDocument; if (doc != null) { @@ -279,10 +304,10 @@ public override Task DidCloseTextDocument(DidCloseTextDocumentParams @params) { // Pick up any changes on disk that we didn't know about EnqueueItem(doc, AnalysisPriority.Low); } - return Task.CompletedTask; } public override async Task DidChangeConfiguration(DidChangeConfigurationParams @params) { + await _analyzerCreationTcs.Task; if (_analyzer == null) { LogMessage(MessageType.Error, "change configuration notification sent to uninitialized server"); return; @@ -296,7 +321,8 @@ public override async Task DidChangeConfiguration(DidChangeConfigurationParams @ } } - public override Task Completion(CompletionParams @params) { + public override async Task Completion(CompletionParams @params) { + await _analyzerCreationTcs.Task; IfTestWaitForAnalysisComplete(); // Make sure document is enqueued for processing _documentChangeProcessingComplete.Wait(200, CancellationToken); @@ -309,7 +335,7 @@ public override Task Completion(CompletionParams @params) { var analysis = entry?.Analysis; if (analysis == null) { TraceMessage($"No analysis found for {uri}"); - return Task.FromResult(new CompletionList { }); + return new CompletionList(); } var opts = GetMemberOptions.None; @@ -386,7 +412,7 @@ public override Task Completion(CompletionParams @params) { if (members == null) { TraceMessage($"No members found in document {uri}"); - return Task.FromResult(new CompletionList { }); + return new CompletionList(); } var filtered = members.Select(m => ToCompletionItem(m, opts)); @@ -398,7 +424,7 @@ public override Task Completion(CompletionParams @params) { var res = new CompletionList { items = filtered.ToArray() }; LogMessage(MessageType.Info, $"Found {res.items.Length} completions for {uri} at {@params.position} after filtering"); - return Task.FromResult(res); + return res; } public override Task CompletionItemResolve(CompletionItem item) { @@ -406,7 +432,8 @@ public override Task CompletionItemResolve(CompletionItem item) return Task.FromResult(item); } - public override Task SignatureHelp(TextDocumentPositionParams @params) { + public override async Task SignatureHelp(TextDocumentPositionParams @params) { + await _analyzerCreationTcs.Task; IfTestWaitForAnalysisComplete(); var uri = @params.textDocument.uri; @@ -417,7 +444,7 @@ public override Task SignatureHelp(TextDocumentPositionParams @pa var analysis = entry?.Analysis; if (analysis == null) { TraceMessage($"No analysis found for {uri}"); - return Task.FromResult(new SignatureHelp { }); + return new SignatureHelp(); } IEnumerable overloads; @@ -439,7 +466,7 @@ public override Task SignatureHelp(TextDocumentPositionParams @pa } } else { LogMessage(MessageType.Info, $"No signatures found in {uri} at {@params.position}"); - return Task.FromResult(new SignatureHelp { }); + return new SignatureHelp(); } } @@ -463,10 +490,12 @@ public override Task SignatureHelp(TextDocumentPositionParams @pa activeParameter = activeParameter }; _displayTextBuilder.BuildMarkdownSignature(sh); - return Task.FromResult(sh); + return sh; } - public override Task FindReferences(ReferencesParams @params) { + public override async Task FindReferences(ReferencesParams @params) { + await _analyzerCreationTcs.Task; + var uri = @params.textDocument.uri; GetAnalysis(@params.textDocument, @params.position, @params._version, out var entry, out var tree); @@ -475,7 +504,7 @@ public override Task FindReferences(ReferencesParams @params) { var analysis = entry?.Analysis; if (analysis == null) { TraceMessage($"No analysis found for {uri}"); - return Task.FromResult(Array.Empty()); + return Array.Empty(); } int? version = null; @@ -526,7 +555,7 @@ public override Task FindReferences(ReferencesParams @params) { result = analysis.GetVariables(expr, @params.position); } else { LogMessage(MessageType.Info, $"No references found in {uri} at {@params.position}"); - return Task.FromResult(Array.Empty()); + return Array.Empty(); } } @@ -551,10 +580,11 @@ public override Task FindReferences(ReferencesParams @params) { .GroupBy(r => r, ReferenceComparer.Instance) .Select(g => g.OrderByDescending(r => (SourceLocation)r.range.end).ThenBy(r => (int?)r._kind ?? int.MaxValue).First()) .ToArray(); - return Task.FromResult(res); + return res; } - public override Task Hover(TextDocumentPositionParams @params) { + public override async Task Hover(TextDocumentPositionParams @params) { + await _analyzerCreationTcs.Task; IfTestWaitForAnalysisComplete(); var uri = @params.textDocument.uri; @@ -565,7 +595,7 @@ public override Task Hover(TextDocumentPositionParams @params) { var analysis = entry?.Analysis; if (analysis == null) { TraceMessage($"No analysis found for {uri}"); - return Task.FromResult(default(Hover)); + return default(Hover); } tree = GetParseTree(entry, uri, _clientCaps?.python?.completionsTimeout ?? Timeout.Infinite, out var version) ?? tree; @@ -577,7 +607,7 @@ public override Task Hover(TextDocumentPositionParams @params) { _analyzer.Modules.TryImport(w.ImportedName, out var modRef)) { var contents = _displayTextBuilder.MakeModuleHoverText(modRef); if (contents != null) { - return Task.FromResult(new Hover { contents = contents }); + return new Hover { contents = contents }; } } @@ -599,7 +629,7 @@ public override Task Hover(TextDocumentPositionParams @params) { } if (expr == null) { LogMessage(MessageType.Info, $"No hover info found in {uri} at {@params.position}"); - return Task.FromResult(default(Hover)); + return default(Hover); } TraceMessage($"Getting hover for {expr.ToCodeString(tree, CodeFormattingOptions.Traditional)}"); @@ -626,10 +656,11 @@ public override Task Hover(TextDocumentPositionParams @params) { _version = version, _typeNames = names }; - return Task.FromResult(res); + return res; } - public override Task WorkspaceSymbols(WorkspaceSymbolParams @params) { + public override async Task WorkspaceSymbols(WorkspaceSymbolParams @params) { + await _analyzerCreationTcs.Task; var members = Enumerable.Empty(); var opts = GetMemberOptions.ExcludeBuiltins | GetMemberOptions.DeclaredOnly; @@ -640,7 +671,7 @@ public override Task WorkspaceSymbols(WorkspaceSymbolParams } members = members.GroupBy(mr => mr.Name).Select(g => g.First()); - return Task.FromResult(members.Select(m => ToSymbolInformation(m)).ToArray()); + return members.Select(m => ToSymbolInformation(m)).ToArray(); } #endregion diff --git a/Python/Product/Analysis/LanguageServer/Structures.cs b/Python/Product/Analysis/LanguageServer/Structures.cs index 3a5ca605dc..fc8fab23a6 100644 --- a/Python/Product/Analysis/LanguageServer/Structures.cs +++ b/Python/Product/Analysis/LanguageServer/Structures.cs @@ -235,7 +235,14 @@ public struct Interpreter { } public Interpreter interpreter; public string[] searchPaths; + /// + /// Controls tooltip display appearance. Different between VS and VS Code. + /// public InformationDisplayOptions displayOptions; + /// + /// If true, analyzer will be created asynchronously. Used in VS Code. + /// + public bool asyncStartup; } From 6d4772dad7bba36cea88e876787e365bdfa1ebf5 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 16:07:31 -0700 Subject: [PATCH 05/14] Move VS Code-specific signature to VSC LS --- .../LanguageServer/DisplayTextBuilder.cs | 33 ------------- .../Product/Analysis/LanguageServer/Server.cs | 1 - .../VSCode/AnalysisVsc/LanguageServer.cs | 48 +++++++++++++++++-- 3 files changed, 44 insertions(+), 38 deletions(-) diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index 8a4bb93ec9..19f821cfa4 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -24,39 +24,6 @@ namespace Microsoft.PythonTools.Analysis.LanguageServer { sealed class DisplayTextBuilder { private readonly RestTextConverter _textConverter = new RestTextConverter(); - public void BuildMarkdownSignature(SignatureHelp signatureHelp) { - foreach (var s in signatureHelp.signatures) { - // Recostruct full signature so editor can display current parameter - var sb = new StringBuilder(); - - if (s.documentation != null) { - s.documentation.value = _textConverter.ToMarkdown(s.documentation.value); - } - sb.Append(s.label); - sb.Append('('); - if (s.parameters != null) { - foreach (var p in s.parameters) { - if (sb[sb.Length - 1] != '(') { - sb.Append(", "); - } - sb.Append(p.label); - if (!string.IsNullOrEmpty(p._type)) { - sb.Append(':'); - sb.Append(p._type); - } - if(!string.IsNullOrEmpty(p._defaultValue)) { - sb.Append('='); - sb.Append(p._defaultValue); - } - if (p.documentation != null) { - p.documentation.value = _textConverter.ToMarkdown(p.documentation.value); - } - } - } - sb.Append(')'); - s.label = sb.ToString(); - } - } public string MakeHoverText(IEnumerable values, string originalExpression, InformationDisplayOptions displayOptions) { string firstLongDescription = null; diff --git a/Python/Product/Analysis/LanguageServer/Server.cs b/Python/Product/Analysis/LanguageServer/Server.cs index 45c58049ab..2f34335fa6 100644 --- a/Python/Product/Analysis/LanguageServer/Server.cs +++ b/Python/Product/Analysis/LanguageServer/Server.cs @@ -490,7 +490,6 @@ public override async Task SignatureHelp(TextDocumentPositionPara activeSignature = activeSignature, activeParameter = activeParameter }; - _displayTextBuilder.BuildMarkdownSignature(sh); return sh; } diff --git a/Python/Product/VSCode/AnalysisVsc/LanguageServer.cs b/Python/Product/VSCode/AnalysisVsc/LanguageServer.cs index 7800fdb291..0089666ce8 100644 --- a/Python/Product/VSCode/AnalysisVsc/LanguageServer.cs +++ b/Python/Product/VSCode/AnalysisVsc/LanguageServer.cs @@ -17,11 +17,13 @@ using System; using System.Diagnostics; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.DsTools.Core.Disposables; using Microsoft.DsTools.Core.Services; using Microsoft.DsTools.Core.Services.Shell; +using Microsoft.PythonTools.Analysis; using Microsoft.PythonTools.Analysis.Infrastructure; using Microsoft.PythonTools.Analysis.LanguageServer; using Microsoft.PythonTools.VsCode.Core.Shell; @@ -39,6 +41,7 @@ public sealed class LanguageServer : IDisposable { private readonly DisposableBag _disposables = new DisposableBag(nameof(LanguageServer)); private readonly Server _server = new Server(); private readonly CancellationTokenSource _sessionTokenSource = new CancellationTokenSource(); + private readonly RestTextConverter _textConverter = new RestTextConverter(); private IUIService _ui; private ITelemetryService _telemetry; private JsonRpc _vscode; @@ -137,9 +140,9 @@ public async Task Exit() { private void MonitorParentProcess(Process process) { Task.Run(async () => { - while(!_sessionTokenSource.IsCancellationRequested) { + while (!_sessionTokenSource.IsCancellationRequested) { await Task.Delay(2000); - if(process.HasExited) { + if (process.HasExited) { _sessionTokenSource.Cancel(); } } @@ -211,8 +214,11 @@ public Task Hover(JToken token) => _server.Hover(token.ToObject()); [JsonRpcMethod("textDocument/signatureHelp")] - public Task SignatureHelp(JToken token) - => _server.SignatureHelp(token.ToObject()); + public async Task SignatureHelp(JToken token) { + var sh = await _server.SignatureHelp(token.ToObject()); + BuildMarkdownSignature(sh); + return sh; + } [JsonRpcMethod("textDocument/definition")] public Task GotoDefinition(JToken token) @@ -266,5 +272,39 @@ public Task DocumentOnTypeFormatting(JToken token) public Task Rename(JToken token) => _server.Rename(token.ToObject()); #endregion + + private void BuildMarkdownSignature(SignatureHelp signatureHelp) { + foreach (var s in signatureHelp.signatures) { + // Recostruct full signature so editor can display current parameter + var sb = new StringBuilder(); + + if (s.documentation != null) { + s.documentation.value = _textConverter.ToMarkdown(s.documentation.value); + } + sb.Append(s.label); + sb.Append('('); + if (s.parameters != null) { + foreach (var p in s.parameters) { + if (sb[sb.Length - 1] != '(') { + sb.Append(", "); + } + sb.Append(p.label); + if (!string.IsNullOrEmpty(p._type)) { + sb.Append(':'); + sb.Append(p._type); + } + if (!string.IsNullOrEmpty(p._defaultValue)) { + sb.Append('='); + sb.Append(p._defaultValue); + } + if (p.documentation != null) { + p.documentation.value = _textConverter.ToMarkdown(p.documentation.value); + } + } + } + sb.Append(')'); + s.label = sb.ToString(); + } + } } } From 5b31cc6868db96b10fc99fb890446baaa44e4ba9 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Apr 2018 16:34:37 -0700 Subject: [PATCH 06/14] Tooltip improvements --- .../LanguageServer/DisplayTextBuilder.cs | 5 +- Python/Product/Analysis/MemberResult.cs | 66 ------------------- .../Product/Analysis/Values/FunctionInfo.cs | 4 +- Python/Product/Analysis/Values/Utils.cs | 23 +++++-- Python/Product/Analysis/scrape_module.py | 14 +++- .../VSCode/AnalysisVsc/LanguageServer.cs | 41 +----------- 6 files changed, 34 insertions(+), 119 deletions(-) diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index 19f821cfa4..c519e8f1d2 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -69,8 +69,9 @@ public string MakeHoverText(IEnumerable values, string originalEx } if (!string.IsNullOrEmpty(originalExpression)) { - if (originalExpression.Length > 4096) { - originalExpression = originalExpression.Substring(0, 4093) + "..."; + if (originalExpression.Length > displayOptions.maxDocumentationTextLength) { + originalExpression = originalExpression.Substring(0, + Math.Max(3, displayOptions.maxDocumentationTextLength - 3)) + "..."; } if (multiline) { result.Insert(0, $"{originalExpression}:{Environment.NewLine}"); diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index 842c0e4d32..d9474881e4 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -171,72 +171,6 @@ private static IEnumerable SeparateMultipleMembers(IEnumerable>(); - // var allTypes = new HashSet(); - - // foreach (var ns in SeparateMultipleMembers(Values)) { - // var docString = ns.Documentation?.TrimDocumentation(); - // var typeString = GetDescription(ns); - // if (string.IsNullOrEmpty(docString)) { - // docString = ""; - // } - // if (!docs.TryGetValue(docString, out var docTypes)) { - // docs[docString] = docTypes = new HashSet(); - // } - // if (!string.IsNullOrEmpty(typeString)) { - // docTypes.Add(typeString); - // allTypes.Add(typeString); - // } - // } - - // var doc = new StringBuilder(); - - // if (allTypes.Count == 0) { - // return "unknown type"; - // } else if (allTypes.Count == 1) { - // doc.AppendLine(allTypes.First()); - // doc.AppendLine(); - // } else { - // var types = allTypes.OrderBy(s => s).ToList(); - // var orStr = types.Count == 2 ? " or " : ", or "; - // doc.AppendLine(string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last()); - // doc.AppendLine(); - // } - - // var typeToDoc = new Dictionary(); - // foreach (var docType in docs) { - // if (string.IsNullOrEmpty(docType.Key)) { - // continue; - // } - - // string typeDisplay = "unknown type"; - // var types = docType.Value.OrderBy(s => s).ToList(); - // if (types.Count == 0) { - // typeDisplay = ""; - // } else if (types.Count == 1) { - // if (allTypes.Count > 1) { - // typeDisplay = types.First(); - // } else { - // typeDisplay = ""; - // } - // } else { - // var orStr = types.Count == 2 ? " or " : ", or "; - // typeDisplay = string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last(); - // } - // typeToDoc[string.Join(",", types)] = typeDisplay + ": " + docType.Key; - // } - - // foreach (var typeDoc in typeToDoc.OrderBy(kv => kv.Key)) { - // doc.AppendLine(typeDoc.Value); - // doc.AppendLine(); - // } - - // return Utils.CleanDocumentation(doc.ToString()); - // } - //} - public PythonMemberType MemberType => _type.Value; private PythonMemberType GetMemberType() { diff --git a/Python/Product/Analysis/Values/FunctionInfo.cs b/Python/Product/Analysis/Values/FunctionInfo.cs index 8875a1d210..927d836c0f 100644 --- a/Python/Product/Analysis/Values/FunctionInfo.cs +++ b/Python/Product/Analysis/Values/FunctionInfo.cs @@ -204,11 +204,11 @@ internal IEnumerable> GetParameterString() { yield return new KeyValuePair(WellKnownRichDescriptionKinds.Parameter, name); if (!string.IsNullOrWhiteSpace(annotation)) { - yield return new KeyValuePair(WellKnownRichDescriptionKinds.Misc, " : "); + yield return new KeyValuePair(WellKnownRichDescriptionKinds.Misc, ": "); yield return new KeyValuePair(WellKnownRichDescriptionKinds.Type, annotation); } if (!string.IsNullOrWhiteSpace(defaultValue)) { - yield return new KeyValuePair(WellKnownRichDescriptionKinds.Misc, " = "); + yield return new KeyValuePair(WellKnownRichDescriptionKinds.Misc, "="); yield return new KeyValuePair(WellKnownRichDescriptionKinds.Misc, defaultValue); } } diff --git a/Python/Product/Analysis/Values/Utils.cs b/Python/Product/Analysis/Values/Utils.cs index 7ce5207444..58e6a27626 100644 --- a/Python/Product/Analysis/Values/Utils.cs +++ b/Python/Product/Analysis/Values/Utils.cs @@ -47,15 +47,24 @@ internal static string StripDocumentation(string doc) { } internal static string CleanDocumentation(string doc) { - int ctr = 0; + // Remove excessive line breaks and remove line breaks inside + // the body of documentation so text flows inside the tooltip. + var ctr = 0; + var seenParagraphGap = false; var result = new StringBuilder(doc.Length); - foreach (char c in doc) { + foreach (var c in doc) { if (c == '\r') { - // pass - } else if (c == '\n') { - ctr++; - if (ctr < 3) { - result.Append("\r\n"); + continue; + } + if (c == '\n') { + if (seenParagraphGap) { + result.Append(' '); + } else { + ctr++; + if (ctr < 3) { + result.AppendLine(); + seenParagraphGap = ctr == 2; + } } } else { result.Append(c); diff --git a/Python/Product/Analysis/scrape_module.py b/Python/Product/Analysis/scrape_module.py index 552117091a..4271844502 100644 --- a/Python/Product/Analysis/scrape_module.py +++ b/Python/Product/Analysis/scrape_module.py @@ -383,17 +383,23 @@ def _init_restype_fromknown(self, scope_alias): return "return " + restype def _init_argspec_fromdocstring(self, defaults, doc=None, override_name=None): - allow_name_mismatch = True if not doc: doc = getattr(self.callable, '__doc__', None) - allow_name_mismatch = False if not isinstance(doc, str): return + if doc.startswith(' Hover(JToken token) => _server.Hover(token.ToObject()); [JsonRpcMethod("textDocument/signatureHelp")] - public async Task SignatureHelp(JToken token) { - var sh = await _server.SignatureHelp(token.ToObject()); - BuildMarkdownSignature(sh); - return sh; - } + public Task SignatureHelp(JToken token) + => _server.SignatureHelp(token.ToObject()); [JsonRpcMethod("textDocument/definition")] public Task GotoDefinition(JToken token) @@ -272,39 +269,5 @@ public Task DocumentOnTypeFormatting(JToken token) public Task Rename(JToken token) => _server.Rename(token.ToObject()); #endregion - - private void BuildMarkdownSignature(SignatureHelp signatureHelp) { - foreach (var s in signatureHelp.signatures) { - // Recostruct full signature so editor can display current parameter - var sb = new StringBuilder(); - - if (s.documentation != null) { - s.documentation.value = _textConverter.ToMarkdown(s.documentation.value); - } - sb.Append(s.label); - sb.Append('('); - if (s.parameters != null) { - foreach (var p in s.parameters) { - if (sb[sb.Length - 1] != '(') { - sb.Append(", "); - } - sb.Append(p.label); - if (!string.IsNullOrEmpty(p._type)) { - sb.Append(':'); - sb.Append(p._type); - } - if (!string.IsNullOrEmpty(p._defaultValue)) { - sb.Append('='); - sb.Append(p._defaultValue); - } - if (p.documentation != null) { - p.documentation.value = _textConverter.ToMarkdown(p.documentation.value); - } - } - } - sb.Append(')'); - s.label = sb.ToString(); - } - } } } From c2ce82171883950002728cba12c70bff58921002 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 7 Apr 2018 15:26:16 -0700 Subject: [PATCH 07/14] Tooltip display --- .../LanguageServer/DisplayTextBuilder.cs | 2 +- Python/Product/Analysis/MemberResult.cs | 52 +++++++++++++++---- Python/Product/Analysis/Values/Utils.cs | 28 ---------- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index c519e8f1d2..924f9294c8 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -71,7 +71,7 @@ public string MakeHoverText(IEnumerable values, string originalEx if (!string.IsNullOrEmpty(originalExpression)) { if (originalExpression.Length > displayOptions.maxDocumentationTextLength) { originalExpression = originalExpression.Substring(0, - Math.Max(3, displayOptions.maxDocumentationTextLength - 3)) + "..."; + Math.Max(3, displayOptions.maxDocumentationTextLength) - 3) + "..."; } if (multiline) { result.Insert(0, $"{originalExpression}:{Environment.NewLine}"); diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index d9474881e4..e2655c0272 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -87,7 +87,7 @@ public string Documentation { var docs = new Dictionary>(); foreach (var ns in SeparateMultipleMembers(Values)) { - var docString = ns.Documentation?.TrimDocumentation() ?? string.Empty; + var docString = GetDocumentation(ns); var typeString = GetDescription(ns); // If first line of doc is already in the type string, then filter it out. @@ -126,16 +126,10 @@ public string Documentation { foreach (var typeDoc in typeToDoc.OrderBy(kv => kv.Key)) { doc.Append(typeDoc.Value.Item1); - if (!string.IsNullOrEmpty(typeDoc.Value.Item2)) { - var cleaned = Utils.CleanDocumentation(typeDoc.Value.Item2); - if (!string.IsNullOrEmpty(cleaned)) { - if (cleaned.IndexOf('\n') >= 0) { - doc.AppendLine(":"); - } else { - doc.Append(": "); - } - doc.Append(cleaned); - } + var details = typeDoc.Value.Item2; + if (!string.IsNullOrEmpty(details)) { + doc.AppendLine(":"); + doc.Append(details); } doc.AppendLine(); doc.AppendLine(); @@ -145,6 +139,42 @@ public string Documentation { } } + private static string GetDocumentation(AnalysisValue ns) { + var doc = ns.Documentation?.TrimDocumentation() ?? string.Empty; + if (ns.MemberType == PythonMemberType.Module) { + // Module doc does not nave example/signature lines like a function + // so just make it flow nicely in the tooltip by removing like breaks. + return doc.Replace('\n', ' ').Replace("\r", string.Empty); + } + // Documentation can contain something like + // func(a, b, c)\n\nThis function... + // i.e. with paragraph. We want to remove line breaks + // in the text after the paragraph breaks and tooltip UI + // to decide of the wrap and flow. + var ctr = 0; + var seenParagraphGap = false; + var result = new StringBuilder(doc.Length); + foreach (var c in doc) { + if (c == '\r') { + continue; + } + if (c == '\n') { + if (seenParagraphGap) { + result.Append(' '); + } else { + ctr++; + if (ctr < 3) { + result.AppendLine(); + seenParagraphGap = ctr == 2; + } + } + } else { + result.Append(c); + ctr = 0; + } + } + return result.ToString().Trim(); + } private static string GetDescription(AnalysisValue ns) { var d = ns?.ShortDescription; if (string.IsNullOrEmpty(d)) { diff --git a/Python/Product/Analysis/Values/Utils.cs b/Python/Product/Analysis/Values/Utils.cs index 58e6a27626..d07e700d44 100644 --- a/Python/Product/Analysis/Values/Utils.cs +++ b/Python/Product/Analysis/Values/Utils.cs @@ -46,34 +46,6 @@ internal static string StripDocumentation(string doc) { return result.ToString(); } - internal static string CleanDocumentation(string doc) { - // Remove excessive line breaks and remove line breaks inside - // the body of documentation so text flows inside the tooltip. - var ctr = 0; - var seenParagraphGap = false; - var result = new StringBuilder(doc.Length); - foreach (var c in doc) { - if (c == '\r') { - continue; - } - if (c == '\n') { - if (seenParagraphGap) { - result.Append(' '); - } else { - ctr++; - if (ctr < 3) { - result.AppendLine(); - seenParagraphGap = ctr == 2; - } - } - } else { - result.Append(c); - ctr = 0; - } - } - return result.ToString().Trim(); - } - internal static IAnalysisSet GetReturnTypes(IPythonFunction func, PythonAnalyzer projectState) { return AnalysisSet.UnionAll(func.Overloads .Where(fn => fn.ReturnType != null) From b2f03da943a8e6700c4f80f68144538a3e90e816 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 7 Apr 2018 17:03:11 -0700 Subject: [PATCH 08/14] Fix tootip trimming --- Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index 924f9294c8..47270f6d78 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -69,7 +69,7 @@ public string MakeHoverText(IEnumerable values, string originalEx } if (!string.IsNullOrEmpty(originalExpression)) { - if (originalExpression.Length > displayOptions.maxDocumentationTextLength) { + if (displayOptions.trimDocumentationText && originalExpression.Length > displayOptions.maxDocumentationTextLength) { originalExpression = originalExpression.Substring(0, Math.Max(3, displayOptions.maxDocumentationTextLength) - 3) + "..."; } From 5025beba0b97898d51fc9577b0aacaf4cb75b537 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 9 Apr 2018 13:16:03 -0700 Subject: [PATCH 09/14] Fix tests --- Python/Product/Analysis/scrape_module.py | 5 +---- Python/Tests/Analysis/AnalysisTest.cs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/Python/Product/Analysis/scrape_module.py b/Python/Product/Analysis/scrape_module.py index 4271844502..b06a4f422b 100644 --- a/Python/Product/Analysis/scrape_module.py +++ b/Python/Product/Analysis/scrape_module.py @@ -388,9 +388,6 @@ def _init_argspec_fromdocstring(self, defaults, doc=None, override_name=None): if not isinstance(doc, str): return - if doc.startswith(' str"); - entry.AssertDescription("fob.f", "test-module.fob.f(self : fob)\r\ndeclared in fob"); + entry.AssertDescription("fob.f", "test-module.fob.f(self: fob)\r\ndeclared in fob"); entry.AssertDescription("fob().g", "method g of test-module.fob objects "); entry.AssertDescription("fob", "class test-module.fob(object)"); //AssertUtil.ContainsExactly(entry.GetVariableDescriptionsByIndex("System.StringSplitOptions.RemoveEmptyEntries", 1), "field of type StringSplitOptions"); @@ -5844,13 +5844,13 @@ def with_params_default_starargs(*args, **kwargs): entry.AssertDescription("docstr_func", "test-module.docstr_func() -> int\r\nuseful documentation"); entry.AssertDescription("with_params", "test-module.with_params(a, b, c)"); - entry.AssertDescription("with_params_default", "test-module.with_params_default(a, b, c : int = 100)"); - entry.AssertDescription("with_params_default_2", "test-module.with_params_default_2(a, b, c : list = [])"); - entry.AssertDescription("with_params_default_3", "test-module.with_params_default_3(a, b, c : tuple = ())"); - entry.AssertDescription("with_params_default_4", "test-module.with_params_default_4(a, b, c : dict = {})"); - entry.AssertDescription("with_params_default_2a", "test-module.with_params_default_2a(a, b, c : list = [...])"); - entry.AssertDescription("with_params_default_3a", "test-module.with_params_default_3a(a, b, c : tuple = (...))"); - entry.AssertDescription("with_params_default_4a", "test-module.with_params_default_4a(a, b, c : dict = {...})"); + entry.AssertDescription("with_params_default", "test-module.with_params_default(a, b, c: int=100)"); + entry.AssertDescription("with_params_default_2", "test-module.with_params_default_2(a, b, c: list=[])"); + entry.AssertDescription("with_params_default_3", "test-module.with_params_default_3(a, b, c: tuple=())"); + entry.AssertDescription("with_params_default_4", "test-module.with_params_default_4(a, b, c: dict={})"); + entry.AssertDescription("with_params_default_2a", "test-module.with_params_default_2a(a, b, c: list=[...])"); + entry.AssertDescription("with_params_default_3a", "test-module.with_params_default_3a(a, b, c: tuple=(...))"); + entry.AssertDescription("with_params_default_4a", "test-module.with_params_default_4a(a, b, c: dict={...})"); entry.AssertDescription("with_params_default_starargs", "test-module.with_params_default_starargs(*args, **kwargs)"); // method which returns itself, we shouldn't stack overflow producing the help... @@ -6601,7 +6601,7 @@ def fn(self): var entry = ProcessText(code); Assert.AreEqual( - "test-module.A.fn(self : A) -> lambda: 123 -> int\ndeclared in A", + "test-module.A.fn(self: A) -> lambda: 123 -> int\ndeclared in A", entry.GetDescriptions("A.fn", 0).Single().Replace("\r\n", "\n") ); } From c6e7e9a614744ccd3b71a92d04e2cab5c19661e7 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 9 Apr 2018 13:55:19 -0700 Subject: [PATCH 10/14] Fix test baselines --- .../Product/Analysis/LanguageServer/Server.cs | 62 +++++++++---------- .../Analysis/LanguageServer/Structures.cs | 1 + Python/Product/Analysis/Values/Protocols.cs | 2 +- Python/Tests/Analysis/LanguageServerTests.cs | 8 ++- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/Python/Product/Analysis/LanguageServer/Server.cs b/Python/Product/Analysis/LanguageServer/Server.cs index 599f69631b..194881149b 100644 --- a/Python/Product/Analysis/LanguageServer/Server.cs +++ b/Python/Product/Analysis/LanguageServer/Server.cs @@ -69,6 +69,12 @@ public Server() { _projectFiles = new ConcurrentDictionary(); _pendingChanges = new ConcurrentDictionary>(UriEqualityComparer.IncludeFragment); _lastReportedDiagnostics = new ConcurrentDictionary>(); + _displayOptions = new InformationDisplayOptions { + trimDocumentationLines = true, + maxDocumentationLineLength = 200, + trimDocumentationText = true, + maxDocumentationTextLength = 4096 + }; } private void Analysis_UnhandledException(object sender, UnhandledExceptionEventArgs e) { @@ -89,31 +95,28 @@ private void TraceMessage(IFormattable message) { #region Client message handling public override async Task Initialize(InitializeParams @params) { - _testEnvironment = @params.initializationOptions.interpreter.properties.ContainsKey("TestEnvironment"); - if (_testEnvironment) { - // Test environment needs predictable initialization. - // Tests can only proceed when analysis is fully done. - _analyzer = CreateAnalyzer(@params.initializationOptions.interpreter).Result; - } else { - if (@params.initializationOptions.asyncStartup) { - CreateAnalyzer(@params.initializationOptions.interpreter).ContinueWith(t => { - if (t.IsFaulted) { - _analyzerCreationTcs.TrySetException(t.Exception); - } else { - try { - _analyzer = t.Result; - OnAnalyzerCreated(@params); - _analyzerCreationTcs.TrySetResult(true); - } catch (Exception ex) { - _analyzerCreationTcs.TrySetException(ex); - } + _testEnvironment = @params.initializationOptions.testEnvironment; + // Test environment needs predictable initialization. + if (@params.initializationOptions.asyncStartup && !_testEnvironment) { + CreateAnalyzer(@params.initializationOptions.interpreter).ContinueWith(t => { + if (t.IsFaulted) { + _analyzerCreationTcs.TrySetException(t.Exception); + } else { + try { + _analyzer = t.Result; + OnAnalyzerCreated(@params); + _analyzerCreationTcs.TrySetResult(true); + } catch (Exception ex) { + _analyzerCreationTcs.TrySetException(ex); } - }).DoNotWait(); - } else { - _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); - _analyzerCreationTcs.TrySetResult(true); - } + } + }).DoNotWait(); + } else { + _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); + OnAnalyzerCreated(@params); + _analyzerCreationTcs.TrySetResult(true); } + return new InitializeResult { capabilities = new ServerCapabilities { textDocumentSync = new TextDocumentSyncOptions { openClose = true, change = TextDocumentSyncKind.Incremental }, @@ -131,12 +134,9 @@ public override async Task Initialize(InitializeParams @params } private void OnAnalyzerCreated(InitializeParams @params) { - _displayOptions = @params.initializationOptions.displayOptions ?? new InformationDisplayOptions { - trimDocumentationLines = true, - maxDocumentationLineLength = 200, - trimDocumentationText = true, - maxDocumentationTextLength = 4096 - }; + if (@params.initializationOptions.displayOptions != null) { + _displayOptions = @params.initializationOptions.displayOptions; + } if (string.IsNullOrEmpty(_analyzer.InterpreterFactory?.Configuration?.InterpreterPath)) { LogMessage(MessageType.Log, "Initializing for generic interpreter"); @@ -1343,11 +1343,11 @@ private MarkupKind SelectBestMarkup(IEnumerable requested, params Ma private string FormatParameter(ParameterResult p) { var res = new StringBuilder(p.Name); if (!string.IsNullOrEmpty(p.Type)) { - res.Append(" : "); + res.Append(": "); res.Append(p.Type); } if (!string.IsNullOrEmpty(p.DefaultValue)) { - res.Append(" = "); + res.Append('='); res.Append(p.DefaultValue); } return res.ToString(); diff --git a/Python/Product/Analysis/LanguageServer/Structures.cs b/Python/Product/Analysis/LanguageServer/Structures.cs index 916d93a679..72f9ea1458 100644 --- a/Python/Product/Analysis/LanguageServer/Structures.cs +++ b/Python/Product/Analysis/LanguageServer/Structures.cs @@ -235,6 +235,7 @@ public struct Interpreter { } public Interpreter interpreter; public string[] searchPaths; + public bool testEnvironment; /// /// Controls tooltip display appearance. Different between VS and VS Code. /// diff --git a/Python/Product/Analysis/Values/Protocols.cs b/Python/Product/Analysis/Values/Protocols.cs index 49a325fa99..e00ba3405f 100644 --- a/Python/Product/Analysis/Values/Protocols.cs +++ b/Python/Product/Analysis/Values/Protocols.cs @@ -670,7 +670,7 @@ public override void SetMember(Node node, AnalysisUnit unit, string name, IAnaly public override IEnumerable> GetRichDescription() { yield return new KeyValuePair(WellKnownRichDescriptionKinds.Name, Name); - foreach (var kv in _values.Types.GetRichDescriptions(prefix: " : ", unionPrefix: "{", unionSuffix: "}")) { + foreach (var kv in _values.Types.GetRichDescriptions(prefix: ": ", unionPrefix: "{", unionSuffix: "}")) { yield return kv; } } diff --git a/Python/Tests/Analysis/LanguageServerTests.cs b/Python/Tests/Analysis/LanguageServerTests.cs index f96dddce7b..250d33a2e5 100644 --- a/Python/Tests/Analysis/LanguageServerTests.cs +++ b/Python/Tests/Analysis/LanguageServerTests.cs @@ -85,7 +85,9 @@ await s.Initialize(new InitializeParams { assembly = typeof(AstPythonInterpreterFactory).Assembly.Location, typeName = typeof(AstPythonInterpreterFactory).FullName, properties = properties - } + }, + asyncStartup = false, + testEnvironment = true }, capabilities = new ClientCapabilities { python = new PythonClientCapabilities { @@ -342,7 +344,7 @@ def f(a, *b, **c): pass "); await AssertSignature(s, mod, new SourceLocation(1, 3), - new string[] { "f()", "f(a)", "f(a, b)", "f(a, *b : tuple)", "f(a, **b : dict)", "f(a, *b : tuple, **c : dict)" }, + new string[] { "f()", "f(a)", "f(a, b)", "f(a, *b: tuple)", "f(a, **b: dict)", "f(a, *b: tuple, **c: dict)" }, new string[0] ); @@ -500,7 +502,7 @@ def g(self): await AssertHover(s, mod, new SourceLocation(13, 1), "c: C", new[] { "test-module.C" }, new SourceSpan(13, 1, 13, 2)); await AssertHover(s, mod, new SourceLocation(14, 7), "c: C", new[] { "test-module.C" }, new SourceSpan(14, 7, 14, 8)); await AssertHover(s, mod, new SourceLocation(14, 9), "c.f: method f of test-module.C objects*", new[] { "test-module.C.f" }, new SourceSpan(14, 7, 14, 10)); - await AssertHover(s, mod, new SourceLocation(14, 1), "c_g: test-module.C.f.g(self)\r\ndeclared in C.f", new[] { "test-module.C.f.g" }, new SourceSpan(14, 1, 14, 4)); + await AssertHover(s, mod, new SourceLocation(14, 1), $"c_g: test-module.C.f.g(self) {Environment.NewLine}declared in C.f", new[] { "test-module.C.f.g" }, new SourceSpan(14, 1, 14, 4)); await AssertHover(s, mod, new SourceLocation(16, 1), "x: int, float", new[] { "int", "float" }, new SourceSpan(16, 1, 16, 2)); } From 17d41298c663735530314ea39e82634c544d105f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 9 Apr 2018 14:19:54 -0700 Subject: [PATCH 11/14] Test fixes --- Python/Product/Analysis/MemberResult.cs | 14 ++++++++++++++ Python/Tests/Analysis/AnalysisSaveTest.cs | 2 +- Python/Tests/Analysis/LanguageServerTests.cs | 2 +- Python/Tests/Analysis/TypeAnnotationTests.cs | 6 +++--- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index e2655c0272..bdaf9a63c5 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -85,6 +85,7 @@ public MemberResult FilterCompletion(string completion) { public string Documentation { get { var docs = new Dictionary>(); + var allTypes = new HashSet(); foreach (var ns in SeparateMultipleMembers(Values)) { var docString = GetDocumentation(ns); @@ -103,10 +104,23 @@ public string Documentation { } if (!string.IsNullOrEmpty(typeString)) { docTypes.Add(typeString); + allTypes.Add(typeString); } } var doc = new StringBuilder(); + if (allTypes.Count == 0) { + return "unknown type"; + } else if (allTypes.Count == 1) { + doc.AppendLine(allTypes.First()); + doc.AppendLine(); + } else { + var types = allTypes.OrderBy(s => s).ToList(); + var orStr = types.Count == 2 ? " or " : ", or "; + doc.AppendLine(string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last()); + doc.AppendLine(); + } + var typeToDoc = new Dictionary>(); foreach (var docType in docs) { if (!docType.Value.Any()) { diff --git a/Python/Tests/Analysis/AnalysisSaveTest.cs b/Python/Tests/Analysis/AnalysisSaveTest.cs index 1770a8abd4..8d1dc94899 100644 --- a/Python/Tests/Analysis/AnalysisSaveTest.cs +++ b/Python/Tests/Analysis/AnalysisSaveTest.cs @@ -175,7 +175,7 @@ import test var allMembers = newMod.Analysis.GetAllAvailableMembersByIndex(pos, GetMemberOptions.None); Assert.AreEqual( - "class test.Aliased or function Aliased(fob)\r\n\r\nclass test.Aliased: class doc\r\n\r\nfunction Aliased(fob): function doc", + "class test.Aliased or function Aliased(fob)\r\n\r\nclass test.Aliased:\r\nclass doc\r\n\r\nfunction Aliased(fob):\r\nfunction doc", allMembers.First(x => x.Name == "Aliased").Documentation ); newPs.Analyzer.AssertHasParameters("FunctionNoRetType", "value"); diff --git a/Python/Tests/Analysis/LanguageServerTests.cs b/Python/Tests/Analysis/LanguageServerTests.cs index 250d33a2e5..9ad3fa514c 100644 --- a/Python/Tests/Analysis/LanguageServerTests.cs +++ b/Python/Tests/Analysis/LanguageServerTests.cs @@ -363,7 +363,7 @@ def f(a = 2, b): pass "); await AssertSignature(s, mod, new SourceLocation(1, 3), - new string[] { "f(a : int)", "f(a : int = 2, b : int)", "f(x : str, y : str)" }, + new string[] { "f(a: int)", "f(a: int=2, b: int)", "f(x: str, y: str)" }, new string[0] ); } diff --git a/Python/Tests/Analysis/TypeAnnotationTests.cs b/Python/Tests/Analysis/TypeAnnotationTests.cs index 2dc6d8feb4..44cdb8ccfa 100644 --- a/Python/Tests/Analysis/TypeAnnotationTests.cs +++ b/Python/Tests/Analysis/TypeAnnotationTests.cs @@ -249,8 +249,8 @@ public void TypingModuleNamedTupleAnalysis() { analyzer.WaitForAnalysis(); analyzer.AssertDescription("n", "tuple"); - analyzer.AssertDescription("n1", "n1(x : int, y : float)"); - analyzer.AssertDescription("n2", "n2(x : int, y : float)"); + analyzer.AssertDescription("n1", "n1(x: int, y: float)"); + analyzer.AssertDescription("n2", "n2(x: int, y: float)"); analyzer.AssertIsInstance("n1_x", BuiltinTypeId.Int); analyzer.AssertIsInstance("n1_y", BuiltinTypeId.Float); @@ -294,7 +294,7 @@ public void TypingModuleNamedTypeAlias() { analyzer.AssertIsInstance("i", BuiltinTypeId.Int); analyzer.AssertIsInstance("sl", BuiltinTypeId.List); analyzer.AssertIsInstance("sl_0", BuiltinTypeId.Str); - analyzer.AssertDescription("n1", "MyNamedTuple(x : int)"); + analyzer.AssertDescription("n1", "MyNamedTuple(x: int)"); analyzer.AssertIsInstance("n1.x", BuiltinTypeId.Int); } From 47bc1f63e705820076ace13cd759f2fa24d2de9a Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 9 Apr 2018 15:50:27 -0700 Subject: [PATCH 12/14] Test fix --- Python/Product/Analysis/MemberResult.cs | 14 -------------- Python/Tests/Analysis/AnalysisSaveTest.cs | 2 +- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/Python/Product/Analysis/MemberResult.cs b/Python/Product/Analysis/MemberResult.cs index bdaf9a63c5..e2655c0272 100644 --- a/Python/Product/Analysis/MemberResult.cs +++ b/Python/Product/Analysis/MemberResult.cs @@ -85,7 +85,6 @@ public MemberResult FilterCompletion(string completion) { public string Documentation { get { var docs = new Dictionary>(); - var allTypes = new HashSet(); foreach (var ns in SeparateMultipleMembers(Values)) { var docString = GetDocumentation(ns); @@ -104,23 +103,10 @@ public string Documentation { } if (!string.IsNullOrEmpty(typeString)) { docTypes.Add(typeString); - allTypes.Add(typeString); } } var doc = new StringBuilder(); - if (allTypes.Count == 0) { - return "unknown type"; - } else if (allTypes.Count == 1) { - doc.AppendLine(allTypes.First()); - doc.AppendLine(); - } else { - var types = allTypes.OrderBy(s => s).ToList(); - var orStr = types.Count == 2 ? " or " : ", or "; - doc.AppendLine(string.Join(", ", types.Take(types.Count - 1)) + orStr + types.Last()); - doc.AppendLine(); - } - var typeToDoc = new Dictionary>(); foreach (var docType in docs) { if (!docType.Value.Any()) { diff --git a/Python/Tests/Analysis/AnalysisSaveTest.cs b/Python/Tests/Analysis/AnalysisSaveTest.cs index 8d1dc94899..d9f3428c42 100644 --- a/Python/Tests/Analysis/AnalysisSaveTest.cs +++ b/Python/Tests/Analysis/AnalysisSaveTest.cs @@ -175,7 +175,7 @@ import test var allMembers = newMod.Analysis.GetAllAvailableMembersByIndex(pos, GetMemberOptions.None); Assert.AreEqual( - "class test.Aliased or function Aliased(fob)\r\n\r\nclass test.Aliased:\r\nclass doc\r\n\r\nfunction Aliased(fob):\r\nfunction doc", + "class test.Aliased:\r\nclass doc\r\n\r\nfunction Aliased(fob):\r\nfunction doc", allMembers.First(x => x.Name == "Aliased").Documentation ); newPs.Analyzer.AssertHasParameters("FunctionNoRetType", "value"); From 9632f4fac725f37b6211a832ff1d62c530a74f7d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 10 Apr 2018 12:32:20 -0700 Subject: [PATCH 13/14] #4031 Concurrency issue accessing analysis dictionary --- .../LanguageServer/AnalysisQueueWorkItem.cs | 32 +++++++++++++++++++ .../LanguageServer/DisplayTextBuilder.cs | 25 ++++----------- .../Product/Analysis/LanguageServer/Server.cs | 15 ++++++--- 3 files changed, 49 insertions(+), 23 deletions(-) create mode 100644 Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs diff --git a/Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs b/Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs new file mode 100644 index 0000000000..9c96c75f4f --- /dev/null +++ b/Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs @@ -0,0 +1,32 @@ +// Python Tools for Visual Studio +// Copyright(c) Microsoft Corporation +// All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the License); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS +// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY +// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABLITY OR NON-INFRINGEMENT. +// +// See the Apache Version 2.0 License for specific language governing +// permissions and limitations under the License. + +using System; +using System.Threading; + +namespace Microsoft.PythonTools.Analysis.LanguageServer { + class AnalysisQueueWorkItem : IAnalyzable { + private readonly Action _action; + public AnalysisQueueWorkItem(Action action) { + _action = action; + } + public void Analyze(CancellationToken cancel) { + if(!cancel.IsCancellationRequested) { + _action(); + } + } + } +} diff --git a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs index 47270f6d78..4f63525d36 100644 --- a/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs +++ b/Python/Product/Analysis/LanguageServer/DisplayTextBuilder.cs @@ -26,40 +26,27 @@ sealed class DisplayTextBuilder { private readonly RestTextConverter _textConverter = new RestTextConverter(); public string MakeHoverText(IEnumerable values, string originalExpression, InformationDisplayOptions displayOptions) { - string firstLongDescription = null; - var multiline = false; var result = new StringBuilder(); var documentations = new HashSet(); foreach (var v in values) { - var doc = GetDocString(v); - firstLongDescription = firstLongDescription ?? doc; + if(result.Length > 0) { + result.AppendLine(); + } + var doc = GetDocString(v); doc = displayOptions.trimDocumentationLines ? LimitLines(doc) : doc; if (string.IsNullOrEmpty(doc)) { continue; } if (documentations.Add(doc)) { - if (documentations.Count > 1) { - if (result.Length == 0) { - // Nop - } else if (result[result.Length - 1] != '\n') { - result.Append(", "); - } else { - multiline = true; - } - } - result.Append(doc); + result.AppendLine(doc); } } - if (documentations.Count == 1 && !string.IsNullOrEmpty(firstLongDescription)) { - result.Clear(); - result.Append(firstLongDescription); - } - var displayText = result.ToString(); + var multiline = displayText.IndexOf('\n') >= 0; if (displayOptions.trimDocumentationText && displayText.Length > displayOptions.maxDocumentationTextLength) { displayText = displayText.Substring(0, Math.Max(3, displayOptions.maxDocumentationTextLength) - 3) + "..."; diff --git a/Python/Product/Analysis/LanguageServer/Server.cs b/Python/Product/Analysis/LanguageServer/Server.cs index 194881149b..31627f7623 100644 --- a/Python/Product/Analysis/LanguageServer/Server.cs +++ b/Python/Product/Analysis/LanguageServer/Server.cs @@ -108,13 +108,19 @@ public override async Task Initialize(InitializeParams @params _analyzerCreationTcs.TrySetResult(true); } catch (Exception ex) { _analyzerCreationTcs.TrySetException(ex); + throw; } } }).DoNotWait(); } else { - _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); - OnAnalyzerCreated(@params); - _analyzerCreationTcs.TrySetResult(true); + try { + _analyzer = await CreateAnalyzer(@params.initializationOptions.interpreter); + OnAnalyzerCreated(@params); + _analyzerCreationTcs.TrySetResult(true); + } catch (Exception ex) { + _analyzerCreationTcs.TrySetException(ex); + throw; + } } return new InitializeResult { @@ -307,6 +313,7 @@ public override async Task DidCloseTextDocument(DidCloseTextDocumentParams @para } } + public override async Task DidChangeConfiguration(DidChangeConfigurationParams @params) { await _analyzerCreationTcs.Task; if (_analyzer == null) { @@ -314,7 +321,7 @@ public override async Task DidChangeConfiguration(DidChangeConfigurationParams @ return; } - await _analyzer.ReloadModulesAsync(); + _queue.Enqueue(new AnalysisQueueWorkItem(() => _analyzer.ReloadModulesAsync().WaitAndUnwrapExceptions()), AnalysisPriority.Normal); // re-analyze all of the modules when we get a new set of modules loaded... foreach (var entry in _analyzer.ModulesByFilename) { From 30d66ef7c3631a0f095cbd0123d9a928cc0f9ff1 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 11 Apr 2018 10:07:27 -0700 Subject: [PATCH 14/14] CR feedback --- .../LanguageServer/AnalysisQueueWorkItem.cs | 32 ---------------- .../Product/Analysis/LanguageServer/Server.cs | 37 ++++++++++++++++++- 2 files changed, 36 insertions(+), 33 deletions(-) delete mode 100644 Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs diff --git a/Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs b/Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs deleted file mode 100644 index 9c96c75f4f..0000000000 --- a/Python/Product/Analysis/LanguageServer/AnalysisQueueWorkItem.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Python Tools for Visual Studio -// Copyright(c) Microsoft Corporation -// All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the License); you may not use -// this file except in compliance with the License. You may obtain a copy of the -// License at http://www.apache.org/licenses/LICENSE-2.0 -// -// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS -// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY -// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, -// MERCHANTABLITY OR NON-INFRINGEMENT. -// -// See the Apache Version 2.0 License for specific language governing -// permissions and limitations under the License. - -using System; -using System.Threading; - -namespace Microsoft.PythonTools.Analysis.LanguageServer { - class AnalysisQueueWorkItem : IAnalyzable { - private readonly Action _action; - public AnalysisQueueWorkItem(Action action) { - _action = action; - } - public void Analyze(CancellationToken cancel) { - if(!cancel.IsCancellationRequested) { - _action(); - } - } - } -} diff --git a/Python/Product/Analysis/LanguageServer/Server.cs b/Python/Product/Analysis/LanguageServer/Server.cs index 31627f7623..ae774c1def 100644 --- a/Python/Product/Analysis/LanguageServer/Server.cs +++ b/Python/Product/Analysis/LanguageServer/Server.cs @@ -34,6 +34,35 @@ namespace Microsoft.PythonTools.Analysis.LanguageServer { public sealed class Server : ServerBase, IDisposable { + /// + /// Implements ability to execute module reload on the analyzer thread + /// + private sealed class ReloadModulesQueueItem : IAnalyzable { + private readonly PythonAnalyzer _analyzer; + private TaskCompletionSource _tcs = new TaskCompletionSource(); + public Task Task => _tcs.Task; + + public ReloadModulesQueueItem(PythonAnalyzer analyzer) { + _analyzer = analyzer; + } + public void Analyze(CancellationToken cancel) { + if (cancel.IsCancellationRequested) { + return; + } + + var currentTcs = Interlocked.Exchange(ref _tcs, new TaskCompletionSource()); + var task = Task.Run(() => _analyzer.ReloadModulesAsync(), cancel); + try { + task.WaitAndUnwrapExceptions(); + currentTcs.TrySetResult(true); + } catch (OperationCanceledException oce) { + currentTcs.TrySetCanceled(oce.CancellationToken); + } catch (Exception ex) { + currentTcs.TrySetException(ex); + } + } + } + internal readonly AnalysisQueue _queue; internal readonly ParseQueue _parseQueue; private readonly Dictionary _pendingParse; @@ -56,6 +85,7 @@ public sealed class Server : ServerBase, IDisposable { private InformationDisplayOptions _displayOptions; private bool _traceLogging; private bool _testEnvironment; + private ReloadModulesQueueItem _reloadModulesQueueItem; // If null, all files must be added manually private string _rootDir; @@ -140,6 +170,8 @@ public override async Task Initialize(InitializeParams @params } private void OnAnalyzerCreated(InitializeParams @params) { + _reloadModulesQueueItem = new ReloadModulesQueueItem(_analyzer); + if (@params.initializationOptions.displayOptions != null) { _displayOptions = @params.initializationOptions.displayOptions; } @@ -321,7 +353,10 @@ public override async Task DidChangeConfiguration(DidChangeConfigurationParams @ return; } - _queue.Enqueue(new AnalysisQueueWorkItem(() => _analyzer.ReloadModulesAsync().WaitAndUnwrapExceptions()), AnalysisPriority.Normal); + // Make sure reload modules is executed on the analyzer thread. + var task = _reloadModulesQueueItem.Task; + _queue.Enqueue(_reloadModulesQueueItem, AnalysisPriority.Normal); + await task; // re-analyze all of the modules when we get a new set of modules loaded... foreach (var entry in _analyzer.ModulesByFilename) {