Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PSAvoidTrailingWhitespace should ignore empty lines indented to match the current block #1033

Open
poshcodebear opened this issue Jul 11, 2018 · 18 comments · May be fixed by #1048
Open

PSAvoidTrailingWhitespace should ignore empty lines indented to match the current block #1033

poshcodebear opened this issue Jul 11, 2018 · 18 comments · May be fixed by #1048

Comments

@poshcodebear
Copy link

Currently, PSAvoidTrailingWhitespace checks all lines for trailing whitespace, even when that line is effectively empty and the "trailing" whitespace is actually leading whitespace for the current indentation level. If the recommended action is taken (remove the whitespace), then adding new code in those spots requires re-indenting that spot to the current code indentation level.

I propose that it should respect the current indentation level for an empty line and not generate a note for those lines, as long as they match the indentation of the current block of code.

What is the latest version of PSScriptAnalyzer at the point of writing
1.17.1, shipping with the vscode-powershell extension version 1.8.1

@bergmeister
Copy link
Collaborator

bergmeister commented Jul 11, 2018

Thanks for the feedback. Which editor do you use that results in having such lines? I think the default behaviour is maybe ok but having a configuration option for the rule to ignore lines with whitespace only would make sense to me.

@poshcodebear
Copy link
Author

As implied, I use Visual Studio Code; that being said, every modern editor I've used (VSCode, ISE, Notepad++, Atom, Visual Studio) does the exact same thing: retains the current indentation line when I press enter. If I hit enter twice, I have an empty line at the same indentation level as the code around it.

I would be fine with that solution, assuming it doesn't mean having to create a settings file for every repo I work in just for that one setting.

@bergmeister
Copy link
Collaborator

bergmeister commented Jul 11, 2018

Ok. I see. In VSCode one can set the setting files.trimTrailingWhitespace to true and on saving the document the trailing whitespace will get removed automatically.
One can even tweak the rule to make it only apply to PowerShell files as follows:

 "[powershell]": {
    "files.trimTrailingWhitespace": true
  },

@poshcodebear
Copy link
Author

I'm not so much looking for it to automatically handle it for me; additionally, I want the automatically inserted whitespace that matches the indentation level to remain (this makes future changes easier as I don't have to stop and re-indent lines that should already be indented correctly).

Up until the last update to the PowerShell extension which pushed this latest version of PSScriptAnalyzer, I've not had to create a settings file to control its behavior as it's been fine out of the box. I get an update and all of a sudden it's complaining about perfectly legitimate use of whitespace and I'm finding the only way to get it to quiet down is either to manually turn that check off every time I launch or create a settings file for every PowerShell folder I work with. Neither of these solutions are particularly appealing, and I don't want to turn it off either, but I find the behavior that it insists on baffling and without any explanation (the documentation/description only lists issues with trailing whitespace at the end of code lines, not at the end of blank lines).

Anyway, if the behavior won't be changed, I feel that the description should at least be updated with a defense of that as a standard.

@bergmeister
Copy link
Collaborator

bergmeister commented Jul 11, 2018

OK, I see now where you're coming from and understand your pain. My current thinking is that it might be be better to rather change the defaults in the VSCode extension to use a new customisation of this rule to allow whitespace only lines. I think we could discuss the defaults of this customisation afterwards depending on what other people think but having the customisation and using it in the defaults of the extension (to be precise, it is actually PowerShellEditorServices, which applies to other editor as well) sounds reasonable.
Since this depends on a new PSSA release, would it be better to turn this rule not on by default for the moment @tylerl0706 ?

@SeeminglyScience
Copy link
Contributor

every modern editor I've used (VSCode, ISE, Notepad++, Atom, Visual Studio) does the exact same thing: retains the current indentation line when I press enter. If I hit enter twice, I have an empty line at the same indentation level as the code around it.

The indentation is only retained if something has been typed (including if it was subsequently deleted). It shouldn't stick around with a double tap of enter. At least it doesn't for me, maybe that's a setting.

That said, it can be annoying when it reports the indent of the current line though. I've gotten used to that from TSLint (which works the same way), but it would be cool if we could ignore the warning for the current line. That would have to be done on the PowerShellEditorServices side though, because we don't report cursor status to PSSA.

would it be better to turn this rule not on by default for the moment

👍

@bergmeister
Copy link
Collaborator

Ok, I created a PR in the PSES repo to not have the rule enabled by default for the moment as a quick solution

@stetan
Copy link

stetan commented Jul 14, 2018

Yes, this was quite a surprise when, all of a sudden, I saw hundreds of alerts in my code. I'm amazed no one picked this up before it was pulled into the main code, as every code editor adds tabs/spaces to indent blank lines to the level of the preceding code as a default setting.

@TylerLeonhardt
Copy link
Member

@SeeminglyScience filtering out the PSSA warnings that contain the cursor position shouldn't be too hard to add.

I don't think I've seen any other linters do something like that but it makes sense to me.

@nschonni
Copy link

Not something I'd use, but here is the setting from ESLint https://eslint.org/docs/rules/no-trailing-spaces#options

@rjmholt
Copy link
Contributor

rjmholt commented Aug 20, 2018

Moving the remaining issue to the PSES repo.

@rjmholt rjmholt closed this as completed Aug 20, 2018
@rjmholt
Copy link
Contributor

rjmholt commented Aug 20, 2018

Just my 2 cents -- this is something I'd use this rule exactly to check; I understand there are differing opinions, but both my habitual editing experience (in VSCode, vim, etc.) and my preferred style is to get rid of extraneous, orphaned whitespace.

As a linter, I feel like PSScriptAnalyzer should support both opinions (that's what a linter is in my mind, a way to formally specify and apply code style opinions).

@rjmholt
Copy link
Contributor

rjmholt commented Aug 20, 2018

Going to reopen since @bergmeister has a PR open on this

@rjmholt rjmholt reopened this Aug 20, 2018
@poshcodebear
Copy link
Author

@rjmholt
I see where you're getting at, and agree that it's a personal style choice; I think it's great to have it as an option, it's just one I've not been able to turn off (short of setting up linting rules in all my repos, which I really didn't want to do for this).

The whitespace on a line by itself matching the indentation of the lines around it being flagged, in my opinion, should really be a separate rule from trailing whitespace for just this reason.

The other issue, of course, is that there should be a way to turn rules on and off globally and not have to set up config files manually, but that's a separate issue.

@joeyaiello
Copy link
Contributor

This rule was created because there are lots of problems that arise specifically in PowerShell as a result of hidden whitespace. The number one that we found was trailing spaces after backticks which would escape the space rather than the line break, preventing stuff like this from working correctly:

Get-Foo -Bar Hello`<if there's a space here you have a problem>
        -Baz World

And while whitespace-only lines don't cause the same sorts of issues in PowerShell, it does create issues in source control where people's editors are different (and this is specifically in place to help with that scenario).

So yeah, my take is that this rule is working as designed.

As for globally changing your settings configuration, have you tried using this:

// Specifies the path to a PowerShell Script Analyzer settings file. To override the default settings for all projects, enter an absolute path, or enter a path relative to your workspace.
  "powershell.scriptAnalysis.settingsPath": ""

@asears
Copy link

asears commented Sep 3, 2020

This is a couple years old and still an open issue.

Can this rule be updated (or a new rule created) in order to fix the VSCode formatting issue? I am applying the change to VSCode as in above, however whitespace on a blank line (to match indenting between two lines) seems like a proper pattern and it shouldn't break backticking...

By default, VSCode will break this rule which causes added issues.

Or can close this with a won't fix and will move on to trying to change the defaults for VSCode's formatter.

@Jaykul
Copy link

Jaykul commented Nov 17, 2020

For what it's worth, I'm in the "a line with nothing but whitespace on it is a mistake and a waste of space" camp.

There are many reasons that those should be cleaned up, not least of which is that they are meaningless.

Are there really linters in any other languages that clean trailing whitespace but don't clean it if it might someday be leading whitespace?

Anyway. I have no problem with giving people the option to ignore this, but it seems there's currently no option to clean them up, so I have to use both ScriptAnalyzer and the VS Code setting to clean them up.

@DennisL68
Copy link

DennisL68 commented Jun 15, 2023

Since there are different opinions, there should be different setting for how AvoidTrailingWhitespace behaves.

Besides, if there is nothing at the start of the line (except white spaces) what exactly is the white spaces trailing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet