Skip to content

Commit c82e04e

Browse files
Mikhail Arkhipovbrettcannon
Mikhail Arkhipov
authored and
brettcannon
committed
Fix spacing of general (non-specific) tokens + tests (#1222)
Fixes #1096
1 parent 905841a commit c82e04e

File tree

4 files changed

+118
-22
lines changed

4 files changed

+118
-22
lines changed

‎src/client/formatters/lineFormatter.ts

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@
55
import Char from 'typescript-char';
66
import { BraceCounter } from '../language/braceCounter';
77
import { TextBuilder } from '../language/textBuilder';
8+
import { TextRangeCollection } from '../language/textRangeCollection';
89
import { Tokenizer } from '../language/tokenizer';
910
import { ITextRangeCollection, IToken, TokenType } from '../language/types';
1011

1112
export class LineFormatter {
12-
private builder: TextBuilder;
13-
private tokens: ITextRangeCollection<IToken>;
14-
private braceCounter: BraceCounter;
15-
private text: string;
13+
private builder = new TextBuilder();
14+
private tokens: ITextRangeCollection<IToken> = new TextRangeCollection<IToken>([]);
15+
private braceCounter = new BraceCounter();
16+
private text = '';
1617

1718
// tslint:disable-next-line:cyclomatic-complexity
1819
public formatLine(text: string): string {
@@ -27,7 +28,7 @@ export class LineFormatter {
2728

2829
const ws = this.text.substr(0, this.tokens.getItemAt(0).start);
2930
if (ws.length > 0) {
30-
this.builder.append(ws); // Preserve leading indentation
31+
this.builder.append(ws); // Preserve leading indentation.
3132
}
3233

3334
for (let i = 0; i < this.tokens.count; i += 1) {
@@ -55,24 +56,28 @@ export class LineFormatter {
5556
break;
5657

5758
case TokenType.Colon:
58-
// x: 1 if not in slice, x[1:y] if inside the slice
59+
// x: 1 if not in slice, x[1:y] if inside the slice.
5960
this.builder.append(':');
6061
if (!this.braceCounter.isOpened(TokenType.OpenBracket) && (next && next.type !== TokenType.Colon)) {
61-
// Not inside opened [[ ... ] sequence
62+
// Not inside opened [[ ... ] sequence.
6263
this.builder.softAppendSpace();
6364
}
6465
break;
6566

6667
case TokenType.Comment:
67-
// add space before in-line comment
68+
// Add space before in-line comment.
6869
if (prev) {
6970
this.builder.softAppendSpace();
7071
}
7172
this.builder.append(this.text.substring(t.start, t.end));
7273
break;
7374

75+
case TokenType.Semicolon:
76+
this.builder.append(';');
77+
break;
78+
7479
default:
75-
this.handleOther(t);
80+
this.handleOther(t, i);
7681
break;
7782
}
7883
}
@@ -85,7 +90,7 @@ export class LineFormatter {
8590
const opCode = this.text.charCodeAt(t.start);
8691
switch (opCode) {
8792
case Char.Equal:
88-
if (index >= 2 && this.handleEqual(t, index)) {
93+
if (this.handleEqual(t, index)) {
8994
return;
9095
}
9196
break;
@@ -105,27 +110,66 @@ export class LineFormatter {
105110
}
106111

107112
private handleEqual(t: IToken, index: number): boolean {
108-
if (this.braceCounter.isOpened(TokenType.OpenBrace)) {
109-
// Check if this is = in function arguments. If so, do not
110-
// add spaces around it.
111-
const prev = this.tokens.getItemAt(index - 1);
112-
const prevPrev = this.tokens.getItemAt(index - 2);
113-
if (prev.type === TokenType.Identifier &&
114-
(prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace)) {
115-
this.builder.append('=');
116-
return true;
117-
}
113+
if (this.isMultipleStatements(index) && !this.braceCounter.isOpened(TokenType.OpenBrace)) {
114+
return false; // x = 1; x, y = y, x
115+
}
116+
// Check if this is = in function arguments. If so, do not add spaces around it.
117+
if (this.isEqualsInsideArguments(index)) {
118+
this.builder.append('=');
119+
return true;
118120
}
119121
return false;
120122
}
121123

122-
private handleOther(t: IToken): void {
124+
private handleOther(t: IToken, index: number): void {
123125
if (this.isBraceType(t.type)) {
124126
this.braceCounter.countBrace(t);
127+
this.builder.append(this.text.substring(t.start, t.end));
128+
return;
125129
}
130+
131+
if (this.isEqualsInsideArguments(index - 1)) {
132+
// Don't add space around = inside function arguments.
133+
this.builder.append(this.text.substring(t.start, t.end));
134+
return;
135+
}
136+
137+
if (index > 0) {
138+
const prev = this.tokens.getItemAt(index - 1);
139+
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon) {
140+
// Don't insert space after (, [ or { .
141+
this.builder.append(this.text.substring(t.start, t.end));
142+
return;
143+
}
144+
}
145+
146+
// In general, keep tokens separated.
147+
this.builder.softAppendSpace();
126148
this.builder.append(this.text.substring(t.start, t.end));
127149
}
128150

151+
private isEqualsInsideArguments(index: number): boolean {
152+
if (index < 1) {
153+
return false;
154+
}
155+
const prev = this.tokens.getItemAt(index - 1);
156+
if (prev.type === TokenType.Identifier) {
157+
if (index >= 2) {
158+
// (x=1 or ,x=1
159+
const prevPrev = this.tokens.getItemAt(index - 2);
160+
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
161+
} else if (index < this.tokens.count - 2) {
162+
const next = this.tokens.getItemAt(index + 1);
163+
const nextNext = this.tokens.getItemAt(index + 2);
164+
// x=1, or x=1)
165+
if (this.isValueType(next.type)) {
166+
return nextNext.type === TokenType.Comma || nextNext.type === TokenType.CloseBrace;
167+
}
168+
}
169+
}
170+
return false;
171+
}
172+
129173
private isOpenBraceType(type: TokenType): boolean {
130174
return type === TokenType.OpenBrace || type === TokenType.OpenBracket || type === TokenType.OpenCurly;
131175
}
@@ -135,4 +179,16 @@ export class LineFormatter {
135179
private isBraceType(type: TokenType): boolean {
136180
return this.isOpenBraceType(type) || this.isCloseBraceType(type);
137181
}
182+
private isValueType(type: TokenType): boolean {
183+
return type === TokenType.Identifier || type === TokenType.Unknown ||
184+
type === TokenType.Number || type === TokenType.String;
185+
}
186+
private isMultipleStatements(index: number): boolean {
187+
for (let i = index; i >= 0; i -= 1) {
188+
if (this.tokens.getItemAt(i).type === TokenType.Semicolon) {
189+
return true;
190+
}
191+
}
192+
return false;
193+
}
138194
}

‎src/test/format/extension.lineFormatter.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,20 @@ suite('Formatting - line formatter', () => {
6565
const actual = formatter.formatLine(' # comment');
6666
assert.equal(actual, ' # comment');
6767
});
68+
test('Equals in first argument', () => {
69+
const actual = formatter.formatLine('foo(x =0)');
70+
assert.equal(actual, 'foo(x=0)');
71+
});
72+
test('Equals in second argument', () => {
73+
const actual = formatter.formatLine('foo(x,y= \"a\",');
74+
assert.equal(actual, 'foo(x, y=\"a\",');
75+
});
76+
test('Equals in multiline arguments', () => {
77+
const actual = formatter.formatLine('x = 1,y =-2)');
78+
assert.equal(actual, 'x=1, y=-2)');
79+
});
80+
test('Equals in multiline arguments starting comma', () => {
81+
const actual = formatter.formatLine(',x = 1,y =m)');
82+
assert.equal(actual, ', x=1, y=m)');
83+
});
6884
});

‎src/test/format/extension.onEnterFormat.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,28 @@ suite('Formatting - OnEnter provider', () => {
5959
assert.equal(text, 'x.y', 'Line ending with period was reformatted');
6060
});
6161

62-
test('Formatting line ending in string', async () => {
62+
test('Formatting line with unknown neighboring tokens', async () => {
6363
const text = await formatAtPosition(9, 0);
64+
assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted');
65+
});
66+
67+
test('Formatting line with unknown neighboring tokens', async () => {
68+
const text = await formatAtPosition(10, 0);
69+
assert.equal(text, 'if 1 <= x:', 'Line with unknown neighboring tokens was not formatted');
70+
});
71+
72+
test('Formatting method definition with arguments', async () => {
73+
const text = await formatAtPosition(11, 0);
74+
assert.equal(text, 'def __init__(self, age=23)', 'Method definition with arguments was not formatted');
75+
});
76+
77+
test('Formatting space after open brace', async () => {
78+
const text = await formatAtPosition(12, 0);
79+
assert.equal(text, 'while(1)', 'Space after open brace was not formatted');
80+
});
81+
82+
test('Formatting line ending in string', async () => {
83+
const text = await formatAtPosition(13, 0);
6484
assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted');
6585
});
6686

‎src/test/pythonFiles/formatting/fileToFormatOnEnter.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,8 @@
66
x+1 #
77
@x
88
x.y
9+
if x<=1:
10+
if 1<=x:
11+
def __init__(self, age = 23)
12+
while(1)
913
x+"""

0 commit comments

Comments
 (0)