Skip to content

Commit 6ab2d7a

Browse files
davidwengierdavidwengier
committed
Use a proper Try method with out parameters and clean up code as a result
1 parent 803a7dd commit 6ab2d7a

File tree

3 files changed

+52
-52
lines changed

3 files changed

+52
-52
lines changed

‎src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/RazorCompletionResolveEndpoint.cs‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the MIT license. See License.txt in the project root for license information.
33

4+
using System.Diagnostics.CodeAnalysis;
45
using System.Linq;
56
using System.Threading;
67
using System.Threading.Tasks;
@@ -33,37 +34,8 @@ public void ApplyCapabilities(VSInternalServerCapabilities serverCapabilities, V
3334

3435
public async Task<VSInternalCompletionItem> HandleRequestAsync(VSInternalCompletionItem completionItem, RazorRequestContext requestContext, CancellationToken cancellationToken)
3536
{
36-
if (!completionItem.TryGetCompletionListResultIds(out var resultIds))
37-
{
38-
// Unable to lookup completion item result info
39-
return completionItem;
40-
}
41-
42-
ICompletionResolveContext? originalRequestContext = null;
43-
VSInternalCompletionList? containingCompletionList = null;
44-
foreach (var resultId in resultIds)
45-
{
46-
if (!_completionListCache.TryGet(resultId, out var cacheEntry))
47-
{
48-
continue;
49-
}
50-
51-
// See if this is the right completion list for this corresponding completion item. We cross-check this based on label only given that
52-
// is what users interact with.
53-
if (cacheEntry.CompletionList.Items.Any(completion =>
54-
completionItem.Label == completion.Label
55-
// Check the Kind as well, e.g. we may have a Razor snippet and a C# keyword with the same label, etc.
56-
&& completionItem.Kind == completion.Kind))
57-
{
58-
originalRequestContext = cacheEntry.Context;
59-
containingCompletionList = cacheEntry.CompletionList;
60-
break;
61-
}
62-
}
63-
64-
if (containingCompletionList is null || originalRequestContext is null)
37+
if (!TryGetOriginalRequestData(completionItem, out var containingCompletionList, out var originalRequestContext))
6538
{
66-
// Couldn't find an associated completion list
6739
return completionItem;
6840
}
6941

@@ -81,4 +53,33 @@ public async Task<VSInternalCompletionItem> HandleRequestAsync(VSInternalComplet
8153

8254
return resolvedCompletionItem;
8355
}
56+
57+
private bool TryGetOriginalRequestData(VSInternalCompletionItem completionItem, [NotNullWhen(true)] out VSInternalCompletionList? completionList, [NotNullWhen(true)] out ICompletionResolveContext? context)
58+
{
59+
context = null;
60+
completionList = null;
61+
62+
if (!completionItem.TryGetCompletionListResultIds(out var resultIds))
63+
{
64+
// Unable to lookup completion item result info
65+
return false;
66+
}
67+
68+
foreach (var resultId in resultIds)
69+
{
70+
// See if this is the right completion list for this corresponding completion item. We cross-check this based on label only given that
71+
// is what users interact with.
72+
if (_completionListCache.TryGet(resultId, out completionList, out context) &&
73+
completionList.Items.Any(
74+
completion =>
75+
completionItem.Label == completion.Label &&
76+
// Check the Kind as well, e.g. we may have a Razor snippet and a C# keyword with the same label, etc.
77+
completionItem.Kind == completion.Kind))
78+
{
79+
return true;
80+
}
81+
}
82+
83+
return false;
84+
}
8485
}

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/CompletionListCache.cs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the MIT license. See License.txt in the project root for license information.
33

4-
using System;
4+
using System.Diagnostics.CodeAnalysis;
55
using Microsoft.VisualStudio.LanguageServer.Protocol;
66

77
namespace Microsoft.CodeAnalysis.Razor.Completion;
@@ -23,11 +23,6 @@ private record struct Slot(int Id, VSInternalCompletionList CompletionList, ICom
2323

2424
public int Add(VSInternalCompletionList completionList, ICompletionResolveContext context)
2525
{
26-
if (completionList is null)
27-
{
28-
throw new ArgumentNullException(nameof(completionList));
29-
}
30-
3126
lock (_accessLock)
3227
{
3328
var index = _nextIndex++;
@@ -48,7 +43,7 @@ public int Add(VSInternalCompletionList completionList, ICompletionResolveContex
4843
}
4944
}
5045

51-
public bool TryGet(int id, out (VSInternalCompletionList CompletionList, ICompletionResolveContext Context) result)
46+
public bool TryGet(int id, [NotNullWhen(true)] out VSInternalCompletionList? completionList, [NotNullWhen(true)] out ICompletionResolveContext? context)
5247
{
5348
lock (_accessLock)
5449
{
@@ -72,15 +67,17 @@ public bool TryGet(int id, out (VSInternalCompletionList CompletionList, IComple
7267

7368
if (slot.Id == id)
7469
{
75-
result = (slot.CompletionList, slot.Context);
70+
completionList = slot.CompletionList;
71+
context = slot.Context;
7672
return true;
7773
}
7874

7975
count--;
8076
}
8177

8278
// A cache entry associated with the given id was not found.
83-
result = default;
79+
completionList = null;
80+
context = null;
8481
return false;
8582
}
8683
}

‎src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Completion/CompletionListCacheTest.cs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ public void TryGet_SetCompletionList_ReturnsTrue()
2121
var resultId = _completionListCache.Add(completionList, _context);
2222

2323
// Act
24-
var result = _completionListCache.TryGet(resultId, out var cacheEntry);
24+
var result = _completionListCache.TryGet(resultId, out var cachedCompletionList, out var context);
2525

2626
// Assert
2727
Assert.True(result);
28-
Assert.Same(completionList, cacheEntry.CompletionList);
29-
Assert.Same(_context, cacheEntry.Context);
28+
Assert.Same(completionList, cachedCompletionList);
29+
Assert.Same(_context, context);
3030
}
3131

3232
[Fact]
@@ -44,23 +44,24 @@ public void TryGet_SetCompletionListOnFullCache_ReturnsTrue()
4444
var resultId = _completionListCache.Add(completionList, _context);
4545

4646
// Act
47-
var result = _completionListCache.TryGet(resultId, out var cacheEntry);
47+
var result = _completionListCache.TryGet(resultId, out var cachedCompletionList, out var context);
4848

4949
// Assert
5050
Assert.True(result);
51-
Assert.Same(completionList, cacheEntry.CompletionList);
52-
Assert.Same(_context, cacheEntry.Context);
51+
Assert.Same(completionList, cachedCompletionList);
52+
Assert.Same(_context, context);
5353
}
5454

5555
[Fact]
5656
public void TryGet_UnknownCompletionList_ReturnsTrue()
5757
{
5858
// Act
59-
var result = _completionListCache.TryGet(1234, out var cachedEntry);
59+
var result = _completionListCache.TryGet(1234, out var cachedCompletionList, out var context);
6060

6161
// Assert
6262
Assert.False(result);
63-
Assert.Equal(default, cachedEntry);
63+
Assert.Null(cachedCompletionList);
64+
Assert.Null(context);
6465
}
6566

6667
[Fact]
@@ -77,12 +78,12 @@ public void TryGet_LastCompletionList_ReturnsTrue()
7778
}
7879

7980
// Act
80-
var result = _completionListCache.TryGet(initialCompletionListResultId, out var cachedEntry);
81+
var result = _completionListCache.TryGet(initialCompletionListResultId, out var cachedCompletionList, out var context);
8182

8283
// Assert
8384
Assert.True(result);
84-
Assert.Same(initialCompletionList, cachedEntry.CompletionList);
85-
Assert.Same(_context, cachedEntry.Context);
85+
Assert.Same(initialCompletionList, cachedCompletionList);
86+
Assert.Same(_context, context);
8687
}
8788

8889
[Fact]
@@ -99,10 +100,11 @@ public void TryGet_EvictedCompletionList_ReturnsFalse()
99100
}
100101

101102
// Act
102-
var result = _completionListCache.TryGet(initialCompletionListResultId, out var cachedEntry);
103+
var result = _completionListCache.TryGet(initialCompletionListResultId, out var cachedCompletionList, out var context);
103104

104105
// Assert
105106
Assert.False(result);
106-
Assert.Equal(default, cachedEntry);
107+
Assert.Null(cachedCompletionList);
108+
Assert.Null(context);
107109
}
108110
}

0 commit comments

Comments
 (0)