PowerShell / PSScriptAnalyzer

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

Quotes aren't smartly replaced when used in `#Requires` statements. #2008

Open lzandman opened 3 weeks ago

lzandman commented 3 weeks ago

I noticed quotes aren't smartly replaced when used in #Requires statements.

Steps to reproduce

Look at the following lines. Note that the double quote chars probably don't need to be used on that first line, but I don't think it's an error to use them. And when they are used, they should be processed properly by PSScriptAnalyzer.

#Requires -Modules "Az.Accounts"
#Requires -Modules @{ ModuleName="Pester"; RequiredVersion='4.9.0' }

Expected behavior

I had expected the double quote chars to be replaced by single quote chars, like this:

#Requires -Modules 'Az.Accounts'
#Requires -Modules @{ ModuleName='Pester'; RequiredVersion='4.9.0' }

Actual behavior

The quotes remain unchanged.

#Requires -Modules "Az.Accounts"
#Requires -Modules @{ ModuleName="Pester"; RequiredVersion='4.9.0' }

It seems PSScriptAnalyzer treats #Requires lines as mere comments and doesn't process them.

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.17763.5830
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17763.5830
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

1.22.0
1.22.0
liamjpeters commented 3 weeks ago

I like the idea of this - but it could be a difficult one to implement.

The way the majority of the rules work within PSSA is that they analyze the Abstract Syntax Tree and not the source files directly.

The rule that does the quote replacement, AvoidUsingDoubleQuotesForConstantString, looks for StringConstantExpressionAst - the the type of AST which represents strings (be they single quoted, double quoted, or here-strings).

The AST is useful as it tells you where each part of the code starts and stops as well as other useful information from the PowerShell parser. For instance in the strings example, where each string starts and stops, and the type of string it is - making it trivial to decide if a change is required and then change the type of quotes that string uses.

The #Requires statement is not directly parsed as a node of the AST - it's parsed into the ScriptRequirements of a ScriptBlockAST - no matter where it appears within the source file.

This provides no hint as to where the #Requires statement appears within the source file and also does not parse any strings that make up the #Requires statement into StringConstantExpressionAst (the 'Az.Accounts' in your example for instance).

I believe to implement this, you'd have to do a lot of work to recognise the #requires statement directly in the source text, then manually parse any strings which make this up (including those which appear within hashtables).