PSAvoidUsingPositionalParameters - seriously?

[Jaykul]

If you're going to make warnings like this -- you have to offer tools to automatically fix them. It's a waste of time for me to manually go through and fix hundreds of "Warnings" like that which (realistically) aren't ever going to break anything.

[raghushantha]

Hi Joel.

Providing automatic fixes (or atleast suggestions) is a great idea. We will consider this going forward.

For existing scripts, you can use -ExcludeRule or inline suppression(at function/script level) or profiles to avoid the rule application
(if you feel the rule is noisy)

-Raghu

[Jaykul]

Why is this even a rule?

Does anyone really think that Test-Path $MyPath is wrong? Or Write-Output $Output ?

Why is Where-Object { $ } OK if those are not?

Why is it ok to pipeline input, if it's not ok to use positional parameters?

[nightroman]

+1. This rule makes sense for complex commands. But for simple commands it is overkill. Besides, some commands may be specifically designed for use with positional parameters (e.g. some DSL-like commands). I doubt that PSSA is capable to decide where this rule is really needed.

[joeyaiello]

We'll talk about this more today, but we think it might be a better compromise to whitelist some common positional parameters (-Name, -Path, -ComputerName, etc) for very common, "obvious" situations like what @Jaykul has shown (similar to what we did with aliases).

What we're trying to avoid here with this rule is particularly clever people using exclusively positional parameters instead of splatting (or vertically lined up parameters with backticks, pick your poison) on Exchange-like parameter sets.

[vors]

+1 on this issue.

As a compromise, I suggest to report warning, only if call has more then one parameter.
In this case Test-Path $foo would be ok, but Test-Path $foo -ErrorVariable ev would not. Which is debatable better.

[Jaykul]

Warning vs. Error doesn't really matter -- as a developer, I "treat warnings as errors" all the time.

I don't think this is worth warning over. Changing positional parameters would be a breaking change (more severe than renaming parameters, since you could fix that with aliases). Can you imagine changing stuff like that, ever? The idea is a violation of the whole notion of APIs.

If it's excessive (three or more) then an INFO trace might be worth it, because it's worth having a chance to review whether I've made the line harder to read by leaving those out -- but the fact is that something like this should not cause a warning:

Set-Item data:\Role -Value $Roles -ErrorAction SilentlyContinue

There's just no real reason for me to provide the -Name name. It's just as readable without it, and there's basically zero chance of breaking that position in the future without breaking the world regardless.

[rkeithhill]

Again I agree with @Jaykul. How about you only fire this rule when three or more positional parameters are being used? With three positional parameters you are starting to put more of a burden on the reader to know cmdlet parameter sets. However allowing two would cover many idiomatic cases e.g.:

Copy-Item foo.cs bar.cs
Write-Output 'hello world'
Test-Path C:\temp\foo.cs
Get-ChildItem C:\temp *.cs
Set-Item data:\Role $Roles -ErrorAction SilentlyContinue
Select-String 'class\s+([^{ ])' C:\temp\foo.cs
... | Add-Member NoteProperty Timestamp -Value (Get-Date)

Further, I would love to see a bit more configurability here. Why not give the user control over the number as well? If they want to dictate only one positional parameter (or maybe three) is OK for their code, let them. I've been working with StyleCop for some time now and it does give some degree of flexibility like this - https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/Configuration.md

[raghushantha]

Improvements to this rule is being tracked here:
#301

There is a bug in the original check-in - this will be fixed

[raghushantha]

Fixed in #406