New rule (disabled by default): AvoidUsingDoubleQuotesForConstantString

Engine/Formatter.cs

@@ -44,6 +44,7 @@ public static string Format<TCmdlet>(
                 "PSAlignAssignmentStatement",
                 "PSUseCorrectCasing",
                 "PSAvoidUsingCmdletAliases",
+                "PSAvoidUsingDoubleQuotesForConstantString",
             };
 
             var text = new EditableText(scriptDefinition);

RuleDocumentation/AvoidUsingDoubleQuotesForConstantString.md

@@ -0,0 +1,21 @@
+# AvoidUsingDoubleQuotesForConstantString
+
+**Severity Level: Information**
+
+## Description
+
+When the value of a string is constant (i.e. not being interpolated by injecting variables or expression into such as e.g. `"$PID-$(hostname)"`), then single quotes should be used to express the constant nature of the string. This is not only to make the intent clearer that the string is a constant and makes it easier to use some special characters such as e.g. `$` within that string expression without the need to escape them. There are exceptions to that when double quoted strings are more readable though, e.g. when the string value itself has to contain a single quote (which would require a double single quotes to escape the character itself) or certain very special characters such as e.g. `"\n"` are already being escaped, the rule would not warn in this case as it is up to the author to decide on what is more readable in those cases.
+
+## Example
+
+### Wrong
+
+``` PowerShell
+$constantValue = "I Love PowerShell"
+```
+
+### Correct
+
+``` PowerShell
+$constantValue = 'I Love PowerShell'
+```

RuleDocumentation/README.md

@@ -17,6 +17,7 @@
 |[AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | |
 |[AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | |
 |[UseUsingScopeModifierInNewRunspaces](./UseUsingScopeModifierInNewRunspaces.md) | Warning | |
+|[AvoidUsingDoubleQuotesForConstantString](./AvoidUsingDoubleQuotesForConstantString.md) | Warning | Yes |
 |[AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md) | Warning | Yes |
 |[AvoidUsingComputerNameHardcoded](./AvoidUsingComputerNameHardcoded.md) | Error | |
 |[AvoidUsingConvertToSecureStringWithPlainText](./AvoidUsingConvertToSecureStringWithPlainText.md) | Error | |

Rules/AvoidUsingDoubleQuotesForConstantString.cs

@@ -0,0 +1,161 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+#if !CORECLR
+using System.ComponentModel.Composition;
+#endif
+using System.Globalization;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// AvoidUsingDoubleQuotesForConstantStrings: Checks if a string that uses double quotes contains a constant string, which could be simplified to a single quote.
+    /// </summary>
+#if !CORECLR
+    [Export(typeof(IScriptRule))]
+#endif
+    public class AvoidUsingDoubleQuotesForConstantString : ConfigurableRule
+    {
+        /// <summary>
+        /// Construct an object of type <seealso cref="AvoidUsingDoubleQuotesForConstantStrings"/>.
+        /// </summary>
+        public AvoidUsingDoubleQuotesForConstantString()
+        {
+            Enable = false;
+        }
+
+        /// <summary>
+        /// Analyzes the given ast to find occurences of StringConstantExpressionAst where double quotes are used
+        /// and could be replaced with single quotes.
+        /// </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(nameof(ast));
+            }
+
+            var stringConstantExpressionAsts = ast.FindAll(testAst => testAst is StringConstantExpressionAst, searchNestedScriptBlocks: true);
+            foreach (StringConstantExpressionAst stringConstantExpressionAst in stringConstantExpressionAsts)
+            {
+                switch (stringConstantExpressionAst.StringConstantType)
+                {
+                    /*
+                        If an interpolated string (i.e. using double quotes) uses single quotes (or @' in case of a here-string), then do not warn.
+                        This because one would then have to escape this single quote, which would make the string less readable.
+                        If an interpolated string contains a backtick character, then do not warn as well.
+                        This is because we'd have to replace the backtick in some cases like `$ and in others like `a it would not be possible at all.
+                        Therefore to keep it simple, all interpolated strings with a backtick are being excluded. 
+                    */
+                    case StringConstantType.DoubleQuoted:
+                        if (stringConstantExpressionAst.Value.Contains("'") || stringConstantExpressionAst.Extent.Text.Contains("`"))
+                        {
+                            break;
+                        }
+                        yield return GetDiagnosticRecord(stringConstantExpressionAst,
+                            $"'{stringConstantExpressionAst.Value}'");
+                        break;
+
+                    case StringConstantType.DoubleQuotedHereString:
+                        if (stringConstantExpressionAst.Value.Contains("@'") || stringConstantExpressionAst.Extent.Text.Contains("`"))
+                        {
+                            break;
+                        }
+                        yield return GetDiagnosticRecord(stringConstantExpressionAst,
+                            $"@'{Environment.NewLine}{stringConstantExpressionAst.Value}{Environment.NewLine}'@");
+                        break;
+
+                    default:
+                        break;
+                }
+            }
+        }
+
+        private DiagnosticRecord GetDiagnosticRecord(StringConstantExpressionAst stringConstantExpressionAst,
+            string suggestedCorrection)
+        {
+            return new DiagnosticRecord(
+                Strings.AvoidUsingDoubleQuotesForConstantStringError,
+                stringConstantExpressionAst.Extent,
+                GetName(),
+                GetDiagnosticSeverity(),
+                stringConstantExpressionAst.Extent.File,
+                suggestedCorrections: new[] {
+                    new CorrectionExtent(
+                        stringConstantExpressionAst.Extent,
+                        suggestedCorrection,
+                        stringConstantExpressionAst.Extent.File
+                    )
+                }
+            );
+        }
+
+        /// <summary>
+        /// Retrieves the common name of this rule.
+        /// </summary>
+        public override string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingDoubleQuotesForConstantStringCommonName);
+        }
+
+        /// <summary>
+        /// Retrieves the description of this rule.
+        /// </summary>
+        public override string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingDoubleQuotesForConstantStringDescription);
+        }
+
+        /// <summary>
+        /// Retrieves the name of this rule.
+        /// </summary>
+        public override string GetName()
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.AvoidUsingDoubleQuotesForConstantStringName);
+        }
+
+        /// <summary>
+        /// Retrieves the severity of the rule: error, warning or information.
+        /// </summary>
+        public override RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Information;
+        }
+
+        /// <summary>
+        /// Gets the severity of the returned diagnostic record: error, warning, or information.
+        /// </summary>
+        /// <returns></returns>
+        public DiagnosticSeverity GetDiagnosticSeverity()
+        {
+            return DiagnosticSeverity.Information;
+        }
+
+        /// <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

@@ -1137,4 +1137,16 @@
   <data name="UseUsingScopeModifierInNewRunspacesCorrectionDescription" xml:space="preserve">
     <value>Replace {0} with {1}</value>
   </data>
-</root>
+  <data name="AvoidUsingDoubleQuotesForConstantStringCommonName" xml:space="preserve">
+    <value>Avoid using double quotes if the string is constant.</value>
+  </data>
+  <data name="AvoidUsingDoubleQuotesForConstantStringDescription" xml:space="preserve">
+    <value>Use single quotes if the string is constant.</value>
+  </data>
+  <data name="AvoidUsingDoubleQuotesForConstantStringName" xml:space="preserve">
+    <value>AvoidUsingDoubleQuotesForConstantString</value>
+  </data>
+  <data name="AvoidUsingDoubleQuotesForConstantStringError" xml:space="preserve">
+    <value>Use single quotes when a string is constant.</value>
+  </data>
+</root>

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

@@ -63,7 +63,7 @@ Describe "Test Name parameters" {
 
         It "get Rules with no parameters supplied" {
             $defaultRules = Get-ScriptAnalyzerRule
-            $expectedNumRules = 64
+            $expectedNumRules = 65
             if ($IsCoreCLR -or ($PSVersionTable.PSVersion.Major -eq 3) -or ($PSVersionTable.PSVersion.Major -eq 4))
             {
                 # for PSv3 PSAvoidGlobalAliases is not shipped because
@@ -162,7 +162,7 @@ Describe "TestSeverity" {
 
     It "filters rules based on multiple severity inputs"{
         $rules = Get-ScriptAnalyzerRule -Severity Error,Information
-        $rules.Count | Should -Be 17
+        $rules.Count | Should -Be 18
     }
 
         It "takes lower case inputs" {

Tests/Rules/AvoidUsingDoubleQuotesForConstantString.tests.ps1

@@ -0,0 +1,102 @@
+$settings = @{
+    IncludeRules = @('PSAvoidUsingDoubleQuotesForConstantString')
+    Rules        = @{
+        PSAvoidUsingDoubleQuotesForConstantString = @{
+            Enable = $true
+        }
+    }
+}
+
+Describe 'AvoidUsingDoubleQuotesForConstantString' {
+    Context 'One line string' {
+        It 'Should warn if string is constant and double quotes are used' {
+            (Invoke-ScriptAnalyzer -ScriptDefinition '$item = "value"' -Settings $settings).Count | Should -Be 1
+        }
+
+        It 'Should correctly format if string is constant and double quotes are used' {
+            Invoke-Formatter -ScriptDefinition '$item = "value"' -Settings $settings | Should -Be "`$item = 'value'"
+        }
+
+        It 'Should not warn if string is interpolated and double quotes are used but single quotes are in value' {
+            Invoke-ScriptAnalyzer -ScriptDefinition "`$item = 'value'" -Settings $settings | Should -BeNullOrEmpty
+        }
+
+        It 'Should not warn if string is interpolated and double quotes are used but backtick character is in value' {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$item = "foo-`$-bar"' -Settings $settings | Should -BeNullOrEmpty
+        }
+
+        It 'Should not warn if string is constant and single quotes are used' {
+            Invoke-ScriptAnalyzer -ScriptDefinition "`$item = 'value'" -Settings $settings | Should -BeNullOrEmpty
+        }
+
+        It 'Should not warn if string is interpolated and double quotes are used' {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$item = "$(Get-Item)"' -Settings $settings | Should -BeNullOrEmpty
+        }
+
+        It 'Should not warn if string is interpolated and double quotes are used' {
+            Invoke-ScriptAnalyzer -ScriptDefinition '$item = "$(Get-Item)"' -Settings $settings | Should -BeNullOrEmpty
+        }
+    }
+
+    # TODO: check escape strings
+
+    Context 'Here string' {
+        It 'Should warn if string is constant and double quotes are used' {
+            $scriptDefinition = @'
+$item=@"
+value
+"@
+'@
+            (Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings).Count | Should -Be 1
+        }
+
+        It 'Should correctly format if string is constant and double quotes are used' {
+            $scriptDefinition = @'
+$item=@"
+value
+"@
+'@
+            Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be @"
+`$item=@'
+value
+'@
+"@
+        }
+
+        It 'Should not warn if string is constant and double quotes are used but @'' is used in value' {
+            $scriptDefinition = @'
+$item=@"
+value1@'value2
+"@
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings | Should -BeNullOrEmpty
+        }
+        It 'Should not warn if string is constant and double quotes are used but backtick is used in value' {
+            $scriptDefinition = @'
+$item=@"
+foo-`$-bar
+"@
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings | Should -BeNullOrEmpty
+        }
+
+        It 'Should not warn if string is constant and single quotes are used' {
+            $scriptDefinition = @"
+`$item=@'
+value
+'@
+"@
+            Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings | Should -BeNullOrEmpty
+        }
+
+        It 'Should not warn if string is interpolated' {
+            $scriptDefinition = @'
+$item=@"
+$(Get-Process)
+"@
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings | Should -BeNullOrEmpty
+        }
+    }
+
+}