From 7f0a54f9e65742078d97985ff69a613d2ad0405e Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Fri, 23 Nov 2018 13:54:28 -0700 Subject: [PATCH 1/3] Code cleanup of the start script and ESHost.cs file Address some PSSA warnings, version guard access to $Is vars, rename Test-NamedPipeName-OrCreate-IfNull to Get-ValidatedNamedPipeName. Not 100% sold on the new name so open to suggestions. --- .../Start-EditorServices.ps1 | 98 ++++++++++--------- .../EditorServicesHost.cs | 8 +- 2 files changed, 59 insertions(+), 47 deletions(-) diff --git a/module/PowerShellEditorServices/Start-EditorServices.ps1 b/module/PowerShellEditorServices/Start-EditorServices.ps1 index 79e2db18c..b3db8912e 100644 --- a/module/PowerShellEditorServices/Start-EditorServices.ps1 +++ b/module/PowerShellEditorServices/Start-EditorServices.ps1 @@ -1,20 +1,27 @@ -# PowerShell Editor Services Bootstrapper Script -# ---------------------------------------------- -# This script contains startup logic for the PowerShell Editor Services -# module when launched by an editor. It handles the following tasks: -# -# - Verifying the existence of dependencies like PowerShellGet -# - Verifying that the expected version of the PowerShellEditorServices module is installed -# - Installing the PowerShellEditorServices module if confirmed by the user -# - Creating named pipes for the language and debug services to use (if using named pipes) -# - Starting the language and debug services from the PowerShellEditorServices module -# -# NOTE: If editor integration authors make modifications to this -# script, please consider contributing changes back to the -# canonical version of this script at the PowerShell Editor -# Services GitHub repository: -# -# https://github.com/PowerShell/PowerShellEditorServices/blob/master/module/PowerShellEditorServices/Start-EditorServices.ps1 +<# +.SYNOPSIS + Starts the language and debug services from the PowerShellEditorServices module. +.DESCRIPTION + PowerShell Editor Services Bootstrapper Script + ---------------------------------------------- + This script contains startup logic for the PowerShell Editor Services + module when launched by an editor. It handles the following tasks: + + - Verifying the existence of dependencies like PowerShellGet + - Verifying that the expected version of the PowerShellEditorServices module is installed + - Installing the PowerShellEditorServices module if confirmed by the user + - Creating named pipes for the language and debug services to use (if using named pipes) + - Starting the language and debug services from the PowerShellEditorServices module +.INPUTS + None +.OUTPUTS + None +.NOTES + If editor integration authors make modifications to this script, please + consider contributing changes back to the canonical version of this script + at the PowerShell Editor Services GitHub repository: + https://github.com/PowerShell/PowerShellEditorServices/blob/master/module/PowerShellEditorServices/Start-EditorServices.ps1' +#> [CmdletBinding(DefaultParameterSetName="NamedPipe")] param( [Parameter(Mandatory=$true)] @@ -65,7 +72,7 @@ param( [switch] $ConfirmInstall, - [Parameter(ParameterSetName="Stdio",Mandatory=$true)] + [Parameter(ParameterSetName="Stdio", Mandatory=$true)] [switch] $Stdio, @@ -154,9 +161,6 @@ if ($host.Runspace.LanguageMode -eq 'ConstrainedLanguage') { ExitWithError "PowerShell is configured with an unsupported LanguageMode (ConstrainedLanguage), language features are disabled." } -# Are we running in PowerShell 5 or later? -$isPS5orLater = $PSVersionTable.PSVersion.Major -ge 5 - # If PSReadline is present in the session, remove it so that runspace # management is easier if ((Microsoft.PowerShell.Core\Get-Module PSReadline).Count -gt 0) { @@ -173,8 +177,8 @@ function Test-ModuleAvailable($ModuleName, $ModuleVersion) { Log "Testing module availability $ModuleName $ModuleVersion" $modules = Microsoft.PowerShell.Core\Get-Module -ListAvailable $moduleName - if ($modules -ne $null) { - if ($ModuleVersion -ne $null) { + if ($null -ne $modules) { + if ($null -ne $ModuleVersion) { foreach ($module in $modules) { if ($module.Version.Equals($moduleVersion)) { Log "$ModuleName $ModuleVersion found" @@ -193,7 +197,6 @@ function Test-ModuleAvailable($ModuleName, $ModuleVersion) { } function New-NamedPipeName { - # We try 10 times to find a valid pipe name for ($i = 0; $i -lt 10; $i++) { $PipeName = "PSES_$([System.IO.Path]::GetRandomFileName())" @@ -202,6 +205,7 @@ function New-NamedPipeName { return $PipeName } } + ExitWithError "Could not find valid a pipe name." } @@ -213,7 +217,7 @@ function Get-NamedPipePath { $PipeName ) - if (-not $IsLinux -and -not $IsMacOS) { + if (($PSVersionTable.PSVersion.Major -le 5) -or $IsWindows) { return "\\.\pipe\$PipeName"; } else { @@ -221,7 +225,6 @@ function Get-NamedPipePath { # the Unix Domain Sockets live in the tmp directory and are prefixed with "CoreFxPipe_" return (Join-Path -Path ([System.IO.Path]::GetTempPath()) -ChildPath "CoreFxPipe_$PipeName") } - } # Returns True if it's a valid pipe name @@ -236,7 +239,7 @@ function Test-NamedPipeName { ) $path = Get-NamedPipePath -PipeName $PipeName - return -not (Test-Path $path) + return !(Test-Path $path) } function Set-NamedPipeMode { @@ -247,16 +250,16 @@ function Set-NamedPipeMode { $PipeFile ) - if ($IsWindows) { + if (($PSVersionTable.PSVersion.Major -le 5) -or $IsWindows) { return } chmod $DEFAULT_USER_MODE $PipeFile - if ($IsLinux) { + if (($PSVersionTable.PSVersion.Major -ge 6) -and $IsLinux) { $mode = /usr/bin/stat -c "%a" $PipeFile } - elseif ($IsMacOS) { + elseif (($PSVersionTable.PSVersion.Major -ge 6) -and $IsMacOS) { $mode = /usr/bin/stat -f "%A" $PipeFile } @@ -268,19 +271,19 @@ function Set-NamedPipeMode { LogSection "Console Encoding" Log $OutputEncoding -function Test-NamedPipeName-OrCreate-IfNull { +function Get-ValidatedNamedPipeName { param( [string] $PipeName ) - if (-not $PipeName) { + + if (!$PipeName) { $PipeName = New-NamedPipeName } - else { - if (-not (Test-NamedPipeName -PipeName $PipeName)) { - ExitWithError "Pipe name supplied is already taken: $PipeName" - } + elseif (!(Test-NamedPipeName -PipeName $PipeName)) { + ExitWithError "Pipe name supplied is already in use: $PipeName" } + return $PipeName } @@ -288,13 +291,16 @@ function Set-PipeFileResult { param ( [Hashtable] $ResultTable, + [string] $PipeNameKey, + [string] $PipeNameValue ) + $ResultTable[$PipeNameKey] = Get-NamedPipePath -PipeName $PipeNameValue - if ($IsLinux -or $IsMacOS) { + if (($PSVersionTable.PSVersion.Major -ge 6) -and ($IsLinux -or $IsMacOS)) { Set-NamedPipeMode -PipeFile $ResultTable[$PipeNameKey] } } @@ -349,11 +355,12 @@ try { -WaitForDebugger:$WaitForDebugger.IsPresent break } + "NamedPipeSimplex" { - $LanguageServiceInPipeName = Test-NamedPipeName-OrCreate-IfNull $LanguageServiceInPipeName - $LanguageServiceOutPipeName = Test-NamedPipeName-OrCreate-IfNull $LanguageServiceOutPipeName - $DebugServiceInPipeName = Test-NamedPipeName-OrCreate-IfNull $DebugServiceInPipeName - $DebugServiceOutPipeName = Test-NamedPipeName-OrCreate-IfNull $DebugServiceOutPipeName + $LanguageServiceInPipeName = Get-ValidatedNamedPipeName $LanguageServiceInPipeName + $LanguageServiceOutPipeName = Get-ValidatedNamedPipeName $LanguageServiceOutPipeName + $DebugServiceInPipeName = Get-ValidatedNamedPipeName $DebugServiceInPipeName + $DebugServiceOutPipeName = Get-ValidatedNamedPipeName $DebugServiceOutPipeName $editorServicesHost = Start-EditorServicesHost ` -HostName $HostName ` @@ -377,9 +384,10 @@ try { Set-PipeFileResult $resultDetails "debugServiceWritePipeName" $DebugServiceOutPipeName break } + Default { - $LanguageServicePipeName = Test-NamedPipeName-OrCreate-IfNull $LanguageServicePipeName - $DebugServicePipeName = Test-NamedPipeName-OrCreate-IfNull $DebugServicePipeName + $LanguageServicePipeName = Get-ValidatedNamedPipeName $LanguageServicePipeName + $DebugServicePipeName = Get-ValidatedNamedPipeName $DebugServicePipeName $editorServicesHost = Start-EditorServicesHost ` -HostName $HostName ` @@ -417,7 +425,7 @@ catch [System.Exception] { Log "ERRORS caught starting up EditorServicesHost" - while ($e -ne $null) { + while ($null -ne $e) { $errorString = $errorString + ($e.Message + "`r`n" + $e.StackTrace + "`r`n") $e = $e.InnerException; Log $errorString @@ -438,7 +446,7 @@ catch [System.Exception] { Log "ERRORS caught while waiting for EditorServicesHost to complete execution" - while ($e -ne $null) { + while ($null -ne $e) { $errorString = $errorString + ($e.Message + "`r`n" + $e.StackTrace + "`r`n") $e = $e.InnerException; Log $errorString diff --git a/src/PowerShellEditorServices.Host/EditorServicesHost.cs b/src/PowerShellEditorServices.Host/EditorServicesHost.cs index f30ffc4ec..4f9ac0864 100644 --- a/src/PowerShellEditorServices.Host/EditorServicesHost.cs +++ b/src/PowerShellEditorServices.Host/EditorServicesHost.cs @@ -45,8 +45,11 @@ public class EditorServiceTransportConfig /// For NamedPipe it's the pipe name. /// public string InOutPipeName { get; set; } + public string OutPipeName { get; set; } + public string InPipeName { get; set; } + internal string Endpoint => OutPipeName != null && InPipeName != null ? $"In pipe: {InPipeName} Out pipe: {OutPipeName}" : $" InOut pipe: {InOutPipeName}"; } @@ -243,7 +246,7 @@ await this.editorSession.PowerShellContext.ImportCommandsModule( foreach (string module in this.additionalModules) { await this.editorSession.PowerShellContext.ExecuteCommand( - new System.Management.Automation.PSCommand().AddCommand("Import-Module").AddArgument(module), + new System.Management.Automation.PSCommand().AddCommand("Microsoft.PowerShell.Core\\Import-Module").AddArgument(module), false, true); } @@ -455,6 +458,7 @@ private void CurrentDomain_UnhandledException( e.ExceptionObject.ToString())); } #endif + private IServerListener CreateServiceListener(MessageProtocolType protocol, EditorServiceTransportConfig config) { switch (config.TransportType) @@ -466,7 +470,7 @@ private IServerListener CreateServiceListener(MessageProtocolType protocol, Edit case EditorServiceTransportType.NamedPipe: { - if (config.OutPipeName !=null && config.InPipeName !=null) + if ((config.OutPipeName != null) && (config.InPipeName != null)) { this.logger.Write(LogLevel.Verbose, $"Creating NamedPipeServerListener for ${protocol} protocol with two pipes: In: '{config.InPipeName}'. Out: '{config.OutPipeName}'"); return new NamedPipeServerListener(protocol, config.InPipeName, config.OutPipeName, this.logger); From 0eb17e8dc71e4c95e3b85d841d6f35dc53c86a66 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sat, 24 Nov 2018 10:55:47 -0700 Subject: [PATCH 2/3] Address PR feedback - remove redundant version checking, add comment --- module/PowerShellEditorServices/Start-EditorServices.ps1 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/module/PowerShellEditorServices/Start-EditorServices.ps1 b/module/PowerShellEditorServices/Start-EditorServices.ps1 index b3db8912e..499bf816f 100644 --- a/module/PowerShellEditorServices/Start-EditorServices.ps1 +++ b/module/PowerShellEditorServices/Start-EditorServices.ps1 @@ -256,10 +256,10 @@ function Set-NamedPipeMode { chmod $DEFAULT_USER_MODE $PipeFile - if (($PSVersionTable.PSVersion.Major -ge 6) -and $IsLinux) { + if ($IsLinux) { $mode = /usr/bin/stat -c "%a" $PipeFile } - elseif (($PSVersionTable.PSVersion.Major -ge 6) -and $IsMacOS) { + elseif ($IsMacOS) { $mode = /usr/bin/stat -f "%A" $PipeFile } @@ -277,6 +277,7 @@ function Get-ValidatedNamedPipeName { $PipeName ) + # If no PipeName is passed in, then we create one that's guaranteed to be valid if (!$PipeName) { $PipeName = New-NamedPipeName } From 3743b0552715327276ff14544bfbf3d92e7dc45f Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sat, 24 Nov 2018 13:24:11 -0700 Subject: [PATCH 3/3] Address PR feedback - use parameter labels and named parameters --- .../EditorServicesHost.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/PowerShellEditorServices.Host/EditorServicesHost.cs b/src/PowerShellEditorServices.Host/EditorServicesHost.cs index 4f9ac0864..503e03068 100644 --- a/src/PowerShellEditorServices.Host/EditorServicesHost.cs +++ b/src/PowerShellEditorServices.Host/EditorServicesHost.cs @@ -245,10 +245,15 @@ await this.editorSession.PowerShellContext.ImportCommandsModule( // gets initialized when that is done earlier than LanguageServer.Initialize foreach (string module in this.additionalModules) { + var command = + new System.Management.Automation.PSCommand() + .AddCommand("Microsoft.PowerShell.Core\\Import-Module") + .AddParameter("Name", module); + await this.editorSession.PowerShellContext.ExecuteCommand( - new System.Management.Automation.PSCommand().AddCommand("Microsoft.PowerShell.Core\\Import-Module").AddArgument(module), - false, - true); + command, + sendOutputToHost: false, + sendErrorToHost: true); } protocolEndpoint.Start();