diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 9bd256f50177..b7a6a13aa29b 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -10,6 +10,21 @@ import { TextRangeCollection } from '../language/textRangeCollection'; import { Tokenizer } from '../language/tokenizer'; import { ITextRangeCollection, IToken, TokenType } from '../language/types'; +const keywordsWithSpaceBeforeBrace = [ + 'and', 'as', 'assert', 'await', + 'del', + 'except', 'elif', + 'for', 'from', + 'global', + 'if', 'import', 'in', 'is', + 'lambda', + 'nonlocal', 'not', + 'or', + 'raise', 'return', + 'while', 'with', + 'yield' +]; + export class LineFormatter { private builder = new TextBuilder(); private tokens: ITextRangeCollection = new TextRangeCollection([]); @@ -59,7 +74,7 @@ export class LineFormatter { } const id = this.text.substring(t.start, t.end); this.builder.append(id); - if (this.keywordWithSpaceAfter(id) && next && this.isOpenBraceType(next.type)) { + if (this.isKeywordWithSpaceBeforeBrace(id) && next && this.isOpenBraceType(next.type)) { // for x in () this.builder.softAppendSpace(); } @@ -75,9 +90,9 @@ export class LineFormatter { break; case TokenType.Comment: - // Add space before in-line comment. + // Add 2 spaces before in-line comment per PEP guidelines. if (prev) { - this.builder.softAppendSpace(); + this.builder.softAppendSpace(2); } this.builder.append(this.text.substring(t.start, t.end)); break; @@ -98,28 +113,35 @@ export class LineFormatter { private handleOperator(index: number): void { const t = this.tokens.getItemAt(index); const prev = index > 0 ? this.tokens.getItemAt(index - 1) : undefined; + const opCode = this.text.charCodeAt(t.start); const next = index < this.tokens.count - 1 ? this.tokens.getItemAt(index + 1) : undefined; if (t.length === 1) { - const opCode = this.text.charCodeAt(t.start); switch (opCode) { case Char.Equal: - if (this.handleEqual(t, index)) { - return; - } - break; + this.handleEqual(t, index); + return; case Char.Period: if (prev && this.isKeyword(prev, 'from')) { this.builder.softAppendSpace(); } - this.builder.append(this.text[t.start]); + this.builder.append('.'); if (next && this.isKeyword(next, 'import')) { this.builder.softAppendSpace(); } return; case Char.At: + if (prev) { + // Binary case + this.builder.softAppendSpace(); + this.builder.append('@'); + this.builder.softAppendSpace(); + } else { + this.builder.append('@'); + } + return; case Char.ExclamationMark: - this.builder.append(this.text[t.start]); + this.builder.append('!'); return; case Char.Asterisk: if (prev && this.isKeyword(prev, 'lambda')) { @@ -153,19 +175,34 @@ export class LineFormatter { this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); + + // Check unary case + if (prev && prev.type === TokenType.Operator) { + if (opCode === Char.Hyphen || opCode === Char.Plus || opCode === Char.Tilde) { + return; + } + } this.builder.softAppendSpace(); } - private handleEqual(t: IToken, index: number): boolean { + private handleEqual(t: IToken, index: number): void { if (this.isMultipleStatements(index) && !this.braceCounter.isOpened(TokenType.OpenBrace)) { - return false; // x = 1; x, y = y, x + // x = 1; x, y = y, x + this.builder.softAppendSpace(); + this.builder.append('='); + this.builder.softAppendSpace(); + return; } + // Check if this is = in function arguments. If so, do not add spaces around it. if (this.isEqualsInsideArguments(index)) { this.builder.append('='); - return true; + return; } - return false; + + this.builder.softAppendSpace(); + this.builder.append('='); + this.builder.softAppendSpace(); } private handleOther(t: IToken, index: number): void { @@ -188,6 +225,12 @@ export class LineFormatter { return; } + if (t.type === TokenType.Number && prev && prev.type === TokenType.Operator && prev.length === 1 && this.text.charCodeAt(prev.start) === Char.Tilde) { + // Special case for ~ before numbers + this.builder.append(this.text.substring(t.start, t.end)); + return; + } + if (t.type === TokenType.Unknown) { this.handleUnknown(t); } else { @@ -224,6 +267,10 @@ export class LineFormatter { return false; } + if (index > 1 && this.tokens.getItemAt(index - 2).type === TokenType.Colon) { + return false; // Type hint should have spaces around like foo(x: int = 1) per PEP 8 + } + const first = this.tokens.getItemAt(0); if (first.type === TokenType.Comma) { return true; // Line starts with commma @@ -278,11 +325,9 @@ export class LineFormatter { } return false; } - private keywordWithSpaceAfter(s: string): boolean { - return s === 'in' || s === 'return' || s === 'and' || - s === 'or' || s === 'not' || s === 'from' || - s === 'import' || s === 'except' || s === 'for' || - s === 'as' || s === 'is'; + + private isKeywordWithSpaceBeforeBrace(s: string): boolean { + return keywordsWithSpaceBeforeBrace.indexOf(s) >= 0; } private isKeyword(t: IToken, keyword: string): boolean { return t.type === TokenType.Identifier && t.length === keyword.length && this.text.substr(t.start, t.length) === keyword; diff --git a/src/client/language/textBuilder.ts b/src/client/language/textBuilder.ts index aebe6187696b..e11f2a1299c4 100644 --- a/src/client/language/textBuilder.ts +++ b/src/client/language/textBuilder.ts @@ -16,8 +16,14 @@ export class TextBuilder { return this.segments.join(''); } - public softAppendSpace(): void { - if (!this.isLastWhiteSpace() && this.segments.length > 0) { + public softAppendSpace(count: number = 1): void { + if (this.segments.length === 0) { + return; + } + if (this.isLastWhiteSpace()) { + count = count - 1; + } + for (let i = 0; i < count; i += 1) { this.segments.push(' '); } } diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index 7ceafdccb0e6..2574c388aeb1 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -280,6 +280,8 @@ export class Tokenizer implements ITokenizer { case Char.Caret: case Char.Equal: case Char.ExclamationMark: + case Char.Percent: + case Char.Tilde: length = nextChar === Char.Equal ? 2 : 1; break; @@ -350,23 +352,40 @@ export class Tokenizer implements ITokenizer { this.tokens.push(new Token(TokenType.Comment, start, this.cs.position - start)); } + // tslint:disable-next-line:cyclomatic-complexity private getStringPrefixLength(): number { - if (this.cs.currentChar === Char.f && (this.cs.nextChar === Char.SingleQuote || this.cs.nextChar === Char.DoubleQuote)) { - return 1; // f-string + if (this.cs.currentChar === Char.SingleQuote || this.cs.currentChar === Char.DoubleQuote) { + return 0; // Simple string, no prefix } - if (this.cs.currentChar === Char.b || this.cs.currentChar === Char.B || this.cs.currentChar === Char.u || this.cs.currentChar === Char.U) { - if (this.cs.nextChar === Char.SingleQuote || this.cs.nextChar === Char.DoubleQuote) { - // b-string or u-string - return 1; + + if (this.cs.nextChar === Char.SingleQuote || this.cs.nextChar === Char.DoubleQuote) { + switch (this.cs.currentChar) { + case Char.f: + case Char.F: + case Char.r: + case Char.R: + case Char.b: + case Char.B: + case Char.u: + case Char.U: + return 1; // single-char prefix like u"" or r"" + default: + break; } - if (this.cs.nextChar === Char.r || this.cs.nextChar === Char.R) { - // b-string or u-string with 'r' suffix - if (this.cs.lookAhead(2) === Char.SingleQuote || this.cs.lookAhead(2) === Char.DoubleQuote) { - return 2; - } + } + + if (this.cs.lookAhead(2) === Char.SingleQuote || this.cs.lookAhead(2) === Char.DoubleQuote) { + const prefix = this.cs.getText().substr(this.cs.position, 2).toLowerCase(); + switch (prefix) { + case 'rf': + case 'ur': + case 'br': + return 2; + default: + break; } } - return this.cs.currentChar === Char.SingleQuote || this.cs.currentChar === Char.DoubleQuote ? 0 : -1; + return -1; } private getQuoteType(): QuoteType { diff --git a/src/test/format/extension.lineFormatter.test.ts b/src/test/format/extension.lineFormatter.test.ts index 656f22c3ffd6..acb86d2c5715 100644 --- a/src/test/format/extension.lineFormatter.test.ts +++ b/src/test/format/extension.lineFormatter.test.ts @@ -59,7 +59,7 @@ suite('Formatting - line formatter', () => { testFormatLine('[ 1 :[2: (x,),y]]{1}', '[1:[2:(x,), y]]{1}'); }); test('Trailing comment', () => { - testFormatLine('x=1 # comment', 'x = 1 # comment'); + testFormatLine('x=1 # comment', 'x = 1 # comment'); }); test('Single comment', () => { testFormatLine('# comment', '# comment'); @@ -87,9 +87,14 @@ suite('Formatting - line formatter', () => { }); test('Brace after keyword', () => { testFormatLine('for x in(1,2,3)', 'for x in (1, 2, 3)'); + testFormatLine('assert(1,2,3)', 'assert (1, 2, 3)'); + testFormatLine('if (True|False)and(False/True)not (! x )', 'if (True | False) and (False / True) not (!x)'); + testFormatLine('while (True|False)', 'while (True | False)'); + testFormatLine('yield(a%b)', 'yield (a % b)'); }); test('Dot operator', () => { testFormatLine('x.y', 'x.y'); + testFormatLine('5 .y', '5.y'); }); test('Unknown tokens no space', () => { testFormatLine('abc\\n\\', 'abc\\n\\'); @@ -121,6 +126,32 @@ suite('Formatting - line formatter', () => { test('from..x import', () => { testFormatLine('from..x import', 'from ..x import'); }); + test('Raw strings', () => { + testFormatLine('z=r""', 'z = r""'); + testFormatLine('z=rf""', 'z = rf""'); + testFormatLine('z=R""', 'z = R""'); + testFormatLine('z=RF""', 'z = RF""'); + }); + test('Binary @', () => { + testFormatLine('a@ b', 'a @ b'); + }); + test('Unary operators', () => { + testFormatLine('x= - y', 'x = -y'); + testFormatLine('x= + y', 'x = +y'); + testFormatLine('x= ~ y', 'x = ~y'); + testFormatLine('x=-1', 'x = -1'); + testFormatLine('x= +1', 'x = +1'); + testFormatLine('x= ~1 ', 'x = ~1'); + }); + test('Equals with type hints', () => { + testFormatLine('def foo(x:int=3,x=100.)', 'def foo(x: int = 3, x=100.)'); + }); + test('Trailing comma', () => { + testFormatLine('a, =[1]', 'a, = [1]'); + }); + test('if()', () => { + testFormatLine('if(True) :', 'if (True):'); + }); test('Grammar file', () => { const content = fs.readFileSync(grammarFile).toString('utf8'); const lines = content.splitLines({ trim: false, removeEmptyEntries: false }); diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index d7119b7b4f6f..397a1f9f398d 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -185,7 +185,7 @@ suite('Language.Tokenizer', () => { }); test('Unknown token', () => { const t = new Tokenizer(); - const tokens = t.tokenize('~$'); + const tokens = t.tokenize('`$'); assert.equal(tokens.count, 1); assert.equal(tokens.getItemAt(0).type, TokenType.Unknown); @@ -301,20 +301,37 @@ suite('Language.Tokenizer', () => { assert.equal(tokens.getItemAt(5).type, TokenType.Number); assert.equal(tokens.getItemAt(5).length, 5); }); + test('Simple expression, leading minus', () => { + const t = new Tokenizer(); + const tokens = t.tokenize('x == -y'); + assert.equal(tokens.count, 4); + + assert.equal(tokens.getItemAt(0).type, TokenType.Identifier); + assert.equal(tokens.getItemAt(0).length, 1); + + assert.equal(tokens.getItemAt(1).type, TokenType.Operator); + assert.equal(tokens.getItemAt(1).length, 2); + + assert.equal(tokens.getItemAt(2).type, TokenType.Operator); + assert.equal(tokens.getItemAt(2).length, 1); + + assert.equal(tokens.getItemAt(3).type, TokenType.Identifier); + assert.equal(tokens.getItemAt(3).length, 1); + }); test('Operators', () => { const text = '< <> << <<= ' + '== != > >> >>= >= <=' + - '+ -' + + '+ - ~ %' + '* ** / /= //=' + - '*= += -= **= ' + + '*= += -= ~= %= **= ' + '& &= | |= ^ ^= ->'; const tokens = new Tokenizer().tokenize(text); const lengths = [ 1, 2, 2, 3, 2, 2, 1, 2, 3, 2, 2, - 1, 1, + 1, 1, 1, 1, 1, 2, 1, 2, 3, - 2, 2, 2, 3, + 2, 2, 2, 2, 2, 3, 1, 2, 1, 2, 1, 2, 2]; assert.equal(tokens.count, lengths.length); for (let i = 0; i < tokens.count; i += 1) { diff --git a/src/test/pythonFiles/formatting/pythonGrammar.py b/src/test/pythonFiles/formatting/pythonGrammar.py index 1a17d94302b5..937cba401d3f 100644 --- a/src/test/pythonFiles/formatting/pythonGrammar.py +++ b/src/test/pythonFiles/formatting/pythonGrammar.py @@ -236,7 +236,7 @@ def test_eof_error(self): compile(s, "", "exec") self.assertIn("unexpected EOF", str(cm.exception)) -var_annot_global: int # a global annotated is necessary for test_var_annot +var_annot_global: int # a global annotated is necessary for test_var_annot # custom namespace for testing __annotations__ @@ -643,7 +643,7 @@ def test_lambdef(self): ### lambdef: 'lambda' [varargslist] ':' test l1 = lambda: 0 self.assertEqual(l1(), 0) - l2 = lambda: a[d] # XXX just testing the expression + l2 = lambda: a[d] # XXX just testing the expression l3 = lambda: [2 < x for x in [-1, 3, 0]] self.assertEqual(l3(), [0, 1, 0]) l4 = lambda x=lambda y=lambda z=1: z: y(): x() @@ -1492,7 +1492,7 @@ def __imatmul__(self, o): self.other = o return self m = M() - self.assertEqual(m@m, 4) + self.assertEqual(m @ m, 4) m @= 42 self.assertEqual(m.other, 42)