PowerShell / PSScriptAnalyzer

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

Invoke-ScriptAnalyzer required Path parameter #1887

Open ThomasNieto opened 1 year ago

ThomasNieto commented 1 year ago

The required -Path parameter is a usability concern. It should default to . removing the need for the required parameter.

Is there a reason or design choice of why this parameter is required without a default value?

Steps to reproduces

Invoke-ScriptAnalyzer

Expected behavior

# just runs
Invoke-ScriptAnalyzer

Actual behavior

Invoke-ScriptAnalyzer

cmdlet Invoke-ScriptAnalyzer at command pipeline position 1
Supply values for the following parameters:
Path:

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.3.2
PSEdition                      Core
GitCommitId                    7.3.2
OS                             Microsoft Windows 10.0.19044
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.21.0
1.20.0
1.19.1
1.18.3
bergmeister commented 1 year ago

I don't think there was a particular reason other than ask user to be explicit but I'd be ok with having such a default.

SydneyhSmith commented 1 year ago

Thanks @ThomasNieto could you please elaborate on the usability concern...this required path is a pattern in PowerShell...it also could be costly to run PSSA on the . path

ThomasNieto commented 1 year ago

@SydneyhSmith because PowerShell best practices is to use a default value over a required parameter when a reasonable default value can be derived from expected use. I would think a common use case for any PowerShell cmdlet and PSSA included is to default to the current working directory.

I don't understand your "costly" statement. Please explain why it is "costly" to run the script analyzer in the current working directory? PSSA defaults to not running recursively. We're not talking about defaulting to the Windows directory with Recurse enabled.

Specifically my use case I ran into is trying to run PSSA in a GitHub action and since GitHub actions current working directory is the GitHub project I would have expected that it would "just work" but it didn't and upon inspecting it was due to this required parameter.

There is no issue from my prospective changing it to be a default value. All existing users have already defined the required path parameter and new users can make use of the default if they so desire.

bergmeister commented 1 year ago

I still agree with Thomas and he has good arguments like Recurse not being the default. Can you give examples Sydney where Path is explicitly required but not necessary (Test-Path is not an example as Path parameter is necessary for it)? At first I was sceptical around not being explicit but then I looked at cmdlets like Get-ChildItem or Invoke-Pester that default to current working directory as well

ghost commented 1 year ago

Closing due to inactivity

ThomasNieto commented 1 year ago

@bergmeister / @SydneyhSmith why was this closed?

bergmeister commented 1 year ago

The bot did it because of the 'Need repro info' tag