Skip to content

Add some necessary contents for diagnostic suppressions. #1694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 36 additions & 15 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands
/// </summary>
[Cmdlet(VerbsLifecycle.Invoke,
"ScriptAnalyzer",
DefaultParameterSetName = "File",
DefaultParameterSetName = "FilePartial",
SupportsShouldProcess = true,
HelpUri = "https://go.microsoft.com/fwlink/?LinkId=525914")]
[OutputType(typeof(DiagnosticRecord))]
Expand All @@ -37,7 +37,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 = "FilePartial",
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
[Parameter(Position = 0,
ParameterSetName = "FileAll",
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
Expand All @@ -54,7 +59,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 = "ScriptDefinitionPartial",
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
[Parameter(Position = 0,
ParameterSetName = "ScriptDefinitionAll",
Mandatory = true,
ValueFromPipeline = true,
ValueFromPipelineByPropertyName = true)]
Expand Down Expand Up @@ -158,19 +168,24 @@ public SwitchParameter Recurse
/// <summary>
/// ShowSuppressed: Show the suppressed message
/// </summary>
[Parameter(Mandatory = false)]
[Parameter(Mandatory = false, ParameterSetName = "FilePartial")]
[Parameter(Mandatory = false, ParameterSetName = "ScriptDefinitionPartial")]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public SwitchParameter SuppressedOnly
{
get { return suppressedOnly; }
set { suppressedOnly = value; }
}
private bool suppressedOnly;
public SwitchParameter SuppressedOnly { get; set; }

/// <summary>
/// ShowAll: Show the suppressed and non-suppressed message
/// </summary>
[Parameter(Mandatory = true, ParameterSetName = "FileAll")]
[Parameter(Mandatory = true, ParameterSetName = "ScriptDefinitionAll")]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public SwitchParameter IncludeSuppressions { get; set; }

/// <summary>
/// Resolves rule violations automatically where possible.
/// </summary>
[Parameter(Mandatory = false, ParameterSetName = "File")]
[Parameter(Mandatory = false, ParameterSetName = "FilePartial")]
[Parameter(Mandatory = false, ParameterSetName = "FileAll")]
public SwitchParameter Fix
{
get { return fix; }
Expand Down Expand Up @@ -341,7 +356,8 @@ protected override void BeginProcessing()
this.excludeRule,
this.severity,
combRulePaths == null || combIncludeDefaultRules,
this.suppressedOnly);
this.SuppressedOnly,
this.IncludeSuppressions);
}

/// <summary>
Expand Down Expand Up @@ -420,7 +436,7 @@ private void ProcessInput()
WriteToOutput(diagnosticsList);
}
}
else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase))
else if (IsScriptParameterSet())
{
diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _);
WriteToOutput(diagnosticsList);
Expand Down Expand Up @@ -499,7 +515,12 @@ private void ProcessPath()

private bool IsFileParameterSet()
{
return String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase);
return String.Equals(this.ParameterSetName, "FilePartial", StringComparison.OrdinalIgnoreCase) || String.Equals(this.ParameterSetName, "FileAll", StringComparison.OrdinalIgnoreCase);
}

private bool IsScriptParameterSet()
{
return String.Equals(this.ParameterSetName, "ScriptDefinitionPartial", StringComparison.OrdinalIgnoreCase) || String.Equals(this.ParameterSetName, "ScriptDefinitionAll", StringComparison.OrdinalIgnoreCase);
}

private bool OverrideSwitchParam(bool paramValue, string paramName)
Expand All @@ -511,4 +532,4 @@ private bool OverrideSwitchParam(bool paramValue, string paramName)

#endregion // Private Methods
}
}
}
19 changes: 19 additions & 0 deletions Engine/Generic/RuleSuppression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ public string Justification
set;
}

/// <summary>
/// Returns the kind of the suppression
/// </summary>
public SuppressionKind Kind
{
get;
set;
}

private static HashSet<string> scopeSet;

/// <summary>
Expand Down Expand Up @@ -376,5 +385,15 @@ public static List<RuleSuppression> GetSuppressions(IEnumerable<AttributeAst> at

return result;
}

/// <summary>
/// A string that indicates where the suppression is persisted.
/// </summary>
public enum SuppressionKind
{
None,
InSource,
External
}
}
}
21 changes: 16 additions & 5 deletions Engine/Generic/SuppressedRecord.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
{
/// <summary>
Expand All @@ -11,20 +13,29 @@ public class SuppressedRecord : DiagnosticRecord
/// <summary>
/// The rule suppression of this record
/// </summary>
public RuleSuppression Suppression
public IList<RuleSuppression> Suppressions
{
get;
set;
get
{
if (suppressions == null) suppressions = new List<RuleSuppression>();

return suppressions;
}
set
{
suppressions = value;
}
}
private IList<RuleSuppression> suppressions;

/// <summary>
/// Creates a suppressed record based on a diagnostic record and the rule suppression
/// </summary>
/// <param name="record"></param>
/// <param name="Suppression"></param>
public SuppressedRecord(DiagnosticRecord record, RuleSuppression suppression)
public SuppressedRecord(DiagnosticRecord record, IList<RuleSuppression> suppressions)
{
Suppression = suppression;
Suppressions = suppressions;
if (record != null)
{
RuleName = record.RuleName;
Expand Down
151 changes: 67 additions & 84 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,50 +1348,47 @@ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
}

List<RuleSuppression> ruleSuppressions = ruleSuppressionsDict[ruleName];
var offsetArr = GetOffsetArray(diagnostics);
int recordIndex = 0;
int startRecord = 0;
bool[] suppressed = new bool[diagnostics.Count];
foreach (RuleSuppression ruleSuppression in ruleSuppressions)
{
int suppressionCount = 0;
while (startRecord < diagnostics.Count
// && diagnostics[startRecord].Extent.StartOffset < ruleSuppression.StartOffset)
// && diagnostics[startRecord].Extent.StartLineNumber < ruleSuppression.st)
&& offsetArr[startRecord] != null && offsetArr[startRecord].Item1 < ruleSuppression.StartOffset)
{
startRecord += 1;
}
bool[] applied = new bool[ruleSuppressions.Count];

// at this point, start offset of startRecord is greater or equals to rulesuppression.startoffset
recordIndex = startRecord;

while (recordIndex < diagnostics.Count)
foreach(DiagnosticRecord diagnostic in diagnostics)
{
var curOffset = GetOffsetArray(diagnostic);
List<RuleSuppression> suppressions = new List<RuleSuppression>();
for (int ruleSuppressionIndex = 0; ruleSuppressionIndex < ruleSuppressions.Count; ruleSuppressionIndex++)
{
DiagnosticRecord record = diagnostics[recordIndex];
var curOffset = offsetArr[recordIndex];

//if (record.Extent.EndOffset > ruleSuppression.EndOffset)
if (curOffset != null && curOffset.Item2 > ruleSuppression.EndOffset)
RuleSuppression ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
//if (diagnostic.Extent.StartOffset < ruleSuppression.StartOffset||diagnostic.Extent.EndOffset > ruleSuppression.EndOffset)
if (curOffset != null && (curOffset.Item1 < ruleSuppression.StartOffset || curOffset.Item2 > ruleSuppression.EndOffset))
{
break;
continue;
}

// we suppress if there is no suppression id or if there is suppression id and it matches
if (string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID)
|| (!String.IsNullOrWhiteSpace(record.RuleSuppressionID) &&
string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase)))
if (string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) ||
(!String.IsNullOrWhiteSpace(diagnostic.RuleSuppressionID) &&
string.Equals(ruleSuppression.RuleSuppressionID, diagnostic.RuleSuppressionID, StringComparison.OrdinalIgnoreCase)))
{
suppressed[recordIndex] = true;
suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression));
suppressionCount += 1;
ruleSuppression.Kind = RuleSuppression.SuppressionKind.InSource;
suppressions.Add(ruleSuppression);
applied[ruleSuppressionIndex] = true;
}
}

recordIndex += 1;
if (suppressions.Count() != 0)
{
suppressedRecords.Add(new SuppressedRecord(diagnostic, suppressions));
}
else
{
unSuppressedRecords.Add(diagnostic);
}
}

// If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly
if (!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && suppressionCount == 0)
// If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly
for (int ruleSuppressionIndex = 0; ruleSuppressionIndex < ruleSuppressions.Count; ruleSuppressionIndex++)
{
RuleSuppression ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
if ((!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID)) && !applied[ruleSuppressionIndex])
{
// checks whether are given a string or a file path
if (String.IsNullOrWhiteSpace(diagnostics.First().Extent.File))
Expand All @@ -1409,70 +1406,56 @@ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
}
}

for (int i = 0; i < suppressed.Length; i += 1)
{
if (!suppressed[i])
{
unSuppressedRecords.Add(diagnostics[i]);
}
}

return result;
}

private Tuple<int,int>[] GetOffsetArray(List<DiagnosticRecord> diagnostics)
private Tuple<int,int> GetOffsetArray(DiagnosticRecord diagnostic)
{
Func<int,int,Tuple<int,int>> GetTuple = (x, y) => new Tuple<int, int>(x, y);
Func<Tuple<int, int>> GetDefaultTuple = () => GetTuple(0, 0);
var offsets = new Tuple<int, int>[diagnostics.Count];
for (int k = 0; k < diagnostics.Count; k++)
var offset = new Tuple<int, int>(0, 0);
var ext = diagnostic.Extent;
if (ext == null)
{
var ext = diagnostics[k].Extent;
if (ext == null)
return null;
}
if (ext.StartOffset == 0 && ext.EndOffset == 0)
{
// check if line and column number correspond to 0 offsets
if (ext.StartLineNumber == 1
&& ext.StartColumnNumber == 1
&& ext.EndLineNumber == 1
&& ext.EndColumnNumber == 1)
{
continue;
return offset;
}
if (ext.StartOffset == 0 && ext.EndOffset == 0)
// created using the ScriptExtent constructor, which sets
// StartOffset and EndOffset to 0
// find the token the corresponding start line and column number
var startToken = Tokens.Where(x
=> x.Extent.StartLineNumber == ext.StartLineNumber
&& x.Extent.StartColumnNumber == ext.StartColumnNumber)
.FirstOrDefault();
if (startToken == null)
{
// check if line and column number correspond to 0 offsets
if (ext.StartLineNumber == 1
&& ext.StartColumnNumber == 1
&& ext.EndLineNumber == 1
&& ext.EndColumnNumber == 1)
{
offsets[k] = GetDefaultTuple();
continue;
}
// created using the ScriptExtent constructor, which sets
// StartOffset and EndOffset to 0
// find the token the corresponding start line and column number
var startToken = Tokens.Where(x
=> x.Extent.StartLineNumber == ext.StartLineNumber
&& x.Extent.StartColumnNumber == ext.StartColumnNumber)
.FirstOrDefault();
if (startToken == null)
{
offsets[k] = GetDefaultTuple();
continue;
}
var endToken = Tokens.Where(x
=> x.Extent.EndLineNumber == ext.EndLineNumber
&& x.Extent.EndColumnNumber == ext.EndColumnNumber)
.FirstOrDefault();
if (endToken == null)
{
offsets[k] = GetDefaultTuple();
continue;
}
offsets[k] = GetTuple(startToken.Extent.StartOffset, endToken.Extent.EndOffset);
return offset;
}
else
var endToken = Tokens.Where(x
=> x.Extent.EndLineNumber == ext.EndLineNumber
&& x.Extent.EndColumnNumber == ext.EndColumnNumber)
.FirstOrDefault();
if (endToken == null)
{
// Extent has valid offsets
offsets[k] = GetTuple(ext.StartOffset, ext.EndOffset);
return offset;
}
offset = GetTuple(startToken.Extent.StartOffset, endToken.Extent.EndOffset);
}
else
{
// Extent has valid offsets
offset = GetTuple(ext.StartOffset, ext.EndOffset);
}
return offsets;

return offset;
}

public static string[] ProcessCustomRulePaths(string[] rulePaths, SessionState sessionState, bool recurse = false)
Expand Down
Loading