Skip to content

Commit eb6cebe

Browse files
iRon7CopilotCopilotsdwheeler
authored
Add new InvalidMultiDotValue rule (#2180)
* Add new InvalidMultiDotValue rule * Fixed several issues as commented by Liam. Co-authored-by: Copilot <copilot@github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Improved wording based on Copilot suggestions. * Update docs/Rules/InvalidMultiDotValue.md Co-authored-by: Sean Wheeler <sean.wheeler@microsoft.com> * Update docs/Rules/InvalidMultiDotValue.md Co-authored-by: Sean Wheeler <sean.wheeler@microsoft.com> * **opt-in by default.** * Remove dead code and change `PSAvoidExclaimOperator` from documentation --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Sean Wheeler <sean.wheeler@microsoft.com>
1 parent 6dadae3 commit eb6cebe

5 files changed

Lines changed: 424 additions & 0 deletions

File tree

‎Rules/InvalidMultiDotValue.cs‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Globalization;
8+
using System.Management.Automation.Language;
9+
using System.Linq;
10+
11+
12+
13+
#if !CORECLR
14+
using System.ComponentModel.Composition;
15+
#endif
16+
17+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
18+
{
19+
#if !CORECLR
20+
[Export(typeof(IScriptRule))]
21+
#endif
22+
23+
/// <summary>
24+
/// Rule that reports an error when an unquoted value contains multiple dots,
25+
/// which is likely an attempt to construct a version number (e.g., 1.2.3)
26+
/// that is not properly quoted and thus misinterpreted as a double with member access.
27+
/// </summary>
28+
public class InvalidMultiDotValue : ConfigurableRule
29+
{
30+
31+
/// <summary>
32+
/// Construct an object of InvalidMultiDotValue type.
33+
/// </summary>
34+
public InvalidMultiDotValue() {
35+
Enable = false;
36+
}
37+
38+
/// <summary>
39+
/// Analyzes the given ast to find the [violation]
40+
/// </summary>
41+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
42+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
43+
/// <returns>A an enumerable type containing the violations</returns>
44+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
45+
{
46+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
47+
48+
// Find all MemberExpressionAst nodes representing invalid unquoted multi-dot values
49+
IEnumerable<MemberExpressionAst> invalidAsts = ast.FindAll(testAst =>
50+
// An expression with 3 or more dots is seen as a double with an additional property
51+
testAst is MemberExpressionAst memberAst &&
52+
// The first two values are seen as a double
53+
memberAst.Expression.StaticType == typeof(double) &&
54+
// the rest is seen as a member of type int or double
55+
memberAst.Member is ConstantExpressionAst constantAst &&
56+
(
57+
constantAst.StaticType == typeof(int) || // e.g.: [Version]1.2.3
58+
constantAst.StaticType == typeof(double) // e.g.: [Version]1.2.3.4
59+
),
60+
true
61+
).Cast<MemberExpressionAst>();
62+
63+
var correctionDescription = Strings.InvalidMultiDotValueCorrectionDescription;
64+
foreach (MemberExpressionAst invalidAst in invalidAsts)
65+
{
66+
var corrections = new List<CorrectionExtent> {
67+
new CorrectionExtent(
68+
invalidAst.Extent.StartLineNumber,
69+
invalidAst.Extent.EndLineNumber,
70+
invalidAst.Extent.StartColumnNumber,
71+
invalidAst.Extent.EndColumnNumber,
72+
"'" + invalidAst.Extent.Text + "'",
73+
fileName,
74+
correctionDescription
75+
)
76+
};
77+
yield return new DiagnosticRecord(
78+
string.Format(
79+
CultureInfo.CurrentCulture,
80+
Strings.InvalidMultiDotValueError,
81+
invalidAst.Extent.Text
82+
),
83+
invalidAst.Extent,
84+
GetName(),
85+
DiagnosticSeverity.Error,
86+
fileName,
87+
invalidAst.Extent.Text,
88+
suggestedCorrections: corrections
89+
);
90+
}
91+
}
92+
93+
/// <summary>
94+
/// Retrieves the common name of this rule.
95+
/// </summary>
96+
public override string GetCommonName()
97+
{
98+
return string.Format(CultureInfo.CurrentCulture, Strings.InvalidMultiDotValueCommonName);
99+
}
100+
101+
/// <summary>
102+
/// Retrieves the description of this rule.
103+
/// </summary>
104+
public override string GetDescription()
105+
{
106+
return string.Format(CultureInfo.CurrentCulture, Strings.InvalidMultiDotValueDescription);
107+
}
108+
109+
/// <summary>
110+
/// Retrieves the name of this rule.
111+
/// </summary>
112+
public override string GetName()
113+
{
114+
return string.Format(
115+
CultureInfo.CurrentCulture,
116+
Strings.NameSpaceFormat,
117+
GetSourceName(),
118+
Strings.InvalidMultiDotValueName);
119+
}
120+
121+
/// <summary>
122+
/// Retrieves the severity of the rule: error, warning or information.
123+
/// </summary>
124+
public override RuleSeverity GetSeverity()
125+
{
126+
return RuleSeverity.Warning;
127+
}
128+
129+
/// <summary>
130+
/// Gets the severity of the returned diagnostic record: error, warning, or information.
131+
/// </summary>
132+
/// <returns></returns>
133+
public DiagnosticSeverity GetDiagnosticSeverity()
134+
{
135+
return DiagnosticSeverity.Warning;
136+
}
137+
138+
/// <summary>
139+
/// Retrieves the name of the module/assembly the rule is from.
140+
/// </summary>
141+
public override string GetSourceName()
142+
{
143+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
144+
}
145+
146+
/// <summary>
147+
/// Retrieves the type of the rule, Builtin, Managed or Module.
148+
/// </summary>
149+
public override SourceType GetSourceType()
150+
{
151+
return SourceType.Builtin;
152+
}
153+
}
154+
}
155+

‎Rules/Strings.resx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,21 @@
12451245
<data name="AvoidUsingAllowUnencryptedAuthenticationError" xml:space="preserve">
12461246
<value>The insecure AllowUnencryptedAuthentication switch was used. This should be avoided except for compatibility with legacy systems.</value>
12471247
</data>
1248+
<data name="InvalidMultiDotValueCommonName" xml:space="preserve">
1249+
<value>Invalid Multi-Dot Value</value>
1250+
</data>
1251+
<data name="InvalidMultiDotValueDescription" xml:space="preserve">
1252+
<value>PowerShell does not support an implicit value with multiple dots. Any unquoted value with 2 or more dots will result in `$null`.</value>
1253+
</data>
1254+
<data name="InvalidMultiDotValueName" xml:space="preserve">
1255+
<value>InvalidMultiDotValue</value>
1256+
</data>
1257+
<data name="InvalidMultiDotValueError" xml:space="preserve">
1258+
<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>
1259+
</data>
1260+
<data name="InvalidMultiDotValueCorrectionDescription" xml:space="preserve">
1261+
<value>Quote the value that contains multiple dots</value>
1262+
</data>
12481263
<data name="AvoidUsingAllowUnencryptedAuthenticationName" xml:space="preserve">
12491264
<value>AvoidUsingAllowUnencryptedAuthentication</value>
12501265
</data>

‎Tests/Rules/InvalidMultiDotValue.tests.ps1‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')]
5+
param()
6+
7+
BeforeAll {
8+
$ruleName = "PSInvalidMultiDotValue"
9+
$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."
10+
$correctionDescription = 'Quote the value that contains multiple dots'
11+
}
12+
13+
Describe "InvalidMultiDotValue" {
14+
15+
BeforeAll {
16+
$Settings = @{
17+
IncludeRules = @($ruleName)
18+
Rules = @{ $ruleName = @{ Enable = $true } }
19+
}
20+
}
21+
22+
Context "Violates" {
23+
It "3 version components" {
24+
$scriptDefinition = { $version = 1.2.3 }.ToString()
25+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
26+
$violations.Count | Should -Be 1
27+
$violations.Severity | Should -Be Error
28+
$violations.Extent.Text | Should -Be '1.2.3'
29+
$violations.Message | Should -Be ($ruleMessage -f '1.2.3')
30+
$violations.RuleSuppressionID | Should -Be '1.2.3'
31+
$violations.SuggestedCorrections.Text | Should -Be "'1.2.3'"
32+
$violations.SuggestedCorrections.Description | Should -Be $correctionDescription
33+
}
34+
35+
It "4 version components" {
36+
$scriptDefinition = { $version = 1.2.3.4 }.ToString()
37+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
38+
$violations.Count | Should -Be 1
39+
$violations.Severity | Should -Be Error
40+
$violations.Extent.Text | Should -Be '1.2.3.4'
41+
$violations.Message | Should -Be ($ruleMessage -f '1.2.3.4')
42+
$violations.RuleSuppressionID | Should -Be '1.2.3.4'
43+
$violations.SuggestedCorrections.Text | Should -Be "'1.2.3.4'"
44+
$violations.SuggestedCorrections.Description | Should -Be $correctionDescription
45+
}
46+
47+
48+
It "With class initializer" {
49+
$scriptDefinition = { $version = [Version]1.2.3 }.ToString()
50+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
51+
$violations.Count | Should -Be 1
52+
$violations.Severity | Should -Be Error
53+
$violations.Extent.Text | Should -Be '1.2.3'
54+
$violations.Message | Should -Be ($ruleMessage -f '1.2.3')
55+
$violations.RuleSuppressionID | Should -Be '1.2.3'
56+
$violations.SuggestedCorrections.Text | Should -Be "'1.2.3'"
57+
$violations.SuggestedCorrections.Description | Should -Be $correctionDescription
58+
}
59+
60+
It "As parameter" {
61+
$scriptDefinition = {
62+
param(
63+
[Version]$version = 1.2.3
64+
)
65+
Write-Verbose $version
66+
}.ToString()
67+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
68+
$violations.Count | Should -Be 1
69+
$violations.Severity | Should -Be Error
70+
$violations.Extent.Text | Should -Be '1.2.3'
71+
$violations.Message | Should -Be ($ruleMessage -f '1.2.3')
72+
$violations.RuleSuppressionID | Should -Be '1.2.3'
73+
$violations.SuggestedCorrections.Text | Should -Be "'1.2.3'"
74+
$violations.SuggestedCorrections.Description | Should -Be $correctionDescription
75+
}
76+
77+
# Even an IP address is apparently expect below.
78+
# The violation message and description presume a version
79+
# is expected because this is the more commonly used type.
80+
It "IP Address" {
81+
$scriptDefinition = { $IP = [System.Net.IPAddress]127.0.0.1 }.ToString()
82+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
83+
$violations.Count | Should -Be 1
84+
$violations.Severity | Should -Be Error
85+
$violations.Extent.Text | Should -Be '127.0.0.1'
86+
$violations.Message | Should -Be ($ruleMessage -f '127.0.0.1')
87+
$violations.RuleSuppressionID | Should -Be '127.0.0.1'
88+
$violations.SuggestedCorrections.Text | Should -Be "'127.0.0.1'"
89+
$violations.SuggestedCorrections.Description | Should -Be $correctionDescription
90+
}
91+
}
92+
93+
Context "Compliant" {
94+
It "From string" {
95+
$scriptDefinition = { $Version = [Version]'1.2.3' }.ToString()
96+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
97+
$violations | Should -BeNullOrEmpty
98+
}
99+
100+
It "From version components" {
101+
$scriptDefinition = { $Version = [Version]::new(1, 2, 3, 4) }.ToString()
102+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
103+
$violations | Should -BeNullOrEmpty
104+
}
105+
106+
It "From (bare) double" {
107+
$scriptDefinition = { $Version = [Version]1.2 }.ToString()
108+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
109+
$violations | Should -BeNullOrEmpty
110+
}
111+
112+
113+
It "Dot notation" { #PowerShell:27356
114+
$scriptDefinition = {
115+
$1.2.3.4
116+
$intKeys = @{ 1 = @{ 2 = @{ 3 = @{ 4 = 'test' } } } }
117+
$intKeys.1.2.3.4
118+
}.ToString()
119+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
120+
$violations | Should -BeNullOrEmpty
121+
}
122+
}
123+
124+
Context "Disabled" {
125+
126+
BeforeAll {
127+
$Settings = @{
128+
IncludeRules = @($ruleName)
129+
Rules = @{ $ruleName = @{ Enable = $false } }
130+
}
131+
}
132+
133+
It "ConvertFrom-SecureString -AsPlainText" {
134+
$scriptDefinition = { $version = 1.2.3 }.ToString()
135+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
136+
$violations | Should -BeNullOrEmpty
137+
}
138+
}
139+
140+
Context "Suppressed" {
141+
It "All" {
142+
$scriptDefinition = {
143+
[Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '', Justification = 'Test')]
144+
param()
145+
$version = 1.2.3
146+
$IP = [System.Net.IPAddress]127.0.0.1
147+
}.ToString()
148+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
149+
$violations | Should -BeNullOrEmpty
150+
}
151+
152+
It "1.2.3" {
153+
$scriptDefinition = {
154+
[Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '1.2.3', Justification = 'Test')]
155+
param()
156+
$version = 1.2.3
157+
}.ToString()
158+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
159+
$violations | Should -BeNullOrEmpty
160+
}
161+
162+
It "127.0.0.1" {
163+
$scriptDefinition = {
164+
[Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '127.0.0.1', Justification = 'Test')]
165+
param()
166+
$IP = [System.Net.IPAddress]127.0.0.1
167+
}.ToString()
168+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
169+
$violations | Should -BeNullOrEmpty
170+
}
171+
}
172+
173+
Context "Fixing" {
174+
175+
BeforeAll { # See request: #1938
176+
$tempFile = Join-Path $TestDrive 'TestScript.ps1'
177+
}
178+
179+
It "Version" {
180+
Set-Content -LiteralPath $tempFile -Value {$version = 1.2.3}.ToString() -NoNewLine
181+
$violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix
182+
Get-Content -LiteralPath $tempFile -Raw | Should -Be {$version = '1.2.3'}.ToString()
183+
}
184+
185+
It "IP Address" {
186+
Set-Content -LiteralPath $tempFile -Value {$IP = [System.Net.IPAddress]127.0.0.1}.ToString() -NoNewLine
187+
$violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix
188+
Get-Content -LiteralPath $tempFile -Raw | Should -Be {$IP = [System.Net.IPAddress]'127.0.0.1'}.ToString()
189+
}
190+
}
191+
}

0 commit comments

Comments
 (0)