Format on type fixes

src/client/formatters/lineFormatter.ts

@@ -10,6 +10,20 @@ import { TextRangeCollection } from '../language/textRangeCollection';
 import { Tokenizer } from '../language/tokenizer';
 import { ITextRangeCollection, IToken, TokenType } from '../language/types';
 
+const keywordsWithSpaceBeforeBrace = [
+    'and', 'as', 'assert',
+    'del',
+    'except', 'elif',
+    'for', 'from',
+    'global',
+    'if', 'import', 'in', 'is',
+    'nonlocal', 'not',
+    'or',
+    'raise', 'return',
+    'while', 'with',
+    'yield'
+];
+
 export class LineFormatter {
     private builder = new TextBuilder();
     private tokens: ITextRangeCollection<IToken> = new TextRangeCollection<IToken>([]);
@@ -59,7 +73,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 +89,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 +112,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 +174,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 +224,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 +266,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 +324,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;

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(' ');
         }
     }

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 {

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,15 @@ 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');
+        //testFormatLine('- 135 .bit_length()', '-135.bit_length()');
     });
     test('Unknown tokens no space', () => {
         testFormatLine('abc\\n\\', 'abc\\n\\');
@@ -121,6 +127,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 });

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) {

src/test/pythonFiles/formatting/pythonGrammar.py

@@ -236,7 +236,7 @@ def test_eof_error(self):
                 compile(s, "<test>", "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)