Skip to content

Commit 3485d15

Browse files
committed
Merge pull request #258 from PowerShell/master
Take latest Master to Development
2 parents 6300b22 + b4e890c commit 3485d15

File tree

44 files changed

+482
-225
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+482
-225
lines changed

CHANGELOG.MD

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,30 @@
1+
## Released v1.0.2 (June.24, 2015)
2+
###Features:
3+
- Perf improvements in the Engine to execute rules concurrently.
4+
5+
6+
###Rules:
7+
- New rule to validate the presence of deprecated module manifest fields.
8+
- Removed PSAvoidTrapStatement rule from the builtin set – since this is not deprecated and using trap is a better choice in certain scenarios.
9+
10+
11+
###Fixes:
12+
- Verbose Message rule applies to only DSC cmdlet based resources.
13+
- Multiple fixes to AvoidUninitializedVariable to work with non-mandatory parameters, fix in the flow graphs for throw statements; support for missing preference variables; support for automatic variables.
14+
- PSAvoidUsingInternalsURLs to work with xPath expressions.
15+
- UseSingularNouns rule to raise warnings for plural phrases.
16+
- Added .gitignore to exclude certain files from being reported as untracked.
17+
- Revisited severity for DSC rules.
18+
- PSUseOutputTypeCorrectly rule not to get triggered for functions returning system.void type.
19+
- PSAvoidDefaultTrueValueSwitchParameter to work with switch attribute when supplied as fully qualified.
20+
- Ignore PSObject and PSCustomObject for UseOutputTypeCorrectly rule.
21+
- Only raise NullComparisonRule if the RHS is an array or has unknown type.
22+
- PSUseDeclaredVarsMoreThanAssignments rule to be raised for script variables and for setting the property of a variable.
23+
- Support for using PSUseCmdletCorrectly rule when mandatory parameters are supplied in a splatted hashtable.
24+
- AvoidUsingPlainTextForPassword rule to be raised only strings or object types.
25+
- Fix for PositionalParameterUsed method (Helper.cs) uses unsafe method to exclude ForEach-Object and Where-Object.
26+
27+
128
## Released v1.0.1 (May.8, 2015)
229
###Features:
330
- Integrated with waffle.io for Project Management.
@@ -18,7 +45,6 @@
1845

1946

2047
##Released v1.0.0 on Apr.24, 2015
21-
2248
###Features:
2349
- Finalized three levels of Severity - Error/Warning/Information.
2450
- Improved PSScriptAnalyzer engine behavior: emits non-terminating errors (Ex: for failed ast parse) and continues rule application when running on multiple scripts.

Engine/Commands/InvokeScriptAnalyzerCommand.cs

Lines changed: 110 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
using System.Text.RegularExpressions;
1414
using System;
15+
using System.ComponentModel;
1516
using System.Collections.Generic;
1617
using System.Diagnostics.CodeAnalysis;
1718
using System.Globalization;
@@ -20,6 +21,9 @@
2021
using System.Management.Automation.Language;
2122
using System.IO;
2223
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
24+
using System.Threading.Tasks;
25+
using System.Collections.Concurrent;
26+
using System.Threading;
2327

2428
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands
2529
{
@@ -251,12 +255,12 @@ private void ProcessPath(string path)
251255
}
252256
}
253257
else if (File.Exists(path))
254-
{
255-
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, path));
258+
{
256259
if ((path.Length > ps1Suffix.Length && path.Substring(path.Length - ps1Suffix.Length).Equals(ps1Suffix, StringComparison.OrdinalIgnoreCase)) ||
257260
(path.Length > psm1Suffix.Length && path.Substring(path.Length - psm1Suffix.Length).Equals(psm1Suffix, StringComparison.OrdinalIgnoreCase)) ||
258261
(path.Length > psd1Suffix.Length && path.Substring(path.Length - psd1Suffix.Length).Equals(psd1Suffix, StringComparison.OrdinalIgnoreCase)))
259262
{
263+
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, path));
260264
AnalyzeFile(path);
261265
}
262266
}
@@ -267,6 +271,13 @@ private void ProcessPath(string path)
267271

268272
}
269273

274+
ConcurrentBag<DiagnosticRecord> diagnostics;
275+
ConcurrentBag<SuppressedRecord> suppressed;
276+
Dictionary<string, List<RuleSuppression>> ruleSuppressions;
277+
List<Regex> includeRegexList;
278+
List<Regex> excludeRegexList;
279+
ConcurrentDictionary<string, List<object>> ruleDictionary;
280+
270281
/// <summary>
271282
/// Analyzes a single script file.
272283
/// </summary>
@@ -275,15 +286,16 @@ private void AnalyzeFile(string filePath)
275286
{
276287
Token[] tokens = null;
277288
ParseError[] errors = null;
278-
List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
279-
List<SuppressedRecord> suppressed = new List<SuppressedRecord>();
289+
ConcurrentBag<DiagnosticRecord> diagnostics = new ConcurrentBag<DiagnosticRecord>();
290+
ConcurrentBag<SuppressedRecord> suppressed = new ConcurrentBag<SuppressedRecord>();
291+
BlockingCollection<List<object>> verboseOrErrors = new BlockingCollection<List<object>>();
280292

281293
// Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash
282294
List<KeyValuePair<CommandInfo, IScriptExtent>> cmdInfoTable = new List<KeyValuePair<CommandInfo, IScriptExtent>>();
283295

284296
//Check wild card input for the Include/ExcludeRules and create regex match patterns
285-
List<Regex> includeRegexList = new List<Regex>();
286-
List<Regex> excludeRegexList = new List<Regex>();
297+
includeRegexList = new List<Regex>();
298+
excludeRegexList = new List<Regex>();
287299
if (includeRule != null)
288300
{
289301
foreach (string rule in includeRule)
@@ -331,7 +343,7 @@ private void AnalyzeFile(string filePath)
331343
return;
332344
}
333345

334-
Dictionary<string, List<RuleSuppression>> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
346+
ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
335347

336348
foreach (List<RuleSuppression> ruleSuppressionsList in ruleSuppressions.Values)
337349
{
@@ -360,43 +372,75 @@ private void AnalyzeFile(string filePath)
360372

361373
if (ScriptAnalyzer.Instance.ScriptRules != null)
362374
{
363-
foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules)
364-
{
365-
bool includeRegexMatch = false;
366-
bool excludeRegexMatch = false;
367-
foreach (Regex include in includeRegexList)
375+
var tasks = ScriptAnalyzer.Instance.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() =>
368376
{
369-
if (include.IsMatch(scriptRule.GetName()))
377+
bool includeRegexMatch = false;
378+
bool excludeRegexMatch = false;
379+
380+
foreach (Regex include in includeRegexList)
370381
{
371-
includeRegexMatch = true;
372-
break;
382+
if (include.IsMatch(scriptRule.GetName()))
383+
{
384+
includeRegexMatch = true;
385+
break;
386+
}
373387
}
374-
}
375388

376-
foreach (Regex exclude in excludeRegexList)
377-
{
378-
if (exclude.IsMatch(scriptRule.GetName()))
389+
foreach (Regex exclude in excludeRegexList)
379390
{
380-
excludeRegexMatch = true;
381-
break;
391+
if (exclude.IsMatch(scriptRule.GetName()))
392+
{
393+
excludeRegexMatch = true;
394+
break;
395+
}
382396
}
383-
}
384-
385-
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
386-
{
387-
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
388397

389-
// Ensure that any unhandled errors from Rules are converted to non-terminating errors
390-
// We want the Engine to continue functioning even if one or more Rules throws an exception
391-
try
398+
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
392399
{
393-
var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList());
394-
diagnostics.AddRange(records.Item2);
395-
suppressed.AddRange(records.Item1);
400+
List<object> result = new List<object>();
401+
402+
result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
403+
404+
// Ensure that any unhandled errors from Rules are converted to non-terminating errors
405+
// We want the Engine to continue functioning even if one or more Rules throws an exception
406+
try
407+
{
408+
var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList());
409+
foreach (var record in records.Item2)
410+
{
411+
diagnostics.Add(record);
412+
}
413+
foreach (var suppressedRec in records.Item1)
414+
{
415+
suppressed.Add(suppressedRec);
416+
}
417+
}
418+
catch (Exception scriptRuleException)
419+
{
420+
result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File));
421+
}
422+
423+
verboseOrErrors.Add(result);
396424
}
397-
catch (Exception scriptRuleException)
425+
}));
426+
427+
Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding());
428+
429+
while (!verboseOrErrors.IsCompleted)
430+
{
431+
List<object> data = null;
432+
try
433+
{
434+
data = verboseOrErrors.Take();
435+
}
436+
catch (InvalidOperationException) { }
437+
438+
if (data != null)
439+
{
440+
WriteVerbose(data[0] as string);
441+
if (data.Count == 2)
398442
{
399-
WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
443+
WriteError(data[1] as ErrorRecord);
400444
}
401445
}
402446
}
@@ -437,8 +481,14 @@ private void AnalyzeFile(string filePath)
437481
try
438482
{
439483
var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList());
440-
diagnostics.AddRange(records.Item2);
441-
suppressed.AddRange(records.Item1);
484+
foreach (var record in records.Item2)
485+
{
486+
diagnostics.Add(record);
487+
}
488+
foreach (var suppressedRec in records.Item1)
489+
{
490+
suppressed.Add(suppressedRec);
491+
}
442492
}
443493
catch (Exception tokenRuleException)
444494
{
@@ -489,8 +539,14 @@ private void AnalyzeFile(string filePath)
489539
try
490540
{
491541
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList());
492-
diagnostics.AddRange(records.Item2);
493-
suppressed.AddRange(records.Item1);
542+
foreach (var record in records.Item2)
543+
{
544+
diagnostics.Add(record);
545+
}
546+
foreach (var suppressedRec in records.Item1)
547+
{
548+
suppressed.Add(suppressedRec);
549+
}
494550
}
495551
catch (Exception dscResourceRuleException)
496552
{
@@ -532,8 +588,14 @@ private void AnalyzeFile(string filePath)
532588
try
533589
{
534590
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
535-
diagnostics.AddRange(records.Item2);
536-
suppressed.AddRange(records.Item1);
591+
foreach (var record in records.Item2)
592+
{
593+
diagnostics.Add(record);
594+
}
595+
foreach (var suppressedRec in records.Item1)
596+
{
597+
suppressed.Add(suppressedRec);
598+
}
537599
}
538600
catch (Exception dscResourceRuleException)
539601
{
@@ -573,15 +635,20 @@ private void AnalyzeFile(string filePath)
573635
}
574636
}
575637

576-
diagnostics.AddRange(ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName));
638+
foreach (var record in ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName))
639+
{
640+
diagnostics.Add(record);
641+
}
577642
}
578643

579644
#endregion
580645

646+
IEnumerable<DiagnosticRecord> diagnosticsList = diagnostics;
647+
581648
if (severity != null)
582649
{
583650
var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true));
584-
diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList();
651+
diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity));
585652
}
586653

587654
//Output through loggers
@@ -596,7 +663,7 @@ private void AnalyzeFile(string filePath)
596663
}
597664
else
598665
{
599-
foreach (DiagnosticRecord diagnostic in diagnostics)
666+
foreach (DiagnosticRecord diagnostic in diagnosticsList)
600667
{
601668
logger.LogObject(diagnostic, this);
602669
}

Engine/Helper.cs

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,21 @@ item is TypeDefinitionAst
219219
return false;
220220
}
221221

222+
/// <summary>
223+
/// Given a commandast, checks whether it uses splatted variable
224+
/// </summary>
225+
/// <param name="cmdAst"></param>
226+
/// <returns></returns>
227+
public bool HasSplattedVariable(CommandAst cmdAst)
228+
{
229+
if (cmdAst == null || cmdAst.CommandElements == null)
230+
{
231+
return false;
232+
}
233+
234+
return cmdAst.CommandElements.Any(cmdElem => cmdElem is VariableExpressionAst && (cmdElem as VariableExpressionAst).Splatted);
235+
}
236+
222237
/// <summary>
223238
/// Given a commandast, checks whether positional parameters are used or not.
224239
/// </summary>
@@ -234,23 +249,17 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
234249
CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName());
235250

236251
IEnumerable<ParameterMetadata> switchParams = null;
237-
IEnumerable<CommandParameterSetInfo> scriptBlocks = null;
238-
bool hasScriptBlockSet = false;
252+
253+
if (HasSplattedVariable(cmdAst))
254+
{
255+
return false;
256+
}
239257

240258
if (commandInfo != null && commandInfo.CommandType == System.Management.Automation.CommandTypes.Cmdlet)
241259
{
242260
try
243261
{
244262
switchParams = commandInfo.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter);
245-
scriptBlocks = commandInfo.ParameterSets;
246-
foreach (CommandParameterSetInfo cmdParaset in scriptBlocks)
247-
{
248-
if (String.Equals(cmdParaset.Name, "ScriptBlockSet", StringComparison.OrdinalIgnoreCase))
249-
{
250-
hasScriptBlockSet = true;
251-
}
252-
}
253-
254263
}
255264
catch (Exception)
256265
{
@@ -264,8 +273,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
264273

265274
foreach (CommandElementAst ceAst in cmdAst.CommandElements)
266275
{
267-
if (!hasScriptBlockSet)
268-
{
269276
if (ceAst is CommandParameterAst)
270277
{
271278
// Skip if it's a switch parameter
@@ -286,17 +293,17 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
286293
}
287294
else
288295
{
289-
//Skip if splatting "@" is used
290-
if (ceAst is VariableExpressionAst)
291-
{
292-
if ((ceAst as VariableExpressionAst).Splatted)
293-
{
294-
continue;
295-
}
296-
}
297296
arguments += 1;
298297
}
299-
}
298+
299+
}
300+
301+
// if not the first element in a pipeline, increase the number of arguments by 1
302+
PipelineAst parent = cmdAst.Parent as PipelineAst;
303+
304+
if (parent != null && parent.PipelineElements.Count > 1 && parent.PipelineElements[0] != cmdAst)
305+
{
306+
arguments += 1;
300307
}
301308

302309
return arguments > parameters;
@@ -672,7 +679,7 @@ internal string GetTypeFromMemberExpressionAstHelper(MemberExpressionAst memberA
672679
/// <param name="varAst"></param>
673680
/// <param name="ast"></param>
674681
/// <returns></returns>
675-
internal Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast)
682+
public Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast)
676683
{
677684
try
678685
{

0 commit comments

Comments
 (0)