From 5dc3cd3929670d6d25511df34a1f41f57e438b6e Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Fri, 7 Aug 2020 02:08:49 -0500 Subject: [PATCH 1/9] UseConsistentWhitespace - Create option to ignore assignment operator inside hash table --- Engine/Settings/CodeFormatting.psd1 | 19 +++---- Engine/Settings/CodeFormattingAllman.psd1 | 19 +++---- Engine/Settings/CodeFormattingOTBS.psd1 | 19 +++---- Engine/Settings/CodeFormattingStroustrup.psd1 | 19 +++---- RuleDocumentation/UseConsistentWhitespace.md | 23 ++++---- Rules/UseConsistentWhitespace.cs | 26 +++++++++ Tests/Rules/UseConsistentWhitespace.tests.ps1 | 54 +++++++++++++++++++ 7 files changed, 134 insertions(+), 45 deletions(-) diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 0676fccf1..b1ef46f15 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -31,15 +31,16 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index a0c649457..5c92565a3 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -31,15 +31,16 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 21f4c0995..c17b51d8f 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -31,15 +31,16 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 34ea924a5..4a6d4866a 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -32,15 +32,16 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/RuleDocumentation/UseConsistentWhitespace.md b/RuleDocumentation/UseConsistentWhitespace.md index 0e7509d1e..e25ed6717 100644 --- a/RuleDocumentation/UseConsistentWhitespace.md +++ b/RuleDocumentation/UseConsistentWhitespace.md @@ -13,15 +13,16 @@ ```powershell Rules = @{ PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $false } } ``` @@ -64,3 +65,7 @@ Checks if a pipe is surrounded by redundant whitespace (i.e. more than 1 whitesp Checks if there is more than one space between parameters and values. E.g. `foo -bar $baz -bat` instead of `foo -bar $baz -bat`. This eliminates redundant whitespace that was probably added unintentionally. The rule does not check for whitespace between parameter and value when the colon syntax `-ParameterName:$ParameterValue` is used as some users prefer either 0 or 1 whitespace in this case. + +#### IgnoreAssignmentOperatorInsideHashTable: bool (Default value is `$false`) + +When `CheckOperator` is set, ignore whitespace around assignment operators within multi-line hash tables. Set this option in order to use the `AlignAssignmentStatement` rule but still check whitespace around operators everywhere else. \ No newline at end of file diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index aad4d3f46..a9d963b64 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -63,6 +63,9 @@ private List>> violationFind [ConfigurableRuleProperty(defaultValue: false)] public bool CheckParameter { get; protected set; } + [ConfigurableRuleProperty(defaultValue: false)] + public bool IgnoreAssignmentOperatorInsideHashTable { get; protected set; } + public override void ConfigureRule(IDictionary paramValueMap) { base.ConfigureRule(paramValueMap); @@ -530,6 +533,29 @@ private IEnumerable FindOperatorViolations(TokenOperations tok continue; } + // exclude assignment operator inside of multi-line hash tables if requested + if (IgnoreAssignmentOperatorInsideHashTable && tokenNode.Value.Kind == TokenKind.Equals) + { + var enclosingHashTables = tokenOperations.Ast.FindAll(a => a.Extent.StartOffset <= tokenNode.Value.Extent.StartOffset && a.Extent.EndOffset >= tokenNode.Value.Extent.EndOffset && a is HashtableAst, true); + if (enclosingHashTables.Count() > 0) + { + Ast innermostEnclosingHashTable = enclosingHashTables.First(); + int smallestSizeSoFar = int.MaxValue; + foreach (var hashTable in enclosingHashTables){ + int currentHashTableSize = hashTable.Extent.EndOffset - hashTable.Extent.StartOffset; + if (currentHashTableSize < smallestSizeSoFar) + { + innermostEnclosingHashTable = hashTable; + smallestSizeSoFar = currentHashTableSize; + } + } + if (innermostEnclosingHashTable.Extent.StartLineNumber != innermostEnclosingHashTable.Extent.EndLineNumber) + { + continue; + } + } + } + var hasWhitespaceBefore = IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode); var hasWhitespaceAfter = tokenNode.Next.Value.Kind == TokenKind.NewLine || IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode.Next); diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 77d320830..681fc4027 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -127,6 +127,7 @@ function foo($param) { $ruleConfiguration.CheckOperator = $true $ruleConfiguration.CheckPipe = $false $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.IgnoreAssignmentOperatorInsideHashTable = $false } It "Should find a violation if no whitespace around an assignment operator" { @@ -180,6 +181,59 @@ $x = $true -and Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } + It "Should find violation if not asked to ignore assignment operator in hash table" { + $def = @' +$ht = @{ + variable = 3 + other = 4 +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + } + + Context "When asked to ignore assignment operator inside hash table" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOperator = $true + $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.IgnoreAssignmentOperatorInsideHashTable = $true + } + It "Should not find violation if assignment operator is in multi-line hash table" { + $def = @' +$ht = @{ + variable = 3 + other = 4 +} +'@ + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should find violation if assignment operator has extra space in single-line hash table" { + $def = @' +$h = @{ + ht = @{a = 3; b = 4} + eb = 33 +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + + It "Should find violation for extra space around non-assignment operator inside hash table" { + $def = @' +$ht = @{ + variable = 3 + other = 4 + 7 +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } } Context "When a comma is not followed by a space" { From b9aa6e7824ae888e5873ba72a60522251ddcf3d0 Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Fri, 7 Aug 2020 21:23:35 -0500 Subject: [PATCH 2/9] Undo whitespace changes to settings files --- Engine/Settings/CodeFormatting.psd1 | 18 +++++++++--------- Engine/Settings/CodeFormattingAllman.psd1 | 18 +++++++++--------- Engine/Settings/CodeFormattingOTBS.psd1 | 18 +++++++++--------- Engine/Settings/CodeFormattingStroustrup.psd1 | 18 +++++++++--------- RuleDocumentation/UseConsistentWhitespace.md | 18 +++++++++--------- 5 files changed, 45 insertions(+), 45 deletions(-) diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index b1ef46f15..8b53c1ae4 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -31,15 +31,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false IgnoreAssignmentOperatorInsideHashTable = $true } diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index 5c92565a3..188b2c651 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -31,15 +31,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false IgnoreAssignmentOperatorInsideHashTable = $true } diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index c17b51d8f..0e966bc46 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -31,15 +31,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false IgnoreAssignmentOperatorInsideHashTable = $true } diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 4a6d4866a..d9bb16be5 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -32,15 +32,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false IgnoreAssignmentOperatorInsideHashTable = $true } diff --git a/RuleDocumentation/UseConsistentWhitespace.md b/RuleDocumentation/UseConsistentWhitespace.md index e25ed6717..a100e9957 100644 --- a/RuleDocumentation/UseConsistentWhitespace.md +++ b/RuleDocumentation/UseConsistentWhitespace.md @@ -13,15 +13,15 @@ ```powershell Rules = @{ PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckPipeForRedundantWhitespace = $false - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false IgnoreAssignmentOperatorInsideHashTable = $false } } From 759c71248ae85f605500d91787c18f05da631ff0 Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Tue, 11 Aug 2020 13:42:06 -0500 Subject: [PATCH 3/9] Begin implementation of visitor to find token in AST --- Engine/TokenOperations.cs | 80 ++++++++++++++++++++++++++ Rules/UseConsistentWhitespace.cs | 19 +----- Tests/Engine/TokenOperations.tests.ps1 | 21 +++++++ 3 files changed, 104 insertions(+), 16 deletions(-) create mode 100644 Tests/Engine/TokenOperations.tests.ps1 diff --git a/Engine/TokenOperations.cs b/Engine/TokenOperations.cs index 0a47ab3cc..6414f7109 100644 --- a/Engine/TokenOperations.cs +++ b/Engine/TokenOperations.cs @@ -232,5 +232,85 @@ private bool OnSameLine(Token token1, Token token2) { return token1.Extent.StartLineNumber == token2.Extent.EndLineNumber; } + + /// + /// Finds the position of a given token in the AST. + /// + /// The to search for. + /// The Ast node directly containing the provided . + public Ast GetAstPosition(Token token) + { + FindAstPostitionVisitor findAstVisitor = new FindAstPostitionVisitor(token.Extent.StartScriptPosition); + ast.Visit(findAstVisitor); + return findAstVisitor.AstPosition; + } + + } + + /// + /// Provides an efficient way to find the position in the AST corresponding to a given script position. + /// + public class FindAstPostitionVisitor : AstVisitor2 + { + private IScriptPosition searchPosition; + + /// + /// Contains the position in the AST corresponding to the provided upon completion of the method. + /// + public Ast AstPosition { get; private set; } + + /// + /// Initializes a new instance of the class with the postition to search for. + /// + /// The script position to search for. + public FindAstPostitionVisitor(IScriptPosition position) + { + this.searchPosition = position; + } + + /// + /// Traverses the AST based on offests to find the leaf node which contains the provided . + /// This method implements the entire functionality of this visitor. All methods are overridden to simply invoke this one. + /// + /// Current AST node to process. + /// An indicating whether to visit children of the current node. + private AstVisitAction Visit(Ast ast) + { + if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset < searchPosition.Offset) + { + return AstVisitAction.SkipChildren; + } + else + { + AstPosition = ast; + return AstVisitAction.Continue; + } + } + + public override AstVisitAction VisitScriptBlock(ScriptBlockAst scriptBlockAst) + { + return Visit(scriptBlockAst); + } + + public override AstVisitAction VisitNamedBlock(NamedBlockAst namedBlockAst) + { + return Visit(namedBlockAst); + } + + public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst assignmentStatementAst) + { + return Visit(assignmentStatementAst); + } + + public override AstVisitAction VisitCommandExpression(CommandExpressionAst commandExpressionAst) + { + return Visit(commandExpressionAst); + } + + public override AstVisitAction VisitHashtable(HashtableAst hashtableAst) + { + return Visit(hashtableAst); + } + } } diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index a9d963b64..ba127e739 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -536,23 +536,10 @@ private IEnumerable FindOperatorViolations(TokenOperations tok // exclude assignment operator inside of multi-line hash tables if requested if (IgnoreAssignmentOperatorInsideHashTable && tokenNode.Value.Kind == TokenKind.Equals) { - var enclosingHashTables = tokenOperations.Ast.FindAll(a => a.Extent.StartOffset <= tokenNode.Value.Extent.StartOffset && a.Extent.EndOffset >= tokenNode.Value.Extent.EndOffset && a is HashtableAst, true); - if (enclosingHashTables.Count() > 0) + Ast containingAst = tokenOperations.GetAstPosition(tokenNode.Value); + if (containingAst is HashtableAst && containingAst.Extent.EndLineNumber != containingAst.Extent.StartLineNumber) { - Ast innermostEnclosingHashTable = enclosingHashTables.First(); - int smallestSizeSoFar = int.MaxValue; - foreach (var hashTable in enclosingHashTables){ - int currentHashTableSize = hashTable.Extent.EndOffset - hashTable.Extent.StartOffset; - if (currentHashTableSize < smallestSizeSoFar) - { - innermostEnclosingHashTable = hashTable; - smallestSizeSoFar = currentHashTableSize; - } - } - if (innermostEnclosingHashTable.Extent.StartLineNumber != innermostEnclosingHashTable.Extent.EndLineNumber) - { - continue; - } + continue; } } diff --git a/Tests/Engine/TokenOperations.tests.ps1 b/Tests/Engine/TokenOperations.tests.ps1 new file mode 100644 index 000000000..401f06b88 --- /dev/null +++ b/Tests/Engine/TokenOperations.tests.ps1 @@ -0,0 +1,21 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +Describe "TokenOperations" { + It "Should return correct AST position for assignment operator in hash table" { + $scriptText = @' +$h = @{ + a = 72 + b = @{ z = "hi" } +} +'@ + $tokens = $null + $parseErrors = $null + $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors) + $tokenOperations = New-Object Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations -ArgumentList @($tokens, $scriptAst) + $operatorToken = $tokens | Where-Object { $_.Extent.StartOffset -eq 32 } + $hashTableAst = $tokenOperations.GetAstPosition($operatorToken) + $hashTableAst | Should -BeOfType [System.Management.Automation.Language.HashTableAst] + $hashTableAst.Extent.Text | Should -Be '@{ z = "hi" }' + } +} \ No newline at end of file From 7ef643b78d858cfb0eadec12d831556f2ec163d0 Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Tue, 11 Aug 2020 18:51:44 -0500 Subject: [PATCH 4/9] Fleshed out FindAstPositionVistor and fixed test --- Engine/FindAstPositionVisitor.cs | 372 +++++++++++++++++++++++++ Engine/TokenOperations.cs | 67 ----- Tests/Engine/TokenOperations.tests.ps1 | 2 +- 3 files changed, 373 insertions(+), 68 deletions(-) create mode 100644 Engine/FindAstPositionVisitor.cs diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs new file mode 100644 index 000000000..baa5c4801 --- /dev/null +++ b/Engine/FindAstPositionVisitor.cs @@ -0,0 +1,372 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Management.Automation.Language; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer +{ + /// + /// Provides an efficient way to find the position in the AST corresponding to a given script position. + /// +#if !(PSV3 || PSV4) + public class FindAstPostitionVisitor : AstVisitor2 +#else + public class FindAstPostitionVisitor : AstVisitor +#endif + { + private IScriptPosition searchPosition; + + /// + /// Contains the position in the AST corresponding to the provided upon completion of the method. + /// + public Ast AstPosition { get; private set; } + + /// + /// Initializes a new instance of the class with the postition to search for. + /// + /// The script position to search for. + public FindAstPostitionVisitor(IScriptPosition position) + { + this.searchPosition = position; + } + + /// + /// Traverses the AST based on offests to find the leaf node which contains the provided . + /// This method implements the entire functionality of this visitor. All methods are overridden to simply invoke this one. + /// + /// Current AST node to process. + /// An indicating whether to visit children of the current node. + private AstVisitAction Visit(Ast ast) + { + if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset < searchPosition.Offset) + { + return AstVisitAction.SkipChildren; + } + else + { + AstPosition = ast; + return AstVisitAction.Continue; + } + } + + public override AstVisitAction VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) + { + return Visit(arrayExpressionAst); + } + + public override AstVisitAction VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) + { + return Visit(arrayLiteralAst); + } + + public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst assignmentStatementAst) + { + return Visit(assignmentStatementAst); + } + + public override AstVisitAction VisitAttribute(AttributeAst attributeAst) + { + return Visit(attributeAst); + } + + public override AstVisitAction VisitAttributedExpression(AttributedExpressionAst attributedExpressionAst) + { + return Visit(attributedExpressionAst); + } + + public override AstVisitAction VisitBinaryExpression(BinaryExpressionAst binaryExpressionAst) + { + return Visit(binaryExpressionAst); + } + + public override AstVisitAction VisitBlockStatement(BlockStatementAst blockStatementAst) + { + return Visit(blockStatementAst); + } + + public override AstVisitAction VisitBreakStatement(BreakStatementAst breakStatementAst) + { + return Visit(breakStatementAst); + } + + public override AstVisitAction VisitCatchClause(CatchClauseAst catchClauseAst) + { + return Visit(catchClauseAst); + } + + public override AstVisitAction VisitCommand(CommandAst commandAst) + { + return Visit(commandAst); + } + + public override AstVisitAction VisitCommandExpression(CommandExpressionAst commandExpressionAst) + { + return Visit(commandExpressionAst); + } + + public override AstVisitAction VisitCommandParameter(CommandParameterAst commandParameterAst) + { + return Visit(commandParameterAst); + } + + public override AstVisitAction VisitConstantExpression(ConstantExpressionAst constantExpressionAst) + { + return Visit(constantExpressionAst); + } + + public override AstVisitAction VisitContinueStatement(ContinueStatementAst continueStatementAst) + { + return Visit(continueStatementAst); + } + + public override AstVisitAction VisitConvertExpression(ConvertExpressionAst convertExpressionAst) + { + return Visit(convertExpressionAst); + } + + public override AstVisitAction VisitDataStatement(DataStatementAst dataStatementAst) + { + return Visit(dataStatementAst); + } + + public override AstVisitAction VisitDoUntilStatement(DoUntilStatementAst doUntilStatementAst) + { + return Visit(doUntilStatementAst); + } + + public override AstVisitAction VisitDoWhileStatement(DoWhileStatementAst doWhileStatementAst) + { + return Visit(doWhileStatementAst); + } + + public override AstVisitAction VisitErrorExpression(ErrorExpressionAst errorExpressionAst) + { + return Visit(errorExpressionAst); + } + + public override AstVisitAction VisitErrorStatement(ErrorStatementAst errorStatementAst) + { + return Visit(errorStatementAst); + } + + public override AstVisitAction VisitExitStatement(ExitStatementAst exitStatementAst) + { + return Visit(exitStatementAst); + } + + public override AstVisitAction VisitExpandableStringExpression(ExpandableStringExpressionAst expandableStringExpressionAst) + { + return Visit(expandableStringExpressionAst); + } + + public override AstVisitAction VisitFileRedirection(FileRedirectionAst fileRedirectionAst) + { + return Visit(fileRedirectionAst); + } + + public override AstVisitAction VisitForEachStatement(ForEachStatementAst forEachStatementAst) + { + return Visit(forEachStatementAst); + } + + public override AstVisitAction VisitForStatement(ForStatementAst forStatementAst) + { + return Visit(forStatementAst); + } + + public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst) + { + return Visit(functionDefinitionAst); + } + + public override AstVisitAction VisitHashtable(HashtableAst hashtableAst) + { + return Visit(hashtableAst); + } + + public override AstVisitAction VisitIfStatement(IfStatementAst ifStatementAst) + { + return Visit(ifStatementAst); + } + + public override AstVisitAction VisitIndexExpression(IndexExpressionAst indexExpressionAst) + { + return Visit(indexExpressionAst); + } + + public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMemberExpressionAst) + { + return Visit(invokeMemberExpressionAst); + } + + public override AstVisitAction VisitMemberExpression(MemberExpressionAst memberExpressionAst) + { + return Visit(memberExpressionAst); + } + + public override AstVisitAction VisitMergingRedirection(MergingRedirectionAst mergingRedirectionAst) + { + return Visit(mergingRedirectionAst); + } + + public override AstVisitAction VisitNamedAttributeArgument(NamedAttributeArgumentAst namedAttributeArgumentAst) + { + return Visit(namedAttributeArgumentAst); + } + + public override AstVisitAction VisitNamedBlock(NamedBlockAst namedBlockAst) + { + return Visit(namedBlockAst); + } + + public override AstVisitAction VisitParamBlock(ParamBlockAst paramBlockAst) + { + return Visit(paramBlockAst); + } + + public override AstVisitAction VisitParameter(ParameterAst parameterAst) + { + return Visit(parameterAst); + } + + public override AstVisitAction VisitParenExpression(ParenExpressionAst parenExpressionAst) + { + return Visit(parenExpressionAst); + } + + public override AstVisitAction VisitPipeline(PipelineAst pipelineAst) + { + return Visit(pipelineAst); + } + + public override AstVisitAction VisitReturnStatement(ReturnStatementAst returnStatementAst) + { + return Visit(returnStatementAst); + } + + public override AstVisitAction VisitScriptBlock(ScriptBlockAst scriptBlockAst) + { + return Visit(scriptBlockAst); + } + + public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionAst scriptBlockExpressionAst) + { + return Visit(scriptBlockExpressionAst); + } + + public override AstVisitAction VisitStatementBlock(StatementBlockAst statementBlockAst) + { + return Visit(statementBlockAst); + } + + public override AstVisitAction VisitStringConstantExpression(StringConstantExpressionAst stringConstantExpressionAst) + { + return Visit(stringConstantExpressionAst); + } + + public override AstVisitAction VisitSubExpression(SubExpressionAst subExpressionAst) + { + return Visit(subExpressionAst); + } + + public override AstVisitAction VisitSwitchStatement(SwitchStatementAst switchStatementAst) + { + return Visit(switchStatementAst); + } + + public override AstVisitAction VisitThrowStatement(ThrowStatementAst throwStatementAst) + { + return Visit(throwStatementAst); + } + + public override AstVisitAction VisitTrap(TrapStatementAst trapStatementAst) + { + return Visit(trapStatementAst); + } + + public override AstVisitAction VisitTryStatement(TryStatementAst tryStatementAst) + { + return Visit(tryStatementAst); + } + + public override AstVisitAction VisitTypeConstraint(TypeConstraintAst typeConstraintAst) + { + return Visit(typeConstraintAst); + } + + public override AstVisitAction VisitTypeExpression(TypeExpressionAst typeExpressionAst) + { + return Visit(typeExpressionAst); + } + + public override AstVisitAction VisitUnaryExpression(UnaryExpressionAst unaryExpressionAst) + { + return Visit(unaryExpressionAst); + } + + public override AstVisitAction VisitUsingExpression(UsingExpressionAst usingExpressionAst) + { + return Visit(usingExpressionAst); + } + + public override AstVisitAction VisitVariableExpression(VariableExpressionAst variableExpressionAst) + { + return Visit(variableExpressionAst); + } + + public override AstVisitAction VisitWhileStatement(WhileStatementAst whileStatementAst) + { + return Visit(whileStatementAst); + } + +#if !(PSV3 || PSV4) + public override AstVisitAction VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst) + { + return Visit(baseCtorInvokeMemberExpressionAst); + } + + public override AstVisitAction VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst) + { + return Visit(configurationDefinitionAst); + } + + public override AstVisitAction VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordStatementAst) + { + return Visit(dynamicKeywordStatementAst); + } + + public override AstVisitAction VisitFunctionMember(FunctionMemberAst functionMemberAst) + { + return Visit(functionMemberAst); + } + + public override AstVisitAction VisitPropertyMember(PropertyMemberAst propertyMemberAst) + { + return Visit(propertyMemberAst); + } + + public override AstVisitAction VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst) + { + return Visit(typeDefinitionAst); + } + + public override AstVisitAction VisitUsingStatement(UsingStatementAst usingStatementAst) + { + return Visit(usingStatementAst); + } +#endif + +#if !(NET452 || PSV6) // NET452 includes V3,4,5 + public override AstVisitAction VisitPipelineChain(PipelineChainAst pipelineChainAst) + { + return Visit(pipelineChainAst); + } + + public override AstVisitAction VisitTernaryExpression(TernaryExpressionAst ternaryExpressionAst) + { + return Visit(ternaryExpressionAst); + } +#endif + + } +} \ No newline at end of file diff --git a/Engine/TokenOperations.cs b/Engine/TokenOperations.cs index 6414f7109..8e2ba3c27 100644 --- a/Engine/TokenOperations.cs +++ b/Engine/TokenOperations.cs @@ -246,71 +246,4 @@ public Ast GetAstPosition(Token token) } } - - /// - /// Provides an efficient way to find the position in the AST corresponding to a given script position. - /// - public class FindAstPostitionVisitor : AstVisitor2 - { - private IScriptPosition searchPosition; - - /// - /// Contains the position in the AST corresponding to the provided upon completion of the method. - /// - public Ast AstPosition { get; private set; } - - /// - /// Initializes a new instance of the class with the postition to search for. - /// - /// The script position to search for. - public FindAstPostitionVisitor(IScriptPosition position) - { - this.searchPosition = position; - } - - /// - /// Traverses the AST based on offests to find the leaf node which contains the provided . - /// This method implements the entire functionality of this visitor. All methods are overridden to simply invoke this one. - /// - /// Current AST node to process. - /// An indicating whether to visit children of the current node. - private AstVisitAction Visit(Ast ast) - { - if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset < searchPosition.Offset) - { - return AstVisitAction.SkipChildren; - } - else - { - AstPosition = ast; - return AstVisitAction.Continue; - } - } - - public override AstVisitAction VisitScriptBlock(ScriptBlockAst scriptBlockAst) - { - return Visit(scriptBlockAst); - } - - public override AstVisitAction VisitNamedBlock(NamedBlockAst namedBlockAst) - { - return Visit(namedBlockAst); - } - - public override AstVisitAction VisitAssignmentStatement(AssignmentStatementAst assignmentStatementAst) - { - return Visit(assignmentStatementAst); - } - - public override AstVisitAction VisitCommandExpression(CommandExpressionAst commandExpressionAst) - { - return Visit(commandExpressionAst); - } - - public override AstVisitAction VisitHashtable(HashtableAst hashtableAst) - { - return Visit(hashtableAst); - } - - } } diff --git a/Tests/Engine/TokenOperations.tests.ps1 b/Tests/Engine/TokenOperations.tests.ps1 index 401f06b88..97fef3958 100644 --- a/Tests/Engine/TokenOperations.tests.ps1 +++ b/Tests/Engine/TokenOperations.tests.ps1 @@ -13,7 +13,7 @@ $h = @{ $parseErrors = $null $scriptAst = [System.Management.Automation.Language.Parser]::ParseInput($scriptText, [ref] $tokens, [ref] $parseErrors) $tokenOperations = New-Object Microsoft.Windows.PowerShell.ScriptAnalyzer.TokenOperations -ArgumentList @($tokens, $scriptAst) - $operatorToken = $tokens | Where-Object { $_.Extent.StartOffset -eq 32 } + $operatorToken = $tokens | Where-Object { $_.Extent.StartLineNumber -eq 3 -and $_.Extent.StartColumnNumber -eq 14} $hashTableAst = $tokenOperations.GetAstPosition($operatorToken) $hashTableAst | Should -BeOfType [System.Management.Automation.Language.HashTableAst] $hashTableAst.Extent.Text | Should -Be '@{ z = "hi" }' From 047ae5e80fbae761d76c500364bc684faca48666 Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Wed, 12 Aug 2020 10:52:57 -0500 Subject: [PATCH 5/9] Make visitor internal --- Engine/FindAstPositionVisitor.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs index baa5c4801..c957e8d03 100644 --- a/Engine/FindAstPositionVisitor.cs +++ b/Engine/FindAstPositionVisitor.cs @@ -9,9 +9,9 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer /// Provides an efficient way to find the position in the AST corresponding to a given script position. /// #if !(PSV3 || PSV4) - public class FindAstPostitionVisitor : AstVisitor2 + internal class FindAstPostitionVisitor : AstVisitor2 #else - public class FindAstPostitionVisitor : AstVisitor + internal class FindAstPostitionVisitor : AstVisitor #endif { private IScriptPosition searchPosition; From 94329b08b72077a38df4a25541f34593b5fbef9e Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Tue, 25 Aug 2020 16:32:29 -0500 Subject: [PATCH 6/9] correct spelling --- Engine/FindAstPositionVisitor.cs | 8 ++++---- Engine/TokenOperations.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs index c957e8d03..75ee33aad 100644 --- a/Engine/FindAstPositionVisitor.cs +++ b/Engine/FindAstPositionVisitor.cs @@ -9,9 +9,9 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer /// Provides an efficient way to find the position in the AST corresponding to a given script position. /// #if !(PSV3 || PSV4) - internal class FindAstPostitionVisitor : AstVisitor2 + internal class FindAstPositionVisitor : AstVisitor2 #else - internal class FindAstPostitionVisitor : AstVisitor + internal class FindAstPositionVisitor : AstVisitor #endif { private IScriptPosition searchPosition; @@ -22,10 +22,10 @@ internal class FindAstPostitionVisitor : AstVisitor public Ast AstPosition { get; private set; } /// - /// Initializes a new instance of the class with the postition to search for. + /// Initializes a new instance of the class with the postition to search for. /// /// The script position to search for. - public FindAstPostitionVisitor(IScriptPosition position) + public FindAstPositionVisitor(IScriptPosition position) { this.searchPosition = position; } diff --git a/Engine/TokenOperations.cs b/Engine/TokenOperations.cs index 8e2ba3c27..fa9a1978a 100644 --- a/Engine/TokenOperations.cs +++ b/Engine/TokenOperations.cs @@ -240,7 +240,7 @@ private bool OnSameLine(Token token1, Token token2) /// The Ast node directly containing the provided . public Ast GetAstPosition(Token token) { - FindAstPostitionVisitor findAstVisitor = new FindAstPostitionVisitor(token.Extent.StartScriptPosition); + FindAstPositionVisitor findAstVisitor = new FindAstPositionVisitor(token.Extent.StartScriptPosition); ast.Visit(findAstVisitor); return findAstVisitor.AstPosition; } From cc9ebd7af7f927fbd8ccdd3c727afa00c977d80d Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Thu, 22 Oct 2020 16:12:02 -0500 Subject: [PATCH 7/9] fix off-by-one error in visitor --- Engine/FindAstPositionVisitor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs index 75ee33aad..0d163d9de 100644 --- a/Engine/FindAstPositionVisitor.cs +++ b/Engine/FindAstPositionVisitor.cs @@ -38,7 +38,7 @@ public FindAstPositionVisitor(IScriptPosition position) /// An indicating whether to visit children of the current node. private AstVisitAction Visit(Ast ast) { - if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset < searchPosition.Offset) + if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset <= searchPosition.Offset) { return AstVisitAction.SkipChildren; } From ff96591d7b9c4a485db613df20daad25db1da426 Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Mon, 26 Oct 2020 14:51:07 -0500 Subject: [PATCH 8/9] Simplify code in visitor --- Engine/FindAstPositionVisitor.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs index 0d163d9de..e8d5ada12 100644 --- a/Engine/FindAstPositionVisitor.cs +++ b/Engine/FindAstPositionVisitor.cs @@ -42,11 +42,8 @@ private AstVisitAction Visit(Ast ast) { return AstVisitAction.SkipChildren; } - else - { - AstPosition = ast; - return AstVisitAction.Continue; - } + AstPosition = ast; + return AstVisitAction.Continue; } public override AstVisitAction VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) From 3594ed5af30d9f866b18f4e343b8edc59291dfca Mon Sep 17 00:00:00 2001 From: Joel Davies <12704625+daviesj@users.noreply.github.com> Date: Thu, 19 Nov 2020 02:39:10 -0600 Subject: [PATCH 9/9] Move private method to the bottom & small summary correction (leaf => leaf-most) --- Engine/FindAstPositionVisitor.cs | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs index e8d5ada12..5816f7e59 100644 --- a/Engine/FindAstPositionVisitor.cs +++ b/Engine/FindAstPositionVisitor.cs @@ -30,23 +30,7 @@ public FindAstPositionVisitor(IScriptPosition position) this.searchPosition = position; } - /// - /// Traverses the AST based on offests to find the leaf node which contains the provided . - /// This method implements the entire functionality of this visitor. All methods are overridden to simply invoke this one. - /// - /// Current AST node to process. - /// An indicating whether to visit children of the current node. - private AstVisitAction Visit(Ast ast) - { - if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset <= searchPosition.Offset) - { - return AstVisitAction.SkipChildren; - } - AstPosition = ast; - return AstVisitAction.Continue; - } - - public override AstVisitAction VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) + public override AstVisitAction VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) { return Visit(arrayExpressionAst); } @@ -365,5 +349,21 @@ public override AstVisitAction VisitTernaryExpression(TernaryExpressionAst terna } #endif + /// + /// Traverses the AST based on offests to find the leaf-most node which contains the provided . + /// This method implements the entire functionality of this visitor. All methods are overridden to simply invoke this one. + /// + /// Current AST node to process. + /// An indicating whether to visit children of the current node. + private AstVisitAction Visit(Ast ast) + { + if (ast.Extent.StartOffset > searchPosition.Offset || ast.Extent.EndOffset <= searchPosition.Offset) + { + return AstVisitAction.SkipChildren; + } + AstPosition = ast; + return AstVisitAction.Continue; + } + } } \ No newline at end of file