Skip to content

Commit e9310f9

Browse files
Merge pull request #2000 from PowerShell/andschwa/code-review
Code clean-up and fixing end-to-end tests
2 parents 97ae16a + 50ee6ad commit e9310f9

27 files changed

+82
-200
lines changed

.editorconfig

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ dotnet_diagnostic.CS0649.severity = error
3939
# CS1570: Parameter has no matching param tag in the XML comment
4040
dotnet_diagnostic.CS1570.severity = silent
4141
# CS1574: XML comment has cref attribute that could not be resolved.
42-
dotnet_diagnostic.CS1574.severity = suggestion
42+
dotnet_diagnostic.CS1574.severity = silent
4343
# CS1591: Missing XML comment for publicly visible type or member
4444
dotnet_diagnostic.CS1591.severity = silent
4545
# CS1998: This async method lacks 'await' operators and will run synchronously

PowerShellEditorServices.build.ps1

+2-2
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ Task TestE2E Build, SetupHelpForTests, {
203203
Set-Location .\test\PowerShellEditorServices.Test.E2E\
204204

205205
$env:PWSH_EXE_NAME = if ($IsCoreCLR) { "pwsh" } else { "powershell" }
206-
Invoke-BuildExec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS72 }
206+
Invoke-BuildExec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS73 }
207207

208208
if (!$script:IsNix) {
209209
if (-not [Security.Principal.WindowsIdentity]::GetCurrent().Owner.IsWellKnown("BuiltInAdministratorsSid")) {
@@ -214,7 +214,7 @@ Task TestE2E Build, SetupHelpForTests, {
214214
try {
215215
Write-Host "Running end-to-end tests in Constrained Language Mode."
216216
[System.Environment]::SetEnvironmentVariable("__PSLockdownPolicy", "0x80000007", [System.EnvironmentVariableTarget]::Machine);
217-
Invoke-BuildExec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS72 }
217+
Invoke-BuildExec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS73 }
218218
} finally {
219219
[System.Environment]::SetEnvironmentVariable("__PSLockdownPolicy", $null, [System.EnvironmentVariableTarget]::Machine);
220220
}

src/PowerShellEditorServices/Extensions/EditorContext.cs

+4-15
Original file line numberDiff line numberDiff line change
@@ -86,26 +86,15 @@ public void SetSelection(
8686
/// </summary>
8787
/// <param name="startPosition">The starting position of the selection.</param>
8888
/// <param name="endPosition">The ending position of the selection.</param>
89-
public void SetSelection(
90-
FilePosition startPosition,
91-
FilePosition endPosition)
92-
{
93-
SetSelection(
94-
new FileRange(
95-
startPosition,
96-
endPosition));
97-
}
89+
public void SetSelection(FilePosition startPosition, FilePosition endPosition) => SetSelection(new FileRange(startPosition, endPosition));
9890

9991
/// <summary>
10092
/// Sets a selection in the host editor's active buffer.
10193
/// </summary>
10294
/// <param name="selectionRange">The range of the selection.</param>
103-
public void SetSelection(FileRange selectionRange)
104-
{
105-
editorOperations
106-
.SetSelectionAsync(selectionRange.ToBufferRange())
107-
.Wait();
108-
}
95+
#pragma warning disable VSTHRD002
96+
public void SetSelection(FileRange selectionRange) => editorOperations.SetSelectionAsync(selectionRange.ToBufferRange()).Wait();
97+
#pragma warning restore VSTHRD002
10998

11099
#endregion
111100
}

src/PowerShellEditorServices/Extensions/EditorFileRanges.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ public interface ILspCurrentFileContext : IFileContext
274274
ILspFileRange SelectionRange { get; }
275275
}
276276

277-
internal struct OmnisharpLspPosition : ILspFilePosition, IEquatable<OmnisharpLspPosition>
277+
internal readonly struct OmnisharpLspPosition : ILspFilePosition, IEquatable<OmnisharpLspPosition>
278278
{
279279
private readonly Position _position;
280280

@@ -287,7 +287,7 @@ internal struct OmnisharpLspPosition : ILspFilePosition, IEquatable<OmnisharpLsp
287287
public bool Equals(OmnisharpLspPosition other) => _position == other._position;
288288
}
289289

290-
internal struct OmnisharpLspRange : ILspFileRange, IEquatable<OmnisharpLspRange>
290+
internal readonly struct OmnisharpLspRange : ILspFileRange, IEquatable<OmnisharpLspRange>
291291
{
292292
private readonly Range _range;
293293

@@ -300,7 +300,7 @@ internal struct OmnisharpLspRange : ILspFileRange, IEquatable<OmnisharpLspRange>
300300
public bool Equals(OmnisharpLspRange other) => _range == other._range;
301301
}
302302

303-
internal struct BufferFilePosition : IFilePosition, IEquatable<BufferFilePosition>
303+
internal readonly struct BufferFilePosition : IFilePosition, IEquatable<BufferFilePosition>
304304
{
305305
private readonly BufferPosition _position;
306306

@@ -317,7 +317,7 @@ public bool Equals(BufferFilePosition other)
317317
}
318318
}
319319

320-
internal struct BufferFileRange : IFileRange, IEquatable<BufferFileRange>
320+
internal readonly struct BufferFileRange : IFileRange, IEquatable<BufferFileRange>
321321
{
322322
private readonly BufferRange _range;
323323

src/PowerShellEditorServices/Extensions/EditorObject.cs

+2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ internal EditorObject(
115115
/// at the time this method is invoked.
116116
/// </summary>
117117
/// <returns>A instance of the EditorContext class.</returns>
118+
#pragma warning disable VSTHRD002, VSTHRD104
118119
public EditorContext GetEditorContext() => _editorOperations.GetEditorContextAsync().Result;
120+
#pragma warning restore VSTHRD002, VSTHRD104
119121

120122
internal void SetAsStaticInstance()
121123
{

src/PowerShellEditorServices/Extensions/EditorWindow.cs

+2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ internal EditorWindow(IEditorOperations editorOperations)
3939
#endregion
4040

4141
#region Public Methods
42+
#pragma warning disable VSTHRD002 // These are public APIs that use async internal methods.
4243

4344
/// <summary>
4445
/// Shows an informational message to the user.
@@ -71,6 +72,7 @@ internal EditorWindow(IEditorOperations editorOperations)
7172
/// <param name="timeout">A timeout in milliseconds for how long the message should remain visible.</param>
7273
public void SetStatusBarMessage(string message, int timeout) => editorOperations.SetStatusBarMessageAsync(message, timeout).Wait();
7374

75+
#pragma warning restore VSTHRD002
7476
#endregion
7577
}
7678
}

src/PowerShellEditorServices/Extensions/EditorWorkspace.cs

+2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public sealed class EditorWorkspace
3131
#endregion
3232

3333
#region Public Methods
34+
#pragma warning disable VSTHRD002 // These are public APIs that use async internal methods.
3435

3536
/// <summary>
3637
/// Creates a new file in the editor
@@ -53,6 +54,7 @@ public sealed class EditorWorkspace
5354
/// <param name="preview">Determines wether the file is opened as a preview or as a durable editor.</param>
5455
public void OpenFile(string filePath, bool preview) => editorOperations.OpenFileAsync(filePath, preview).Wait();
5556

57+
#pragma warning restore VSTHRD002
5658
#endregion
5759
}
5860
}

src/PowerShellEditorServices/Extensions/FileContext.cs

+5-1
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,14 @@ public void InsertText(
209209
/// </summary>
210210
/// <param name="textToInsert">The text string to insert.</param>
211211
/// <param name="insertRange">The buffer range which will be replaced by the string.</param>
212+
#pragma warning disable VSTHRD002
212213
public void InsertText(string textToInsert, IFileRange insertRange)
213214
{
214215
editorOperations
215216
.InsertTextAsync(scriptFile.DocumentUri.ToString(), textToInsert, insertRange.ToBufferRange())
216217
.Wait();
217218
}
219+
#pragma warning restore VSTHRD002
218220

219221
#endregion
220222

@@ -232,6 +234,7 @@ public void InsertText(string textToInsert, IFileRange insertRange)
232234
/// the path where the file should be saved,
233235
/// including the file name with extension as the leaf
234236
/// </param>
237+
#pragma warning disable VSTHRD002
235238
public void SaveAs(string newFilePath)
236239
{
237240
// Do some validation here so that we can provide a helpful error if the path won't work
@@ -244,8 +247,9 @@ public void SaveAs(string newFilePath)
244247
throw new IOException(string.Format("The file '{0}' already exists", absolutePath));
245248
}
246249

247-
editorOperations.SaveFileAsync(scriptFile.FilePath, newFilePath);
250+
editorOperations.SaveFileAsync(scriptFile.FilePath, newFilePath).Wait();
248251
}
252+
#pragma warning restore VSTHRD002
249253

250254
#endregion
251255
}

src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public static EditorServicesServerFactory Create(string logPath, int minimumLogL
8383
/// <param name="inputStream">The protocol transport input stream.</param>
8484
/// <param name="outputStream">The protocol transport output stream.</param>
8585
/// <param name="hostStartupInfo">The host details configuration for Editor Services
86-
/// instantation.</param>
86+
/// instantiation.</param>
8787
/// <returns>A new, unstarted language server instance.</returns>
8888
public PsesLanguageServer CreateLanguageServer(
8989
Stream inputStream,

src/PowerShellEditorServices/Logging/LoggerExtensions.cs

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ namespace Microsoft.PowerShell.EditorServices.Logging
99
{
1010
internal static class LoggerExtensions
1111
{
12+
// TODO: These need to be fixed (and used consistently).
1213
public static void LogException(
1314
this ILogger logger,
1415
string message,

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

-13
Original file line numberDiff line numberDiff line change
@@ -559,19 +559,6 @@ public StackFrameDetails[] GetStackFrames()
559559
}
560560
}
561561

562-
internal StackFrameDetails[] GetStackFrames(CancellationToken cancellationToken)
563-
{
564-
debugInfoHandle.Wait(cancellationToken);
565-
try
566-
{
567-
return stackFrameDetails;
568-
}
569-
finally
570-
{
571-
debugInfoHandle.Release();
572-
}
573-
}
574-
575562
internal async Task<StackFrameDetails[]> GetStackFramesAsync()
576563
{
577564
await debugInfoHandle.WaitAsync().ConfigureAwait(false);

src/PowerShellEditorServices/Services/DebugAdapter/Debugging/InvalidPowerShellExpressionException.cs

+5-4
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ public class InvalidPowerShellExpressionException : Exception
1414
/// Initializes a new instance of the SetVariableExpressionException class.
1515
/// </summary>
1616
/// <param name="message">Message indicating why the expression is invalid.</param>
17-
public InvalidPowerShellExpressionException(string message)
18-
: base(message)
19-
{
20-
}
17+
public InvalidPowerShellExpressionException(string message) : base(message) { }
18+
19+
public InvalidPowerShellExpressionException() { }
20+
21+
public InvalidPowerShellExpressionException(string message, Exception innerException) : base(message, innerException) { }
2122
}
2223
}

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/StackTraceHandler.cs

+8-14
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,19 @@ internal class StackTraceHandler : IStackTraceHandler
1919

2020
public StackTraceHandler(DebugService debugService) => _debugService = debugService;
2121

22-
public Task<StackTraceResponse> Handle(StackTraceArguments request, CancellationToken cancellationToken)
22+
public async Task<StackTraceResponse> Handle(StackTraceArguments request, CancellationToken cancellationToken)
2323
{
24-
StackFrameDetails[] stackFrameDetails =
25-
_debugService.GetStackFrames(cancellationToken);
24+
StackFrameDetails[] stackFrameDetails = await _debugService.GetStackFramesAsync(cancellationToken).ConfigureAwait(false);
2625

2726
// Handle a rare race condition where the adapter requests stack frames before they've
2827
// begun building.
29-
if (stackFrameDetails == null)
28+
if (stackFrameDetails is null)
3029
{
31-
return Task.FromResult(new StackTraceResponse
30+
return new StackTraceResponse
3231
{
3332
StackFrames = Array.Empty<StackFrame>(),
3433
TotalFrames = 0
35-
});
34+
};
3635
}
3736

3837
List<StackFrame> newStackFrames = new();
@@ -52,19 +51,14 @@ public Task<StackTraceResponse> Handle(StackTraceArguments request, Cancellation
5251
{
5352
// Create the new StackFrame object with an ID that can
5453
// be referenced back to the current list of stack frames
55-
//newStackFrames.Add(
56-
// StackFrame.Create(
57-
// stackFrameDetails[i],
58-
// i));
59-
newStackFrames.Add(
60-
LspDebugUtils.CreateStackFrame(stackFrameDetails[i], id: i));
54+
newStackFrames.Add(LspDebugUtils.CreateStackFrame(stackFrameDetails[i], id: i));
6155
}
6256

63-
return Task.FromResult(new StackTraceResponse
57+
return new StackTraceResponse
6458
{
6559
StackFrames = newStackFrames,
6660
TotalFrames = newStackFrames.Count
67-
});
61+
};
6862
}
6963
}
7064
}

src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public static Task<DscBreakpointCapability> GetDscCapabilityAsync(
9797
return Task.FromResult<DscBreakpointCapability>(null);
9898
}
9999

100-
Func<SMA.PowerShell, CancellationToken, DscBreakpointCapability> getDscBreakpointCapabilityFunc = (pwsh, _) =>
100+
DscBreakpointCapability getDscBreakpointCapabilityFunc(SMA.PowerShell pwsh, CancellationToken _)
101101
{
102102
PSInvocationSettings invocationSettings = new()
103103
{
@@ -159,7 +159,7 @@ public static Task<DscBreakpointCapability> GetDscCapabilityAsync(
159159
logger.LogTrace($"DSC resources found: {resourcePaths.Count}");
160160

161161
return capability;
162-
};
162+
}
163163

164164
return psesHost.ExecuteDelegateAsync(
165165
nameof(getDscBreakpointCapabilityFunc),

src/PowerShellEditorServices/Services/PowerShell/Handlers/GetVersionHandler.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ namespace Microsoft.PowerShell.EditorServices.Handlers
1010
{
1111
internal class GetVersionHandler : IGetVersionHandler
1212
{
13-
public async Task<PowerShellVersion> Handle(GetVersionParams request, CancellationToken cancellationToken)
13+
public Task<PowerShellVersion> Handle(GetVersionParams request, CancellationToken cancellationToken)
1414
{
15-
return new PowerShellVersion
15+
return Task.FromResult(new PowerShellVersion
1616
{
1717
Version = VersionUtils.PSVersionString,
1818
Edition = VersionUtils.PSEdition,
1919
Commit = VersionUtils.GitCommitId,
2020
Architecture = VersionUtils.Architecture
21-
};
21+
});
2222
}
2323
}
2424
}

src/PowerShellEditorServices/Services/PowerShell/Handlers/PSHostProcessAndRunspaceHandlers.cs

+1-4
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,7 @@ public async Task<RunspaceResponse[]> Handle(GetRunspaceParams request, Cancella
6262
{
6363
IEnumerable<PSObject> runspaces = null;
6464

65-
if (request.ProcessId == null)
66-
{
67-
request.ProcessId = "current";
68-
}
65+
request.ProcessId ??= "current";
6966

7067
// If the processId is a valid int, we need to run Get-Runspace within that process
7168
// otherwise just use the current runspace.

src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Concurrent;
66
using System.Collections.Generic;
7+
using System.Linq;
78
using System.Management.Automation;
89
using System.Threading;
910
using System.Threading.Tasks;
@@ -241,7 +242,7 @@ public static async Task<AliasMap> GetAliasesAsync(
241242
.AddParameter("CommandType", CommandTypes.Alias),
242243
cancellationToken).ConfigureAwait(false);
243244

244-
foreach (AliasInfo aliasInfo in aliases)
245+
foreach (AliasInfo aliasInfo in aliases.Cast<AliasInfo>())
245246
{
246247
// TODO: When we move to netstandard2.1, we can use another overload which generates
247248
// static delegates and thus reduces allocations.

src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs

+1-10
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,7 @@ public PsesCodeActionHandler(ILoggerFactory factory, AnalysisService analysisSer
3535
CodeActionKinds = new CodeActionKind[] { CodeActionKind.QuickFix }
3636
};
3737

38-
// TODO: Either fix or ignore "method lacks 'await'" warning.
39-
public override async Task<CodeAction> Handle(CodeAction request, CancellationToken cancellationToken)
40-
{
41-
// TODO: How on earth do we handle a CodeAction? This is new...
42-
if (cancellationToken.IsCancellationRequested)
43-
{
44-
_logger.LogDebug("CodeAction request canceled for: {Title}", request.Title);
45-
}
46-
return request;
47-
}
38+
public override Task<CodeAction> Handle(CodeAction request, CancellationToken cancellationToken) => Task.FromResult(request);
4839

4940
public override async Task<CommandOrCodeActionContainer> Handle(CodeActionParams request, CancellationToken cancellationToken)
5041
{

src/PowerShellEditorServices/Services/TextDocument/TokenOperations.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ internal static FoldingReferenceList FoldableReferences(Token[] tokens)
136136
refList.SafeAdd(CreateFoldingReference(blockStartToken, blockNextLine - 1, FoldingRangeKind.Comment));
137137
blockStartToken = token;
138138
}
139-
if (blockStartToken == null) { blockStartToken = token; }
139+
blockStartToken ??= token;
140140
blockNextLine = thisLine + 1;
141141
}
142142

src/PowerShellEditorServices/Services/Workspace/Handlers/WorkspaceSymbolsHandler.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public override async Task<Container<SymbolInformation>> Handle(WorkspaceSymbolP
3939

4040
foreach (ScriptFile scriptFile in _workspaceService.GetOpenedFiles())
4141
{
42+
_logger.LogDebug($"Handling workspace symbols request for: {request.Query}");
4243
IEnumerable<SymbolReference> foundSymbols = _symbolsService.FindSymbolsInFile(scriptFile);
4344

4445
// TODO: Need to compute a relative path that is based on common path for all workspace files
@@ -89,7 +90,7 @@ public override async Task<Container<SymbolInformation>> Handle(WorkspaceSymbolP
8990
});
9091
}
9192
}
92-
_logger.LogWarning("Logging in a handler works now.");
93+
9394
return new Container<SymbolInformation>(symbols);
9495
}
9596

0 commit comments

Comments
 (0)