diff --git a/Engine/FindAstPositionVisitor.cs b/Engine/FindAstPositionVisitor.cs new file mode 100644 index 000000000..5816f7e59 --- /dev/null +++ b/Engine/FindAstPositionVisitor.cs @@ -0,0 +1,369 @@ +// 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) + internal class FindAstPositionVisitor : AstVisitor2 +#else + internal class FindAstPositionVisitor : 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 FindAstPositionVisitor(IScriptPosition position) + { + this.searchPosition = position; + } + + 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 + + /// + /// 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 diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 0676fccf1..8b53c1ae4 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -40,6 +40,7 @@ CheckPipeForRedundantWhitespace = $false CheckSeparator = $true CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index a0c649457..188b2c651 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -40,6 +40,7 @@ CheckPipeForRedundantWhitespace = $false CheckSeparator = $true CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 21f4c0995..0e966bc46 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -40,6 +40,7 @@ CheckPipeForRedundantWhitespace = $false CheckSeparator = $true CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 34ea924a5..d9bb16be5 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -41,6 +41,7 @@ CheckPipeForRedundantWhitespace = $false CheckSeparator = $true CheckParameter = $false + IgnoreAssignmentOperatorInsideHashTable = $true } PSAlignAssignmentStatement = @{ diff --git a/Engine/TokenOperations.cs b/Engine/TokenOperations.cs index 0a47ab3cc..fa9a1978a 100644 --- a/Engine/TokenOperations.cs +++ b/Engine/TokenOperations.cs @@ -232,5 +232,18 @@ 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) + { + FindAstPositionVisitor findAstVisitor = new FindAstPositionVisitor(token.Extent.StartScriptPosition); + ast.Visit(findAstVisitor); + return findAstVisitor.AstPosition; + } + } } diff --git a/RuleDocumentation/UseConsistentWhitespace.md b/RuleDocumentation/UseConsistentWhitespace.md index 0e7509d1e..a100e9957 100644 --- a/RuleDocumentation/UseConsistentWhitespace.md +++ b/RuleDocumentation/UseConsistentWhitespace.md @@ -22,6 +22,7 @@ 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..ba127e739 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,16 @@ private IEnumerable FindOperatorViolations(TokenOperations tok continue; } + // exclude assignment operator inside of multi-line hash tables if requested + if (IgnoreAssignmentOperatorInsideHashTable && tokenNode.Value.Kind == TokenKind.Equals) + { + Ast containingAst = tokenOperations.GetAstPosition(tokenNode.Value); + if (containingAst is HashtableAst && containingAst.Extent.EndLineNumber != containingAst.Extent.StartLineNumber) + { + continue; + } + } + var hasWhitespaceBefore = IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode); var hasWhitespaceAfter = tokenNode.Next.Value.Kind == TokenKind.NewLine || IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode.Next); diff --git a/Tests/Engine/TokenOperations.tests.ps1 b/Tests/Engine/TokenOperations.tests.ps1 new file mode 100644 index 000000000..97fef3958 --- /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.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" }' + } +} \ No newline at end of file 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" {