Inaccurate PSScriptAnalyzer "Line has trailing whitespace" errors since 1.8.0

[stukey]

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.

[rjmholt]

Hi @stukey, for the rule itself you should open an issue in the PSScriptAnalyzer repo.

There is also a way to persist PSSA configurations that @rkeithhill has detailed in #1432 (comment).

It sounds like there are some issues with the Select PSScriptAnalyzerRules dialog:

• It does not persist changes (may be by design, but we might want to change the design?)
• Poor performance (likely related to other performance issues we are currently working on resolving).

[stukey]

Thanks @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]

@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]

@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]

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]

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]

We could have make the PSSA settings be a user/workspace configuration set of settings like the markdown linter uses e.g.:

    "markdownlint.config": {
      "MD024": false
    }

But this has a number of issues. We already have a file for settings - PSScriptAnalyzerSetting.psd1. Having the settings in .vscode\settings.json kind 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]

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]

Both issues seem pretty straightforward to implement too

[rjmholt]

Yeah I think there are a couple of fairly large design questions here (as in, changing them later will break things), which are:

1. If we persist UI-configured settings, where do we persist them to? Do we overwrite existing settings or do we create a new file?
2. If we allow a second configuration path (e.g. per-user or per-{user+workspace}), what is its precedence wrt a workspace configuration file?

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]

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.

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

1. 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.

2. 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

• Use the user based setting if no other settings are applied
• If there is a workspace level settings file, discard all user based settings and use the workspace level settings.

Otherwise it'd be too hard to comply with settings vastly different than your own.

[rkeithhill]

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]

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]

You could do this now with a user setting of:

"powershell.scriptAnalysis.settingsPath": "C:\\Users\\hillr\\PSScriptAnalyzerSettings.psd1"

That would apply to everywhere except where a .vscode\settings.json file overrides that user setting.

[SeeminglyScience]

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]

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:

• Should disable override the local settings file?
• Should the presence of a local settings file override these settings entirely?
• Should these settings be "played on top of" a settings file?
• Should the disable only affect defaults and enable always add in? (This seems complicated, but worth enumerating I thought)

[SeeminglyScience]

My thoughts

Should disable override the local settings file?

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.

Should the presence of a local settings file override these settings entirely?

Yes (see above for reasoning)

Should these settings be "played on top of" a settings file?

No (see above for reasoning)

Should the disable only affect defaults and enable always add in? (This seems complicated, but worth enumerating I thought)

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]

We should distinguish between VSCode workspace setting file .vscode\settings.json and the PSSA settings file .\PSScriptAnalyzerSettings.psd1 file. 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]

@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 the setting.json as 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]

I thought I was interpreting your intent correctly. Just wanted to make sure. :-)

[rjmholt]

Ok, I'm sold (on @SeeminglyScience's suggestion)

[stukey]

@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]

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 @bergmeister knows.

[rjmholt]

Is there a command to execute in the integrated console maybe? Like Get-EnabledRules?

[bergmeister]

PSSA itself has all rules enabled by default when calling Invoke-ScriptAnalyzer, it is PSES that decided to use the -IncludeRules parameter 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]

This has been resolved by PowerShell/PowerShellEditorServices#704 and will be in the next update to the extension.

[stukey]

@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]

@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-ScriptAnalyzer or 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,