Skip to content

Commit 4052861

Browse files
committed
Fix indentation for arrays in else if and object literals
- Prevent double indentation for arrays/objects when they appear on the same line as another active indentor (e.g. `else if ArrayContains([`). - Ensure object literals assigned to array keys (e.g. `m["key"] = {`) are still indented correctly by tracking active indentors on the line.
1 parent 8aa8114 commit 4052861

File tree

2 files changed

+101
-8
lines changed

2 files changed

+101
-8
lines changed

src/formatters/IndentFormatter.spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,64 @@ describe('IndentFormatter', () => {
103103
);
104104
});
105105
});
106+
107+
it('handles array indentation in else if block', () => {
108+
const input = undent`
109+
sub test()
110+
if true then
111+
print "true"
112+
else if ArrayContains([
113+
"addToWatchlist",
114+
"removeFromWatchlist",
115+
"removeFromContinueWatching",
116+
"markAsWatched",
117+
"markAsUnwatched"
118+
], action.id) then
119+
Metrics().ReportEvent(action.metrics?.click)
120+
end if
121+
end sub
122+
`;
123+
124+
const expected = [
125+
'sub test()',
126+
' if true then',
127+
' print "true"',
128+
' else if ArrayContains([',
129+
' "addToWatchlist",',
130+
' "removeFromWatchlist",',
131+
' "removeFromContinueWatching",',
132+
' "markAsWatched",',
133+
' "markAsUnwatched"',
134+
' ], action.id) then',
135+
' Metrics().ReportEvent(action.metrics?.click)',
136+
' end if',
137+
'end sub'
138+
].join('\n');
139+
140+
const actual = format(input);
141+
expect(actual).to.equal(expected);
142+
});
143+
144+
it('handles object literal indentation after array access', () => {
145+
const input = undent`
146+
sub test()
147+
m["mainGroupCurrentBasePosition"] = {
148+
"x": 0,
149+
"y": 0
150+
}
151+
end sub
152+
`;
153+
154+
const expected = [
155+
'sub test()',
156+
' m["mainGroupCurrentBasePosition"] = {',
157+
' "x": 0,',
158+
' "y": 0',
159+
' }',
160+
'end sub'
161+
].join('\n');
162+
163+
const actual = format(input);
164+
expect(actual).to.equal(expected);
165+
});
106166
});

src/formatters/IndentFormatter.ts

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,17 @@ import { OutdentSpacerTokenKinds, IndentSpacerTokenKinds, CallableKeywordTokenKi
55
import type { FormattingOptions } from '../FormattingOptions';
66
import { util } from '../util';
77

8+
interface IndentState {
9+
kind: TokenKind;
10+
causedIndent: boolean;
11+
}
12+
13+
const StructureIndentTokenKinds = [
14+
TokenKind.LeftSquareBracket,
15+
TokenKind.LeftCurlyBrace,
16+
TokenKind.QuestionLeftSquare
17+
];
18+
819
export class IndentFormatter {
920
/**
1021
* Handle indentation for an array of tokens
@@ -19,7 +30,7 @@ export class IndentFormatter {
1930
//get a map of all if statements for easier lookups
2031
const ifStatements = this.getAllIfStatements(parser);
2132

22-
let parentIndentTokenKinds: TokenKind[] = [];
33+
let parentIndentTokenKinds: IndentState[] = [];
2334

2435
//the list of output tokens
2536
let result: Token[] = [];
@@ -50,16 +61,21 @@ export class IndentFormatter {
5061
lineTokens: Token[],
5162
tokens: Token[],
5263
ifStatements: Map<Token, IfStatement>,
53-
parentIndentTokenKinds: TokenKind[]
64+
parentIndentTokenKinds: IndentState[]
5465
): { currentLineOffset: number; nextLineOffset: number } {
5566
const getParentIndentTokenKind = () => {
56-
const parentIndentTokenKind = parentIndentTokenKinds.length > 0 ? parentIndentTokenKinds[parentIndentTokenKinds.length - 1] : undefined;
67+
const parentIndentTokenKind = parentIndentTokenKinds.length > 0 ? parentIndentTokenKinds[parentIndentTokenKinds.length - 1].kind : undefined;
5768
return parentIndentTokenKind;
5869
};
5970

6071
let currentLineOffset = 0;
6172
let nextLineOffset = 0;
6273
let foundIndentorThisLine = false;
74+
// Track the number of open indentors created on this line.
75+
// This is used to determine if we should suppress indentation for structure indentors (like `[` or `{`).
76+
// We only suppress if there is an *active* indentor on this line (e.g. `if ArrayContains([`).
77+
// If an indentor was closed (e.g. `m["key"] = {`), we should NOT suppress.
78+
let activeIndentorsOnThisLine = 0;
6379

6480
for (let i = 0; i < lineTokens.length; i++) {
6581
let token = lineTokens[i];
@@ -73,6 +89,10 @@ export class IndentFormatter {
7389

7490
//if the previous token was `else` and this token is `if`, skip this token. (we used to have a single token for `elseif` but it got split out in an update of brighterscript)
7591
if (previousNonWhitespaceToken?.kind === TokenKind.Else && token.kind === TokenKind.If) {
92+
foundIndentorThisLine = true;
93+
// `else if` implies an indentor (the `if` condition), even though the `If` token is skipped.
94+
// So we treat it as an active indentor for the purpose of suppression.
95+
activeIndentorsOnThisLine++;
7696
continue;
7797
}
7898

@@ -113,7 +133,7 @@ export class IndentFormatter {
113133

114134
// check for specifically mentioned tokens to NOT indent
115135
const parentIndentTokenKind = getParentIndentTokenKind();
116-
const parentIndentTokenKindsContainsSubOrFunction = parentIndentTokenKinds.includes(TokenKind.Sub) || parentIndentTokenKinds.includes(TokenKind.Function);
136+
const parentIndentTokenKindsContainsSubOrFunction = parentIndentTokenKinds.some(x => x.kind === TokenKind.Sub || x.kind === TokenKind.Function);
117137

118138
const tokenKindIsClass = token.kind === TokenKind.Class;
119139
const tokenKindIsEnum = token.kind === TokenKind.Enum;
@@ -135,9 +155,19 @@ export class IndentFormatter {
135155
continue;
136156
}
137157

138-
nextLineOffset++;
158+
// Don't indent if this is a structure indentor (like `[` or `{`) and we've already found an indentor on this line.
159+
// This prevents double indentation for things like `if ArrayContains([` or `[[`
160+
let causedIndent = true;
161+
if (activeIndentorsOnThisLine > 0 && StructureIndentTokenKinds.includes(token.kind)) {
162+
causedIndent = false;
163+
}
164+
165+
if (causedIndent) {
166+
nextLineOffset++;
167+
activeIndentorsOnThisLine++;
168+
}
139169
foundIndentorThisLine = true;
140-
parentIndentTokenKinds.push(token.kind);
170+
parentIndentTokenKinds.push({ kind: token.kind, causedIndent: causedIndent });
141171

142172
//don't double indent if this is `[[...\n...]]` or `[{...\n...}]`
143173
if (
@@ -167,11 +197,14 @@ export class IndentFormatter {
167197
continue;
168198
}
169199

170-
nextLineOffset--;
200+
const popped = parentIndentTokenKinds.pop();
201+
if (popped?.causedIndent) {
202+
nextLineOffset--;
203+
activeIndentorsOnThisLine--;
204+
}
171205
if (foundIndentorThisLine === false) {
172206
currentLineOffset--;
173207
}
174-
parentIndentTokenKinds.pop();
175208

176209
//don't double un-indent if this is `[[...\n...]]` or `[{...\n...}]`
177210
if (

0 commit comments

Comments
 (0)