Skip to content

Commit

Permalink
Avoid showing completions inside strings (#1933)
Browse files Browse the repository at this point in the history
  • Loading branch information
anthony-c-martin authored Mar 19, 2021
1 parent 968d68b commit 7a958ef
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,20 @@
"command": "editor.action.triggerParameterHints"
}
},
{
"label": "bracketAtBeginning",
"kind": "variable",
"detail": "bracketAtBeginning",
"deprecated": false,
"preselect": false,
"sortText": "2_bracketAtBeginning",
"insertTextFormat": "plainText",
"insertTextMode": "asIs",
"textEdit": {
"range": {},
"newText": "bracketAtBeginning"
}
},
{
"label": "bracketInTheMiddle",
"kind": "variable",
Expand Down Expand Up @@ -517,20 +531,6 @@
"newText": "emptyJsonArray"
}
},
{
"label": "enclosingBrackets",
"kind": "variable",
"detail": "enclosingBrackets",
"deprecated": false,
"preselect": false,
"sortText": "2_enclosingBrackets",
"insertTextFormat": "plainText",
"insertTextMode": "asIs",
"textEdit": {
"range": {},
"newText": "enclosingBrackets"
}
},
{
"label": "endsWith",
"kind": "function",
Expand Down
5 changes: 3 additions & 2 deletions src/Bicep.Core.Samples/Files/Variables_LF/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ var curliesInInterp = '{${123}{0}${true}}'

// verify correct bracket escaping
var bracketInTheMiddle = 'a[b]'
// #completionTest(25) -> symbolsPlusTypes
// #completionTest(25) -> empty
var bracketAtBeginning = '[test'
// #completionTest(23) -> symbolsPlusTypes
var enclosingBrackets = '[test]'
var emptyJsonArray = '[]'
var interpolatedBrackets = '[${myInt}]'
Expand Down Expand Up @@ -287,4 +288,4 @@ module.exports = function (context) {
// BUG #2: Data is not guaranteed to be read before the Azure Function's invocation ends
context.done();
}
'''
'''
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ var curliesInInterp = '{${123}{0}${true}}'

// verify correct bracket escaping
var bracketInTheMiddle = 'a[b]'
// #completionTest(25) -> symbolsPlusTypes
// #completionTest(25) -> empty
var bracketAtBeginning = '[test'
// #completionTest(23) -> symbolsPlusTypes
var enclosingBrackets = '[test]'
var emptyJsonArray = '[]'
var interpolatedBrackets = '[${myInt}]'
Expand Down Expand Up @@ -288,3 +289,4 @@ module.exports = function (context) {
context.done();
}
'''

Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ var curliesInInterp = '{${123}{0}${true}}'

// verify correct bracket escaping
var bracketInTheMiddle = 'a[b]'
// #completionTest(25) -> symbolsPlusTypes
// #completionTest(25) -> empty
var bracketAtBeginning = '[test'
// #completionTest(23) -> symbolsPlusTypes
var enclosingBrackets = '[test]'
var emptyJsonArray = '[]'
var interpolatedBrackets = '[${myInt}]'
Expand Down
4 changes: 3 additions & 1 deletion src/Bicep.Core.Samples/Files/Variables_LF/main.symbols.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ var curliesInInterp = '{${123}{0}${true}}'
// verify correct bracket escaping
var bracketInTheMiddle = 'a[b]'
//@[4:22) Variable bracketInTheMiddle. Type: 'a[b]'. Declaration start char: 0, length: 31
// #completionTest(25) -> symbolsPlusTypes
// #completionTest(25) -> empty
var bracketAtBeginning = '[test'
//@[4:22) Variable bracketAtBeginning. Type: '[test'. Declaration start char: 0, length: 32
// #completionTest(23) -> symbolsPlusTypes
var enclosingBrackets = '[test]'
//@[4:21) Variable enclosingBrackets. Type: '[test]'. Declaration start char: 0, length: 32
var emptyJsonArray = '[]'
Expand Down Expand Up @@ -381,3 +382,4 @@ module.exports = function (context) {
context.done();
}
'''

10 changes: 7 additions & 3 deletions src/Bicep.Core.Samples/Files/Variables_LF/main.syntax.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ var bracketInTheMiddle = 'a[b]'
//@[25:31) StringSyntax
//@[25:31) StringComplete |'a[b]'|
//@[31:32) NewLine |\n|
// #completionTest(25) -> symbolsPlusTypes
//@[42:43) NewLine |\n|
// #completionTest(25) -> empty
//@[31:32) NewLine |\n|
var bracketAtBeginning = '[test'
//@[0:32) VariableDeclarationSyntax
//@[0:3) Identifier |var|
Expand All @@ -151,6 +151,8 @@ var bracketAtBeginning = '[test'
//@[25:32) StringSyntax
//@[25:32) StringComplete |'[test'|
//@[32:33) NewLine |\n|
// #completionTest(23) -> symbolsPlusTypes
//@[42:43) NewLine |\n|
var enclosingBrackets = '[test]'
//@[0:32) VariableDeclarationSyntax
//@[0:3) Identifier |var|
Expand Down Expand Up @@ -2309,4 +2311,6 @@ module.exports = function (context) {
context.done();
}
'''
//@[3:3) EndOfFile ||
//@[3:4) NewLine |\n|
//@[0:0) EndOfFile ||
10 changes: 7 additions & 3 deletions src/Bicep.Core.Samples/Files/Variables_LF/main.tokens.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,16 @@ var bracketInTheMiddle = 'a[b]'
//@[23:24) Assignment |=|
//@[25:31) StringComplete |'a[b]'|
//@[31:32) NewLine |\n|
// #completionTest(25) -> symbolsPlusTypes
//@[42:43) NewLine |\n|
// #completionTest(25) -> empty
//@[31:32) NewLine |\n|
var bracketAtBeginning = '[test'
//@[0:3) Identifier |var|
//@[4:22) Identifier |bracketAtBeginning|
//@[23:24) Assignment |=|
//@[25:32) StringComplete |'[test'|
//@[32:33) NewLine |\n|
// #completionTest(23) -> symbolsPlusTypes
//@[42:43) NewLine |\n|
var enclosingBrackets = '[test]'
//@[0:3) Identifier |var|
//@[4:21) Identifier |enclosingBrackets|
Expand Down Expand Up @@ -1470,4 +1472,6 @@ module.exports = function (context) {
context.done();
}
'''
//@[3:3) EndOfFile ||
//@[3:4) NewLine |\n|
//@[0:0) EndOfFile ||
47 changes: 47 additions & 0 deletions src/Bicep.LangServer.IntegrationTests/CompletionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@
using System.Threading.Tasks;
using Bicep.Core.Extensions;
using Bicep.Core.FileSystem;
using Bicep.Core.Parsing;
using Bicep.Core.Samples;
using Bicep.Core.Syntax;
using Bicep.Core.Text;
using Bicep.Core.TypeSystem.Az;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Utils;
using Bicep.LangServer.IntegrationTests.Completions;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
Expand Down Expand Up @@ -84,6 +88,49 @@ public async Task CompletionRequestShouldProduceExpectedCompletions(DataSet data
ValidateCompletions(dataSet, setName, intermediate);
}

[TestMethod]
public async Task String_segments_do_not_return_completions()
{
var fileWithCursors = @"
var completeString = |'he|llo'|
var interpolatedString = |'abc${|true}|de|f${|false}|gh|i'|
var multilineString = |'''|
hel|lo
'''|
";
var bicepFile = fileWithCursors.Replace("|", "");
var syntaxTree = SyntaxTree.Create(new Uri("file:///main.bicep"), bicepFile);

var cursors = new List<int>();
for (var i = 0; i < fileWithCursors.Length; i++)
{
if (fileWithCursors[i] == '|')
{
cursors.Add(i - cursors.Count);
}
}

using var client = await IntegrationTestHelper.StartServerWithTextAsync(bicepFile, syntaxTree.FileUri, resourceTypeProvider: TypeProvider);

foreach (var cursor in cursors)
{
using var assertionScope = new AssertionScope();
assertionScope.AddReportable(
"completion context",
PrintHelper.PrintWithAnnotations(syntaxTree, new [] {
new PrintHelper.Annotation(new TextSpan(cursor, 0), "cursor position"),
}, 1, true));

var completions = await client.RequestCompletion(new CompletionParams
{
TextDocument = new TextDocumentIdentifier(syntaxTree.FileUri),
Position = TextCoordinateConverter.GetPosition(syntaxTree.LineStarts, cursor),
});

completions.Should().BeEmpty();
}
}

private void ValidateCompletions(DataSet dataSet, string setName, List<(Position position, JToken actual)> intermediate)
{
// if the test author asserts on the wrong completion set in their tests
Expand Down
19 changes: 18 additions & 1 deletion src/Bicep.LangServer/Completions/BicepCompletionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,24 @@ parent is OutputDeclarationSyntax ||
/// Determines if we are inside an expression. Will not produce a correct result if context kind is set is already set to something.
/// </summary>
/// <param name="matchingNodes">The matching nodes</param>
private static bool IsInnerExpressionContext(List<SyntaxBase> matchingNodes) => matchingNodes.OfType<ExpressionSyntax>().Any();
private static bool IsInnerExpressionContext(List<SyntaxBase> matchingNodes)
{
var isInStringSegment = SyntaxMatcher.IsTailMatch<StringSyntax, Token>(matchingNodes, (_, token) => token.Type switch {
TokenType.StringComplete => true,
TokenType.StringLeftPiece => true,
TokenType.StringMiddlePiece => true,
TokenType.StringRightPiece => true,
TokenType.MultilineString => true,
_ => false,
});

if (isInStringSegment)
{
return false;
}

return matchingNodes.OfType<ExpressionSyntax>().Any();
}

private static Range GetReplacementRange(SyntaxTree syntaxTree, SyntaxBase innermostMatchingNode, int offset)
{
Expand Down

0 comments on commit 7a958ef

Please sign in to comment.