Repository navigation
Inaccurate PSScriptAnalyzer "Line has trailing whitespace" errors since 1.8.0 #1429
Description
Since the update to the PowerShell Extension 1.8.0 (and 1.8.1) I am seeing 100s of alerts about my scripts in the 'Problems' section of VSCode indicating "[PSScriptAnalyzer] Line has trailing whitespace (PSAvoidTrailingWhitespace)". This rule in the script analzyer seems to be extremely unreliable since it's highlighting many lines of code that do not have any trailing or leading spaces - mainly in code that is tab intended or where text strings span across multiple lines (word wrap enabled). Example from a Param section in a custom function:
[CmdletBinding()]
Param(
[ValidateNotNullOrEmpty()][Parameter(Mandatory=$True,Position=1)][string]$ExchangeGuid
)
The analyzer indicates that the closing parenthesis has trailing whitespace, but it does not. The file is a .ps1 using Windows 1252 encoding and CRLF.
Also disabling this rule via the command palette, PowerShell: Select PsScriptAnalyzerRules doesn’t seem to stay disabled across restarts. Each time I restart VSCode the option is re-enabled. And most of the time I can’t actually load the Select PsScriptAnalyzerRules dialog - it never appears or takes many minutes to appear which is very frustrating.
Activity
rjmholt commentedon Jul 12, 2018
Hi Stuart Chapman (@stukey), for the rule itself you should open an issue in the PSScriptAnalyzer repo.
There is also a way to persist PSSA configurations that Keith Hill (@rkeithhill) has detailed in #1432 (comment).
It sounds like there are some issues with the
Select PSScriptAnalyzerRulesdialog:stukey commentedon Jul 12, 2018
Thanks Rob Holt (@rjmholt). I will open a separate issue. But I do think that we should be able to persist those rule settings across restarts. The other option of having to create files at the root of each directory seems kinda clunky.
rkeithhill commentedon Jul 12, 2018
Stuart Chapman (@stukey) That's perhaps a personal preference but I certainly prefer having settings like this committed into my SCM system (Git, svn, etc). Ideally, the PSSA UI would act as a UI for this settings file - creating it for you.
stukey commentedon Jul 12, 2018
Keith Hill (@rkeithhill) Yep, understood, but we’re not using an SCM currently. Our scripts are stored in numerous folders across multiple servers. So for us the new behaviour is extremely frustrating. Each script we open in VSCode now shows 100s of errors due to the new whitespace rule which we can never resolve or disable.
rjmholt commentedon Jul 12, 2018
The problem there is that the standard with linters (pyflakes, tslint, hlint, etc.) is to have a configuration file in the code base. The reason that's standard is because linting configurations tend to be per-codebase, not per-user (because multiple users with different linting configurations contributing to the same code would end badly).
But then the extension allows you to override that at runtime as a convenience feature, with the idea in mind that if you want it persisted you write it to the config file.
With all that said... You should be able to configure the
"powershell.scriptAnalysis.settingsPath"setting to be any path you like; it will honour an absolute path.Anyway, I've just noticed that the question about supporting all of this better is better covered by #899.
SeeminglyScience commentedon Jul 12, 2018
I'd like to see the ability to configure global defaults that take affect only if there is no workspace rule set defined. I also prefer to have the rule set in source control in general, but an untitled workspace or random drafts folder is a pretty common occurrence in PowerShell.
rkeithhill commentedon Jul 12, 2018
We could have make the PSSA settings be a user/workspace configuration set of settings like the markdown linter uses e.g.:
But this has a number of issues. We already have a file for settings - PSScriptAnalyzerSetting.psd1. Having the settings in
.vscode\settings.jsonkind of duplicates the PSSASettings files. Also, PSSA can directly use PSSASettings.psd1 but not.vscode\settings.json. PSSA rules are pretty fluid atm - with new rules being added and existing rules being deprecated (removed). That also presents a challenge for settings persistence. And we have this UI for rule selection that is "in-memory" only. We could make that a UI for editing settings but which settings would we pick - PSSASettings.psd1, workspace settings or user settings?bergmeister commentedon Jul 12, 2018
If the extension fixed/implemented #823 then this would make it easier for most users, which is more important than the already referenced #1429
rjmholt commentedon Jul 12, 2018
Both issues seem pretty straightforward to implement too
rjmholt commentedon Jul 12, 2018
Yeah I think there are a couple of fairly large design questions here (as in, changing them later will break things), which are:
I can see many different valid opinions here and I worry that the more we try to juggle multiple configurations, the more we'll just end up causing pain.
SeeminglyScience commentedon Jul 12, 2018
We could kind of get around that by not actually verifying the setting, similar to how feature flags work.
The way I picture it is either
Having a setting that is just a list of disabled rules. That makes it easy to disable rules you disagree with (at least in the context of an anonymous workspace), but also makes it so new default rules don't need to be explicitly opt-in. And it'd be dead easy to tie to the UI.
Just having a user level setting that specifies a default setting file. This one would allow for more customization, but it would be harder to get started with and harder to tie to the UI.
The way precedence makes sense to me is
Otherwise it'd be too hard to comply with settings vastly different than your own.
rkeithhill commentedon Jul 12, 2018
Are you not concerned that if the user wants to run PSSA outside of VSCode, PSSA will not be able to use the VSCode settings? This is why I've liked having a PSScriptAnalyzerSettings.psd1 file. You can use it inside VSCode and outside as well.
SeeminglyScience commentedon Jul 12, 2018
I would still expect workspace settings to be controlled via a the same psd1 file. The only difference is an additional setting that may control the default experience only if the workspace does not have a
PSScriptAnalyzerSettings.psd1.rkeithhill commentedon Jul 12, 2018
You could do this now with a user setting of:
That would apply to everywhere except where a
.vscode\settings.jsonfile overrides that user setting.SeeminglyScience commentedon Jul 13, 2018
Oh cool! I don't know why I had it in my head that that was a workspace only setting :) Now that I think about it, workspace only settings aren't even a thing are they? :P
A generic "disabled rules" setting that only applies when there are no other settings (workspace or otherwise) may still be a good idea. Or maybe just an easier way to configure that file.
rjmholt commentedon Jul 13, 2018
Yeah, now I think about it, having a set of "default rules" configured by the editor probably means we should have at least a way to disable them, and maybe also a VSCode configuration to add more in too. Probably a user setting rather than a workspace one?
Like:
{ "powershell.scriptAnalysis.rules": { "disable": [ "PSAvoidTrailingWhitespace" ], "enable": [ "RuleThatILikeButMyColleaguesDont", ] } }Questions are:
disableonly affect defaults andenablealways add in? (This seems complicated, but worth enumerating I thought)SeeminglyScience commentedon Jul 13, 2018
My thoughts
I don't think so. If you have a user scoped setting file, you can already control everything there. If you have a workspace setting file, that should take precedence over everything. Otherwise it'd be too much of a pain to adhere to different repo's rule sets.
Yes (see above for reasoning)
No (see above for reasoning)
Yes. I think this should be very light adjustments to defaults, those who need more fine control would be better off making a full setting file. Enable always adding in is important imo, because otherwise new defaults would be significantly less likely to be seen.
rkeithhill commentedon Jul 13, 2018
We should distinguish between VSCode workspace setting file
.vscode\settings.jsonand the PSSA settings file.\PSScriptAnalyzerSettings.psd1file. The latter can be used by both VSCode and PSSA directly. The former by VSCode only. Sorry if I sound like a broken record but when I see "workspace setting" I can't quite tell which of the two is being referred to.SeeminglyScience commentedon Jul 13, 2018
Keith Hill (@rkeithhill) that's a really good point. Every instance I've said "settings file" I'm referring to
PSScriptAnalyzerSettings.psd1. For some reason I picture thesetting.jsonas a settings menu in any other GUI app even though I edit the json directly. So I don't typically think of it as a settings file. That's probably made everything I've said incredibly confusing :)rkeithhill commentedon Jul 13, 2018
I thought I was interpreting your intent correctly. Just wanted to make sure. :-)
rjmholt commentedon Jul 13, 2018
Ok, I'm sold (on Patrick Meinecke (@SeeminglyScience)'s suggestion)
stukey commentedon Jul 13, 2018
Keith Hill (@rkeithhill) thanks for the suggestion. I have created a psScriptAnalyzerSettings.psd1 file and specified it in my user settings. That is working as desired to disable the new white space rule. I did have issues figuring out what default rules I should enable. The template psd1 file didn’t seem to contain all of the default rules that are enabled by the Powershell extension. So I manually added them one by one by comparing to the list of rules displayed in the command palette. Is there an up-to-date list of default rules anywhere? I want them all on except the white space rule. Also since activating the rule set in user prefs and upgrading to vsCode 1.2.5.1 I’ve noticed that the script analyser seems to be very slow to respond to changes in the script. e.g if I create a new variable but have not used it the standard error about a variable being defined but not used appears. If I then use the variable in the next line of code the error continues to appear for several minutes before eventually disappearing.
rkeithhill commentedon Jul 13, 2018
If you specify only rules to exclude, you'll get the default rules minus the rules you have excluded. As to how to tell which rules are enabled by default, maybe Christoph Bergmeister (@bergmeister) knows.
rjmholt commentedon Jul 13, 2018
Is there a command to execute in the integrated console maybe? Like
Get-EnabledRules?bergmeister commentedon Jul 13, 2018
PSSA itself has all rules enabled by default when calling
Invoke-ScriptAnalyzer, it is PSES that decided to use the-IncludeRulesparameter where PSSA runs only the specified rules. IncludeRules means 'run only those rules' and 'excluderules' means that the default minus the specified rules are used (its usage becomes more apparent when used together with customrules., only in this case would it make sense to use both IncludeRules and excluderules together). The example that I give in this example repo here explains some of it but it is probably too generic and I need to create more examples to cover different scenarios...rkeithhill commentedon Jul 14, 2018
This has been resolved by PowerShell/PowerShellEditorServices#704 and will be in the next update to the extension.
stukey commentedon Jul 17, 2018
Keith Hill (@rkeithhill) Thanks again. I just wanted to point out that specifying only the excludes in the PSScriptAnalyzerSettings.psd1 file doesn’t in fact provide the default rules. I did this and several other rules were enabled which I had to disable by adding to the exclude list. These are the ones I had to exclude:
ExcludeRules = @('PSPossibleIncorrectComparisionWithNull',
'PSAvoidTrailingWhitespace',
'PSUseShouldProcessForStateChangingFunctions',
'PSUseSingularNouns',
'PSAvoidUsingInvokeExpression')
bergmeister commentedon Jul 17, 2018
Stuart Chapman (@stukey) Once you specify a settings file, then the VSCode defaults do not apply any more at all. The point of this is to get the same result when running
Invoke-ScriptAnalyzeror seeing the results in VSCode. PSScriptAnalyzer enables all rules by default, it is only the VSCode extension that does not when not specifying anything, hence why 2 other issues were referenced as the extension/PSES does not offer any build in way of changing those settings and at the moment the only customisation is to use a PSSA settings file, from then on the PSSA docs apply,