search

PowerShell / vscode-powershell

Provides PowerShell language and debugging support for Visual Studio Code
https://marketplace.visualstudio.com/items/ms-vscode.PowerShell
MIT License
1.71k stars 492 forks source link

Extension's Formatter ignores config file. #3024

Open ninmonkey opened 4 years ago

ninmonkey commented 4 years ago

Issue Description

The extension's formatter is ignoring the config file: PSScriptAnalyzerSettings.psd1 I didn't file this in /PSScriptAnalyzer because it (Invoke-Formatter) works as expected with the same config file.

Properties set under "powershell.codeFormatting.* are working. Except it's ignoring "powershell.scriptAnalysis.settingsPath": "PSScriptAnalyzerSettings.psd1" when formatting.

I can disable changing process using Invoke-Formatter using this config

@{
    'Rules' = @{
        'PSAvoidUsingCmdletAliases' = @{
            'Whitelist' = @('process')
        }
    }
}

Expected Behaviour

Input example:

function foo {
    param ()

    Process {
        ls
        process
    }
}

Should format as

function foo {
    param ()

    Process {
        Get-ChildItem
        process
    }
}

Actual Behaviour

function foo {
    param ()

    Process {
        Get-ChildItem
        Get-Process
    }
}

System Details

A tiny workspace reproduces the example: 2020-10-20 - Invoke-Formatter - ignores config.zip

System Details Output (Click to Expand) ``` ### VSCode version: 1.50.1 d2e414d9e4239a252d1ab117bd7067f125afd80a x64 ### VSCode extensions: alefragnani.Bookmarks@11.4.0 arcticicestudio.nord-visual-studio-code@0.14.0 bungcip.better-toml@0.3.2 christian-kohler.path-intellisense@2.3.0 CoenraadS.bracket-pair-colorizer@1.0.61 CoenraadS.bracket-pair-colorizer-2@0.2.0 DavidAnson.vscode-markdownlint@0.37.1 donjayamanne.githistory@0.6.12 DotJoshJohnson.xml@2.5.1 eamodio.gitlens@10.2.2 eamodio.tsl-problem-matcher@0.3.1 Equinusocio.vsc-community-material-theme@1.4.2 Equinusocio.vsc-material-theme@33.0.0 equinusocio.vsc-material-theme-icons@1.2.0 esbenp.prettier-vscode@5.7.1 evan-buss.font-switcher@3.1.0 firefox-devtools.vscode-firefox-debug@2.9.1 formulahendry.code-runner@0.11.1 juanmnl.vscode-theme-hydra@3.1.0 matangover.mypy@0.1.4 mechatroner.rainbow-csv@1.7.1 medo64.code-point@1.7.1 ms-dotnettools.csharp@1.23.4 ms-mssql.mssql@1.9.0 ms-python.python@2020.9.114305 ms-python.vscode-pylance@2020.10.2 ms-vscode-remote.remote-wsl@0.50.1 ms-vscode.cpptools@1.0.1 ms-vscode.hexeditor@1.3.0 ms-vscode.powershell@2020.6.0 [disabled] ms-vscode.powershell-preview@2020.9.0 ms-vscode.vscode-typescript-tslint-plugin@1.2.3 msjsdiag.debugger-for-chrome@4.12.11 octref.vscode-json-transform@0.1.2 PKief.material-icon-theme@4.3.0 PowerQuery.vscode-powerquery@0.1.5 RandomFractalsInc.vscode-data-preview@2.2.0 redhat.vscode-xml@0.13.0 RobbOwen.synthwave-vscode@0.1.8 rogalmic.bash-debug@0.3.9 rust-lang.rust@0.7.8 sallar.vscode-duotone-dark@0.3.3 shakram02.bash-beautify@0.1.1 shd101wyy.markdown-preview-enhanced@0.5.13 stansw.vscode-odata@0.1.0 stuart.unique-window-colors@1.0.51 vadimcn.vscode-lldb@1.6.0 VisualStudioExptTeam.vscodeintellicode@1.2.10 webfreak.debug@0.25.0 wwm.better-align@1.1.6 yzhang.markdown-all-in-one@3.3.0 zhuangtongfa.material-theme@3.9.3 ### PSES version: 2.3.0.0 Name Version ---- ------- PowerShellEditorServices.Commands 0.2.0 PowerShellEditorServices.VSCode 0.2.0 ### PowerShell version: Name Value ---- ----- PSVersion 7.0.3 PSEdition Core GitCommitId 7.0.3 OS Microsoft Windows 10.0.19041 Platform Win32NT PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…} PSRemotingProtocolVersion 2.3 SerializationVersion 1.1.0.1 WSManStackVersion 3.0 ```

Attached Logs

log - editorServices.log

Environment Information

Visual Studio Code

Name Version
Operating System Windows_NT x64 10.0.19041
VSCode 1.50.1
PowerShell Extension Version 2020.9.0

PowerShell Information

Name Value
PSVersion 7.0.3
PSEdition Core
GitCommitId 7.0.3
OS Microsoft Windows 10.0.19041
Platform Win32NT
PSCompatibleVersions 1.0 2.0 3.0 4.0 5.0 5.1.10032.0 6.0.0 6.1.0 6.2.0 7.0.3
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

Visual Studio Code Extensions

Visual Studio Code Extensions(Click to Expand) | Extension | Author | Version | | ------------------------------- | -------------------- | ------------- | | better-toml | bungcip | 0.3.2 | | Bookmarks | alefragnani | 11.4.0 | | code-point | medo64 | 1.7.1 | | code-runner | formulahendry | 0.11.1 | | cpptools | ms-vscode | 1.0.1 | | csharp | ms-dotnettools | 1.23.4 | | debug | webfreak | 0.25.0 | | debugger-for-chrome | msjsdiag | 4.12.11 | | font-switcher | evan-buss | 3.1.0 | | githistory | donjayamanne | 0.6.12 | | gitlens | eamodio | 10.2.2 | | hexeditor | ms-vscode | 1.3.0 | | markdown-all-in-one | yzhang | 3.3.0 | | material-icon-theme | PKief | 4.3.0 | | material-theme | zhuangtongfa | 3.9.3 | | mssql | ms-mssql | 1.9.0 | | mypy | matangover | 0.1.4 | | nord-visual-studio-code | arcticicestudio | 0.14.0 | | path-intellisense | christian-kohler | 2.3.0 | | powershell-preview | ms-vscode | 2020.9.0 | | prettier-vscode | esbenp | 5.7.1 | | python | ms-python | 2020.9.114305 | | rainbow-csv | mechatroner | 1.7.1 | | remote-wsl | ms-vscode-remote | 0.50.1 | | rust | rust-lang | 0.7.8 | | synthwave-vscode | RobbOwen | 0.1.8 | | tsl-problem-matcher | eamodio | 0.3.1 | | vsc-community-material-theme | Equinusocio | 1.4.2 | | vsc-material-theme | Equinusocio | 33.0.0 | | vsc-material-theme-icons | equinusocio | 1.2.0 | | vscode-data-preview | RandomFractalsInc | 2.2.0 | | vscode-duotone-dark | sallar | 0.3.3 | | vscode-firefox-debug | firefox-devtools | 2.9.1 | | vscode-json-transform | octref | 0.1.2 | | vscode-lldb | vadimcn | 1.6.0 | | vscode-odata | stansw | 0.1.0 | | vscode-powerquery | PowerQuery | 0.1.5 | | vscode-pylance | ms-python | 2020.10.2 | | vscode-theme-hydra | juanmnl | 3.1.0 | | vscode-typescript-tslint-plugin | ms-vscode | 1.2.3 | | vscode-xml | redhat | 0.13.0 | | vscodeintellicode | VisualStudioExptTeam | 1.2.10 |
manual_invoke_formatter.ps1 (Click to Expand) ```powershell $settings = 'PSScriptAnalyzerSettings.psd1' $src = @' function foo { param () Process { ls process } } '@ $expected = @' function foo { param () Process { Get-ChildItem process } } '@ Write-Host -fore red "Input" $src Write-Host -fore red "Result" $result = Invoke-Formatter -ScriptDefinition $src -Settings $settings $result "Expected?" $result -eq $expected ```
corbob commented 4 years ago

It looks like @bergmeister may have answered this in the #vscode channel on Discord/Slack Reproduced here to provide context within the issue: @ninmonkey The settings path settings only applies for script analysis, i.e. Invoke-ScriptAnalyzer and not formatting. All formatting related rule properties are settable via the powershell.codeformatting.* settings

I'll add: if you're looking for project specific formatting options, you might want to use a workspace settings file .vscode/settings.json so that anyone pulling down the code gets your formatting options.

ninmonkey commented 3 years ago

@corbob:

All formatting related rule properties are settable via the powershell.codeformatting.* settings

My original examples weren't great. This new one shows that

I cannot find a powershell.formatting setting that lets me change the formatting setting PSAvoidUsingCmdletAliases

Problem

Formatter breaks code when there's parse errors.

Invoke-Formatter runs even on parse errors. Because of that, it's indirectly mutating code. After fixing the error (like a missing comma), you also have to fix the (now missing) process block

vscode - formatter - ignores config - part 2 - 2021-04-15

Proposed Solution

( I filed this under vscode-powershell because it's invoking the formatter with its own config)

How to Reproduce

PSScriptAnalyzerSettings.psd1

@{
    'Rules' = @{
        # keep the rule *except* 'process' because tiny typo-s mutates code
        'PSAvoidUsingCmdletAliases' = @{
            Enable      = $True
            'Whitelist' = @('process')
        }
    }
}

To Reproduce

Case1: Error Case

function DoStuff {
    <#
    .description
        example trigger type 1

        A few cases the script becomes synatically invalid
        Like accidentally deleting a ',' in the parameter block

        The formatter should not modify the script when  **error severity level** errors are found

        As a work-around I whitelisted 'process', so I could keep the rule on
    #>
    param(
        [Parameter()][int]$Num
        [Parameter()][int]$Num2
    )
    process {
    }
}

Case2: Unexpected, but correct behavior.

function OtherStuff {
    <#
    .description
    example trigger type 2

    By accident, there exists code outside the process/begin/end blocks

    The code is technically synatically valid. The User doesn't expect doesn't expect `process` ( or `get-process` ) to be called with a script block.

    Meaning it's formatting using the correct behavior.
    #>
    'accidental outer code'
    process {

    }
}
rjmholt commented 3 years ago

See https://github.com/PowerShell/PSScriptAnalyzer/issues/1402#issuecomment-704418988

ninmonkey commented 3 years ago

See PowerShell/PSScriptAnalyzer#1402 (comment)

@rjmholt I think I found another case. Going through that thread it didn't have a case with a syntax error in param() blocks. (they were always empty) (Edit: it was fixed)

I thought maybe it's not the same answer because of these differences:

Format example - 2021-05-06

Semantic highlighting made me this might be a different situation AST wise , that it's not actually parsing as a function with 2 parameters, where the parenthesis were redundant, ex:

param $a $b

Part2

If I put process before param, Then I get the token outside of blocks begin/end/etc

unexpected token 'param', expected 'begin', 'process', 'end', or 'dynamicparam'.

image

Should this occur as the same when the token is a language keyword?

ninmonkey commented 3 years ago

Describe bug or problem

Update: There's currently 2 parts:

  1. bug : The formatter is ran when the script has parse errors / can't run. (Verses the linked thread where the case was valid) like https://github.com/PowerShell/PSScriptAnalyzer/issues/1402#issuecomment-704418988

quoting @rjmholt:

Running the formatter on scripts with errors on them is inherently risky, since what the user thinks they're looking at and what the parser sees are possibly quite different in those scenarios.

  1. feature request: vscode-powershell preferences are missing a formatting setting. Using the rules file powershell.scriptAnalysis.settingsPath when calling Invoke-formatter would solve that.

Is there a reason not to use the config file? IIRC It there were api limitations at the time.

To reproduce

input

function F {
    param( $x $y )
    process {}
}

rules

$settingsPathRule = @{
    'Rules' = @{
        'PSAvoidUsingCmdletAliases' = @{
            Enable      = $true
            'Whitelist' = @('process')
        }
    }
}

$vsCodeRule = @{
    'Rules' = @{
        'PSAvoidUsingCmdletAliases' = @{
            Enable = $true
        }
    }
}

Expected Behavior

Don't format code when Invoke-ScriptAnalyzer returns severity level ParseError

Actual

function F {
    param( $x $y )
    Get-Process {}
}

ScriptAnalyzer output

Invoke-ScriptAnalyzer -ScriptDefinition 'function F {
    param( $x $y )
    process {}
}' | ft -AutoSize -Wrap

RuleName                                     Severity   ScriptName Line Message
--------                                     --------   ---------- ---- -------
MissingEndParenthesisInFunctionParameterList ParseError            2    Missing ')' in function parameter list.
MissingEndCurlyBrace                         ParseError            1    Missing closing '}' in statement block or type definition.
UnexpectedToken                              ParseError            2    Unexpected token ')' in expression or statement.
UnexpectedToken                              ParseError            4    Unexpected token '}' in expression or statement.
PSAvoidUsingCmdletAliases                    Warning               3    'process' is implicitly aliasing 'Get-process' because it is missing the 'Get-' prefix.
                                                                        This can introduce possible problems and make scripts hard to maintain. Please consider
                                                                        changing command to its full name.
PSReviewUnusedParameter                      Warning               2    The parameter 'x' has been declared but not used.

Sample Cases:

SydneyhSmith commented 1 year ago

Wanted to check in and see if you are still having this issue @ninmonkey also wanted to confirm that you still hit this issue when there is no "s" in "settings" i.e. "powershell.scriptAnalysis.settingsPath": "PSScriptAnalyzerSetting.psd1"

bergmeister commented 1 year ago

This issue has completely deviated from that it was before. The powershell.scriptAnalysis.settingsPath setting in VS-Code is just used for script analysis and not formatting. Its name makes this clear. There is no such equivalent setting for the formatter, which is currently controlled by a set of powershell.codeFormatting.* settings.