Skip to content

Commit d265818

Browse files
raghushantharaghushantha
committed
Merge pull request #337 from PowerShell/master
Merge Branches
2 parents 24f7858 + 33cd389 commit d265818

11 files changed

+66
-37
lines changed

‎Engine/Helper.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,9 @@ public bool HasSplattedVariable(CommandAst cmdAst)
283283
/// Given a commandast, checks whether positional parameters are used or not.
284284
/// </summary>
285285
/// <param name="cmdAst"></param>
286+
/// <param name="moreThanThreePositional">only return true if more than three positional parameters are used</param>
286287
/// <returns></returns>
287-
public bool PositionalParameterUsed(CommandAst cmdAst)
288+
public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositional = false)
288289
{
289290
if (cmdAst == null || cmdAst.GetCommandName() == null)
290291
{
@@ -351,6 +352,11 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
351352
arguments += 1;
352353
}
353354

355+
if (moreThanThreePositional && arguments < 3)
356+
{
357+
return false;
358+
}
359+
354360
return arguments > parameters;
355361
}
356362

‎Engine/ScriptAnalyzer.cs

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ private void Initialize(
183183
this.includeRegexList = new List<Regex>();
184184
this.excludeRegexList = new List<Regex>();
185185

186+
#region ParseProfile
186187
if (!String.IsNullOrWhiteSpace(profile))
187188
{
188189
try
@@ -191,9 +192,7 @@ private void Initialize(
191192
}
192193
catch
193194
{
194-
this.outputWriter.WriteError(new ErrorRecord(new FileNotFoundException(),
195-
string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, profile),
196-
ErrorCategory.InvalidArgument, this));
195+
this.outputWriter.WriteWarning(string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, profile));
197196
}
198197

199198
if (File.Exists(profile))
@@ -202,15 +201,23 @@ private void Initialize(
202201
ParseError[] parserErrors = null;
203202
Ast profileAst = Parser.ParseFile(profile, out parserTokens, out parserErrors);
204203
IEnumerable<Ast> hashTableAsts = profileAst.FindAll(item => item is HashtableAst, false);
205-
foreach (HashtableAst hashTableAst in hashTableAsts)
204+
205+
// no hashtable, raise warning
206+
if (hashTableAsts.Count() == 0)
207+
{
208+
this.outputWriter.WriteWarning(string.Format(CultureInfo.CurrentCulture, Strings.InvalidProfile, profile));
209+
}
210+
else
206211
{
212+
HashtableAst hashTableAst = hashTableAsts.First() as HashtableAst;
213+
207214
foreach (var kvp in hashTableAst.KeyValuePairs)
208215
{
209216
if (!(kvp.Item1 is StringConstantExpressionAst))
210217
{
211-
this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(),
212-
string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyFormat, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile),
213-
ErrorCategory.InvalidArgument, this));
218+
// first item (the key) should be a string
219+
this.outputWriter.WriteWarning(
220+
string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyFormat, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile));
214221
continue;
215222
}
216223

@@ -257,11 +264,11 @@ private void Initialize(
257264
{
258265
foreach (var element in arrayLitAst.Elements)
259266
{
267+
// all the values in the array needs to be string
260268
if (!(element is StringConstantExpressionAst))
261269
{
262-
this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(),
263-
string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, element.Extent.StartLineNumber, element.Extent.StartColumnNumber, profile),
264-
ErrorCategory.InvalidArgument, this));
270+
this.outputWriter.WriteWarning(
271+
string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, element.Extent.StartLineNumber, element.Extent.StartColumnNumber, profile));
265272
continue;
266273
}
267274

@@ -273,9 +280,8 @@ private void Initialize(
273280

274281
if (rhsList.Count == 0)
275282
{
276-
this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(),
277-
string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, kvp.Item2.Extent.StartLineNumber, kvp.Item2.Extent.StartColumnNumber, profile),
278-
ErrorCategory.InvalidArgument, this));
283+
this.outputWriter.WriteWarning(
284+
string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, kvp.Item2.Extent.StartLineNumber, kvp.Item2.Extent.StartColumnNumber, profile));
279285
break;
280286
}
281287

@@ -312,16 +318,17 @@ private void Initialize(
312318
}
313319
break;
314320
default:
315-
this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(),
316-
string.Format(CultureInfo.CurrentCulture, Strings.WrongKey, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile),
317-
ErrorCategory.InvalidArgument, this));
321+
this.outputWriter.WriteWarning(
322+
string.Format(CultureInfo.CurrentCulture, Strings.WrongKey, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile));
318323
break;
319324
}
320325
}
321326
}
322327
}
323328
}
324329

330+
#endregion
331+
325332
//Check wild card input for the Include/ExcludeRules and create regex match patterns
326333
if (this.includeRule != null)
327334
{

‎Engine/Strings.Designer.cs

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Engine/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,4 +201,7 @@
201201
<data name="WrongValueFormat" xml:space="preserve">
202202
<value>Value in the profile hashtable should be a string or an array of strings: line {0} column {1} in file {2}</value>
203203
</data>
204+
<data name="InvalidProfile" xml:space="preserve">
205+
<value>Profile file '{0}' is invalid because it does not contain a hashtable.</value>
206+
</data>
204207
</root>

‎Rules/AvoidPositionalParameters.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4545
if (cmdAst.GetCommandName() == null) continue;
4646

4747
if (Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()) != null
48-
&& Helper.Instance.PositionalParameterUsed(cmdAst))
48+
&& Helper.Instance.PositionalParameterUsed(cmdAst, true))
4949
{
5050
PipelineAst parent = cmdAst.Parent as PipelineAst;
5151

@@ -55,14 +55,14 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5555
if (parent.PipelineElements[0] == cmdAst)
5656
{
5757
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()),
58-
cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName());
58+
cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, cmdAst.GetCommandName());
5959
}
6060
}
6161
// not in pipeline so just raise it normally
6262
else
6363
{
6464
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()),
65-
cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName());
65+
cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, cmdAst.GetCommandName());
6666
}
6767
}
6868
}
@@ -109,7 +109,7 @@ public SourceType GetSourceType()
109109
/// <returns></returns>
110110
public RuleSeverity GetSeverity()
111111
{
112-
return RuleSeverity.Warning;
112+
return RuleSeverity.Information;
113113
}
114114

115115
/// <summary>

‎Tests/Engine/GetScriptAnalyzerRule.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ Describe "TestSeverity" {
118118

119119
It "filters rules based on multiple severity inputs"{
120120
$rules = Get-ScriptAnalyzerRule -Severity Error,Information
121-
$rules.Count | Should be 13
121+
$rules.Count | Should be 14
122122
}
123123

124124
It "takes lower case inputs" {

‎Tests/Engine/InvokeScriptAnalyzer.tests.ps1

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,12 @@ Describe "Test IncludeRule" {
179179
Context "IncludeRule supports wild card" {
180180
It "includes 1 wildcard rule"{
181181
$includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules
182-
$includeWildcard.Count | Should be 3
182+
$includeWildcard.Count | Should be 0
183183
}
184184

185185
it "includes 2 wildcardrules" {
186186
$includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules, $useRules
187-
$includeWildcard.Count | Should be 7
187+
$includeWildcard.Count | Should be 4
188188
}
189189
}
190190
}
@@ -210,12 +210,12 @@ Describe "Test Severity" {
210210

211211
It "works with 2 arguments" {
212212
$errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity Information, Warning
213-
$errors.Count | Should Be 2
213+
$errors.Count | Should Be 1
214214
}
215215

216216
It "works with lowercase argument"{
217217
$errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity information, warning
218-
$errors.Count | Should Be 2
218+
$errors.Count | Should Be 1
219219
}
220220
}
221221

‎Tests/Engine/RuleSuppression.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ Describe "RuleSuppressionWithScope" {
5050
Context "FunctionScope" {
5151
It "Does not raise violations" {
5252
$suppression = $violations | Where-Object {$_.RuleName -eq "PSAvoidUsingPositionalParameters" }
53-
$suppression.Count | Should Be 1
53+
$suppression.Count | Should Be 0
5454
}
5555
}
5656

‎Tests/Rules/AvoidPositionalParameters.ps1

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,2 @@
1-
Get-Content Test
2-
Get-ChildItem Tests
3-
Write-Output "I don't want to use positional parameters"
4-
Split-Path "RandomPath" -Leaf
5-
Get-Process | ForEach-Object {Write-Host $_.name -foregroundcolor cyan}
1+
# give it 3 positional parameters
2+
Get-Command "abc" 4 4.3

‎Tests/Rules/AvoidPositionalParameters.tests.ps1

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Import-Module PSScriptAnalyzer
2-
$violationMessage = "Cmdlet 'Write-Host' has positional parameter. Please use named parameters instead of positional parameters when calling a command."
2+
$violationMessage = "Cmdlet 'Get-Command' has positional parameter. Please use named parameters instead of positional parameters when calling a command."
33
$violationName = "PSAvoidUsingPositionalParameters"
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\AvoidPositionalParameters.ps1 | Where-Object {$_.RuleName -eq $violationName}
@@ -8,8 +8,8 @@ $noViolationsDSC = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $director
88

99
Describe "AvoidPositionalParameters" {
1010
Context "When there are violations" {
11-
It "has 4 avoid positional parameters violation" {
12-
$violations.Count | Should Be 5
11+
It "has 1 avoid positional parameters violation" {
12+
$violations.Count | Should Be 1
1313
}
1414

1515
It "has the correct description message" {

0 commit comments

Comments
 (0)