#840, #844 (#848)

Mikhail Arkhipov · web-flow · commit b70e8a2d46ae · 2018-02-21T14:05:38.000-08:00
* Fix pylint search * Handle quote escapes in strings * Escapes in strings * CR feedback * Missing pip * Test * Tests * Tests * Mac python path * Tests * Tests * Test * "Go To Python object" doesn't work * Proper detection and type population in virtual env * Test fixes * Simplify venv check * Remove duplicates * Test * Discover pylintrc better + tests * Undo change * CR feedback * Set interprereter before checking install * Fix typo and path compare on Windows * Rename method * #815 - 'F' in flake8 means warning * 730 - same folder temp * Properly resolve ~ * Test * Test * Fix dot spacing * Remove banner * Delete banner code
diff --git a/src/client/banner.ts b/src/client/banner.ts
diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts
@@ -35,8 +35,10 @@ export class PythonInstaller {
             return true;
         }
 
-        await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.');
-        this.shell.openUrl('https://www.python.org/downloads');
+        const download = 'Download';
+        if (await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.', download) === download) {
+            this.shell.openUrl('https://www.python.org/downloads');
+        }
         return false;
     }
 }
diff --git a/src/client/extension.ts b/src/client/extension.ts
@@ -8,7 +8,6 @@ if ((Reflect as any).metadata === undefined) {
 import { Container } from 'inversify';
 import * as vscode from 'vscode';
 import { Disposable, Memento, OutputChannel, window } from 'vscode';
-import { BannerService } from './banner';
 import { PythonSettings } from './common/configSettings';
 import * as settings from './common/configSettings';
 import { STANDARD_OUTPUT_CHANNEL } from './common/constants';
@@ -183,9 +182,6 @@ export async function activate(context: vscode.ExtensionContext) {
     });
     activationDeferred.resolve();
 
-    // tslint:disable-next-line:no-unused-expression
-    new BannerService(persistentStateFactory);
-
     const deprecationMgr = new FeatureDeprecationManager(persistentStateFactory, !!jupyterExtension);
     deprecationMgr.initialize();
     context.subscriptions.push(new FeatureDeprecationManager(persistentStateFactory, !!jupyterExtension));
diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts
@@ -48,7 +48,7 @@ export class LineFormatter {
                     break;
 
                 case TokenType.Identifier:
-                    if (!prev || (!this.isOpenBraceType(prev.type) && prev.type !== TokenType.Colon)) {
+                    if (prev && !this.isOpenBraceType(prev.type) && prev.type !== TokenType.Colon && prev.type !== TokenType.Operator) {
                         this.builder.softAppendSpace();
                     }
                     this.builder.append(this.text.substring(t.start, t.end));
@@ -81,24 +81,44 @@ export class LineFormatter {
 
     private handleOperator(index: number): void {
         const t = this.tokens.getItemAt(index);
-        if (index >= 2 && t.length === 1 && this.text.charCodeAt(t.start) === Char.Equal) {
-            if (this.braceCounter.isOpened(TokenType.OpenBrace)) {
-                // Check if this is = in function arguments. If so, do not
-                // add spaces around it.
-                const prev = this.tokens.getItemAt(index - 1);
-                const prevPrev = this.tokens.getItemAt(index - 2);
-                if (prev.type === TokenType.Identifier &&
-                    (prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace)) {
-                    this.builder.append('=');
+        if (t.length === 1) {
+            const opCode = this.text.charCodeAt(t.start);
+            switch (opCode) {
+                case Char.Equal:
+                    if (index >= 2 && this.handleEqual(t, index)) {
+                        return;
+                    }
+                    break;
+                case Char.Period:
+                    this.builder.append('.');
+                    return;
+                case Char.At:
+                    this.builder.append('@');
                     return;
-                }
+                default:
+                    break;
             }
         }
         this.builder.softAppendSpace();
         this.builder.append(this.text.substring(t.start, t.end));
         this.builder.softAppendSpace();
     }
 
+    private handleEqual(t: IToken, index: number): boolean {
+        if (this.braceCounter.isOpened(TokenType.OpenBrace)) {
+            // Check if this is = in function arguments. If so, do not
+            // add spaces around it.
+            const prev = this.tokens.getItemAt(index - 1);
+            const prevPrev = this.tokens.getItemAt(index - 2);
+            if (prev.type === TokenType.Identifier &&
+                (prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace)) {
+                this.builder.append('=');
+                return true;
+            }
+        }
+        return false;
+    }
+
     private handleOther(t: IToken): void {
         if (this.isBraceType(t.type)) {
             this.braceCounter.countBrace(t);
diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts
@@ -127,9 +127,9 @@ export class Tokenizer implements ITokenizer {
             case Char.Colon:
                 this.tokens.push(new Token(TokenType.Colon, this.cs.position, 1));
                 break;
-            case Char.Period:
             case Char.At:
-                this.tokens.push(new Token(TokenType.Unknown, this.cs.position, 1));
+            case Char.Period:
+                this.tokens.push(new Token(TokenType.Operator, this.cs.position, 1));
                 break;
             default:
                 if (this.isPossibleNumber()) {
diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts
@@ -12,47 +12,64 @@ const unformattedFile = path.join(formatFilesPath, 'fileToFormatOnEnter.py');
 
 suite('Formatting - OnEnter provider', () => {
     let document: vscode.TextDocument;
+    let editor: vscode.TextEditor;
 
     suiteSetup(initialize);
     setup(async () => {
         document = await vscode.workspace.openTextDocument(unformattedFile);
-        await vscode.window.showTextDocument(document);
+        editor = await vscode.window.showTextDocument(document);
     });
     suiteTeardown(closeActiveWindows);
     teardown(closeActiveWindows);
 
-    test('Regular string', async () => {
-        const edits = await formatAtPosition(1, 0);
-        assert.notEqual(edits!.length, 0, 'Line was not formatted');
+    test('Simple statement', async () => {
+        const text = await formatAtPosition(1, 0);
+        assert.equal(text, 'x = 1', 'Line was not formatted');
     });
 
     test('No formatting inside strings', async () => {
-        const edits = await formatAtPosition(2, 0);
-        assert.equal(edits!.length, 0, 'Text inside string was formatted');
+        let text = await formatAtPosition(2, 0);
+        assert.equal(text, '"""x=1', 'Text inside string was formatted');
+        text = await formatAtPosition(3, 0);
+        assert.equal(text, '"""', 'Text inside string was formatted');
     });
 
     test('Whitespace before comment', async () => {
-        const edits = await formatAtPosition(4, 0);
-        assert.equal(edits!.length, 0, 'Whitespace before comment was formatted');
+        const text = await formatAtPosition(4, 0);
+        assert.equal(text, '  # comment', 'Whitespace before comment was not preserved');
     });
 
     test('No formatting of comment', async () => {
-        const edits = await formatAtPosition(5, 0);
-        assert.equal(edits!.length, 0, 'Text inside comment was formatted');
+        const text = await formatAtPosition(5, 0);
+        assert.equal(text, '# x=1', 'Text inside comment was formatted');
     });
 
     test('Formatting line ending in comment', async () => {
-        const edits = await formatAtPosition(6, 0);
-        assert.notEqual(edits!.length, 0, 'Line ending in comment was not formatted');
+        const text = await formatAtPosition(6, 0);
+        assert.equal(text, 'x + 1 # ', 'Line ending in comment was not formatted');
+    });
+
+    test('Formatting line with @', async () => {
+        const text = await formatAtPosition(7, 0);
+        assert.equal(text, '@x', 'Line with @ was reformatted');
+    });
+
+    test('Formatting line with @', async () => {
+        const text = await formatAtPosition(8, 0);
+        assert.equal(text, 'x.y', 'Line ending with period was reformatted');
     });
 
     test('Formatting line ending in string', async () => {
-        const edits = await formatAtPosition(7, 0);
-        assert.notEqual(edits!.length, 0, 'Line ending in multilint string was not formatted');
+        const text = await formatAtPosition(9, 0);
+        assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted');
     });
 
-    async function formatAtPosition(line: number, character: number): Promise<vscode.TextEdit[] | undefined> {
-        return await vscode.commands.executeCommand<vscode.TextEdit[]>('vscode.executeFormatOnTypeProvider',
+    async function formatAtPosition(line: number, character: number): Promise<string> {
+        const edits = await vscode.commands.executeCommand<vscode.TextEdit[]>('vscode.executeFormatOnTypeProvider',
             document.uri, new vscode.Position(line, character), '\n', { insertSpaces: true, tabSize: 2 });
+        if (edits) {
+            await editor.edit(builder => edits.forEach(e => builder.replace(e.range, e.newText)));
+        }
+        return document.lineAt(line - 1).text;
     }
 });
diff --git a/src/test/install/pythonInstallation.test.ts b/src/test/install/pythonInstallation.test.ts
@@ -81,7 +81,11 @@ suite('Installation', () => {
         let openUrlCalled = false;
         let url;
 
-        c.appShell.setup(x => x.showErrorMessage(TypeMoq.It.isAnyString())).callback(() => showErrorMessageCalled = true);
+        const download = 'Download';
+        c.appShell
+            .setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), download))
+            .callback(() => showErrorMessageCalled = true)
+            .returns(() => Promise.resolve(download));
         c.appShell.setup(x => x.openUrl(TypeMoq.It.isAnyString())).callback((s: string) => {
             openUrlCalled = true;
             url = s;
@@ -93,6 +97,17 @@ suite('Installation', () => {
         assert.equal(showErrorMessageCalled, true, 'Error message not shown');
         assert.equal(openUrlCalled, true, 'Python download page not opened');
         assert.equal(url, 'https://www.python.org/downloads', 'Python download page is incorrect');
+
+        showErrorMessageCalled = false;
+        openUrlCalled = false;
+        c.appShell
+            .setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), download))
+            .callback(() => showErrorMessageCalled = true)
+            .returns(() => Promise.resolve(''));
+
+        await c.pythonInstaller.checkPythonInstallation(c.settings.object);
+        assert.equal(showErrorMessageCalled, true, 'Error message not shown');
+        assert.equal(openUrlCalled, false, 'Python download page was opened');
     });
 
     test('Mac: Default Python warning', async () => {
diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts
@@ -91,15 +91,23 @@ suite('Language.Tokenizer', () => {
             assert.equal(tokens.getItemAt(i).type, TokenType.Comment);
         }
     });
-    test('Period/At to unknown token', async () => {
+    test('Period to operator token', async () => {
         const t = new Tokenizer();
-        const tokens = t.tokenize('.@x');
+        const tokens = t.tokenize('x.y');
         assert.equal(tokens.count, 3);
 
-        assert.equal(tokens.getItemAt(0).type, TokenType.Unknown);
-        assert.equal(tokens.getItemAt(1).type, TokenType.Unknown);
+        assert.equal(tokens.getItemAt(0).type, TokenType.Identifier);
+        assert.equal(tokens.getItemAt(1).type, TokenType.Operator);
         assert.equal(tokens.getItemAt(2).type, TokenType.Identifier);
     });
+    test('@ to operator token', async () => {
+        const t = new Tokenizer();
+        const tokens = t.tokenize('@x');
+        assert.equal(tokens.count, 2);
+
+        assert.equal(tokens.getItemAt(0).type, TokenType.Operator);
+        assert.equal(tokens.getItemAt(1).type, TokenType.Identifier);
+    });
     test('Unknown token', async () => {
         const t = new Tokenizer();
         const tokens = t.tokenize('~$');
diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py
@@ -4,4 +4,6 @@
   # comment
 # x=1
 x+1 # 
+@x
+x.y
 x+"""

Original file line number	Diff line number	Diff line change
`@@ -35,8 +35,10 @@ export class PythonInstaller {`
`35`	`35`	`return true;`
`36`	`36`	`}`
`37`	`37`
`38`		`- await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.');`
`39`		`- this.shell.openUrl('https://www.python.org/downloads');`
	`38`	`+ const download = 'Download';`
	`39`	`+ if (await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.', download) === download) {`
	`40`	`+ this.shell.openUrl('https://www.python.org/downloads');`
	`41`	`+ }`
`40`	`42`	`return false;`
`41`	`43`	`}`
`42`	`44`	`}`
-Original file line number
+Diff line change
   # comment
 # x=1
 x+1 #
 +@x
 +x.y
 x+"""