diff --git a/Rules/InvalidMultiDotValue.cs b/Rules/InvalidMultiDotValue.cs new file mode 100644 index 000000000..b6c3dc93f --- /dev/null +++ b/Rules/InvalidMultiDotValue.cs @@ -0,0 +1,100 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Management.Automation.Language; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that reports an error when an unquoted value contains multiple dots, + /// which is likely an attempt to construct a version number (e.g., 1.2.3) + /// that is not properly quoted and thus misinterpreted as a double with member access. + /// + public class InvalidMultiDotValue : IScriptRule + { + /// + /// Analyzes the PowerShell unquoted values that contain multiple dots. + /// + /// The PowerShell Abstract Syntax Tree to analyze. + /// The name of the file being analyzed (for diagnostic reporting). + /// A collection of diagnostic records for each violation. + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Find all MemberExpressionAst nodes representing invalid unquoted multi-dot values + IEnumerable invalidAsts = ast.FindAll(testAst => + // An expression with 3 or more dots is seen as a double with an additional property + testAst is MemberExpressionAst memberAst && + // The first two values are seen as a double + memberAst.Expression.StaticType == typeof(double) && + // the rest is seen as a member of type int or double + memberAst.Member is ConstantExpressionAst constantAst && + ( + constantAst.StaticType == typeof(int) || // e.g.: [Version]1.2.3 + constantAst.StaticType == typeof(double) // e.g.: [Version]1.2.3.4 + ), + true + ); + + if (invalidAsts != null) { + var correctionDescription = Strings.InvalidMultiDotValueCorrectionDescription; + foreach (MemberExpressionAst invalidAst in invalidAsts) + { + var corrections = new List { + new CorrectionExtent( + invalidAst.Extent.StartLineNumber, + invalidAst.Extent.EndLineNumber, + invalidAst.Extent.StartColumnNumber, + invalidAst.Extent.EndColumnNumber, + "'" + invalidAst.Extent.Text + "'", + fileName, + correctionDescription + ) + }; + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.InvalidMultiDotValueError, + invalidAst.Extent.Text + ), + invalidAst.Extent, + GetName(), + DiagnosticSeverity.Error, + fileName, + invalidAst.Extent.Text, + suggestedCorrections: corrections + ); + } + } + } + + public string GetCommonName() => Strings.InvalidMultiDotValueCommonName; + + public string GetDescription() => Strings.InvalidMultiDotValueDescription; + + public string GetName() => string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.InvalidMultiDotValueName); + + public RuleSeverity GetSeverity() => RuleSeverity.Error; + + public string GetSourceName() => Strings.SourceName; + + public SourceType GetSourceType() => SourceType.Builtin; + } +} \ No newline at end of file diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..96f87fdca 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1221,6 +1221,21 @@ The insecure AllowUnencryptedAuthentication switch was used. This should be avoided except for compatibility with legacy systems. + + Invalid Multi-Dot Value + + + PowerShell does not support an implicit value with multiple dots. Any unquoted value with 2 or more dots will result in `$null`. + + + InvalidMultiDotValue + + + The unquoted '{0}' expression is not a valid syntax. Types with multiple dots need to be constructed from either a quoted string or individual components. + + + Quote the value that contains multiple dots + AvoidUsingAllowUnencryptedAuthentication diff --git a/Tests/Rules/InvalidMultiDotValue.tests.ps1 b/Tests/Rules/InvalidMultiDotValue.tests.ps1 new file mode 100644 index 000000000..e3b4a15b1 --- /dev/null +++ b/Tests/Rules/InvalidMultiDotValue.tests.ps1 @@ -0,0 +1,167 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')] +param() + +BeforeAll { + $ruleName = "PSInvalidMultiDotValue" + $ruleMessage = "The unquoted '{0}' expression is not a valid syntax. Types with multiple dots need to be constructed from either a quoted string or individual components." + $correctionDescription = 'Quote the value that contains multiple dots' +} + +Describe "InvalidMultiDotValue" { + Context "Violates" { + It "3 version components" { + $scriptDefinition = { $version = 1.2.3 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3') + $violations.RuleSuppressionID | Should -Be '1.2.3' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + It "4 version components" { + $scriptDefinition = { $version = 1.2.3.4 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3.4' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3.4') + $violations.RuleSuppressionID | Should -Be '1.2.3.4' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3.4'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + + It "With class initializer" { + $scriptDefinition = { $version = [Version]1.2.3 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3') + $violations.RuleSuppressionID | Should -Be '1.2.3' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + It "As parameter" { + $scriptDefinition = { + param( + [Version]$version = 1.2.3 + ) + Write-Verbose $version + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3') + $violations.RuleSuppressionID | Should -Be '1.2.3' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + # Even an IP address is apparently expect below. + # The violation message and description presume a version + # is expected because this is the more commonly used type. + It "IP Address" { + $scriptDefinition = { $IP = [System.Net.IPAddress]127.0.0.1 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '127.0.0.1' + $violations.Message | Should -Be ($ruleMessage -f '127.0.0.1') + $violations.RuleSuppressionID | Should -Be '127.0.0.1' + $violations.SuggestedCorrections.Text | Should -Be "'127.0.0.1'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + } + + Context "Compliant" { + It "From string" { + $scriptDefinition = { $Version = [Version]'1.2.3' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "From version components" { + $scriptDefinition = { $Version = [Version]::new(1, 2, 3, 4) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "From (bare) double" { + $scriptDefinition = { $Version = [Version]1.2 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + + It "Dot notation" { #PowerShell:27356 + $scriptDefinition = { + $1.2.3.4 + $intKeys = @{ 1 = @{ 2 = @{ 3 = @{ 4 = 'test' } } } } + $intKeys.1.2.3.4 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } + + Context "Suppressed" { + It "All" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '', Justification = 'Test')] + param() + $version = 1.2.3 + $IP = [System.Net.IPAddress]127.0.0.1 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "1.2.3" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '1.2.3', Justification = 'Test')] + param() + $version = 1.2.3 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "127.0.0.1" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '127.0.0.1', Justification = 'Test')] + param() + $IP = [System.Net.IPAddress]127.0.0.1 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } + + Context "Fixing" { + + BeforeAll { # See request: #1938 + $tempFile = Join-Path $TestDrive 'TestScript.ps1' + } + + It "Version" { + Set-Content -LiteralPath $tempFile -Value {$version = 1.2.3}.ToString() -NoNewLine + $violations = Invoke-ScriptAnalyzer -Path $tempFile -fix + Get-Content -LiteralPath $tempFile -Raw | Should -Be {$version = '1.2.3'}.ToString() + } + + It "IP Address" { + Set-Content -LiteralPath $tempFile -Value {$IP = [System.Net.IPAddress]127.0.0.1}.ToString() -NoNewLine + $violations = Invoke-ScriptAnalyzer -Path $tempFile -fix + Get-Content -LiteralPath $tempFile -Raw | Should -Be {$IP = [System.Net.IPAddress]'127.0.0.1'}.ToString() + } + } +} \ No newline at end of file diff --git a/docs/Rules/InvalidMultiDotValue.md b/docs/Rules/InvalidMultiDotValue.md new file mode 100644 index 000000000..53f3701bc --- /dev/null +++ b/docs/Rules/InvalidMultiDotValue.md @@ -0,0 +1,45 @@ +--- +description: Invalid unquoted multi-dot value construction +ms.date: 04/24/2024 +ms.topic: reference +title: InvalidMultiDotValue +--- +# InvalidMultiDotValue + +**Severity Level: Error** + +## Description + +PowerShell does not support an implicit value with multiple dots. +Any *unquoted* value with 2 or more dots will not be treated as any special type (like a `version` or `IPAddress`) +but result in `$null`. These objects need to be constructed from either a quoted string (e.g. `[Version]'1.2.3'`) +or their individual components (e.g. `[Version]::new(1, 2, 3)`). + + +## Example + +### Wrong + +```powershell +$version = 1.2.3 +``` + +or even: + +```powershell +$IP = [System.Net.IPAddress]127.0.0.1 +``` + +Where both examples will result in `$null` instead of any specific object. + +### Correct + +```powershell +$version = [Version]'1.2.3' +# or: +$version = [Version]::new(1, 2, 3) +``` + +```powershell +$IP = [System.Net.IPAddress]'127.0.0.1' +``` diff --git a/docs/Rules/README.md b/docs/Rules/README.md index fca031e33..9cc1bea45 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -48,6 +48,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [DSCUseIdenticalMandatoryParametersForDSC](./DSCUseIdenticalMandatoryParametersForDSC.md) | Error | Yes | | | [DSCUseIdenticalParametersForDSC](./DSCUseIdenticalParametersForDSC.md) | Error | Yes | | | [DSCUseVerboseMessageInDSCResource](./DSCUseVerboseMessageInDSCResource.md) | Error | Yes | | +| [InvalidMultiDotValue](./InvalidMultiDotValue.md) | Error | Yes | | | [MisleadingBacktick](./MisleadingBacktick.md) | Warning | Yes | | | [MissingModuleManifestField](./MissingModuleManifestField.md) | Warning | Yes | | | [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes |