PSUseOutputTypeCorrectly/[OutputType()] fires incorrectly, reporting that a fn should return string when the function actually return array of strings

[plastikfan]

Before submitting a bug report:

• Make sure you are able to repro it on the latest released version
• Perform a quick search for existing issues to check if this bug has already been reported

Steps to reproduce

function Split-KeyValuePairFormatter {
  [OutputType('System.String[]')]
  [CmdletBinding()]
  param (
    [Parameter(Mandatory = $true)]
    [string]
    $Format,

    [string]
    $KeyConstituent,

    [string]
    $ValueConstituent,

    [string]
    $KeyPlaceHolder = "<%KEY%>",

    [string]
    $ValuePlaceHolder = "<%VALUE%>"
  )
  [string[]]$constituents = @();

   # IMPLEMENTATION CODE OMITTED, but you can see that the return statement
   # just returns the array declared as string[]

   return $constituents;
}

Expected behavior

No warning expected

Actual behavior

Error:

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseOutputTypeCorrectly            Information  split-key- 152   The cmdlet 'Split-KeyValuePairFormatter' returns an  
                                                 value-pair       object of type 'System.String' but this type is not  
                                                 -formatter       declared in the OutputType attribute.
                                                 .ps1

If I change OutputType to: [OutputType('System.String')], which is clearly wrong as it doesn't reflect the behaviour of the actual code, then there is no error. This does not seem right. As a work around, I'm having to declare an incorrect return type, ie System.string in [OutputType()], and put in a comment to indicate as such. Luckily, there is no effect on the functinality, but I can't omit the OutputType without PSScriptAnalyzer reporting a warning.

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.0.0
PSEdition                      Core
GitCommitId                    7.0.0
OS                             Microsoft Windows 10.0.18362
Platform                       Win32NT
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
1.18.3

[bergmeister]

@plastikfan Thanks for taking the time to open an issue and provide details.
I can repro except your statement that it doesn't happen when changing the OutputType to [OutputType('System.String')], it happens in that case as well for me.

Variable analysis seems to infer the type from the right hand side of the assignment @(), that's why PSSA thinks it is of type System.Object[]. Variable analysis in PSSA is something that got forked off the PowerShell project a few years ago and we are looking into updating this code for PSSA 2.0, which has the potential of being more clever and could fix this issue as-is.

For your example, you can help PSSA though to recognize the OutputType correctly by adding a cast to the return statement and then it won't complain any more

   return [string[]] $constituents;

P.S. If you want, you can also shorten the type declaration a bit as PowerShell always assumes the System namespace: [OutputType([string[]])]

[plastikfan]

I'm just learning how to use PSScriptAnalyzer, and I've just discovered I can suppress the message, so now I'm using this:

[OutputType('System.String[]')]
[CmdletBinding()]
[Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseOutputTypeCorrectly", "")]

So, I've left the OutputType to the correct type, but as you can see I've suppressed the Rule for this function.

[bergmeister]

@plastikfan I don't think you understood, by changing your function as follows, you can help PSScriptAnalyzer recongize the output type correctly, no need to suppress in this case:

function Split-KeyValuePairFormatter {
  [OutputType('System.String[]')]
  [CmdletBinding()]
  param (
    [Parameter(Mandatory = $true)]
    [string]
    $Format,

    [string]
    $KeyConstituent,

    [string]
    $ValueConstituent,

    [string]
    $KeyPlaceHolder = "<%KEY%>",

    [string]
    $ValuePlaceHolder = "<%VALUE%>"
  )
  [string[]]$constituents = @();

   # IMPLEMENTATION CODE OMITTED, but you can see that the return statement
   # just returns the array declared as string[]

  return [string[]] $constituents;
}

[plastikfan]

My previous message crossed-over, so I didnt see your response whilst I was writing that one!
I have made that change and that works, apart from defining the type in OutputType('string[]') did not stop the warning from being issued. I have to use [OutputType('System.String[]')], otherwise the warning pops out.

thanks for your assistance.

[bergmeister]

@plastikfan No, worries, you have to use brackets instead of single quotes around string[], i.e. [OutputType([string[]])], then it will work.

[plastikfan]

Thank you @bergmeister :)

[rjmholt]

Looks like the type inference visitor in PowerShell understands this:

https://github.com/PowerShell/PowerShell/blob/cefbf3d6a972d9f1365c4ce5c5d6285133c2f304/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs#L951-L954

[bergmeister]

Yep @rjmholt, except that its internal. That's exactly the kind of legacy that I was talking about where this 'forked' (aka copy pasted 5 years ago) version of VariableAnalysis has caused and if we could bring the latest version of variable analysis into PSSA for v2, I'd hope subtle issues like this one would magically go away

[rjmholt]

My hope is that we can spin the visitors into their own NuGet package and consume it separately. The issue there is that it might not be possible with framework runtime targeting in some circumstances.

But there are other places where these visitors would be useful and I don't want to reinvent them. I'd much prefer to have a standalone library of AST tools that we can reuse.

[bergmeister]

Yes that would make sense. Even just having it for PS7 only would be good. The more goodies PS7 has, the more it will naturally convince people to move over to PS7

[deadlydog]

I just ran into this today, so it seems it is still a problem. For me, I want to ensure PowerShell does not unroll the array when returning it, so I'm using code like:

return ,$constituents # Note the comma in front to prevent unrolling.

This ensures that an array is always returned, even if it does not contain any elements. Otherwise trying to access the .Count property on the returned object fails, since it is not an array and does not have the property.

I can't figure out how to cast the return statement to return the unrolled array without breaking functionality. I've tried these:

return [string[]] $constituents # breaks tests
return [string[]] ,$constituents # breaks tests
return [string[]](,$constituents) # breaks tests

What does work and satisfies both my tests and PSScriptAnalyzer is using:

Write-Output $constituents -NoEnumerate

I know that this is the more natural syntax for PowerShell, but I prefer to use the return syntax, especially to keep this line of code consistent with the rest of the codebase.

Rather than adjusting the code style to satisfy the meta-info OutputType attribute for PSScriptAnalyzer, I guess for now I'll just suppress the message by using:

[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseOutputTypeCorrectly', '')]

It would be nice to get this issue addressed though.