Skip to content

Commit

Permalink
Implement -IncludeSuppressions parameter (#1701)
Browse files Browse the repository at this point in the history
  • Loading branch information
rjmholt authored Aug 4, 2021
1 parent 9112135 commit d1942a1
Show file tree
Hide file tree
Showing 25 changed files with 319 additions and 155 deletions.
109 changes: 67 additions & 42 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands
/// </summary>
[Cmdlet(VerbsLifecycle.Invoke,
"ScriptAnalyzer",
DefaultParameterSetName = "File",
DefaultParameterSetName = ParameterSet_Path_SuppressedOnly,
SupportsShouldProcess = true,
HelpUri = "https://go.microsoft.com/fwlink/?LinkId=525914")]
[OutputType(typeof(DiagnosticRecord))]
[OutputType(typeof(SuppressedRecord))]
[OutputType(typeof(DiagnosticRecord), typeof(SuppressedRecord))]
public class InvokeScriptAnalyzerCommand : PSCmdlet, IOutputWriter
{
private const string ParameterSet_Path_SuppressedOnly = nameof(Path) + "_" + nameof(SuppressedOnly);
private const string ParameterSet_Path_IncludeSuppressed = nameof(Path) + "_" + nameof(IncludeSuppressed);
private const string ParameterSet_ScriptDefinition_SuppressedOnly = nameof(ScriptDefinition) + "_" + nameof(SuppressedOnly);
private const string ParameterSet_ScriptDefinition_IncludeSuppressed = nameof(ScriptDefinition) + "_" + nameof(IncludeSuppressed);

#region Private variables
List<string> processedPaths;
#endregion // Private variables
Expand All @@ -37,7 +41,12 @@ public class InvokeScriptAnalyzerCommand : PSCmdlet, IOutputWriter
/// Path: The path to the file or folder to invoke PSScriptAnalyzer on.
/// </summary>
[Parameter(Position = 0,
ParameterSetName = "File",
ParameterSetName = ParameterSet_Path_IncludeSuppressed,
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
[Parameter(Position = 0,
ParameterSetName = ParameterSet_Path_SuppressedOnly,
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
Expand All @@ -54,7 +63,12 @@ public string Path
/// ScriptDefinition: a script definition in the form of a string to run rules on.
/// </summary>
[Parameter(Position = 0,
ParameterSetName = "ScriptDefinition",
ParameterSetName = ParameterSet_ScriptDefinition_IncludeSuppressed,
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
[Parameter(Position = 0,
ParameterSetName = ParameterSet_ScriptDefinition_SuppressedOnly,
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
Expand Down Expand Up @@ -84,7 +98,6 @@ public string[] CustomRulePath
/// RecurseCustomRulePath: Find rules within subfolders under the path
/// </summary>
[Parameter(Mandatory = false)]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public SwitchParameter RecurseCustomRulePath
{
get { return recurseCustomRulePath; }
Expand All @@ -96,7 +109,6 @@ public SwitchParameter RecurseCustomRulePath
/// IncludeDefaultRules: Invoke default rules along with Custom rules
/// </summary>
[Parameter(Mandatory = false)]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public SwitchParameter IncludeDefaultRules
{
get { return includeDefaultRules; }
Expand Down Expand Up @@ -143,11 +155,15 @@ public string[] Severity
}
private string[] severity;

// TODO: This should be only in the Path parameter sets, and is ignored otherwise,
// but we already have a test that depends on it being otherwise
//[Parameter(ParameterSetName = ParameterSet_Path_IncludeSuppressed)]
//[Parameter(ParameterSetName = ParameterSet_Path_SuppressedOnly)]
//
/// <summary>
/// Recurse: Apply to all files within subfolders under the path
/// </summary>
[Parameter(Mandatory = false)]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
[Parameter]
public SwitchParameter Recurse
{
get { return recurse; }
Expand All @@ -158,19 +174,22 @@ public SwitchParameter Recurse
/// <summary>
/// ShowSuppressed: Show the suppressed message
/// </summary>
[Parameter(Mandatory = false)]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public SwitchParameter SuppressedOnly
{
get { return suppressedOnly; }
set { suppressedOnly = value; }
}
private bool suppressedOnly;
[Parameter(ParameterSetName = ParameterSet_Path_SuppressedOnly)]
[Parameter(ParameterSetName = ParameterSet_ScriptDefinition_SuppressedOnly)]
public SwitchParameter SuppressedOnly { get; set; }

/// <summary>
/// Include suppressed diagnostics in the output.
/// </summary>
[Parameter(ParameterSetName = ParameterSet_Path_IncludeSuppressed, Mandatory = true)]
[Parameter(ParameterSetName = ParameterSet_ScriptDefinition_IncludeSuppressed, Mandatory = true)]
public SwitchParameter IncludeSuppressed { get; set; }

/// <summary>
/// Resolves rule violations automatically where possible.
/// </summary>
[Parameter(Mandatory = false, ParameterSetName = "File")]
[Parameter(Mandatory = false, ParameterSetName = ParameterSet_Path_IncludeSuppressed)]
[Parameter(Mandatory = false, ParameterSetName = ParameterSet_Path_SuppressedOnly)]
public SwitchParameter Fix
{
get { return fix; }
Expand Down Expand Up @@ -334,14 +353,20 @@ protected override void BeginProcessing()
this.settings));
}

SuppressionPreference suppressionPreference = SuppressedOnly
? SuppressionPreference.SuppressedOnly
: IncludeSuppressed
? SuppressionPreference.Include
: SuppressionPreference.Omit;

ScriptAnalyzer.Instance.Initialize(
this,
combRulePaths,
this.includeRule,
this.excludeRule,
this.severity,
combRulePaths == null || combIncludeDefaultRules,
this.suppressedOnly);
suppressionPreference);
}

/// <summary>
Expand Down Expand Up @@ -402,29 +427,32 @@ protected override void StopProcessing()

private void ProcessInput()
{
IEnumerable<DiagnosticRecord> diagnosticsList = Enumerable.Empty<DiagnosticRecord>();
if (IsFileParameterSet())
WriteToOutput(RunAnalysis());
}

private IEnumerable<DiagnosticRecord> RunAnalysis()
{
if (!IsFileParameterSet())
{
foreach (var p in processedPaths)
{
if (fix)
{
ShouldProcess(p, $"Analyzing and fixing path with Recurse={this.recurse}");
diagnosticsList = ScriptAnalyzer.Instance.AnalyzeAndFixPath(p, this.ShouldProcess, this.recurse);
}
else
{
ShouldProcess(p, $"Analyzing path with Recurse={this.recurse}");
diagnosticsList = ScriptAnalyzer.Instance.AnalyzePath(p, this.ShouldProcess, this.recurse);
}
WriteToOutput(diagnosticsList);
}
return ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _);
}
else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase))

var diagnostics = new List<DiagnosticRecord>();
foreach (string path in this.processedPaths)
{
diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _);
WriteToOutput(diagnosticsList);
if (fix)
{
ShouldProcess(path, $"Analyzing and fixing path with Recurse={this.recurse}");
diagnostics.AddRange(ScriptAnalyzer.Instance.AnalyzeAndFixPath(path, this.ShouldProcess, this.recurse));
}
else
{
ShouldProcess(path, $"Analyzing path with Recurse={this.recurse}");
diagnostics.AddRange(ScriptAnalyzer.Instance.AnalyzePath(path, this.ShouldProcess, this.recurse));
}
}

return diagnostics;
}

private void WriteToOutput(IEnumerable<DiagnosticRecord> diagnosticRecords)
Expand Down Expand Up @@ -497,10 +525,7 @@ private void ProcessPath()
}
}

private bool IsFileParameterSet()
{
return String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase);
}
private bool IsFileParameterSet() => Path is not null;

private bool OverrideSwitchParam(bool paramValue, string paramName)
{
Expand Down
1 change: 1 addition & 0 deletions Engine/Engine.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<AssemblyVersion>1.20.0</AssemblyVersion>
<PackageId>Engine</PackageId>
<RootNamespace>Microsoft.Windows.PowerShell.ScriptAnalyzer</RootNamespace> <!-- Namespace needs to match Assembly name for ressource binding -->
<LangVersion>9.0</LangVersion>
</PropertyGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'net452' ">
Expand Down
2 changes: 1 addition & 1 deletion Engine/Formatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static string Format<TCmdlet>(

var currentSettings = GetCurrentSettings(settings, rule);
ScriptAnalyzer.Instance.UpdateSettings(currentSettings);
ScriptAnalyzer.Instance.Initialize(cmdlet, null, null, null, null, true, false);
ScriptAnalyzer.Instance.Initialize(cmdlet, null, null, null, null, true, SuppressionPreference.Omit);

text = ScriptAnalyzer.Instance.Fix(text, range, skipParsing, out Range updatedRange, out bool fixesWereApplied, ref scriptAst, ref scriptTokens, skipVariableAnalysis: true);
skipParsing = !fixesWereApplied;
Expand Down
13 changes: 10 additions & 3 deletions Engine/Generic/DiagnosticRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,13 @@ public IEnumerable<CorrectionExtent> SuggestedCorrections
set { suggestedCorrections = value; }
}

public bool IsSuppressed { get; protected set; } = false;

/// <summary>
/// DiagnosticRecord: The constructor for DiagnosticRecord class.
/// </summary>
public DiagnosticRecord()
{

}

/// <summary>
Expand All @@ -109,7 +110,14 @@ public DiagnosticRecord()
/// <param name="severity">The severity of this diagnostic</param>
/// <param name="scriptPath">The full path of the script file being analyzed</param>
/// <param name="suggestedCorrections">The correction suggested by the rule to replace the extent text</param>
public DiagnosticRecord(string message, IScriptExtent extent, string ruleName, DiagnosticSeverity severity, string scriptPath, string ruleId = null, IEnumerable<CorrectionExtent> suggestedCorrections = null)
public DiagnosticRecord(
string message,
IScriptExtent extent,
string ruleName,
DiagnosticSeverity severity,
string scriptPath,
string ruleId = null,
IEnumerable<CorrectionExtent> suggestedCorrections = null)
{
Message = message;
RuleName = ruleName;
Expand All @@ -119,7 +127,6 @@ public DiagnosticRecord(string message, IScriptExtent extent, string ruleName, D
RuleSuppressionID = ruleId;
this.suggestedCorrections = suggestedCorrections;
}

}


Expand Down
1 change: 1 addition & 0 deletions Engine/Generic/SuppressedRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class SuppressedRecord : DiagnosticRecord
public SuppressedRecord(DiagnosticRecord record, IReadOnlyList<RuleSuppression> suppressions)
{
Suppression = new ReadOnlyCollection<RuleSuppression>(new List<RuleSuppression>(suppressions));
IsSuppressed = true;
if (record != null)
{
RuleName = record.RuleName;
Expand Down
37 changes: 27 additions & 10 deletions Engine/ScriptAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
{
internal enum SuppressionPreference
{
Omit = 0,
Include = 1,
SuppressedOnly = 2,
}

public sealed class ScriptAnalyzer
{
#region Private members
Expand All @@ -41,7 +48,7 @@ public sealed class ScriptAnalyzer
string[] severity;
List<Regex> includeRegexList;
List<Regex> excludeRegexList;
bool suppressedOnly;
private SuppressionPreference _suppressionPreference;
#if !PSV3
ModuleDependencyHandler moduleHandler;
#endif
Expand Down Expand Up @@ -118,7 +125,7 @@ internal void Initialize<TCmdlet>(
string[] excludeRuleNames = null,
string[] severity = null,
bool includeDefaultRules = false,
bool suppressedOnly = false)
SuppressionPreference suppressionPreference = SuppressionPreference.Omit)
where TCmdlet : PSCmdlet, IOutputWriter
{
if (cmdlet == null)
Expand All @@ -135,7 +142,7 @@ internal void Initialize<TCmdlet>(
excludeRuleNames,
severity,
includeDefaultRules,
suppressedOnly);
suppressionPreference);
}

/// <summary>
Expand All @@ -150,6 +157,7 @@ public void Initialize(
string[] severity = null,
bool includeDefaultRules = false,
bool suppressedOnly = false,
bool includeSuppression = false,
string profile = null)
{
if (runspace == null)
Expand All @@ -163,6 +171,11 @@ public void Initialize(
outputWriter);
Helper.Instance.Initialize();

SuppressionPreference suppressionPreference = suppressedOnly
? SuppressionPreference.SuppressedOnly
: includeSuppression
? SuppressionPreference.Include
: SuppressionPreference.Omit;

this.Initialize(
outputWriter,
Expand All @@ -173,7 +186,7 @@ public void Initialize(
excludeRuleNames,
severity,
includeDefaultRules,
suppressedOnly,
suppressionPreference,
profile);
}

Expand All @@ -187,7 +200,7 @@ public void CleanUp()
severity = null;
includeRegexList = null;
excludeRegexList = null;
suppressedOnly = false;
_suppressionPreference = SuppressionPreference.Omit;
}

/// <summary>
Expand Down Expand Up @@ -671,7 +684,7 @@ private void Initialize(
string[] excludeRuleNames,
string[] severity,
bool includeDefaultRules = false,
bool suppressedOnly = false,
SuppressionPreference suppressionPreference = SuppressionPreference.Omit,
string profile = null)
{
if (outputWriter == null)
Expand Down Expand Up @@ -730,7 +743,7 @@ private void Initialize(
}
}

this.suppressedOnly = suppressedOnly;
_suppressionPreference = suppressionPreference;
this.includeRegexList = new List<Regex>();
this.excludeRegexList = new List<Regex>();

Expand Down Expand Up @@ -2324,9 +2337,13 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
// Need to reverse the concurrentbag to ensure that results are sorted in the increasing order of line numbers
IEnumerable<DiagnosticRecord> diagnosticsList = diagnostics.Reverse();

return this.suppressedOnly ?
suppressed.OfType<DiagnosticRecord>() :
diagnosticsList;
return _suppressionPreference switch
{
SuppressionPreference.SuppressedOnly => suppressed.OfType<DiagnosticRecord>(),
SuppressionPreference.Omit => diagnosticsList,
SuppressionPreference.Include => diagnosticsList.Concat(suppressed.OfType<DiagnosticRecord>()),
_ => throw new ArgumentException($"SuppressionPreference has invalid value '{_suppressionPreference}'"),
};
}
}
}
Loading

0 comments on commit d1942a1

Please sign in to comment.