PowerShell / PSScriptAnalyzer

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

UseCompatibleCommands should recognise partial parameter matches #1201

Open rjmholt opened 5 years ago

rjmholt commented 5 years ago

Summary of the new feature

PowerShell allows for the usage of partial parameter names, like Import-Module 'MyModule' -Pass.

UseCompatibleCommands currently sees this is a compatibility error.

While PSScriptAnalyzer should discourage truncated parameter names (like command aliases), UseCompatibleCommands should be able to recognise the difference between a truncated parameter that will work and something that will not.

Proposed technical implementation details (optional)

The query API currently uses a dictionary to look up parameters. Instead of this, it should lazily compute a trie of parameters that is able to tell whether a parameter is uniquely resolved given an input.

What is the latest version of PSScriptAnalyzer at the point of writing

1.18.0

/cc @JamesWTruher

rjmholt commented 4 years ago

From @bergmeister's comment in https://github.com/PowerShell/PSScriptAnalyzer/issues/1393#issuecomment-572783717:

I vaguely remember that a low level implementation detail is that a dictionary or hashtable is used, both have O(1) to find the element, I think that searching for elements starting with a certain string, the expense to find the item would become linear.

We do indeed currently use a dictionary. A trie would accomplish what we want and change lookup time from O(1) to O(log(n)), but given a reasonable ceiling of 100 parameters per command, the performance difference would likely be low (since we still get O(1) command lookup).

The hard part is implementing the trie. There are some other improvements that are higher priority for me in the compatibility rules in the medium term, and right now I'm concentrating on the extension. But I do see this as a relatively straightforward problem to solve, we just need to find the time to invest.

bergmeister commented 4 years ago

Yes, trie is a good call. There is already a NuGet package for it, which multitargets netstandard2.0 and net461, meaning we'd either have to bump minimum .net version requirements for windows PowerShell or implement it only for PSCore. It's definitely relatively complex as we'd have to adapt the build to include one or more DLLs. PSSA's multithreated nature also means that we'd require the trie implementation to be thread-safe, something that is not easy to say until we try it (or see sporadic failures only in production), therefore this would definitely need to have a rule option for it https://github.com/gmamaladze/trienet