Add some necessary contents for diagnostic suppressions.

[t-lipingma]

PR Summary

Fixes #1691.

Output Suppressions to sarif result file.

• Add a list of suppressions for each diagnostic.

• Add -IncludeSuppressions parameter for output all violations including suppressed one.

• Add Kind attribute in RuleSuppression.

• Add test cases for those changes.

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
• Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

[ghost]

All CLA requirements met.

[t-lipingma]

tests issue needs to solve before merge: #1695

[rjmholt]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rjmholt]

This isn't mutually exclusive with SuppressedOnly and probably should be. What should a user expect when both are provided?

Given that -SuppressedOnly already exists and we don't want to remove or otherwise break it, we probably can't move to an enum parameter like -SuppressionPreference <Exclude | Include | SuppressedOnly>.

An alternative might be to put each switch into its own parameter set, but that starts to get complicated. We should discuss this further.

[rjmholt]

Suggested change

	[Parameter(Mandatory = false)]
	[Parameter]

Mandatory defaults to false

[JamesWTruher]

correct, if it's not mandatory, that should just be omitted. if SuppressedOnly should not be used with this, then parameter sets should be used (and it does get complicated). Although it doesn't look to complicated at the moment it would mean an additional 2 parameter sets.

[t-lipingma]

yeah. I totally agree with you guys on that point.
If you decided to change the parameter set, let's discuss more details on the design, I'd like to help.

[rjmholt]

From our offline conversation:

If the parameter set change is too complicated, can we check in the current solution firstly?

Yeah I think it's a good idea for us to separate out the suppression combining parts from the new parameter/feature. We can then discuss offline or over a call what the best way forward is.

[t-lipingma]

To maintain PSSA's existing design, I think we can make '-SuppressedOnly' / '-IncludeSuppressions' exclusive in the short term. What do you think about it? Look forward to your reply.

[t-lipingma]

Btw, ExcludeRule and IncludeRule should also be mutually exclusive.

[rjmholt]

make '-SuppressedOnly' / '-IncludeSuppressions' exclusive

Yeah, they should be in different parameter sets.

Btw, ExcludeRule and IncludeRule should also be mutually exclusive.

No, those can be used together to include a non-default rule while excluding a default one.

[t-lipingma]

To make -SuppressedOnly / -IncludeSuppressions exclusive.
I used the logic below:

 Function TestParamSet {
    [CmdletBinding(DefaultParameterSetName="File1")]
    Param
    (
	[Parameter(Position = 0,ParameterSetName = 'File1',Mandatory = true)]
        [Parameter(Position = 0,ParameterSetName = 'File2',Mandatory = true]
        [switch] $Path,
        
	[Parameter(Position = 0,ParameterSetName = 'ScriptDefinition1',Mandatory = true)]
        [Parameter(Position = 0,ParameterSetName = 'ScriptDefinition2',Mandatory = true)]
	[switch] $ScriptDefinition,
        
        
        [Parameter(ParameterSetName = 'File1', Mandatory = false)]
        [Parameter(ParameterSetName = 'ScriptDefinition1', Mandatory = false)]
        [switch] $SuppressedOnly,

        [Parameter(ParameterSetName = 'File2', Mandatory = true)]
        [Parameter(ParameterSetName = 'ScriptDefinition2', Mandatory = true)]
        [switch] $IncludeSuppressions

        [Parameter(Position = 0,ParameterSetName = 'File1',Mandatory = false)]
        [Parameter(Position = 0,ParameterSetName = 'File2',Mandatory = false]
        [switch] $Fix,
    )

    Process {
        #Do Nothing
    }
}

so it's to say:

1、Invoke-ScriptAnalyzer C:\build.ps1 --> ParameterSetName==DefaultParameterSetName.

2、Invoke-ScriptAnalyzer -Path C:\build.ps1 --> ParameterSetName==DefaultParameterSetName.

3、Invoke-ScriptAnalyzer -Path C:\build.ps1 -SuppressedOnly --> ParameterSetName=="File1".

4、Invoke-ScriptAnalyzer -Path C:\build.ps1 -IncludeSuppressions --> ParameterSetName=="File2".

5、Invoke-ScriptAnalyzer -ScriptDefinition $tmp --> ParameterSetName=="ScriptDefinition1".

6、Invoke-ScriptAnalyzer -ScriptDefinition $tmp -SuppressedOnly --> ParameterSetName=="ScriptDefinition1"

7、Invoke-ScriptAnalyzer -ScriptDefinition $tmp -IncludeSuppressions -->ParameterSetName=="ScriptDefinition2"

LMK if you have any suggestions, Thanks!

[JamesWTruher]

correct, if it's not mandatory, that should just be omitted. if SuppressedOnly should not be used with this, then parameter sets should be used (and it does get complicated). Although it doesn't look to complicated at the moment it would mean an additional 2 parameter sets.

[JamesWTruher]

@bergmeister could you also please take a look at this?

[rjmholt]

@t-lipingma I've implemented the suppression combination part of this work in #1699.

[bergmeister]

Interesting idea. I wonder if it would also be useful to the user if the DiagnosticRecord gets an additional property whether it would've been suppressed so that the user does not need to run pssa twice and do logic on two sets of records.