From 0de98e90218b1a22ed8e161ec979568f294d0fd4 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 9 Mar 2018 22:35:32 +0000 Subject: [PATCH 1/3] Do not trigger UseDeclaredVarsMoreThanAssignment when using Get-Variable --- Rules/UseDeclaredVarsMoreThanAssignments.cs | 19 +++++++++++++++++++ ...eDeclaredVarsMoreThanAssignments.tests.ps1 | 5 +++++ 2 files changed, 24 insertions(+) diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index 32fb4c153..5e77fe43b 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -16,6 +16,7 @@ #endif using System.Globalization; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.Linq; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { @@ -204,6 +205,24 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip } } + // Detect usages of Get-Variable + var getVariableCmdletNamesAndAliases = Helper.Instance.CmdletNameAndAliases("Get-Variable"); + IEnumerable getVariableCommandAsts = scriptBlockAst.FindAll(testAst => testAst is CommandAst commandAst && + getVariableCmdletNamesAndAliases.Contains(commandAst.GetCommandName(), StringComparer.OrdinalIgnoreCase), true); + foreach (CommandAst getVariableCommandAst in getVariableCommandAsts) + { + var commandElements = getVariableCommandAst.CommandElements.ToList(); + // The following extracts the variable name only in the simplest possibe case 'Get-Variable variableName' + if (commandElements.Count == 2 && commandElements[1] is StringConstantExpressionAst constantExpressionAst) + { + var variableName = constantExpressionAst.Value; + if (assignments.ContainsKey(variableName)) + { + assignments.Remove(variableName); + } + } + } + foreach (string key in assignments.Keys) { yield return new DiagnosticRecord( diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 index daa766b06..23b8107bd 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 @@ -73,5 +73,10 @@ function MyFunc2() { It "returns no violations" { $noViolations.Count | Should -Be 0 } + + It "Using a variable via 'Get-Variable' does not trigger a warning" { + $noViolations = Invoke-ScriptAnalyzer -ScriptDefinition '$a=4; get-variable a' | Where-Object { $_.RuleName -eq $violationName } + $noViolations.Count | Should -Be 0 + } } } \ No newline at end of file From c67fc8d855c7993b52852b7d1a8fd66a3b2042ed Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 11 Mar 2018 19:32:25 +0000 Subject: [PATCH 2/3] update licence text --- Rules/UseDeclaredVarsMoreThanAssignments.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index 5e77fe43b..d7bcf4337 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -1,12 +1,5 @@ -// Copyright (c) Microsoft Corporation. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. using System; using System.Collections.Generic; From 361c0d8c684af4ca7cbd7398bf1d82afa8dbc8e9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 23 Mar 2018 19:36:10 +0000 Subject: [PATCH 3/3] address PR comments --- Rules/UseDeclaredVarsMoreThanAssignments.cs | 22 +++++++++---------- ...eDeclaredVarsMoreThanAssignments.tests.ps1 | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index d7bcf4337..ebe4ae5a6 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -114,7 +114,7 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip IEnumerable varAsts = scriptBlockAst.FindAll(testAst => testAst is VariableExpressionAst, true); IEnumerable varsInAssignment; - Dictionary assignments = new Dictionary(StringComparer.OrdinalIgnoreCase); + Dictionary assignmentsDictionary_OrdinalIgnoreCase = new Dictionary(StringComparer.OrdinalIgnoreCase); string varKey; bool inAssignment; @@ -149,9 +149,9 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip { string variableName = Helper.Instance.VariableNameWithoutScope(assignmentVarAst.VariablePath); - if (!assignments.ContainsKey(variableName)) + if (!assignmentsDictionary_OrdinalIgnoreCase.ContainsKey(variableName)) { - assignments.Add(variableName, assignmentAst); + assignmentsDictionary_OrdinalIgnoreCase.Add(variableName, assignmentAst); } } } @@ -164,9 +164,9 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip varKey = Helper.Instance.VariableNameWithoutScope(varAst.VariablePath); inAssignment = false; - if (assignments.ContainsKey(varKey)) + if (assignmentsDictionary_OrdinalIgnoreCase.ContainsKey(varKey)) { - varsInAssignment = assignments[varKey].Left.FindAll(testAst => testAst is VariableExpressionAst, true); + varsInAssignment = assignmentsDictionary_OrdinalIgnoreCase[varKey].Left.FindAll(testAst => testAst is VariableExpressionAst, true); // Checks if this variableAst is part of the logged assignment foreach (VariableExpressionAst varInAssignment in varsInAssignment) @@ -186,13 +186,13 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip if (!inAssignment) { - assignments.Remove(varKey); + assignmentsDictionary_OrdinalIgnoreCase.Remove(varKey); } // Check if variable belongs to PowerShell built-in variables if (Helper.Instance.HasSpecialVars(varKey)) { - assignments.Remove(varKey); + assignmentsDictionary_OrdinalIgnoreCase.Remove(varKey); } } } @@ -209,18 +209,18 @@ private IEnumerable AnalyzeScriptBlockAst(ScriptBlockAst scrip if (commandElements.Count == 2 && commandElements[1] is StringConstantExpressionAst constantExpressionAst) { var variableName = constantExpressionAst.Value; - if (assignments.ContainsKey(variableName)) + if (assignmentsDictionary_OrdinalIgnoreCase.ContainsKey(variableName)) { - assignments.Remove(variableName); + assignmentsDictionary_OrdinalIgnoreCase.Remove(variableName); } } } - foreach (string key in assignments.Keys) + foreach (string key in assignmentsDictionary_OrdinalIgnoreCase.Keys) { yield return new DiagnosticRecord( string.Format(CultureInfo.CurrentCulture, Strings.UseDeclaredVarsMoreThanAssignmentsError, key), - assignments[key].Left.Extent, + assignmentsDictionary_OrdinalIgnoreCase[key].Left.Extent, GetName(), DiagnosticSeverity.Warning, fileName, diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 index 23b8107bd..0ba7f63e1 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 @@ -75,7 +75,7 @@ function MyFunc2() { } It "Using a variable via 'Get-Variable' does not trigger a warning" { - $noViolations = Invoke-ScriptAnalyzer -ScriptDefinition '$a=4; get-variable a' | Where-Object { $_.RuleName -eq $violationName } + $noViolations = Invoke-ScriptAnalyzer -ScriptDefinition '$a=4; get-variable a' $noViolations.Count | Should -Be 0 } }