Skip to content

Commit e997906

Browse files
author
Kapil Borle
committed
Modify CorrectionExtent class
The class does not implement IScriptExtent interface anymore. The purpose of CorrectionExtent is to emit violation extent and the correction text but this behaviour is not consistent with the IScriptExtent interface.
1 parent 50706a1 commit e997906

7 files changed

Lines changed: 85 additions & 85 deletions

File tree

Engine/Generic/CorrectionExtent.cs

Lines changed: 5 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
1010
{
11-
public class CorrectionExtent : IScriptExtent
11+
public class CorrectionExtent
1212
{
1313
public int EndColumnNumber
1414
{
@@ -26,22 +26,6 @@ public int EndLineNumber
2626
}
2727
}
2828

29-
public int EndOffset
30-
{
31-
get
32-
{
33-
throw new NotImplementedException();
34-
}
35-
}
36-
37-
public IScriptPosition EndScriptPosition
38-
{
39-
get
40-
{
41-
throw new NotImplementedException();
42-
}
43-
}
44-
4529
public string File
4630
{
4731
get
@@ -66,22 +50,6 @@ public int StartLineNumber
6650
}
6751
}
6852

69-
public int StartOffset
70-
{
71-
get
72-
{
73-
throw new NotImplementedException();
74-
}
75-
}
76-
77-
public IScriptPosition StartScriptPosition
78-
{
79-
get
80-
{
81-
throw new NotImplementedException();
82-
}
83-
}
84-
8553
public string Text
8654
{
8755
get
@@ -97,7 +65,7 @@ public string Text
9765
private int endColumnNumber;
9866
private string text;
9967

100-
public CorrectionExtent(string file, int startLineNumber, int endLineNumber, int startColumnNumber, int endColumnNumber, string text)
68+
public CorrectionExtent(int startLineNumber, int endLineNumber, int startColumnNumber, int endColumnNumber, string text, string file)
10169
{
10270
this.startLineNumber = startLineNumber;
10371
this.endLineNumber = endLineNumber;
@@ -113,34 +81,12 @@ private void ThrowIfInvalidArguments()
11381
ThrowIfNull<string>(file, "filename");
11482
ThrowIfNull<string>(text, "text");
11583
ThrowIfDecreasing(startLineNumber, endLineNumber, "start line number cannot be less than end line number");
116-
ThrowIfTextNotConsistent();
117-
}
118-
119-
private void ThrowIfTextNotConsistent()
120-
{
121-
using (var stringReader = new StringReader(text))
84+
if (startLineNumber == endLineNumber)
12285
{
123-
int numLines = 0;
124-
int expectedNumLines = endLineNumber - startLineNumber + 1;
125-
for (string line = stringReader.ReadLine(); line != null; line = stringReader.ReadLine())
126-
{
127-
numLines++;
128-
}
129-
if (numLines != expectedNumLines)
130-
{
131-
throw new ArgumentException("number of lines not consistent with text argument");
132-
}
133-
if (numLines == 1)
134-
{
135-
ThrowIfDecreasing(startColumnNumber, endColumnNumber, "start column number cannot be less then end column number");
136-
if (endColumnNumber - startColumnNumber != text.Length)
137-
{
138-
throw new ArgumentException("column numbers are inconsistent with the length of the string");
139-
}
140-
}
86+
ThrowIfDecreasing(StartColumnNumber, endColumnNumber, "start column number cannot be less than end column number for a one line extent");
14187
}
14288
}
143-
89+
14490
private void ThrowIfDecreasing(int start, int end, string message)
14591
{
14692
if (start > end)

Engine/Generic/DiagnosticRecord.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
// THE SOFTWARE.
1111
//
1212

13+
using System.Collections.Generic;
1314
using System.Management.Automation.Language;
1415

1516
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
@@ -26,7 +27,7 @@ public class DiagnosticRecord
2627
private DiagnosticSeverity severity;
2728
private string scriptName;
2829
private string ruleSuppressionId;
29-
private string suggestedCorrection;
30+
private List<CorrectionExtent> suggestedCorrections;
3031

3132
/// <summary>
3233
/// Represents a string from the rule about why this diagnostic was created.
@@ -96,9 +97,9 @@ public string RuleSuppressionID
9697
/// Returns suggested correction
9798
/// return value can be null
9899
/// </summary>
99-
public string SuggestedCorrection
100+
public List<CorrectionExtent> SuggestedCorrections
100101
{
101-
get { return suggestedCorrection; }
102+
get { return suggestedCorrections; }
102103
}
103104

104105
/// <summary>
@@ -117,16 +118,16 @@ public DiagnosticRecord()
117118
/// <param name="ruleName">The name of the rule that created this diagnostic</param>
118119
/// <param name="severity">The severity of this diagnostic</param>
119120
/// <param name="scriptName">The name of the script file being analyzed</param>
120-
/// <param name="suggestedCorrection">The correction suggested by the rule to replace the extent text</param>
121-
public DiagnosticRecord(string message, IScriptExtent extent, string ruleName, DiagnosticSeverity severity, string scriptName, string ruleId = null, string suggestedCorrection = null)
121+
/// <param name="suggestedCorrections">The correction suggested by the rule to replace the extent text</param>
122+
public DiagnosticRecord(string message, IScriptExtent extent, string ruleName, DiagnosticSeverity severity, string scriptName, string ruleId = null, List<CorrectionExtent> suggestedCorrections = null)
122123
{
123124
Message = string.IsNullOrEmpty(message) ? string.Empty : message;
124125
RuleName = string.IsNullOrEmpty(ruleName) ? string.Empty : ruleName;
125126
Extent = extent;
126127
Severity = severity;
127128
ScriptName = string.IsNullOrEmpty(scriptName) ? string.Empty : scriptName;
128129
ruleSuppressionId = ruleId;
129-
this.suggestedCorrection = suggestedCorrection;
130+
this.suggestedCorrections = suggestedCorrections;
130131
}
131132
}
132133

Rules/AvoidAlias.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,31 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
6060
DiagnosticSeverity.Warning,
6161
fileName,
6262
aliasName,
63-
cmdletName);
63+
suggestedCorrections: GetCorrectionExtent(cmdAst, cmdletName));
6464
}
6565
}
6666
}
6767

68+
/// <summary>
69+
/// Creates a list containing suggested correction
70+
/// </summary>
71+
/// <param name="cmdAst"></param>
72+
/// <param name="cmdletName"></param>
73+
/// <returns>Returns a list of suggested corrections</returns>
74+
private List<CorrectionExtent> GetCorrectionExtent(CommandAst cmdAst, string cmdletName)
75+
{
76+
var corrections = new List<CorrectionExtent>();
77+
var ext = cmdAst.Extent;
78+
corrections.Add(new CorrectionExtent(
79+
ext.StartLineNumber,
80+
ext.EndLineNumber,
81+
ext.StartColumnNumber,
82+
ext.EndColumnNumber,
83+
cmdletName,
84+
ext.File));
85+
return corrections;
86+
}
87+
6888
/// <summary>
6989
/// GetName: Retrieves the name of this rule.
7090
/// </summary>

Rules/MisleadingBacktick.cs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,38 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5555
{
5656
int lineNumber = ast.Extent.StartLineNumber + i;
5757

58-
ScriptPosition start = new ScriptPosition(fileName, lineNumber, match.Index, line);
59-
ScriptPosition end = new ScriptPosition(fileName, lineNumber, match.Index + match.Length, line);
58+
var start = new ScriptPosition(fileName, lineNumber, match.Index, line);
59+
var end = new ScriptPosition(fileName, lineNumber, match.Index + match.Length, line);
60+
var extent = new ScriptExtent(start, end);
6061
yield return new DiagnosticRecord(
6162
string.Format(CultureInfo.CurrentCulture, Strings.MisleadingBacktickError),
62-
new ScriptExtent(start, end),
63+
extent,
6364
GetName(),
6465
DiagnosticSeverity.Warning,
6566
fileName,
66-
suggestedCorrection:string.Empty);
67+
suggestedCorrections: GetCorrectionExtent(extent));
6768
}
6869
}
6970
}
7071

72+
/// <summary>
73+
/// Creates a list containing suggested correction
74+
/// </summary>
75+
/// <param name="cmdAst"></param>
76+
/// <returns>Returns a list of suggested corrections</returns>
77+
private List<CorrectionExtent> GetCorrectionExtent(IScriptExtent violationExtent)
78+
{
79+
var corrections = new List<CorrectionExtent>();
80+
corrections.Add(new CorrectionExtent(
81+
violationExtent.StartLineNumber,
82+
violationExtent.EndLineNumber,
83+
violationExtent.StartColumnNumber,
84+
violationExtent.EndColumnNumber,
85+
String.Empty,
86+
violationExtent.File));
87+
return corrections;
88+
}
89+
7190
/// <summary>
7291
/// GetName: Retrieves the name of this rule.
7392
/// </summary>

Tests/Engine/CorrectionExtent.tests.ps1

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,29 @@ if (!(Get-Module PSScriptAnalyzer))
33
Import-Module PSScriptAnalyzer
44
}
55

6-
76
Describe "Correction Extent" {
8-
Context "It should throw for invalid arguments" {
9-
It "throw if end line number is less than start line number" {
10-
$filename = "newfile"
11-
$startLineNumber = 2
12-
$endLineNumber = 1
13-
$startColumnNumber = 1
7+
$type = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent]
8+
9+
Context "Object construction" {
10+
It "creates the object with correct properties" {
11+
$correctionExtent = $type::new(1, 1, 1, 3, "get-childitem", "newfile")
1412

13+
$correctionExtent.StartLineNumber | Should Be 1
14+
$correctionExtent.EndLineNumber | Should Be 1
15+
$correctionExtent.StartColumnNumber | Should Be 1
16+
$correctionExtent.EndColumnNumber | Should be 3
17+
$correctionExtent.Text | Should Be "get-childitem"
18+
$correctionExtent.File | Should Be "newfile"
19+
}
20+
21+
It "throws if end line number is less than start line number" {
1522
$text = "Get-ChildItem"
16-
$endColumnNumber = $text.Length
17-
{[Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent]::new($filename, $startLineNumber, $startColumnNumber, $endLineNumber, $endColumnNumber, "T")} | Should Throw
23+
{$type::new(2, 1, 1, $text.Length + 1, $text, "newfile")} | Should Throw "start line number"
24+
}
25+
26+
It "throws if end column number is less than start column number for same line" {
27+
$text = "start-process"
28+
{$type::new(1, 1, 2, 1, $text, "newfile")} | Should Throw "start column number"
1829
}
1930
}
2031
}

Tests/Rules/AvoidUsingAlias.tests.ps1

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ Describe "AvoidUsingAlias" {
1515
$violations[1].Message | Should Match $violationMessage
1616
}
1717

18-
It "suggests correction" {
19-
$violations[0].SuggestedCorrection | Should Be 'Invoke-Expression'
20-
$violations[1].SuggestedCorrection | Should Be 'Clear-Host'
21-
}
18+
It "suggests correction" {
19+
$violations[0].SuggestedCorrections.Count | Should Be 1
20+
$violations[0].SuggestedCorrections.Text | Should Be 'Invoke-Expression'
21+
22+
$violations[1].SuggestedCorrections.Count | Should Be 1
23+
$violations[1].SuggestedCorrections.Text | Should Be 'Clear-Host'
24+
}
2225
}
2326

2427
Context "When there are no violations" {

Tests/Rules/MisleadingBacktick.tests.ps1

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ Describe "Avoid Misleading Backticks" {
1111

1212
foreach ($violation in $violations)
1313
{
14-
$violation.SuggestedCorrection | Should Not Be $null
15-
$violation.SuggestedCorrection | Should BeNullOrEmpty
14+
$violation.SuggestedCorrections.Count | Should Be 1
15+
$violation.SuggestedCorrections[0].Text | Should Be ''
1616
}
1717
}
1818
}

0 commit comments

Comments
 (0)