From 13412351cdb198ea7b50fdb8c09ee9ae52585914 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Tue, 24 May 2022 15:08:56 -0400 Subject: [PATCH 1/4] Add missing argument to FromPositions --- .../Commands/Public/ConvertTo-ScriptExtent.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 b/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 index ad08a9626..ba679e83f 100644 --- a/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 +++ b/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 @@ -111,6 +111,7 @@ function ConvertTo-ScriptExtent { if (-not $EndColumnNumber) { $EndColumnNumber = 1 } return [Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]::FromPositions( + $fileContext, $StartLineNumber, $StartColumnNumber, $EndLineNumber, From 8ebedc6f2f06ce4df4f32f85c81f1460e5100fa1 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Wed, 25 May 2022 11:45:37 -0400 Subject: [PATCH 2/4] Fix type resolution error in `Set-ScriptExtent` While resisting the urge to burn the entire file to the ground. Boy did I have some wild style choices five years ago. --- .../Commands/Public/Set-ScriptExtent.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/PowerShellEditorServices/Commands/Public/Set-ScriptExtent.ps1 b/module/PowerShellEditorServices/Commands/Public/Set-ScriptExtent.ps1 index dfefc8fdc..d5bf9dbd1 100644 --- a/module/PowerShellEditorServices/Commands/Public/Set-ScriptExtent.ps1 +++ b/module/PowerShellEditorServices/Commands/Public/Set-ScriptExtent.ps1 @@ -25,7 +25,7 @@ function Set-ScriptExtent { ) begin { $fileContext = $psEditor.GetEditorContext().CurrentFile - $extentList = [System.Collections.Generic.List[Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]]::new() + $extentList = [System.Collections.Generic.List[[Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]]]::new() } process { if ($Extent -isnot [Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]) { From 430387949c784988af2f5c6791fe91aee9e6f1a0 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Wed, 25 May 2022 11:50:21 -0400 Subject: [PATCH 3/4] Fix incorrectly setting end range to 1 When passing a single Line/Column value, we now assume you want a zero length extent of a single point. This was already the case for buffer ranges. Also take a pass at style because it was an abomination and I apologize to the world for doing it. --- .../Public/ConvertTo-ScriptExtent.ps1 | 102 +++++++++--------- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 b/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 index ba679e83f..0186a5b6f 100644 --- a/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 +++ b/module/PowerShellEditorServices/Commands/Public/ConvertTo-ScriptExtent.ps1 @@ -8,77 +8,63 @@ function ConvertTo-ScriptExtent { [CmdletBinding()] [OutputType([System.Management.Automation.Language.IScriptExtent])] param( - [Parameter(Mandatory, ValueFromPipelineByPropertyName, ParameterSetName='ByOffset')] + [Parameter(Mandatory, ValueFromPipelineByPropertyName, ParameterSetName = 'ByOffset')] [Alias('StartOffset', 'Offset')] - [int] - $StartOffsetNumber, + [int] $StartOffsetNumber, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByOffset')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByOffset')] [Alias('EndOffset')] - [int] - $EndOffsetNumber, + [int] $EndOffsetNumber, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByPosition')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByPosition')] [Alias('StartLine', 'Line')] - [int] - $StartLineNumber, + [int] $StartLineNumber, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByPosition')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByPosition')] [Alias('StartColumn', 'Column')] - [int] - $StartColumnNumber, + [int] $StartColumnNumber, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByPosition')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByPosition')] [Alias('EndLine')] - [int] - $EndLineNumber, + [int] $EndLineNumber, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByPosition')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByPosition')] [Alias('EndColumn')] - [int] - $EndColumnNumber, + [int] $EndColumnNumber, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByPosition')] - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByOffset')] - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByBuffer')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByPosition')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByOffset')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByBuffer')] [Alias('File', 'FileName')] - [string] - $FilePath = $psEditor.GetEditorContext().CurrentFile.Path, + [string] $FilePath = $psEditor.GetEditorContext().CurrentFile.Path, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByBuffer')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByBuffer')] [Alias('Start')] - [Microsoft.PowerShell.EditorServices.Extensions.IFilePosition, Microsoft.PowerShell.EditorServices] - $StartBuffer, + [Microsoft.PowerShell.EditorServices.Extensions.IFilePosition, Microsoft.PowerShell.EditorServices] $StartBuffer, - [Parameter(ValueFromPipelineByPropertyName, ParameterSetName='ByBuffer')] + [Parameter(ValueFromPipelineByPropertyName, ParameterSetName = 'ByBuffer')] [Alias('End')] - [Microsoft.PowerShell.EditorServices.Extensions.IFilePosition, Microsoft.PowerShell.EditorServices] - $EndBuffer, - - [Parameter(Mandatory, - ValueFromPipeline, - ValueFromPipelineByPropertyName, - ParameterSetName='ByExtent')] - [System.Management.Automation.Language.IScriptExtent] - $Extent + [Microsoft.PowerShell.EditorServices.Extensions.IFilePosition, Microsoft.PowerShell.EditorServices] $EndBuffer, + + [Parameter(Mandatory, ValueFromPipeline, ValueFromPipelineByPropertyName, ParameterSetName = 'ByExtent')] + [System.Management.Automation.Language.IScriptExtent] $Extent ) begin { $fileContext = $psEditor.GetEditorContext().CurrentFile $emptyExtent = [Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]::Empty } - process { # Already a InternalScriptExtent, FileScriptExtent or is empty. $returnAsIs = $Extent -and - (0 -ne $Extent.StartOffset -or - 0 -ne $Extent.EndOffset -or - $Extent -eq $emptyExtent) + ($Extent.StartOffset -or $Extent.EndOffset -or $Extent -eq $emptyExtent) - if ($returnAsIs) { return $Extent } + if ($returnAsIs) { + return $Extent + } if ($StartOffsetNumber) { $startOffset = $StartOffsetNumber - $endOffset = $EndOffsetNumber + $endOffset = $EndOffsetNumber # Allow creating a single position extent with just the offset parameter. if (-not $EndOffsetNumber) { @@ -92,8 +78,7 @@ function ConvertTo-ScriptExtent { } if ($StartBuffer) { - if (-not $EndBuffer) - { + if (-not $EndBuffer) { $EndBuffer = $StartBuffer } @@ -105,10 +90,31 @@ function ConvertTo-ScriptExtent { $EndBuffer.Column) } - if (-not $StartColumnNumber) { $StartColumnNumber = 1 } - if (-not $StartLineNumber) { $StartLineNumber = 1 } - if (-not $EndLineNumber) { $EndLineNumber = 1 } - if (-not $EndColumnNumber) { $EndColumnNumber = 1 } + # Allow piping a single line and column to get a zero length script extent. + if ($PSBoundParameters.ContainsKey('StartColumnNumber') -and -not $PSBoundParameters.ContainsKey('EndColumnNumber')) { + $EndColumnNumber = $StartColumnNumber + } + + if ($PSBoundParameters.ContainsKey('StartLineNumber') -and -not $PSBoundParameters.ContainsKey('EndLineNumber')) { + $EndLineNumber = $StartLineNumber + } + + # Protect against zero as a value since lines and columns start at 1 + if (-not $StartColumnNumber) { + $StartColumnNumber = 1 + } + + if (-not $StartLineNumber) { + $StartLineNumber = 1 + } + + if (-not $EndLineNumber) { + $EndLineNumber = 1 + } + + if (-not $EndColumnNumber) { + $EndColumnNumber = 1 + } return [Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]::FromPositions( $fileContext, From ec01a45fe29197bfea9b76807814b29181398203 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Wed, 25 May 2022 11:53:19 -0400 Subject: [PATCH 4/4] Fix offset calculation and NREs Offset calculation was hitting the same new line repeatedly as it didn't set the new offset for one *after* the new line char. Also empty extents pass null as `FileContext` so added some null checks. --- .../Extensions/EditorFileRanges.cs | 36 ++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/PowerShellEditorServices/Extensions/EditorFileRanges.cs b/src/PowerShellEditorServices/Extensions/EditorFileRanges.cs index 281e3411e..ed6ae9de5 100644 --- a/src/PowerShellEditorServices/Extensions/EditorFileRanges.cs +++ b/src/PowerShellEditorServices/Extensions/EditorFileRanges.cs @@ -15,12 +15,23 @@ public class FileScriptPosition : IScriptPosition, IFilePosition public static FileScriptPosition FromPosition(FileContext file, int lineNumber, int columnNumber) { + if (file is null) + { + throw new ArgumentNullException(nameof(file)); + } + int offset = 0; int currLine = 1; string fileText = file.Ast.Extent.Text; while (offset < fileText.Length && currLine < lineNumber) { - offset = fileText.IndexOf('\n', offset); + offset = fileText.IndexOf('\n', offset) + 1; + if (offset is 0) + { + // Line and column passed were not valid and the offset can not be determined. + return new FileScriptPosition(file, lineNumber, columnNumber, offset); + } + currLine++; } @@ -31,6 +42,11 @@ public static FileScriptPosition FromPosition(FileContext file, int lineNumber, public static FileScriptPosition FromOffset(FileContext file, int offset) { + if (file is null) + { + throw new ArgumentNullException(nameof(file)); + } + int line = 1; string fileText = file.Ast.Extent.Text; @@ -59,7 +75,7 @@ public static FileScriptPosition FromOffset(FileContext file, int offset) internal FileScriptPosition(FileContext file, int lineNumber, int columnNumber, int offset) { _file = file; - Line = file.GetTextLines()[lineNumber - 1]; + Line = file?.GetTextLines()?[lineNumber - 1] ?? string.Empty; ColumnNumber = columnNumber; LineNumber = lineNumber; Offset = offset; @@ -79,7 +95,7 @@ internal FileScriptPosition(FileContext file, int lineNumber, int columnNumber, int IFilePosition.Line => LineNumber; - public string GetFullScript() => _file.GetText(); + public string GetFullScript() => _file?.GetText() ?? string.Empty; } public class FileScriptExtent : IScriptExtent, IFileRange @@ -94,6 +110,11 @@ public static bool IsEmpty(FileScriptExtent extent) public static FileScriptExtent FromOffsets(FileContext file, int startOffset, int endOffset) { + if (file is null) + { + throw new ArgumentNullException(nameof(file)); + } + return new FileScriptExtent( file, FileScriptPosition.FromOffset(file, startOffset), @@ -102,6 +123,11 @@ public static FileScriptExtent FromOffsets(FileContext file, int startOffset, in public static FileScriptExtent FromPositions(FileContext file, int startLine, int startColumn, int endLine, int endColumn) { + if (file is null) + { + throw new ArgumentNullException(nameof(file)); + } + return new FileScriptExtent( file, FileScriptPosition.FromPosition(file, startLine, startColumn), @@ -127,7 +153,7 @@ public FileScriptExtent(FileContext file, FileScriptPosition start, FileScriptPo public IScriptPosition EndScriptPosition => _end; - public string File => _file.Path; + public string File => _file?.Path ?? string.Empty; public int StartColumnNumber => _start.ColumnNumber; @@ -137,7 +163,7 @@ public FileScriptExtent(FileContext file, FileScriptPosition start, FileScriptPo public IScriptPosition StartScriptPosition => _start; - public string Text => _file.GetText().Substring(_start.Offset, _end.Offset - _start.Offset); + public string Text => _file?.GetText()?.Substring(_start.Offset, _end.Offset - _start.Offset) ?? string.Empty; IFilePosition IFileRange.Start => _start;