Skip to content

Add regression test for untitled scripts in Windows PowerShell #1830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 15, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,12 @@ public Task<ConfigurationDoneResponse> Handle(ConfigurationDoneArguments request
private async Task LaunchScriptAsync(string scriptToLaunch)
{
PSCommand command;
if (ScriptFile.IsUntitledPath(scriptToLaunch))
// Script could an actual script, or a URI to a script file (or untitled document).
if (!System.Uri.IsWellFormedUriString(scriptToLaunch, System.UriKind.RelativeOrAbsolute)
|| ScriptFile.IsUntitledPath(scriptToLaunch))
{
ScriptFile untitledScript = _workspaceService.GetFile(scriptToLaunch);
if (BreakpointApiUtils.SupportsBreakpointApis(_runspaceContext.CurrentRunspace))
bool isScriptFile = _workspaceService.TryGetFile(scriptToLaunch, out ScriptFile untitledScript);
if (isScriptFile && BreakpointApiUtils.SupportsBreakpointApis(_runspaceContext.CurrentRunspace))
{
// Parse untitled files with their `Untitled:` URI as the filename which will
// cache the URI and contents within the PowerShell parser. By doing this, we
Expand Down Expand Up @@ -138,7 +140,11 @@ private async Task LaunchScriptAsync(string scriptToLaunch)
// Command breakpoints and `Wait-Debugger` will work. We must wrap the script
// with newlines so that any included comments don't break the command.
command = PSCommandHelpers.BuildDotSourceCommandWithArguments(
string.Concat("{\n", untitledScript.Contents, "\n}"), _debugStateService.Arguments);
string.Concat(
"{" + System.Environment.NewLine,
isScriptFile ? untitledScript.Contents : scriptToLaunch,
System.Environment.NewLine + "}"),
_debugStateService.Arguments);
}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,18 +195,17 @@ internal static List<string> GetLinesInternal(string text)
}

/// <summary>
/// Deterines whether the supplied path indicates the file is an "untitled:Unitled-X"
/// Determines whether the supplied path indicates the file is an "untitled:Untitled-X"
/// which has not been saved to file.
/// </summary>
/// <param name="path">The path to check.</param>
/// <returns>True if the path is an untitled file, false otherwise.</returns>
internal static bool IsUntitledPath(string path)
{
Validate.IsNotNull(nameof(path), path);
return !string.Equals(
DocumentUri.From(path).Scheme,
Uri.UriSchemeFile,
StringComparison.OrdinalIgnoreCase);
// This may not have been given a URI, so return false instead of throwing.
return Uri.IsWellFormedUriString(path, UriKind.RelativeOrAbsolute) &&
!string.Equals(DocumentUri.From(path).Scheme, Uri.UriSchemeFile, StringComparison.OrdinalIgnoreCase);
Comment on lines +207 to +208
Copy link
Member Author

Choose a reason for hiding this comment

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

Double-check me here please. As far as I can tell, this is expecting a URI?

}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,12 @@ public ScriptFile GetFile(DocumentUri documentUri)
/// </summary>
/// <param name="filePath">The file path at which the script resides.</param>
/// <param name="scriptFile">The out parameter that will contain the ScriptFile object.</param>
public bool TryGetFile(string filePath, out ScriptFile scriptFile) =>
TryGetFile(new Uri(filePath), out scriptFile);
public bool TryGetFile(string filePath, out ScriptFile scriptFile)
{
scriptFile = null;
return Uri.IsWellFormedUriString(filePath, UriKind.RelativeOrAbsolute)
&& TryGetFile(new Uri(filePath), out scriptFile);
}

/// <summary>
/// Tries to get an open file in the workspace. Returns true if it succeeds, false otherwise.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ namespace PowerShellEditorServices.Test.E2E
{
public static class DebugAdapterClientExtensions
{
public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient, string filePath, TaskCompletionSource<object> started)
public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient, string script, TaskCompletionSource<object> started)
{
LaunchResponse launchResponse = await debugAdapterClient.Launch(
new PsesLaunchRequestArguments
{
NoDebug = false,
Script = filePath,
Script = script,
Cwd = "",
CreateTemporaryIntegratedConsole = false
}).ConfigureAwait(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,13 @@ private string GenerateScriptFromLoggingStatements(params string[] logStatements
throw new ArgumentNullException(nameof(logStatements), "Expected at least one argument.");
}

// Have script create/overwrite file first with `>`.
// Clean up side effects from other test runs.
if (File.Exists(s_testOutputPath))
{
File.Delete(s_testOutputPath);
}

// Have script create file first with `>` (but don't rely on overwriting).
StringBuilder builder = new StringBuilder().Append('\'').Append(logStatements[0]).Append("' > '").Append(s_testOutputPath).AppendLine("'");
for (int i = 1; i < logStatements.Length; i++)
{
Expand Down Expand Up @@ -177,7 +183,7 @@ public async Task CanLaunchScriptWithNoBreakpointsAsync()
public async Task CanSetBreakpointsAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode,
PsesStdioProcess.RunningInConstrainedLanguageMode,
"You can't set breakpoints in ConstrainedLanguage mode.");

string filePath = NewTestFile(GenerateScriptFromLoggingStatements(
Expand Down Expand Up @@ -254,7 +260,7 @@ public async Task CanSetBreakpointsAsync()
public async Task CanStepPastSystemWindowsForms()
{
Skip.IfNot(PsesStdioProcess.IsWindowsPowerShell);
Skip.If(PsesStdioProcess.RunningInConstainedLanguageMode);
Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode);

string filePath = NewTestFile(string.Join(Environment.NewLine, new[]
{
Expand Down Expand Up @@ -291,5 +297,32 @@ public async Task CanStepPastSystemWindowsForms()
Assert.NotNull(form);
Assert.Equal("System.Windows.Forms.Form, Text: ", form.Value);
}

// This tests the edge-case where a raw script (or an untitled script) has the last line
// commented. Since in some cases (such as Windows PowerShell, or the script not having a
// backing ScriptFile) we just wrap the script with braces, we had a bug where the last
// brace would be after the comment. We had to ensure we wrapped with newlines instead.
[Trait("Category", "DAP")]
[Fact]
public async Task CanLaunchScriptWithCommentedLastLineAsync()
{
string script = GenerateScriptFromLoggingStatements("a log statement") + "# a comment at the end";
Assert.Contains(Environment.NewLine + "# a comment", script);
Assert.EndsWith("at the end", script);

// NOTE: This is horribly complicated, but the "script" parameter here is assigned to
// PsesLaunchRequestArguments.Script, which is then assigned to
// DebugStateService.ScriptToLaunch in that handler, and finally used by the
// ConfigurationDoneHandler in LaunchScriptAsync.
await PsesDebugAdapterClient.LaunchScript(script, Started).ConfigureAwait(false);

ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
Assert.NotNull(configDoneResponse);

// At this point the script should be running so lets give it time
await Task.Delay(2000).ConfigureAwait(false);

Assert.Collection(GetLog(), (i) => Assert.Equal("a log statement", i));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function CanSendWorkspaceSymbolRequest {
public async Task CanReceiveDiagnosticsFromFileOpenAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

NewTestFile("$a = 4");
Expand All @@ -178,7 +178,7 @@ public async Task WontReceiveDiagnosticsFromFileOpenThatIsNotPowerShellAsync()
public async Task CanReceiveDiagnosticsFromFileChangedAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

string filePath = NewTestFile("$a = 4");
Expand Down Expand Up @@ -230,7 +230,7 @@ public async Task CanReceiveDiagnosticsFromFileChangedAsync()
public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

NewTestFile("gci | % { $_ }");
Expand Down Expand Up @@ -331,7 +331,7 @@ await PsesLanguageClient
public async Task CanSendFormattingRequestAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

string scriptPath = NewTestFile(@"
Expand Down Expand Up @@ -368,7 +368,7 @@ public async Task CanSendFormattingRequestAsync()
public async Task CanSendRangeFormattingRequestAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

string scriptPath = NewTestFile(@"
Expand Down Expand Up @@ -892,7 +892,7 @@ function CanSendReferencesCodeLensRequest {
public async Task CanSendCodeActionRequestAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

string filePath = NewTestFile("gci");
Expand Down Expand Up @@ -1090,7 +1090,7 @@ await PsesLanguageClient
[SkippableFact]
public async Task CanSendGetProjectTemplatesRequestAsync()
{
Skip.If(PsesStdioProcess.RunningInConstainedLanguageMode, "Plaster doesn't work in ConstrainedLanguage mode.");
Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode, "Plaster doesn't work in ConstrainedLanguage mode.");

GetProjectTemplatesResponse getProjectTemplatesResponse =
await PsesLanguageClient
Expand All @@ -1110,7 +1110,7 @@ await PsesLanguageClient
public async Task CanSendGetCommentHelpRequestAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
"Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");

string scriptPath = NewTestFile(@"
Expand Down Expand Up @@ -1183,7 +1183,7 @@ await PsesLanguageClient
public async Task CanSendExpandAliasRequestAsync()
{
Skip.If(
PsesStdioProcess.RunningInConstainedLanguageMode,
PsesStdioProcess.RunningInConstrainedLanguageMode,
"This feature currently doesn't support ConstrainedLanguage Mode.");

ExpandAliasResult expandAliasResult =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ public class PsesStdioProcess : StdioServerProcess

#region public static properties

// NOTE: Just hard-code this to "powershell" when testing with the code lens.
public static string PwshExe { get; } = Environment.GetEnvironmentVariable("PWSH_EXE_NAME") ?? "pwsh";
public static bool IsWindowsPowerShell { get; } = PwshExe.Contains("powershell");
public static bool RunningInConstainedLanguageMode { get; } =
public static bool RunningInConstrainedLanguageMode { get; } =
Environment.GetEnvironmentVariable("__PSLockdownPolicy", EnvironmentVariableTarget.Machine) != null;

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ public void DocumentUriReturnsCorrectStringForAbsolutePath()
[InlineData("vscode-notebook-cell:/Users/me/Documents/test.ps1#0001", true)]
[InlineData("https://microsoft.com", true)]
[InlineData("Untitled:Untitled-1", true)]
[InlineData("'a log statement' > 'c:\\Users\\me\\Documents\\test.txt'\r\n", false)]
public void IsUntitledFileIsCorrect(string path, bool expected) => Assert.Equal(expected, ScriptFile.IsUntitledPath(path));
Copy link
Member Author

Choose a reason for hiding this comment

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

See, this test implies that the function is asking "is this path a URI that indicates we're untitled?"

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps we should expect this case to be true by flipping the logic with IsWellFormedUri. That is, if we're given something that is not a URI, assume it's untitled, but I'm not sure that would work for the first two cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, doesn't work, since /Users/me/Documents/test.ps1 is not a URI.

}
}