Skip to content

Commit 213d512

Browse files
tempora-mutanturtempora-mutantur
committed
Add the AvoidSemicolonsAsLineTerminators (#824)
It's a rule to warn about lines ending with a semicolon
1 parent a94d9f5 commit 213d512

8 files changed

+407
-1
lines changed

‎Engine/Formatter.cs

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public static string Format<TCmdlet>(
4545
"PSUseCorrectCasing",
4646
"PSAvoidUsingCmdletAliases",
4747
"PSAvoidUsingDoubleQuotesForConstantString",
48+
"PSAvoidSemicolonsAsLineTerminators"
4849
};
4950

5051
var text = new EditableText(scriptDefinition);

‎Rules/AvoidSemicolonsAsLineTerminators.cs

+175
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
#if !CORECLR
7+
using System.ComponentModel.Composition;
8+
#endif
9+
using System.Globalization;
10+
using System.Linq;
11+
using System.Management.Automation.Language;
12+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
13+
14+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
15+
{
16+
/// <summary>
17+
/// AvoidSemicolonsAsLineTerminators: Checks for lines to don't end with semicolons
18+
/// </summary>
19+
#if !CORECLR
20+
[Export(typeof(IScriptRule))]
21+
#endif
22+
public class AvoidSemicolonsAsLineTerminators : ConfigurableRule
23+
{
24+
/// <summary>
25+
/// Construct an object of AvoidSemicolonsAsLineTerminators type.
26+
/// </summary>
27+
public AvoidSemicolonsAsLineTerminators()
28+
{
29+
Enable = false;
30+
}
31+
32+
/// <summary>
33+
/// Analyzes the given ast to find violations.
34+
/// </summary>
35+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
36+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
37+
/// <returns>A an enumerable type containing the violations</returns>
38+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
39+
{
40+
if (ast == null)
41+
{
42+
throw new ArgumentNullException(nameof(ast));
43+
}
44+
45+
46+
var diagnosticRecords = new List<DiagnosticRecord>();
47+
48+
IEnumerable<Ast> assignmentStatements = ast.FindAll(item => item is AssignmentStatementAst, true);
49+
50+
var tokens = Helper.Instance.Tokens;
51+
for (int tokenIndex = 0; tokenIndex < tokens.Length; tokenIndex++)
52+
{
53+
54+
var token = tokens[tokenIndex];
55+
var semicolonTokenExtent = token.Extent;
56+
57+
var isSemicolonToken = token.Kind is TokenKind.Semi;
58+
if (!isSemicolonToken)
59+
{
60+
continue;
61+
}
62+
63+
var isPartOfAnyAssignmentStatement = assignmentStatements.Any(assignmentStatement => (assignmentStatement.Extent.EndOffset == semicolonTokenExtent.StartOffset + 1));
64+
if (isPartOfAnyAssignmentStatement)
65+
{
66+
continue;
67+
}
68+
69+
var nextTokenIndex = tokenIndex + 1;
70+
var isNextTokenIsNewLine = tokens[nextTokenIndex].Kind is TokenKind.NewLine;
71+
var isNextTokenIsEndOfInput = tokens[nextTokenIndex].Kind is TokenKind.EndOfInput;
72+
73+
if (!isNextTokenIsNewLine && !isNextTokenIsEndOfInput)
74+
{
75+
continue;
76+
}
77+
78+
var violationExtent = new ScriptExtent(
79+
new ScriptPosition(
80+
ast.Extent.File,
81+
semicolonTokenExtent.StartLineNumber,
82+
semicolonTokenExtent.StartColumnNumber,
83+
semicolonTokenExtent.StartScriptPosition.Line
84+
),
85+
new ScriptPosition(
86+
ast.Extent.File,
87+
semicolonTokenExtent.EndLineNumber,
88+
semicolonTokenExtent.EndColumnNumber,
89+
semicolonTokenExtent.EndScriptPosition.Line
90+
));
91+
92+
var suggestedCorrections = new List<CorrectionExtent>();
93+
suggestedCorrections.Add(new CorrectionExtent(
94+
violationExtent,
95+
string.Empty,
96+
ast.Extent.File
97+
));
98+
99+
var record = new DiagnosticRecord(
100+
String.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsError),
101+
violationExtent,
102+
GetName(),
103+
GetDiagnosticSeverity(),
104+
ast.Extent.File,
105+
null,
106+
suggestedCorrections
107+
);
108+
diagnosticRecords.Add(record);
109+
}
110+
111+
return diagnosticRecords;
112+
}
113+
114+
/// <summary>
115+
/// Retrieves the common name of this rule.
116+
/// </summary>
117+
public override string GetCommonName()
118+
{
119+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsCommonName);
120+
}
121+
122+
/// <summary>
123+
/// Retrieves the description of this rule.
124+
/// </summary>
125+
public override string GetDescription()
126+
{
127+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsDescription);
128+
}
129+
130+
/// <summary>
131+
/// Retrieves the name of this rule.
132+
/// </summary>
133+
public override string GetName()
134+
{
135+
return string.Format(
136+
CultureInfo.CurrentCulture,
137+
Strings.NameSpaceFormat,
138+
GetSourceName(),
139+
Strings.AvoidSemicolonsAsLineTerminatorsName);
140+
}
141+
142+
/// <summary>
143+
/// Retrieves the severity of the rule: error, warning or information.
144+
/// </summary>
145+
public override RuleSeverity GetSeverity()
146+
{
147+
return RuleSeverity.Warning;
148+
}
149+
150+
/// <summary>
151+
/// Gets the severity of the returned diagnostic record: error, warning, or information.
152+
/// </summary>
153+
/// <returns></returns>
154+
public DiagnosticSeverity GetDiagnosticSeverity()
155+
{
156+
return DiagnosticSeverity.Warning;
157+
}
158+
159+
/// <summary>
160+
/// Retrieves the name of the module/assembly the rule is from.
161+
/// </summary>
162+
public override string GetSourceName()
163+
{
164+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
165+
}
166+
167+
/// <summary>
168+
/// Retrieves the type of the rule, Builtin, Managed or Module.
169+
/// </summary>
170+
public override SourceType GetSourceType()
171+
{
172+
return SourceType.Builtin;
173+
}
174+
}
175+
}

‎Rules/Strings.Designer.cs

+36
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rules/Strings.resx

+12
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,18 @@
921921
<data name="AvoidLongLinesError" xml:space="preserve">
922922
<value>Line exceeds the configured maximum length of {0} characters</value>
923923
</data>
924+
<data name="AvoidSemicolonsAsLineTerminatorsName" xml:space="preserve">
925+
<value>AvoidSemicolonsAsLineTerminators</value>
926+
</data>
927+
<data name="AvoidSemicolonsAsLineTerminatorsCommonName" xml:space="preserve">
928+
<value>Avoid semicolons as line terminators</value>
929+
</data>
930+
<data name="AvoidSemicolonsAsLineTerminatorsDescription" xml:space="preserve">
931+
<value>Line should not end with a semicolon</value>
932+
</data>
933+
<data name="AvoidSemicolonsAsLineTerminatorsError" xml:space="preserve">
934+
<value>Line ends with a semicolon</value>
935+
</data>
924936
<data name="PlaceOpenBraceName" xml:space="preserve">
925937
<value>PlaceOpenBrace</value>
926938
</data>

‎Tests/Engine/GetScriptAnalyzerRule.tests.ps1

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Describe "Test Name parameters" {
6363

6464
It "get Rules with no parameters supplied" {
6565
$defaultRules = Get-ScriptAnalyzerRule
66-
$expectedNumRules = 66
66+
$expectedNumRules = 67
6767
if ($PSVersionTable.PSVersion.Major -le 4)
6868
{
6969
# for PSv3 PSAvoidGlobalAliases is not shipped because

‎Tests/Rules/AvoidSemicolonsAsLineTerminators.tests.ps1

+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
BeforeAll {
5+
$ruleName = "PSAvoidSemicolonsAsLineTerminators"
6+
7+
$ruleSettings = @{
8+
Enable = $true
9+
}
10+
$settings = @{
11+
IncludeRules = @($ruleName)
12+
Rules = @{ $ruleName = $ruleSettings }
13+
}
14+
}
15+
16+
Describe "AvoidSemicolonsAsLineTerminators" {
17+
Context "When the rule is not enabled explicitly" {
18+
It "Should not find violations" {
19+
$def = "'test';"
20+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def
21+
$violations.Count | Should -Be 0
22+
}
23+
}
24+
25+
Context "Given a line ending with a semicolon" {
26+
It "Should find one violation" {
27+
$def = "'test';"
28+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
29+
$violations.Count | Should -Be 1
30+
}
31+
}
32+
33+
Context "Given a line with a semicolon in the middle and one at the end" {
34+
It "Should find one violation" {
35+
$def = "'test';'test';"
36+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
37+
$violations[0].Extent.StartLineNumber | Should -Be 1
38+
$violations[0].Extent.EndLineNumber | Should -Be 1
39+
$violations[0].Extent.StartColumnNumber | Should -Be 14
40+
$violations[0].Extent.EndColumnNumber | Should -Be 15
41+
}
42+
}
43+
44+
45+
Context "Given a multiline string with a line ending with a semicolon" {
46+
It "Should get the correct extent of the violation for a single semicolon" {
47+
$def = @"
48+
'this line has no semicolon'
49+
'test';
50+
"@
51+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
52+
$violations[0].Extent.StartLineNumber | Should -Be 2
53+
$violations[0].Extent.EndLineNumber | Should -Be 2
54+
$violations[0].Extent.StartColumnNumber | Should -Be 7
55+
$violations[0].Extent.EndColumnNumber | Should -Be 8
56+
}
57+
}
58+
59+
Context "Given a multiline string with a line having a semicolon in the middle" {
60+
It "Should not find any violations" {
61+
$def = @"
62+
'this line has no semicolon'
63+
'line with a semicolon; in the middle'
64+
"@
65+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
66+
$violations.Count | Should -Be 0
67+
}
68+
}
69+
70+
Context "Given a multiline string with a line having a semicolon in C# code" {
71+
It "Should not find any violations" {
72+
$def = @"
73+
`$calcCode = `@"
74+
public class Calc{
75+
public int Add(int x,int y){
76+
return x+y;
77+
}
78+
}
79+
"`@
80+
81+
Add-Type -TypeDefinition `$calcCode -Language CSharp
82+
83+
`$c = New-Object Calc
84+
`$c.Add(1,2)
85+
"@
86+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
87+
$violations.Count | Should -Be 0
88+
}
89+
}
90+
91+
Context "Given a multiline string with a line having a semicolon in variable assignment" {
92+
It "Should not find any violations" {
93+
$def = @"
94+
`$allowPopupsOption=`@"
95+
lockPref("dom.disable_open_during_load", false);
96+
"`@
97+
Write `$allowPopupsOption | Out-File -Encoding UTF8 -FilePath `$pathToMozillaCfg -Append
98+
"@
99+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
100+
$violations.Count | Should -Be 0
101+
}
102+
}
103+
104+
Context "Given a line ending with a semicolon" {
105+
It "Should remove the semicolon at the end" {
106+
$def = "'test';"
107+
$expected = "'test'"
108+
109+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
110+
}
111+
}
112+
113+
Context "Given a line with a semicolon in the middle and one at the end" {
114+
It "Should remove the semicolon at the end" {
115+
$def = "'test';'test';"
116+
$expected = "'test';'test'"
117+
118+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
119+
}
120+
}
121+
122+
Context "Given a multiline string with a line ending with a semicolon" {
123+
It "Should remove the semicolon at the end of the line with a semicolon" {
124+
$def = @"
125+
'this line has no semicolon at the end'
126+
'test';
127+
"@
128+
$expected = @"
129+
'this line has no semicolon at the end'
130+
'test'
131+
"@
132+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
133+
}
134+
}
135+
}

0 commit comments

Comments
 (0)