PSScriptAnalyzer
PSScriptAnalyzer copied to clipboard
Unable to suppress PSAvoidLongLines warning in a method
I'm unable to suppress the PSAvoidLongLines warning in a method inside a class.
Steps to reproduce
Run Invoke-ScriptAnalyzer -Path . -Recurse
while the following PSScriptAnalyzerSettings.psd1
file is present in the folder:
@{
Severity = @('Error', 'Warning', 'Information')
ExcludeRules = @('PSUseBOMForUnicodeEncodedFile',
'PSAvoidUsingConvertToSecureStringWithPlainText',
'PSAvoidUsingPlainTextForPassword')
Rules = @{
PSAlignAssignmentStatement = @{
Enable = $true
CheckHashtable = $true
}
PSPlaceCloseBrace = @{
Enable = $true
NoEmptyLineBefore = $false
IgnoreOneLineBlock = $true
NewLineAfter = $true
}
PSPlaceOpenBrace = @{
Enable = $true
OnSameLine = $true
NewLineAfter = $true
IgnoreOneLineBlock = $true
}
PSAvoidLongLines = @{
Enable = $true
MaximumLineLength = 80
}
}
}
Class foo {
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidLongLines", "")]
[String]output () {
$output = "a book is a book is a book. but this report on the activities of a company is"
return $output
}
}
Expected behavior
0 warnings
Actual behavior
RuleName Severity ScriptName Line Message
-------- -------- ---------- ---- -------
PSAvoidLongLines Warning test.ps1 4 Line exceeds the configured maximum length of 80 characters
1 rule violations found. Severity distribution: Error = 0, Warning = 1, Information = 0
If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *
Environment data
> $PSVersionTable
Name Value
---- -----
PSVersion 7.0.3
PSEdition Core
GitCommitId 7.0.3
OS Darwin 17.7.0 Darwin Kernel Version 17.7.0: Wed May 27 17:00:02 PDT 2020; root:xnu-4570.71.80.1~1/RELEASE_X86_64
Platform Unix
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0
> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.19.0
See also #1586 for initial report since this issue was closed i opened a new one and provided clear repo steps.
Debugging this, the suppression is properly created but not applied by the suppression logic, which is here:
https://github.com/PowerShell/PSScriptAnalyzer/blob/09b69296753e6127d517b6c0f6a9035a1d53d11b/Engine/Helper.cs#L1357-L1391
It looks like the suppression is not applied because the offset is incorrect, which is because the AvoidLongLines
rule creates its own ScriptPosition
s, which set offsets to 0
:
https://github.com/PowerShell/PSScriptAnalyzer/blob/09b69296753e6127d517b6c0f6a9035a1d53d11b/Rules/AvoidLongLines.cs#L66-L78
To fix this, the AvoidLongLines
rule needs to create an InternalScriptPosition
using reflection, with a correctly calculated offset.
cc @thomasrayner
cc @thomasrayner
I definitely want to take this on but I might need a slightly larger push in the right direction. Any link to an example or doc should (hopefully) be enough.
@thomasrayner We'll have to go to a level that is so low, the only docs is the PowerShell source code itself😅
What @rjmholt is saying is that the ScriptPosition
class that this rules use to create an IScriptPosition
has the its readonly offset property always return zero, which we can see in the code here:
https://github.com/PowerShell/PowerShell/blob/9b39f05557ef1c4fe51b14376a7c3abbcfaebff8/src/System.Management.Automation/engine/parser/Position.cs#L669-L672
However, there is also exist the InternalScriptPosition
class that implements IScriptPosition
, which is defined here:
https://github.com/PowerShell/PowerShell/blob/9b39f05557ef1c4fe51b14376a7c3abbcfaebff8/src/System.Management.Automation/engine/parser/Position.cs#L430
As we can see, that class is not publicly exposed, therefore using reflection (something like this) one is still able to create this class, which allows passing in the offset via its constructor here: https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/parser/Position.cs#L434
An alternative to reflection (which is expensive from a performance point of view) would of course be create our own implementation of InternalScriptPosition
but given this special case, using reflection is OK and means at least less work/code to write write :-)
@bergmeister thank you for a great breakdown! If you folks don't mind this fix taking a little while (fitting it into my spare time), please assign this to me. I'd love to take it on.
That would be great but there is also no pressure, PSSA release cycles are quite long anyway and I don't think this issue is that much of a problem. My explanation only covers the Position part, you probably have to do some experiment to find what the right offset would be. The complex suppression logic that Rob linked above was created by someone else many years above and I luckily never had to touch it or look at it but looking at Rob's comment, you might be able to just ignore it and assume it does its job correctly so that it would just be the case of finding the right offset programmatically.
Just FYI, you can also implement an IScriptPosition
/IScriptExtent
yourself, but then you must calculate the offset, which is error prone. I've written code that does it a few times though. Ideally this should be something that PowerShell fixes.
Just FYI, you can also implement an
IScriptPosition
/IScriptExtent
yourself, but then you must calculate the offset, which is error prone. I've written code that does it a few times though. Ideally this should be something that PowerShell fixes.
@rjmholt do you happen to have a reference for this that I can check out?
We should really add that code to PowerShell itself with tests. Or perhaps to a NuGet package that we can pull in in different places.
Considering that it's gonna be tough for me to prioritize working this issue into my work, do you think it'd be prudent for me to just wait for PS to add it?
do you think it'd be prudent for me to just wait for PS to add it?
Well it's not going to be added to PS 3, 4, 5.1, 7.0 or 7.1 in any case, which PSSA also supports. But there's not exactly a plan to add it to 7.2 either — I'm just thinking out loud.
I'll try and work out some time to poke at it, then. 😁