Add new InvalidMultiDotValue rule

Rules/InvalidMultiDotValue.cs

@@ -0,0 +1,154 @@
+// 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
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public class InvalidMultiDotValue : ConfigurableRule
+    {
+
+        /// <summary>
+        /// Construct an object of InvalidMultiDotValue type.
+        /// </summary>
+        public InvalidMultiDotValue() {
+            Enable = false;
+        }
+
+        /// <summary>
+        /// Analyzes the given ast to find the [violation]
+        /// </summary>
+        /// <param name="ast">AST to be analyzed. This should be non-null</param>
+        /// <param name="fileName">Name of file that corresponds to the input AST.</param>
+        /// <returns>A an enumerable type containing the violations</returns>
+        public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
+
+            // Find all MemberExpressionAst nodes representing invalid unquoted multi-dot values
+            IEnumerable<Ast> 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<CorrectionExtent> {
+                        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
+                    );
+                }
+            }
+        }
+
+        /// <summary>
+        /// Retrieves the common name of this rule.
+        /// </summary>
+        public override string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.InvalidMultiDotValueCommonName);
+        }
+
+        /// <summary>
+        /// Retrieves the description of this rule.
+        /// </summary>
+        public override string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.InvalidMultiDotValueDescription);
+        }
+
+        /// <summary>
+        /// Retrieves the name of this rule.
+        /// </summary>
+        public override string GetName()
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.InvalidMultiDotValueName);
+        }
+
+        /// <summary>
+        /// Retrieves the severity of the rule: error, warning or information.
+        /// </summary>
+        public override RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Gets the severity of the returned diagnostic record: error, warning, or information.
+        /// </summary>
+        /// <returns></returns>
+        public DiagnosticSeverity GetDiagnosticSeverity()
+        {
+            return DiagnosticSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Retrieves the name of the module/assembly the rule is from.
+        /// </summary>
+        public override string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+
+        /// <summary>
+        /// Retrieves the type of the rule, Builtin, Managed or Module.
+        /// </summary>
+        public override SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+    }
+}
+

Rules/Strings.resx

@@ -1221,6 +1221,21 @@
   <data name="AvoidUsingAllowUnencryptedAuthenticationError" xml:space="preserve">
     <value>The insecure AllowUnencryptedAuthentication switch was used. This should be avoided except for compatibility with legacy systems.</value>
   </data>
+  <data name="InvalidMultiDotValueCommonName" xml:space="preserve">
+    <value>Invalid Multi-Dot Value</value>
+  </data>
+  <data name="InvalidMultiDotValueDescription" xml:space="preserve">
+    <value>PowerShell does not support an implicit value with multiple dots. Any unquoted value with 2 or more dots will result in `$null`.</value>
+  </data>
+  <data name="InvalidMultiDotValueName" xml:space="preserve">
+    <value>InvalidMultiDotValue</value>
+  </data>
+  <data name="InvalidMultiDotValueError" xml:space="preserve">
+    <value>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.</value>
+  </data>
+  <data name="InvalidMultiDotValueCorrectionDescription" xml:space="preserve">
+    <value>Quote the value that contains multiple dots</value>
+  </data>
   <data name="AvoidUsingAllowUnencryptedAuthenticationName" xml:space="preserve">
     <value>AvoidUsingAllowUnencryptedAuthentication</value>
   </data>

Tests/Rules/InvalidMultiDotValue.tests.ps1

@@ -0,0 +1,191 @@
+# 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" {
+
+    BeforeAll {
+        $Settings = @{
+            IncludeRules = @($ruleName)
+            Rules        = @{ $ruleName = @{ Enable = $true } }
+        }
+    }
+
+    Context "Violates" {
+        It "3 version components" {
+            $scriptDefinition = { $version = 1.2.3 }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "From version components" {
+            $scriptDefinition = { $Version = [Version]::new(1, 2, 3, 4) }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "From (bare) double" {
+            $scriptDefinition = { $Version = [Version]1.2 }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $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 -Settings $Settings
+            $violations | Should -BeNullOrEmpty
+        }
+    }
+
+    Context "Disabled" {
+
+        BeforeAll {
+            $Settings = @{
+                IncludeRules = @($ruleName)
+                Rules        = @{ $ruleName = @{ Enable = $false } }
+            }
+        }
+
+        It "ConvertFrom-SecureString -AsPlainText" {
+            $scriptDefinition = { $version = 1.2.3 }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings
+            $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 -Settings $Settings -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 -Settings $Settings -fix
+            Get-Content -LiteralPath $tempFile -Raw | Should -Be {$IP = [System.Net.IPAddress]'127.0.0.1'}.ToString()
+        }
+    }
+}