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:

PSScriptAnalyzer/Engine/Helper.cs

Lines 1357 to 1391 in 09b6929

    
           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; 
        
           } 
        
           // at this point, start offset of startRecord is greater or equals to rulesuppression.startoffset 
        
           recordIndex = startRecord; 
        
           while (recordIndex < diagnostics.Count) 
        
           { 
        
               DiagnosticRecord record = diagnostics[recordIndex]; 
        
               var curOffset = offsetArr[recordIndex]; 
        
               //if (record.Extent.EndOffset > ruleSuppression.EndOffset) 
        
               if (curOffset != null && curOffset.Item2 > ruleSuppression.EndOffset) 
        
               { 
        
                   break; 
        
               } 
        
               // 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))) 
        
               { 
        
                   suppressed[recordIndex] = true; 
        
                   suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression)); 
        
                   suppressionCount += 1; 
        
               } 
        
               recordIndex += 1; 
        
           }

It looks like the suppression is not applied because the offset is incorrect, which is because the AvoidLongLines rule creates its own ScriptPositions, which set offsets to 0:

PSScriptAnalyzer/Rules/AvoidLongLines.cs

Lines 66 to 78 in 09b6929

    
           var violationExtent = new ScriptExtent( 
        
               new ScriptPosition( 
        
                   ast.Extent.File, 
        
                   startLine, 
        
                   startColumn, 
        
                   line 
        
               ), 
        
               new ScriptPosition( 
        
                   ast.Extent.File, 
        
                   endLine, 
        
                   endColumn, 
        
                   line 
        
               ));

To fix this, the AvoidLongLines rule needs to create an InternalScriptPosition using reflection, with a correctly calculated offset.

Thomas Rayner (@thomasrayner)

cc Thomas Rayner (@thomasrayner)

Thomas Rayner (@thomasrayner)

cc Thomas Rayner (@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.

Thomas Rayner (@thomasrayner)

Thomas Rayner (@thomasrayner) We'll have to go to a level that is so low, the only docs is the PowerShell source code itself😅
What Rob Holt (@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 :-)

Christoph Bergmeister (@bergmeister)

Christoph Bergmeister (@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.

Rob Holt (@rjmholt)

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.

Rob Holt (@rjmholt) do you happen to have a reference for this that I can check out?

Here you go.

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. 😁

Navigation Menu

Repository navigation

Unable to suppress PSAvoidLongLines warning in a method #1591

Steps to reproduce

Expected behavior

Actual behavior

Environment data

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	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;
	}

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

	while (recordIndex < diagnostics.Count)
	{
	DiagnosticRecord record = diagnostics[recordIndex];
	var curOffset = offsetArr[recordIndex];

	//if (record.Extent.EndOffset > ruleSuppression.EndOffset)
	if (curOffset != null && curOffset.Item2 > ruleSuppression.EndOffset)
	{
	break;
	}

	// 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)))
	{
	suppressed[recordIndex] = true;
	suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression));
	suppressionCount += 1;
	}

	recordIndex += 1;
	}

	var violationExtent = new ScriptExtent(
	new ScriptPosition(
	ast.Extent.File,
	startLine,
	startColumn,
	line
	),
	new ScriptPosition(
	ast.Extent.File,
	endLine,
	endColumn,
	line
	));

Unable to suppress PSAvoidLongLines warning in a method #1591

Description

Steps to reproduce

Expected behavior

Actual behavior

Environment data

Activity

rjmholt commented on Oct 5, 2020

bergmeister commented on Oct 25, 2020

thomasrayner commented on Oct 25, 2020

bergmeister commented on Oct 25, 2020

thomasrayner commented on Oct 25, 2020

bergmeister commented on Oct 25, 2020

rjmholt commented on Mar 11, 2021

thomasrayner commented on Apr 27, 2021

rjmholt commented on Apr 27, 2021

thomasrayner commented on Apr 27, 2021

rjmholt commented on Apr 27, 2021

thomasrayner commented on Apr 28, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions