PowerShell / PSScriptAnalyzer

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

Warn against unicode characters #1350

Open shaneing opened 5 years ago

shaneing commented 5 years ago

Summary of the new feature

When using the double quote in Chinese, PSSA should throw an error or warning.

Proposed technical implementation details (optional)

Three cases as follows:

Invoke-ScriptAnalyzer -ScriptDefinition '$a = "b“; Write-Output $a'
Invoke-ScriptAnalyzer -ScriptDefinition '$a = “b“; Write-Output $a'
Invoke-ScriptAnalyzer -ScriptDefinition '$a = “b"; Write-Output $a'

What is the latest version of PSScriptAnalyzer at the point of writing

v1.18.3

bergmeister commented 5 years ago

Why? What is the problem with double quotes in Chinese?

shaneing commented 5 years ago

Why? What is the problem with double quotes in Chinese?

I run the script in powershell 4.0 (Windows Server 2012 R2) and its output as follows:

+ $a = "b�Write-Output $a
+      ~~~~~~~~~~~~~~~~~~~
The string is missing terminator:
    + CategoryInfo          : ParserError: (:) [], ParseExcept
    + FullyQualifiedErrorId : TerminatorExpectedAtEndOfString
bergmeister commented 5 years ago

Is this a true double quote or a non-standard unicode character that just looks like a double quote? What is the behaviour for PowerShell 5.1 or 6.2? Is this known @rjmholt ?

shaneing commented 5 years ago

It's no problem with PowerShell 6.2.

rjmholt commented 5 years ago

It's because of the encoding of the script. You want PSUseBOMForUnicodeEncodedFile.

The script is in UTF-8, but Windows PowerShell 5.1 and under default to an extended ASCII encoding. PS 6+ handles it fine because it defaults to UTF-8 when there's no BOM. The character itself is a perfectly legal string terminator in all versions of PowerShell.

If you re-save your script as UTF-8 with BOM, WinPS will pick that up and it will work. Please be mindful of source encoding; the PowerShell interpreter only sees bytes, so it's something you must explicitly handle in your editor and the way in which you move scripts around. If you share code, ensure that everyone is saving the file with a portable encoding (the ISE is not good for this).

shaneing commented 5 years ago

It's because of the encoding of the script. You want PSUseBOMForUnicodeEncodedFile.

The script is in UTF-8, but Windows PowerShell 5.1 and under default to an extended ASCII encoding. PS 6+ handles it fine because it defaults to UTF-8 when there's no BOM. The character itself is a perfectly legal string terminator in all versions of PowerShell.

If you re-save your script as UTF-8 with BOM, WinPS will pick that up and it will work. Please be mindful of source encoding; the PowerShell interpreter only sees bytes, so it's something you must explicitly handle in your editor and the way in which you move scripts around. If you share code, ensure that everyone is saving the file with a portable encoding (the ISE is not good for this).

ok, I get it now, thanks. The reason is that the script automated encoding conversion to UTF-8 when I input the double quote in Chinese uncarefully. I do not recommend using it. So ...

rjmholt commented 5 years ago

I do not recommend using it. So ...

That's a fair point. A rule that warns against obscure but accepted characters in PowerShell scripts might be a good idea. Particularly long dashes and styled quotes, which tend to come from MS Word.

bergmeister commented 5 years ago

There is already an issue with a viable solution where someone made a vs code extension, see here https://github.com/PowerShell/PSScriptAnalyzer/issues/981#issuecomment-420852777 https://marketplace.visualstudio.com/items?itemName=GlenBuktenica.unicode-substitutions

There is also another vs code extension to highlight dodgy characters https://marketplace.visualstudio.com/items?itemName=nachocab.highlight-dodgy-characters

rjmholt commented 5 years ago

Going to close this as a duplicate of https://github.com/PowerShell/PSScriptAnalyzer/issues/981