Skip to content

Commit 94f298d

Browse files
author
Kapil Borle
committed
Update CloseBraceRule to use only tokens
1 parent 8593797 commit 94f298d

2 files changed

Lines changed: 132 additions & 126 deletions

File tree

‎Rules/PlaceCloseBrace.cs‎

Lines changed: 125 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -41,186 +41,188 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4141
throw new ArgumentNullException("ast");
4242
}
4343

44-
// TODO Given that we need to make exceptions for
45-
// ScriptBlockExpressionAst and NamedBlockAst, a
46-
// simpler approach using only tokens seems more
47-
// robust - for every close brace, check the position
48-
// of its corresponding open brace. If the open brace
49-
// is on a line by itself, use its identation to decide
50-
// close brace's indentation. If the open brace is
51-
// preceded by any non new line token then find the
52-
// fist keyword on the line and use its indentation for
53-
// the close brace
44+
// TODO Should have the following options
45+
// * no-empty-line-before
46+
// * on-new-line
47+
// * align
5448

55-
var tokens = Helper.Instance.Tokens.ToList();
56-
var astTokenMap = new Dictionary<Ast, List<Token>>();
57-
var violationTokens = new HashSet<Token>();
49+
var tokens = Helper.Instance.Tokens;
5850
var diagnosticRecords = new List<DiagnosticRecord>();
59-
var astItems = ast.FindAll(x => x is ScriptBlockAst
60-
|| x is StatementBlockAst
61-
|| x is NamedBlockAst,
62-
true);
63-
foreach (var astItem in astItems)
51+
var openBracePosStack = new Stack<int>();
52+
53+
for (int k = 0; k < tokens.Length; k++)
6454
{
65-
var astTokens = GetTokens(astItem, tokens, ref astTokenMap);
66-
AddToDiagnosticRecords(
67-
GetViolationForBraceOnSameLine(astItem, astTokens, fileName, ref violationTokens),
68-
ref diagnosticRecords);
55+
var token = tokens[k];
56+
if (token.Kind == TokenKind.LCurly)
57+
{
58+
openBracePosStack.Push(k);
59+
continue;
60+
}
6961

70-
AddToDiagnosticRecords(
71-
GetViolationForEmptyLineBeforeBrace(astItem, astTokens, fileName, ref violationTokens),
72-
ref diagnosticRecords);
62+
if (token.Kind == TokenKind.RCurly)
63+
{
64+
if (openBracePosStack.Count > 0)
65+
{
66+
var openBracePos = openBracePosStack.Pop();
67+
AddToDiagnosticRecords(
68+
GetViolationForBraceOnSameLine(tokens, k, openBracePos, fileName),
69+
ref diagnosticRecords);
70+
AddToDiagnosticRecords(
71+
GetViolationForEmptyLineBeforeBrace(tokens, k, openBracePos, fileName),
72+
ref diagnosticRecords);
73+
}
74+
else
75+
{
76+
break;
77+
}
78+
}
7379
}
7480

7581
return diagnosticRecords;
7682
}
7783

78-
private void AddToDiagnosticRecords(
79-
DiagnosticRecord diagnosticRecord,
80-
ref List<DiagnosticRecord> diagnosticRecords)
84+
private DiagnosticRecord GetViolationForEmptyLineBeforeBrace(Token[] tokens, int closeBracePos, int openBracePos, string fileName)
8185
{
82-
if (diagnosticRecord != null)
86+
if (tokens.Length > 2 && tokens.Length > closeBracePos)
8387
{
84-
diagnosticRecords.Add(diagnosticRecord);
88+
var closeBraceToken = tokens[closeBracePos];
89+
var newLineToken = tokens[closeBracePos - 1];
90+
var extraNewLineToken = tokens[closeBracePos - 2];
91+
if (newLineToken.Kind == TokenKind.NewLine
92+
&& extraNewLineToken.Kind == TokenKind.NewLine)
93+
{
94+
return new DiagnosticRecord(
95+
"Extra new line before close brace",
96+
closeBraceToken.Extent,
97+
GetName(),
98+
GetDiagnosticSeverity(),
99+
fileName,
100+
null,
101+
GetSuggestedCorrectionsForEmptyLineBeforeBrace(tokens, closeBracePos, openBracePos, fileName));
102+
}
85103
}
104+
105+
return null;
86106
}
87107

88-
private List<Token> GetTokens(Ast ast, List<Token> tokens, ref Dictionary<Ast, List<Token>> astTokenMap)
108+
private List<CorrectionExtent> GetSuggestedCorrectionsForEmptyLineBeforeBrace(Token[] tokens, int closeBracePos, int openBracePos, string fileName)
89109
{
90-
if (astTokenMap.Keys.Contains(ast))
91-
{
92-
return astTokenMap[ast];
93-
}
110+
var corrections = new List<CorrectionExtent>();
111+
var newLineToken = tokens[closeBracePos - 2];
112+
var closeBraceToken = tokens[closeBracePos];
113+
corrections.Add(new CorrectionExtent(
114+
newLineToken.Extent.StartLineNumber,
115+
closeBraceToken.Extent.EndLineNumber,
116+
newLineToken.Extent.StartColumnNumber,
117+
closeBraceToken.Extent.EndColumnNumber,
118+
newLineToken.Text + GetIndentation(tokens, closeBracePos, openBracePos) + closeBraceToken.Text,
119+
fileName));
120+
return corrections;
121+
}
94122

95-
// check if any parent upstream is in the cache
96-
var parentAst = ast.Parent;
97-
while (parentAst != null
98-
&& !astTokenMap.Keys.Contains(parentAst))
123+
private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePos)
124+
{
125+
// if open brace on a new line by itself, use its indentation
126+
var openBraceToken = tokens[openBracePos];
127+
if (tokens[openBracePos - 1].Kind == TokenKind.NewLine)
99128
{
100-
parentAst = parentAst.Parent;
129+
return new String(' ', openBraceToken.Extent.StartColumnNumber - 1);
101130
}
102131

103-
List<Token> tokenSuperSet = parentAst == null ? tokens : astTokenMap[parentAst];
104-
var tokenSet = new List<Token>();
105-
foreach (var token in tokenSuperSet)
132+
// if open brace follows any keywords use the identation of the first keyword
133+
// on the line containing the open brace
134+
Token firstTokenOnOpenBraceLine = openBraceToken;
135+
for (int k = openBracePos; k > 0; --k)
106136
{
107-
if (Helper.ContainsExtent(ast.Extent, token.Extent))
137+
if (tokens[k].Extent.StartLineNumber == firstTokenOnOpenBraceLine.Extent.StartLineNumber)
108138
{
109-
tokenSet.Add(token);
139+
firstTokenOnOpenBraceLine = tokens[k];
110140
}
111-
}
112-
113-
astTokenMap[ast] = tokenSet;
114-
return tokenSet;
115-
}
116-
private DiagnosticRecord GetViolationForEmptyLineBeforeBrace(
117-
Ast ast,
118-
List<Token> tokens,
119-
string fileName,
120-
ref HashSet<Token> violationTokens)
121-
{
122-
if (tokens.Count >= 3)
123-
{
124-
var closeBraceToken = tokens.Last();
125-
var extraNewLineToken = tokens[tokens.Count - 2];
126-
var newLineToken = tokens[tokens.Count - 3];
127-
if (!violationTokens.Contains(closeBraceToken)
128-
&& closeBraceToken.Kind == TokenKind.RCurly
129-
&& extraNewLineToken.Kind == TokenKind.NewLine
130-
&& newLineToken.Kind == TokenKind.NewLine)
141+
else
131142
{
132-
violationTokens.Add(closeBraceToken);
133-
return new DiagnosticRecord(
134-
"Extra new line before close brace",
135-
closeBraceToken.Extent,
136-
GetName(),
137-
GetDiagnosticSeverity(),
138-
fileName,
139-
null,
140-
GetSuggestedCorrectionsForEmptyLineBeforeBrace(ast, closeBraceToken, newLineToken, fileName));
143+
break;
141144
}
142145
}
143146

144-
return null;
147+
return new String(' ', firstTokenOnOpenBraceLine.Extent.StartColumnNumber - 1);
145148
}
146149

147-
private DiagnosticRecord GetViolationForBraceOnSameLine(
148-
Ast ast,
149-
List<Token> tokens,
150-
string fileName,
151-
ref HashSet<Token> violationTokens)
150+
private DiagnosticRecord GetViolationForBraceOnSameLine(Token[] tokens, int closeBracePos, int openBracePos, string fileName)
152151
{
153-
if (tokens.Count >= 2)
152+
if (tokens.Length > 1 && tokens.Length > closeBracePos)
154153
{
155-
var closeBraceToken = tokens.Last();
156-
if (!violationTokens.Contains(closeBraceToken)
157-
&& closeBraceToken.Kind == TokenKind.RCurly
158-
&& tokens[tokens.Count - 2].Kind != TokenKind.NewLine)
154+
var closeBraceToken = tokens[closeBracePos];
155+
if (tokens[closeBracePos - 1].Kind != TokenKind.NewLine)
159156
{
160-
violationTokens.Add(closeBraceToken);
161157
return new DiagnosticRecord(
162158
GetError(),
163159
closeBraceToken.Extent,
164160
GetName(),
165161
GetDiagnosticSeverity(),
166162
fileName,
167163
null,
168-
GetSuggestedCorrectionsForBraceOnSameLine(ast, closeBraceToken, fileName));
164+
GetSuggestedCorrectionsForBraceOnSameLine(tokens, closeBracePos, openBracePos, fileName));
169165
}
170166
}
171167

172168
return null;
173169
}
174170

175171
private List<CorrectionExtent> GetSuggestedCorrectionsForBraceOnSameLine(
176-
Ast ast,
177-
Token closeBraceToken,
172+
Token[] tokens,
173+
int closeBracePos,
174+
int openBracePos,
178175
string fileName)
179176
{
180177
var corrections = new List<CorrectionExtent>();
181-
corrections.Add(
182-
new CorrectionExtent(
183-
closeBraceToken.Extent.StartLineNumber,
184-
closeBraceToken.Extent.EndLineNumber,
185-
closeBraceToken.Extent.StartColumnNumber,
186-
closeBraceToken.Extent.EndColumnNumber,
187-
Environment.NewLine + GetIndentation(ast) + closeBraceToken.Text,
188-
fileName));
178+
var closeBraceToken = tokens[closeBracePos];
179+
corrections.Add(new CorrectionExtent(
180+
closeBraceToken.Extent.StartLineNumber,
181+
closeBraceToken.Extent.EndLineNumber,
182+
closeBraceToken.Extent.StartColumnNumber,
183+
closeBraceToken.Extent.EndColumnNumber,
184+
Environment.NewLine + GetIndentation(tokens, closeBracePos, openBracePos) + closeBraceToken.Extent.Text,
185+
fileName));
189186
return corrections;
190187
}
191188

192-
private string GetIndentation(Ast ast)
189+
private void AddToDiagnosticRecords(
190+
DiagnosticRecord diagnosticRecord,
191+
ref List<DiagnosticRecord> diagnosticRecords)
193192
{
194-
var targetAst = ast;
195-
if (!(targetAst is NamedBlockAst)
196-
&& targetAst.Parent != null)
193+
if (diagnosticRecord != null)
197194
{
198-
targetAst = targetAst.Parent;
199-
if (targetAst is ScriptBlockExpressionAst)
200-
{
201-
targetAst = targetAst.Parent ?? targetAst;
202-
}
195+
diagnosticRecords.Add(diagnosticRecord);
203196
}
204-
205-
return new String(' ', targetAst.Extent.StartColumnNumber - 1);
206197
}
207198

208-
private List<CorrectionExtent> GetSuggestedCorrectionsForEmptyLineBeforeBrace(
209-
Ast ast,
210-
Token closeBraceToken,
211-
Token newLineToken,
212-
string fileName)
199+
private List<Token> GetTokens(Ast ast, List<Token> tokens, ref Dictionary<Ast, List<Token>> astTokenMap)
213200
{
214-
var corrections = new List<CorrectionExtent>();
215-
corrections.Add(
216-
new CorrectionExtent(
217-
newLineToken.Extent.StartLineNumber,
218-
closeBraceToken.Extent.EndLineNumber,
219-
newLineToken.Extent.StartColumnNumber,
220-
closeBraceToken.Extent.EndColumnNumber,
221-
Environment.NewLine + GetIndentation(ast) + closeBraceToken.Text,
222-
fileName));
223-
return corrections;
201+
if (astTokenMap.Keys.Contains(ast))
202+
{
203+
return astTokenMap[ast];
204+
}
205+
206+
// check if any parent upstream is in the cache
207+
var parentAst = ast.Parent;
208+
while (parentAst != null
209+
&& !astTokenMap.Keys.Contains(parentAst))
210+
{
211+
parentAst = parentAst.Parent;
212+
}
213+
214+
List<Token> tokenSuperSet = parentAst == null ? tokens : astTokenMap[parentAst];
215+
var tokenSet = new List<Token>();
216+
foreach (var token in tokenSuperSet)
217+
{
218+
if (Helper.ContainsExtent(ast.Extent, token.Extent))
219+
{
220+
tokenSet.Add(token);
221+
}
222+
}
223+
224+
astTokenMap[ast] = tokenSet;
225+
return tokenSet;
224226
}
225227

226228
/// <summary>

‎Rules/PlaceOpenBrace.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
2323
/// <summary>
2424
/// A class to walk an AST to check for [violation]
2525
/// </summary>
26-
#if !CORECLR
26+
#if !CORECLR
2727
[Export(typeof(IScriptRule))]
2828
#endif
2929
class PlaceOpenBrace : IScriptRule
@@ -46,14 +46,18 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4646
throw new ArgumentNullException("ast");
4747
}
4848

49+
// TODO Should have the following options
50+
// * on-same-line
51+
// * on-new-line
52+
// * no-empty-line-after
4953

5054
var lCurlyTokenPositions = new List<int>();
5155
var tokens = Helper.Instance.Tokens;
5256
for (int k = 2; k < tokens.Length; k++)
5357
{
54-
58+
5559
if (tokens[k].Kind == TokenKind.LCurly
56-
&& tokens[k-1].Kind == TokenKind.NewLine)
60+
&& tokens[k - 1].Kind == TokenKind.NewLine)
5761
{
5862
yield return new DiagnosticRecord(
5963
GetError(),

0 commit comments

Comments
 (0)