Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Fix invalid insertTextFormat in CompletionItem instances #344

Merged
merged 3 commits into from
Nov 5, 2018
Merged

Fix invalid insertTextFormat in CompletionItem instances #344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
556e805
fix invalid insertTextFormat in CompletionItem instances
jakebailey Nov 5, 2018
40cdf1f
check insertTextFormat in AssertCompletion, defaulted to PlainText
jakebailey Nov 5, 2018
e7e5184
only check items which are in contains for format matching, not all
jakebailey Nov 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/Analysis/Engine/Test/CompletionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ class oar(list):
var completions = await server.SendCompletion(uri, 2, 8);

completions.Should().HaveItem("append")
.Which.Should().HaveInsertText($"append(self, value):{Environment.NewLine} return super(oar, self).append(value)");
.Which.Should().HaveInsertText($"append(self, value):{Environment.NewLine} return super(oar, self).append(value)")
.And.HaveInsertTextFormat(InsertTextFormat.PlainText);
}

[DataRow(PythonLanguageVersion.V36, "value")]
Expand All @@ -259,7 +260,8 @@ class oar(list):
var completions = await server.SendCompletion(uri, 2, 8);

completions.Should().HaveItem("append")
.Which.Should().HaveInsertText($"append(self, {parameterName}):{Environment.NewLine} return super().append({parameterName})");
.Which.Should().HaveInsertText($"append(self, {parameterName}):{Environment.NewLine} return super().append({parameterName})")
.And.HaveInsertTextFormat(InsertTextFormat.PlainText);
}

[ServerTestMethod(LatestAvailable2X = true), Priority(0)]
Expand Down Expand Up @@ -417,7 +419,8 @@ public async Task AddBracketsEnabled(Server server, string code, int row, int ch
var completion = await server.SendCompletion(uri, row, character);

completion.Should().HaveItem(expectedLabel)
.Which.Should().HaveInsertText(expectedInsertText);
.Which.Should().HaveInsertText(expectedInsertText)
.And.HaveInsertTextFormat(InsertTextFormat.Snippet);
}

[DataRow(PythonLanguageMajorVersion.LatestV2, "foo(self):{0} return super(B, self).foo()")]
Expand All @@ -438,7 +441,8 @@ class B(A):
var completion = await server.SendCompletion(uri, 6, 9);

completion.Should().HaveItem("foo")
.Which.Should().HaveInsertText(expectedInsertText);
.Which.Should().HaveInsertText(expectedInsertText)
.And.HaveInsertTextFormat(InsertTextFormat.PlainText);
}

[TestMethod, Priority(0)]
Expand Down Expand Up @@ -499,7 +503,8 @@ public async Task Completion_PackageRelativeImport(Server server) {
var completion = await server.SendCompletion(uri, 0, 22);

completion.Should().HaveItem("right")
.Which.Should().HaveInsertText("right");
.Which.Should().HaveInsertText("right")
.And.HaveInsertTextFormat(InsertTextFormat.PlainText);
}

[DataRow(true)]
Expand Down Expand Up @@ -1008,7 +1013,7 @@ public async Task MultiPartDocument() {
public async Task WithWhitespaceAroundDot() {
using (var s = await CreateServerAsync()) {
var u = await s.OpenDefaultDocumentAndGetUriAsync("import sys\nsys . version\n");
await AssertCompletion(s, u, new[] { "argv" }, null, new SourceLocation(2, 7),
await AssertCompletion(s, u, new[] { "argv" }, null, new SourceLocation(2, 7),
new CompletionContext { triggerCharacter = ".", triggerKind = CompletionTriggerKind.TriggerCharacter });
}
}
Expand All @@ -1029,7 +1034,7 @@ public async Task MarkupKindValid() {
}
}

private static async Task AssertCompletion(Server s, Uri uri, IReadOnlyCollection<string> contains, IReadOnlyCollection<string> excludes, Position? position = null, CompletionContext? context = null, Func<CompletionItem, string> cmpKey = null, string expr = null, Range? applicableSpan = null) {
private static async Task AssertCompletion(Server s, Uri uri, IReadOnlyCollection<string> contains, IReadOnlyCollection<string> excludes, Position? position = null, CompletionContext? context = null, Func<CompletionItem, string> cmpKey = null, string expr = null, Range? applicableSpan = null, InsertTextFormat? allFormat = InsertTextFormat.PlainText) {
await s.WaitForCompleteAnalysisAsync(CancellationToken.None);
var res = await s.Completion(new CompletionParams {
textDocument = new TextDocumentIdentifier { uri = uri },
Expand All @@ -1040,10 +1045,14 @@ private static async Task AssertCompletion(Server s, Uri uri, IReadOnlyCollectio
DumpDetails(res);

cmpKey = cmpKey ?? (c => c.insertText);
var items = res.items?.Select(cmpKey).ToList() ?? new List<string>();
var items = res.items?.Select(i => (cmpKey(i), i.insertTextFormat)).ToList() ?? new List<(string, InsertTextFormat)>();

if (contains != null && contains.Any()) {
items.Should().Contain(contains);
items.Select(i => i.Item1).Should().Contain(contains);

if (allFormat != null) {
items.Where(i => contains.Contains(i.Item1)).Select(i => i.Item2).Should().AllBeEquivalentTo(allFormat);
}
}

if (excludes != null && excludes.Any()) {
Expand Down Expand Up @@ -1119,4 +1128,4 @@ await s.DidOpenTextDocument(new DidOpenTextDocumentParams {
return uri;
}
}
}
}
11 changes: 10 additions & 1 deletion src/Analysis/Engine/Test/FluentAssertions/CompletionItemAssertions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ public AndConstraint<CompletionItemAssertions> HaveInsertText(string insertText,
return new AndConstraint<CompletionItemAssertions>(this);
}

[CustomAssertion]
public AndConstraint<CompletionItemAssertions> HaveInsertTextFormat(InsertTextFormat insertTextFormat, string because = "", params object[] reasonArgs) {
Execute.Assertion.ForCondition(Subject.insertTextFormat == insertTextFormat)
.BecauseOf(because, reasonArgs)
.FailWith($"Expected '{Subject.label}' completion to have insert text format '{insertTextFormat}'{{reason}}, but it has '{Subject.insertTextFormat}'");

return new AndConstraint<CompletionItemAssertions>(this);
}

[CustomAssertion]
public AndConstraint<CompletionItemAssertions> HaveDocumentation(string documentation, string because = "", params object[] reasonArgs) {
Execute.Assertion.BecauseOf(because, reasonArgs)
Expand All @@ -51,4 +60,4 @@ public AndConstraint<CompletionItemAssertions> HaveDocumentation(string document
return new AndConstraint<CompletionItemAssertions>(this);
}
}
}
}
13 changes: 8 additions & 5 deletions src/LanguageServer/Impl/Implementation/CompletionAnalysis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ public IEnumerable<CompletionItem> GetCompletions() {
return null;
}

return addMetadataArg
? GetCompletionsFromTopLevel().Append(MetadataArgCompletion)
return addMetadataArg
? GetCompletionsFromTopLevel().Append(MetadataArgCompletion)
: GetCompletionsFromTopLevel();

case ForStatement forStatement when TryGetCompletionsInForStatement(forStatement, out var result):
Expand Down Expand Up @@ -269,7 +269,7 @@ private IEnumerable<CompletionItem> GetModules(string[] names, bool includeMembe
return GetModules();
}

private IEnumerable<CompletionItem> GetModules()
private IEnumerable<CompletionItem> GetModules()
=> Analysis.ProjectState.GetModules().Select(ToCompletionItem);

private IEnumerable<CompletionItem> GetModulesFromNode(DottedName name, bool includeMembers = false) => GetModules(GetNamesFromDottedName(name), includeMembers);
Expand Down Expand Up @@ -410,6 +410,7 @@ private CompletionItem ToOverrideCompletionItem(IOverloadResult o, ClassDefiniti
return new CompletionItem {
label = o.Name,
insertText = MakeOverrideCompletionString(indent, o, cd.Name),
insertTextFormat = InsertTextFormat.PlainText,
kind = CompletionItemKind.Method
};
}
Expand Down Expand Up @@ -799,10 +800,11 @@ private CompletionItem ToCompletionItem(IMemberResult m) {

var doc = _textBuilder.GetDocumentation(m.Values, string.Empty);
var kind = ToCompletionItemKind(m.MemberType);

var res = new CompletionItem {
label = m.Name,
insertText = completion,
insertTextFormat = InsertTextFormat.PlainText,
documentation = string.IsNullOrWhiteSpace(doc) ? null : new MarkupContent {
kind = _textBuilder.DisplayOptions.preferredFormat,
value = doc
Expand All @@ -825,6 +827,7 @@ private static CompletionItem ToCompletionItem(string text, PythonMemberType typ
return new CompletionItem {
label = label ?? text,
insertText = text,
insertTextFormat = InsertTextFormat.PlainText,
// Place regular items first, advanced entries last
sortText = char.IsLetter(text, 0) ? "1" : "2",
kind = ToCompletionItemKind(type),
Expand Down Expand Up @@ -976,4 +979,4 @@ private IEnumerable<Token> ReadExpressionTokens(IEnumerable<KeyValuePair<IndexSp
return exprTokens;
}
}
}
}
2 changes: 2 additions & 0 deletions src/LanguageServer/Impl/Implementation/Server.Completion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ private bool HandleOldStyleCompletionExtension(ModuleAnalysis analysis, PythonAs
filterText = x.filterText,
preselect = x.preselect,
insertText = x.insertText,
insertTextFormat = (PythonTools.Analysis.LanguageServer.InsertTextFormat)x.insertTextFormat,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using? Definitions that are in the engine are obsolete and are kept around for backward compat. POR to remove them in a month or so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if any of that is used, but I wanted to be thorough and make sure it's set anywhere it can be. Once all of the old types go away this'll probably also be deleted (and we can probably start setting sane defaults too).

}).ToArray()
};

Expand All @@ -169,6 +170,7 @@ private bool HandleOldStyleCompletionExtension(ModuleAnalysis analysis, PythonAs
filterText = x.filterText,
preselect = x.preselect,
insertText = x.insertText,
insertTextFormat = (InsertTextFormat)x.insertTextFormat,
textEdit = x.textEdit.HasValue
? new TextEdit {
range = new Range {
Expand Down