PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.87k stars 378 forks source link

PSUseOutputTypeCorrectly is triggered when it shouldn't. #676

Open bielawb opened 7 years ago

bielawb commented 7 years ago

It looks to me like PSUseOutputTypeCorrectly check is not taking very basic, crucial, fundamental fact about PowerShell into account: by default it will not keep any simple collection, unless you explicitly tell it to.

Result? I have PInvoke code (taken from here) and I correctly defined OutputType as [Connection] (I've actually created namespace for it, but that's irrelevant). As expected, Get-Member confirms that PowerShell function will return stream of connections, not collection as a whole. Yet, PSScriptAnalyzer will complain about it:

Line: 45 - The cmdlet 'Get-OPNetStat' returns an object of type 'Optiver.Connection[]' but this type is not declared in the OutputType attribute. 
Line: 54 - The cmdlet 'Get-OPNetStat' returns an object of type 'Optiver.Connection[]' but this type is not declared in the OutputType attribute. 

In fact, if I would follow instructions and declare my OutputType as a collection it would be conflicting with what my function actually returns:

Set-StrictMode -Version Latest
Get-OPNetStat | % IsFixedSize
% : The input name "IsFixedSize" cannot be resolved to a member.

IsFixedSize was tab-completed, based on OutputType that PSScriptAnalyzer told me to define. As expected, my Connection object doesn't have it. The only workaround I've found that satisfies both PSScriptAnalyzer and my hope to get nice tab-completion for command I wrote is to pass result of the command thru ForEach-Object:

[Optiver.NetworkUtil]::GetTCP() } | ForEach-Object { $_ }
kapilmb commented 7 years ago

I think I understand what you are saying but would like to have more information. Can you please provide me with an example so that I can reproduce this issue? Thanks!

bielawb commented 7 years ago

Sure thing. This is the code that should help repro the issue:

# Lets define some dummy type...

Add-Type @'
using System;

namespace foo
{
    public class bar
    {
        public bar ()
        {
            this.Name = "Unknown";
        }
        public bar (string name)
        {
            this.Name = name;
        }
        public string Name;

    }
    public class barFactory
    {
        public static bar[] Create ()
        {
            return new bar[] {
                new bar ("one"),
                new bar ("two"),
                new bar ("three")
            };
        }
    }
}
'@

# Function definition...

$def = @'
function Get-Foo {
    [OutputType([foo.bar])]
    [CmdletBinding()]
    param ()
    [foo.barFactory]::Create()    
}

function Get-FooCollection {
    [OutputType([foo.bar[]])]
    [CmdletBinding()]
    param ()
    [foo.barFactory]::Create()    
}
'@

Invoke-ScriptAnalyzer -ScriptDefinition $def -IncludeRule PSUseOutputTypeCorrectly

# Verify that definition, not Script Analyzer is correct...
. ([scriptblock]::Create($def))

# Setting script mode to get errors when accessing non-existing properties...
Set-StrictMode -Version Latest

# Both properties based on OutputType suggested by command...
Get-Foo | ForEach-Object Name
# one, two, three, as expected...
Get-FooCollection | ForEach-Object Count
# ForEach-Object : The input name "Count" cannot be resolved to a member. x3
Clijsters commented 7 years ago

Same here. Look at that short snippet (I removed some boilerplate, this one isn't actually working as is)

As you can see, the ObjectType is not defined by the $tmpList object, because it's piped to a Converter. Also is it only one case, where the ArrayList is used.

function Get-TogglProject {
    [OutputType("PSToggl.Project")]
    param([...])

    $projects = Get-SomeItems

    switch ($PsCmdlet.ParameterSetName) {
        "byObject" {
    #This is where an ArrayList is created
            [System.Collections.ArrayList]$tmpList
            foreach ($item in $InputObject) {
                $tmpList.add($pL.Invoke($item))
            }
            $projects = $tmpList
        }
        "byName" {
            $projects = $projects | Where-Object {$_.Name -Like $Name}
        }
    }

    return $projects | ConvertTo-TogglProject
}

EDIT: Did a quickfix replacing [System.Collections.ArrayList]$tmpList with $tmpList = New-Object -TypeName System.Collections.ArrayList