From 160c960aceb5bcc83d435ec40907c0a93d10eac1 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Sun, 8 Mar 2020 15:10:05 -0400 Subject: [PATCH] Fix for PSES 2.0 GA in PS7 (#51) * Change version numbers, fix type resolution * Fix build issues * Fix bad command splat variable insertion points * Suppress analyzer fixes * Additional fixes for suppress analyzer --- .vscode/extensions.json | 2 +- EditorServicesCommandSuite.build.ps1 | 8 +- build.ps1 | 2 +- module/EditorServicesCommandSuite.psd1 | 29 +--- module/EditorServicesCommandSuite.psm1 | 2 +- src/EditorServicesCommandSuite.Common.props | 1 + .../CodeGeneration/DocumentEditWriter.cs | 14 +- .../CodeGeneration/PowerShellScriptWriter.cs | 50 +++++-- .../Refactors/CommandSplatRefactor.cs | 132 +++++++++++++++++- .../SuppressAnalyzerMessageRefactor.cs | 19 ++- tools/AssertRequiredModule.ps1 | 6 + 11 files changed, 208 insertions(+), 57 deletions(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index e9ff3e7..a79cd68 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -2,7 +2,7 @@ // See http://go.microsoft.com/fwlink/?LinkId=827846 // for the documentation about the extensions.json format "recommendations": [ - "ms-vscode.csharp", + "ms-dotnettools.csharp", "ms-vscode.powershell", "DavidAnson.vscode-markdownlint", ], diff --git a/EditorServicesCommandSuite.build.ps1 b/EditorServicesCommandSuite.build.ps1 index 0691f01..03ba06c 100644 --- a/EditorServicesCommandSuite.build.ps1 +++ b/EditorServicesCommandSuite.build.ps1 @@ -69,17 +69,17 @@ task AssertPowerShellCore { } if ($Force.IsPresent) { - choco install powershell-core --version 6.2.3 -y + choco install powershell-core --version 7.0.0 -y } else { - choco install powershell-core --verison 6.2.3 + choco install powershell-core --verison 7.0.0 } - $script:pwsh = Get-Command $env:ProgramFiles/PowerShell/6/pwsh.exe @FailOnError + $script:pwsh = Get-Command $env:ProgramFiles/PowerShell/7/pwsh.exe @FailOnError } task AssertRequiredModules { $assertRequiredModule = Get-Command $ToolsPath/AssertRequiredModule.ps1 @FailOnError - & $assertRequiredModule platyPS -RequiredVersion 0.12.0 -Force:$Force.IsPresent + & $assertRequiredModule platyPS -RequiredVersion 0.14.0 -Force:$Force.IsPresent } task AssertDotNet { diff --git a/build.ps1 b/build.ps1 index 2a99a40..7b2fabc 100644 --- a/build.ps1 +++ b/build.ps1 @@ -8,7 +8,7 @@ param( [switch] $Force ) end { - & "$PSScriptRoot\tools\AssertRequiredModule.ps1" InvokeBuild 5.4.2 -Force:$Force.IsPresent + & "$PSScriptRoot\tools\AssertRequiredModule.ps1" InvokeBuild 5.5.6 -Force:$Force.IsPresent $invokeBuildSplat = @{ Task = 'PrePublish' File = "$PSScriptRoot/EditorServicesCommandSuite.build.ps1" diff --git a/module/EditorServicesCommandSuite.psd1 b/module/EditorServicesCommandSuite.psd1 index 9156ece..dfb5ebb 100644 --- a/module/EditorServicesCommandSuite.psd1 +++ b/module/EditorServicesCommandSuite.psd1 @@ -12,7 +12,7 @@ RootModule = 'EditorServicesCommandSuite.psm1' # Version number of this module. -ModuleVersion = '0.5.0' +ModuleVersion = '1.0.0' # ID used to uniquely identify this module GUID = '97607afd-d9bd-4a2e-a9f9-70fe1a0a9e4c' @@ -69,30 +69,6 @@ VariablesToExport = @() # Aliases to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no aliases to export. AliasesToExport = 'Add-CommandToManifest' -# List of all files packaged with this module -FileList = 'en-US\EditorServicesCommandSuite-help.xml', - 'EditorServicesCommandSuite.Classes.ps1', - 'EditorServicesCommandSuite.deps.json', - 'EditorServicesCommandSuite.dll', - 'EditorServicesCommandSuite.EditorServices.deps.json', - 'EditorServicesCommandSuite.EditorServices.dll', - 'EditorServicesCommandSuite.EditorServices.pdb', - 'EditorServicesCommandSuite.EditorServices.xml', - 'EditorServicesCommandSuite.format.ps1xml', - 'EditorServicesCommandSuite.pdb', - 'EditorServicesCommandSuite.psd1', - 'EditorServicesCommandSuite.psm1', - 'EditorServicesCommandSuite.PSReadLine.deps.json', - 'EditorServicesCommandSuite.PSReadLine.dll', - 'EditorServicesCommandSuite.PSReadLine.pdb', - 'EditorServicesCommandSuite.PSReadLine.xml', - 'EditorServicesCommandSuite.RefactorCmdlets.cdxml', - 'EditorServicesCommandSuite.xml', - 'System.Buffers.dll', - 'System.Memory.dll', - 'System.Numerics.Vectors.dll', - 'System.Runtime.CompilerServices.Unsafe.dll' - # Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell. PrivateData = @{ @@ -115,6 +91,9 @@ PrivateData = @{ - New editor command ConvertTo-FunctionDefinition for generating functions from selected text. '@ + # Prerelease string of this module + Prerelease = 'beta1' + } # End of PSData hashtable } # End of PrivateData hashtable diff --git a/module/EditorServicesCommandSuite.psm1 b/module/EditorServicesCommandSuite.psm1 index 2dc3c2f..a9d719b 100644 --- a/module/EditorServicesCommandSuite.psm1 +++ b/module/EditorServicesCommandSuite.psm1 @@ -4,7 +4,7 @@ Update-FormatData -AppendPath $PSScriptRoot/EditorServicesCommandSuite.format.ps if ($null -ne $psEditor) { if ($PSVersionTable.PSVersion.Major -ge 6) { - $extensionService = [Microsoft.PowerShell.EditorServices.Extensions.EditorObjectExtensions]:: + $extensionService = [Microsoft.PowerShell.EditorServices.Extensions.EditorObjectExtensions, Microsoft.PowerShell.EditorServices]:: GetExtensionServiceProvider($psEditor) $assembly = $extensionService.LoadAssemblyInPsesLoadContext(( diff --git a/src/EditorServicesCommandSuite.Common.props b/src/EditorServicesCommandSuite.Common.props index acae3a3..2313627 100644 --- a/src/EditorServicesCommandSuite.Common.props +++ b/src/EditorServicesCommandSuite.Common.props @@ -7,6 +7,7 @@ true preview netstandard2.0;netcoreapp2.0 + 1.0.0.0 diff --git a/src/EditorServicesCommandSuite/CodeGeneration/DocumentEditWriter.cs b/src/EditorServicesCommandSuite/CodeGeneration/DocumentEditWriter.cs index b3139aa..b7c608a 100644 --- a/src/EditorServicesCommandSuite/CodeGeneration/DocumentEditWriter.cs +++ b/src/EditorServicesCommandSuite/CodeGeneration/DocumentEditWriter.cs @@ -19,6 +19,8 @@ internal class DocumentEditWriter : SpanEnabledStreamWriter protected char[] _coreTab; + private protected int? _pendingIndent; + private readonly Stack _indentStack = new Stack(); private readonly int _byteOffsetModifier; @@ -35,8 +37,6 @@ internal class DocumentEditWriter : SpanEnabledStreamWriter private readonly byte[] _originalBuffer; - private int? _pendingIndent; - private bool _isWritePending; private long _lastPositionSet; @@ -498,9 +498,17 @@ private IEnumerable ReduceEdits(IEnumerable edits) StartOffset = editGroup.Key, EndOffset = highestOverride.EndOffset, OriginalValue = highestOverride.OriginalValue, + + // The order by is hacky fix for when some refactors insert into offset 0 + // and also generate using namespace statements. A better fix would be to + // implement a weight concept to document edits. NewValue = string.Concat( editGroup - .OrderBy(edit => edit.Id) + .OrderByDescending( + edit => edit.NewValue.StartsWith( + "using namespace ", + StringComparison.OrdinalIgnoreCase)) + .ThenBy(edit => edit.Id) .Select(edit => edit.NewValue)), }; } diff --git a/src/EditorServicesCommandSuite/CodeGeneration/PowerShellScriptWriter.cs b/src/EditorServicesCommandSuite/CodeGeneration/PowerShellScriptWriter.cs index cc4b627..7a9749a 100644 --- a/src/EditorServicesCommandSuite/CodeGeneration/PowerShellScriptWriter.cs +++ b/src/EditorServicesCommandSuite/CodeGeneration/PowerShellScriptWriter.cs @@ -143,11 +143,23 @@ public bool AddUsingStatements(HashSet namespaces, out int replaceLength SetPosition(0); } - Write(UsingUtilities.GetUsingStatementString(usings)); + var oldPendingIndent = _pendingIndent; + var oldIndent = Indent; + try + { + _pendingIndent = null; + Indent = 0; + Write(UsingUtilities.GetUsingStatementString(usings)); - if (!existing.Any()) + if (!existing.Any()) + { + WriteLines(2); + } + } + finally { - WriteLines(2); + _pendingIndent = oldPendingIndent; + Indent = oldIndent; } return true; @@ -762,18 +774,30 @@ internal void CloseParamBlock(bool shouldPopIndent = false) internal void WriteUsingStatement(string name, UsingStatementKind kind) { - char[] kindSymbol = kind switch + var oldPendingIndent = _pendingIndent; + var oldIndent = Indent; + try { - UsingStatementKind.Assembly => Symbols.Assembly, - UsingStatementKind.Module => Symbols.Module, - UsingStatementKind.Namespace => Symbols.Namespace, - _ => throw new InvalidOperationException(), - }; + _pendingIndent = null; + Indent = 0; + char[] kindSymbol = kind switch + { + UsingStatementKind.Assembly => Symbols.Assembly, + UsingStatementKind.Module => Symbols.Module, + UsingStatementKind.Namespace => Symbols.Namespace, + _ => throw new InvalidOperationException(), + }; - Write(Using); - Write(Space); - Write(kindSymbol); - Write(name); + Write(Using); + Write(Space); + Write(kindSymbol); + Write(name); + } + finally + { + _pendingIndent = oldPendingIndent; + Indent = oldIndent; + } } internal Task RegisterWorkspaceChangeAsync(DocumentContextBase context) diff --git a/src/EditorServicesCommandSuite/CodeGeneration/Refactors/CommandSplatRefactor.cs b/src/EditorServicesCommandSuite/CodeGeneration/Refactors/CommandSplatRefactor.cs index 7129209..934ed28 100644 --- a/src/EditorServicesCommandSuite/CodeGeneration/Refactors/CommandSplatRefactor.cs +++ b/src/EditorServicesCommandSuite/CodeGeneration/Refactors/CommandSplatRefactor.cs @@ -24,11 +24,24 @@ internal class CommandSplatRefactor : RefactorProvider private const string AmbiguousParameterSet = "AmbiguousParameterSet"; + private static readonly Type s_ternaryExpressionAstType; + + private static readonly Type s_pipelineChainAstType; + private static readonly HashSet s_allCommonParameters = new HashSet( Cmdlet.CommonParameters.Concat(Cmdlet.OptionalCommonParameters), StringComparer.OrdinalIgnoreCase); + static CommandSplatRefactor() + { + s_ternaryExpressionAstType = typeof(PSObject).Assembly + .GetType("System.Management.Automation.Language.TernaryExpressionAst"); + + s_pipelineChainAstType = typeof(PSObject).Assembly + .GetType("System.Management.Automation.Language.PipelineChainAst"); + } + internal CommandSplatRefactor(IRefactorUI ui) { UI = ui; @@ -174,9 +187,122 @@ await AddAdditionalParameters( return (parameterList, unresolvedPositionalArgs); } + private static Ast FindVariableInjectionTargetAst(CommandAst command) + { + // Search up the tree for the right place to insert the splat variable + // assignment. This is done to avoid inserting it in scenarios like: + // + // $commandValue = $commandSplat = @{ Param = $true } + // Command @commandSplat + // + // or: + // + // ($commandSplat = @{ Param = $true} + // Command @commandSplat) + Ast target = command.FindParent(); + while (true) + { + Ast parent = target.Parent; + if (parent == null) + { + return target; + } + + bool shouldGetParent = false; + if (parent is StatementAst) + { + shouldGetParent = + parent is AssignmentStatementAst + || parent is ThrowStatementAst + || parent is ReturnStatementAst + || parent is ExitStatementAst + || parent is ContinueStatementAst + || parent is CommandBaseAst + || parent is HashtableAst + || parent is BreakStatementAst + || parent is PipelineBaseAst; + } + + if (parent is ExpressionAst) + { + shouldGetParent = + parent is ParenExpressionAst + || parent is BinaryExpressionAst + || parent is UnaryExpressionAst + || parent is AttributedExpressionAst + || parent is MemberExpressionAst + || parent is ExpandableStringExpressionAst + || parent is ArrayLiteralAst + || parent is UsingExpressionAst + || parent is IndexExpressionAst; + } + + if (shouldGetParent) + { + target = parent; + continue; + } + + if (parent is IfStatementAst ifStatementAst) + { + foreach (Tuple clause in ifStatementAst.Clauses) + { + if (target == clause.Item1) + { + return ifStatementAst; + } + } + + return target; + } + + if (parent is ForStatementAst forStatement) + { + if (target == forStatement.Initializer || + target == forStatement.Iterator || + target == forStatement.Condition) + { + return forStatement; + } + + return target; + } + + if (parent is LoopStatementAst loop) + { + if (target == loop.Condition) + { + return loop; + } + + return target; + } + + // These don't actually work yet since AstEnumerable won't find a CommandAst + // hidden in either of these language elements. + if (s_ternaryExpressionAstType != null) + { + Type reflectionType = parent.GetType(); + if (s_ternaryExpressionAstType.IsAssignableFrom(reflectionType)) + { + target = parent; + continue; + } + + if (s_pipelineChainAstType?.IsAssignableFrom(reflectionType) == true) + { + target = parent; + continue; + } + } + + return target; + } + } + private static async Task> GetEdits(CommandSplatArguments args) { - PipelineAst parentStatement = args.Command.FindParent(); + Ast variableInjectionTarget = FindVariableInjectionTargetAst(args.Command); string commandName = args.Command.GetCommandName(); IEnumerable elements = args.Command.CommandElements.Skip(1); IScriptExtent elementsExtent = elements.JoinExtents(); @@ -194,14 +320,14 @@ private static async Task> GetEdits(CommandSplatArgume var splatWriter = new PowerShellScriptWriter(args.Command); var elementsWriter = new PowerShellScriptWriter(args.Command); - splatWriter.SetPosition(parentStatement); + splatWriter.SetPosition(variableInjectionTarget); splatWriter.WriteAssignment( () => splatWriter.WriteVariable(args.VariableName), () => splatWriter.OpenHashtable()); if (elementsExtent is Empty.Extent) { - elementsWriter.SetPosition(parentStatement, atEnd: true); + elementsWriter.SetPosition(variableInjectionTarget, atEnd: true); elementsWriter.Write(Symbols.Space); } else diff --git a/src/EditorServicesCommandSuite/CodeGeneration/Refactors/SuppressAnalyzerMessageRefactor.cs b/src/EditorServicesCommandSuite/CodeGeneration/Refactors/SuppressAnalyzerMessageRefactor.cs index f2941ce..3ea855f 100644 --- a/src/EditorServicesCommandSuite/CodeGeneration/Refactors/SuppressAnalyzerMessageRefactor.cs +++ b/src/EditorServicesCommandSuite/CodeGeneration/Refactors/SuppressAnalyzerMessageRefactor.cs @@ -81,7 +81,9 @@ private CodeAction CreateAction(DiagnosticMarker marker) title: string.Format( CultureInfo.CurrentCulture, sourceAction.Title, - marker.RuleSuppressionId)); + string.IsNullOrEmpty(marker.RuleSuppressionId) + ? marker.RuleName + : marker.RuleSuppressionId)); } private async Task> GetMarkersAsync(DocumentContextBase request) @@ -123,6 +125,7 @@ internal IEnumerable CreateEdits() if (!_statementAcceptsAttributes) { WriteToParamBlock(Markers); + Writer.CreateDocumentEdits(); return Writer.Edits; } @@ -138,9 +141,11 @@ internal IEnumerable CreateEdits() else { WriteToStatement(group); + Writer.WriteIndentIfPending(); } } + Writer.CreateDocumentEdits(); return Writer.Edits; } @@ -179,6 +184,7 @@ private void WriteToParamBlock(IEnumerable markers) { Writer.SetPosition(sbAst.ParamBlock); WriteMarkers(markers); + Writer.WriteIndentIfPending(); return; } @@ -199,12 +205,12 @@ private void WriteToParamBlock(IEnumerable markers) return; } - bool foundToken = Context.Token + bool foundToken = Context.Token.List.First .FindNext() .ContainingStartOf(sbAst) .TryGetResult(out TokenNode entryToken); - if (foundToken) + if (!foundToken) { Writer.SetPosition(sbAst); } @@ -257,7 +263,8 @@ private void WriteMarkers(IEnumerable markers) Writer.WriteStringExpression( StringConstantType.SingleQuoted, marker.RuleName); - Writer.Write(Symbols.Comma + Symbols.Space); + Writer.Write(Symbols.Comma); + Writer.Write(Symbols.Space); Writer.WriteStringExpression( StringConstantType.SingleQuoted, marker.RuleSuppressionId); @@ -273,8 +280,8 @@ private bool DoesStatementAcceptAttributes() return false; } - Token tokenAtStatementStart = Context.Token - .FindNext() + Token tokenAtStatementStart = Context.Token.List.First + .FindNextOrSelf() .ContainingStartOf(_parentStatementAst) .GetResult() .Value; diff --git a/tools/AssertRequiredModule.ps1 b/tools/AssertRequiredModule.ps1 index 955515f..2680217 100644 --- a/tools/AssertRequiredModule.ps1 +++ b/tools/AssertRequiredModule.ps1 @@ -28,6 +28,12 @@ end { # TODO: Install required versions into the tools folder try { Import-Module @importModuleSplat -Force + } catch [System.IO.FileLoadException] { + # Hope this is the FileList manifest bug and move on if the module + # seems like it loaded. + if (-not (Get-Module $Name -ErrorAction Ignore)) { + throw $PSItem + } } catch [System.IO.FileNotFoundException] { Install-Module @importModuleSplat -Force:$Force.IsPresent -Scope $Scope Import-Module @importModuleSplat -Force