From a764bc9822e113a18c954286a62b26fde6514a64 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 14:47:16 -0800 Subject: [PATCH 01/34] Undo changes --- pythonFiles/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index e530be32b367..99f8582c614c 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -88,7 +88,7 @@ def _generate_signature(self, completion): return '' return '%s(%s)' % ( completion.name, - ', '.join(p.description[6:] for p in completion.params if p)) + ', '.join(self._get_param_name(p.description) for p in completion.params if p)) def _get_call_signatures(self, script): """Extract call signatures from jedi.api.Script object in failsafe way. From 9d1b2cc85c3563ccc9b7a242206ae6a5b6d644f6 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 15:44:21 -0800 Subject: [PATCH 02/34] Test fixes --- pythonFiles/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index 99f8582c614c..e530be32b367 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -88,7 +88,7 @@ def _generate_signature(self, completion): return '' return '%s(%s)' % ( completion.name, - ', '.join(self._get_param_name(p.description) for p in completion.params if p)) + ', '.join(p.description[6:] for p in completion.params if p)) def _get_call_signatures(self, script): """Extract call signatures from jedi.api.Script object in failsafe way. From a91291a072bc9730252c199969e7a40a698ca2d2 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 1 Mar 2018 22:03:47 -0800 Subject: [PATCH 03/34] Increase timeout --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 135ca5d8b6c1..22ffd45a0675 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -21,7 +21,7 @@ const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, - timeout: 25000, + timeout: 35000, retries: 3, grep }; From bf266af260b16e1021511a7274c3d6576182179f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 16:26:53 -0800 Subject: [PATCH 04/34] Remove double event listening --- src/client/providers/linterProvider.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index fb66aab3971b..27aa85ffa61f 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -9,7 +9,6 @@ import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; -import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; @@ -17,7 +16,6 @@ export class LinterProvider implements vscode.Disposable { private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; private configMonitor: ConfigSettingMonitor; - private interpreterService: IInterpreterService; private documents: IDocumentManager; private configuration: IConfigurationService; private linterManager: ILinterManager; @@ -31,12 +29,9 @@ export class LinterProvider implements vscode.Disposable { this.fs = serviceContainer.get(IFileSystem); this.engine = serviceContainer.get(ILintingEngine); this.linterManager = serviceContainer.get(ILinterManager); - this.interpreterService = serviceContainer.get(IInterpreterService); this.documents = serviceContainer.get(IDocumentManager); this.configuration = serviceContainer.get(IConfigurationService); - this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles())); - this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); From 7bc6bd643e5ec53eb71a259ad2a5a43a643f7e33 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 16:35:39 -0800 Subject: [PATCH 05/34] Remove test --- src/test/linters/lint.provider.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 023ee86223be..51e49d3d35b9 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -113,16 +113,6 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'save'), TypeMoq.Times.never()); }); - test('Lint on change interpreters', () => { - const e = new vscode.EventEmitter(); - interpreterService.setup(x => x.onDidChangeInterpreter).returns(() => e.event); - - // tslint:disable-next-line:no-unused-variable - const provider = new LinterProvider(context.object, serviceContainer); - e.fire(); - engine.verify(x => x.lintOpenPythonFiles(), TypeMoq.Times.once()); - }); - test('Lint on save pylintrc', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('.pylintrc')); From 8ce8b48db3aedb785026b2fe22071b40d2f6c048 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 17:02:12 -0800 Subject: [PATCH 06/34] Revert "Remove test" This reverts commit e240c3fd117c38b9e6fdcbdd1ba2715789fefe48. --- src/test/linters/lint.provider.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 51e49d3d35b9..023ee86223be 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -113,6 +113,16 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'save'), TypeMoq.Times.never()); }); + test('Lint on change interpreters', () => { + const e = new vscode.EventEmitter(); + interpreterService.setup(x => x.onDidChangeInterpreter).returns(() => e.event); + + // tslint:disable-next-line:no-unused-variable + const provider = new LinterProvider(context.object, serviceContainer); + e.fire(); + engine.verify(x => x.lintOpenPythonFiles(), TypeMoq.Times.once()); + }); + test('Lint on save pylintrc', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('.pylintrc')); From e3a549e58e0f888d79364e353cbcdf00d86b3416 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 17:02:47 -0800 Subject: [PATCH 07/34] Revert "Remove double event listening" This reverts commit af573be27372a79d5589e2134002cc753bb54f2a. --- src/client/providers/linterProvider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index 27aa85ffa61f..fb66aab3971b 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -9,6 +9,7 @@ import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; +import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; @@ -16,6 +17,7 @@ export class LinterProvider implements vscode.Disposable { private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; private configMonitor: ConfigSettingMonitor; + private interpreterService: IInterpreterService; private documents: IDocumentManager; private configuration: IConfigurationService; private linterManager: ILinterManager; @@ -29,9 +31,12 @@ export class LinterProvider implements vscode.Disposable { this.fs = serviceContainer.get(IFileSystem); this.engine = serviceContainer.get(ILintingEngine); this.linterManager = serviceContainer.get(ILinterManager); + this.interpreterService = serviceContainer.get(IInterpreterService); this.documents = serviceContainer.get(IDocumentManager); this.configuration = serviceContainer.get(IConfigurationService); + this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles())); + this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); From 92e8c1ee2e264784b76a250d1f39b157ba198bbe Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 15:32:47 -0700 Subject: [PATCH 08/34] #1096 The if statement is automatically formatted incorrectly --- src/client/formatters/lineFormatter.ts | 10 ++++++---- src/test/format/extension.onEnterFormat.test.ts | 7 ++++++- src/test/pythonFiles/formatting/fileToFormatOnEnter.py | 1 + 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 4b3bff70aa8d..835e7e82b92a 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -5,14 +5,15 @@ import Char from 'typescript-char'; import { BraceCounter } from '../language/braceCounter'; import { TextBuilder } from '../language/textBuilder'; +import { TextRangeCollection } from '../language/textRangeCollection'; import { Tokenizer } from '../language/tokenizer'; import { ITextRangeCollection, IToken, TokenType } from '../language/types'; export class LineFormatter { - private builder: TextBuilder; - private tokens: ITextRangeCollection; - private braceCounter: BraceCounter; - private text: string; + private builder = new TextBuilder(); + private tokens: ITextRangeCollection = new TextRangeCollection([]); + private braceCounter = new BraceCounter(); + private text = ''; // tslint:disable-next-line:cyclomatic-complexity public formatLine(text: string): string { @@ -123,6 +124,7 @@ export class LineFormatter { if (this.isBraceType(t.type)) { this.braceCounter.countBrace(t); } + this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index 74597ce19be7..23e5cbecb8fa 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -59,8 +59,13 @@ suite('Formatting - OnEnter provider', () => { assert.equal(text, 'x.y', 'Line ending with period was reformatted'); }); - test('Formatting line ending in string', async () => { + test('Formatting line with unknown neighboring tokens', async () => { const text = await formatAtPosition(9, 0); + assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted'); + }); + + test('Formatting line ending in string', async () => { + const text = await formatAtPosition(10, 0); assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted'); }); diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py index bbd025363098..67d533125ab2 100644 --- a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py +++ b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py @@ -6,4 +6,5 @@ x+1 # @x x.y +if x<=1: x+""" From b540a1dc43b376e05ddb29bbf6a16dd1fb51843e Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 15:35:31 -0700 Subject: [PATCH 09/34] Merge fix --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 22ffd45a0675..135ca5d8b6c1 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -21,7 +21,7 @@ const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, - timeout: 35000, + timeout: 25000, retries: 3, grep }; From 7b0573ed946ec6ed34d077b0f824f2a7aaaba613 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 16:08:23 -0700 Subject: [PATCH 10/34] Add more tests --- src/client/formatters/lineFormatter.ts | 41 ++++++++++++++++--- .../format/extension.onEnterFormat.test.ts | 12 +++++- .../formatting/fileToFormatOnEnter.py | 2 + 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 835e7e82b92a..7684c9337755 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -73,7 +73,7 @@ export class LineFormatter { break; default: - this.handleOther(t); + this.handleOther(t, i); break; } } @@ -109,10 +109,7 @@ export class LineFormatter { 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)) { + if (this.isEqualsInsideArguments(index)) { this.builder.append('='); return true; } @@ -120,14 +117,46 @@ export class LineFormatter { return false; } - private handleOther(t: IToken): void { + private handleOther(t: IToken, index: number): void { if (this.isBraceType(t.type)) { this.braceCounter.countBrace(t); + this.builder.append(this.text.substring(t.start, t.end)); + return; + } + + if (this.isEqualsInsideArguments(index - 1)) { + // Don't add space around = inside function arguments + this.builder.append(this.text.substring(t.start, t.end)); + return; + } + + if (index > 0) { + const prev = this.tokens.getItemAt(index - 1); + if (this.isOpenBraceType(prev.type)) { + // Don't insert space after (, [ or { + this.builder.append(this.text.substring(t.start, t.end)); + return; + } } + + // In general, keep tokes separated this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } + private isEqualsInsideArguments(index: number): boolean { + if (index < 2) { + return false; + } + 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)) { + return true; + } + return false; + } + private isOpenBraceType(type: TokenType): boolean { return type === TokenType.OpenBrace || type === TokenType.OpenBracket || type === TokenType.OpenCurly; } diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index 23e5cbecb8fa..e34aeb0a0ed3 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -64,8 +64,18 @@ suite('Formatting - OnEnter provider', () => { assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted'); }); - test('Formatting line ending in string', async () => { + test('Formatting method definition with arguments', async () => { const text = await formatAtPosition(10, 0); + assert.equal(text, 'def __init__(self, age=23)', 'Method definition with arguments was not formatted'); + }); + + test('Formatting space after open brace', async () => { + const text = await formatAtPosition(11, 0); + assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted'); + }); + + test('Formatting line ending in string', async () => { + const text = await formatAtPosition(12, 0); assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted'); }); diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py index 67d533125ab2..779167118ffc 100644 --- a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py +++ b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py @@ -7,4 +7,6 @@ @x x.y if x<=1: +def __init__(self, age = 23) +while(1) x+""" From facb10613596b28f82e672266f130e57b4311847 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 16:30:21 -0700 Subject: [PATCH 11/34] More tests --- src/test/format/extension.onEnterFormat.test.ts | 11 ++++++++--- .../pythonFiles/formatting/fileToFormatOnEnter.py | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index e34aeb0a0ed3..689129da78c8 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -64,18 +64,23 @@ suite('Formatting - OnEnter provider', () => { assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted'); }); - test('Formatting method definition with arguments', async () => { + test('Formatting line with unknown neighboring tokens', async () => { const text = await formatAtPosition(10, 0); + assert.equal(text, 'if 1 <= x:', 'Line with unknown neighboring tokens was not formatted'); + }); + + test('Formatting method definition with arguments', async () => { + const text = await formatAtPosition(11, 0); assert.equal(text, 'def __init__(self, age=23)', 'Method definition with arguments was not formatted'); }); test('Formatting space after open brace', async () => { - const text = await formatAtPosition(11, 0); + const text = await formatAtPosition(12, 0); assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted'); }); test('Formatting line ending in string', async () => { - const text = await formatAtPosition(12, 0); + const text = await formatAtPosition(13, 0); assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted'); }); diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py index 779167118ffc..8adfd1fa1233 100644 --- a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py +++ b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py @@ -7,6 +7,7 @@ @x x.y if x<=1: +if 1<=x: def __init__(self, age = 23) while(1) x+""" From f113881370f26a52f159862bb822f0119225013c Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 16:32:06 -0700 Subject: [PATCH 12/34] Typo --- src/client/formatters/lineFormatter.ts | 2 +- src/test/format/extension.onEnterFormat.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 7684c9337755..56e9b6cf7237 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -139,7 +139,7 @@ export class LineFormatter { } } - // In general, keep tokes separated + // In general, keep tokens separated this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index 689129da78c8..8f594d5e2559 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -76,7 +76,7 @@ suite('Formatting - OnEnter provider', () => { test('Formatting space after open brace', async () => { const text = await formatAtPosition(12, 0); - assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted'); + assert.equal(text, 'while(1)', 'Space after open brace was not formatted'); }); test('Formatting line ending in string', async () => { From 3e76718c097e23b52a9ffbe5de27f18f512f3f5f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 27 Mar 2018 18:15:11 -0700 Subject: [PATCH 13/34] Test --- src/client/formatters/lineFormatter.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 56e9b6cf7237..4c30d8c17baa 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -72,6 +72,10 @@ export class LineFormatter { this.builder.append(this.text.substring(t.start, t.end)); break; + case TokenType.Semicolon: + this.builder.append(';'); + break; + default: this.handleOther(t, i); break; @@ -132,7 +136,7 @@ export class LineFormatter { if (index > 0) { const prev = this.tokens.getItemAt(index - 1); - if (this.isOpenBraceType(prev.type)) { + if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon) { // Don't insert space after (, [ or { this.builder.append(this.text.substring(t.start, t.end)); return; From 6e85dc68a3516175546fc6c65cddede03a58bcea Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 27 Mar 2018 21:47:51 -0700 Subject: [PATCH 14/34] Also better handle multiline arguments --- src/client/formatters/lineFormatter.ts | 47 ++++++++++++++----- .../format/extension.lineFormatter.test.ts | 16 +++++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 4c30d8c17baa..694af69ea5ff 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -90,7 +90,7 @@ export class LineFormatter { const opCode = this.text.charCodeAt(t.start); switch (opCode) { case Char.Equal: - if (index >= 2 && this.handleEqual(t, index)) { + if (this.handleEqual(t, index)) { return; } break; @@ -110,13 +110,13 @@ export class LineFormatter { } 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. - if (this.isEqualsInsideArguments(index)) { - this.builder.append('='); - return true; - } + if (this.isMultipleStatements(index) && !this.braceCounter.isOpened(TokenType.OpenBrace)) { + return false; // x = 1; x, y = y, x + } + // 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 false; } @@ -149,14 +149,23 @@ export class LineFormatter { } private isEqualsInsideArguments(index: number): boolean { - if (index < 2) { + if (index < 1) { return false; } 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)) { - return true; + if (prev.type === TokenType.Identifier) { + if (index >= 2) { + // (x=1 or ,x=1 + const prevPrev = this.tokens.getItemAt(index - 2); + return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace; + } else if (index < this.tokens.count - 2) { + const next = this.tokens.getItemAt(index + 1); + const nextNext = this.tokens.getItemAt(index + 2); + // x=1, or x=1) + if (this.isValueType(next.type)) { + return nextNext.type === TokenType.Comma || nextNext.type === TokenType.CloseBrace; + } + } } return false; } @@ -170,4 +179,16 @@ export class LineFormatter { private isBraceType(type: TokenType): boolean { return this.isOpenBraceType(type) || this.isCloseBraceType(type); } + private isValueType(type: TokenType): boolean { + return type === TokenType.Identifier || type === TokenType.Unknown || + type === TokenType.Number || type === TokenType.String; + } + private isMultipleStatements(index: number): boolean { + for (let i = index; i >= 0; i -= 1) { + if (this.tokens.getItemAt(i).type === TokenType.Semicolon) { + return true; + } + } + return false; + } } diff --git a/src/test/format/extension.lineFormatter.test.ts b/src/test/format/extension.lineFormatter.test.ts index 842cb02d735d..79de72c5774a 100644 --- a/src/test/format/extension.lineFormatter.test.ts +++ b/src/test/format/extension.lineFormatter.test.ts @@ -65,4 +65,20 @@ suite('Formatting - line formatter', () => { const actual = formatter.formatLine(' # comment'); assert.equal(actual, ' # comment'); }); + test('Equals in first argument', () => { + const actual = formatter.formatLine('foo(x =0)'); + assert.equal(actual, 'foo(x=0)'); + }); + test('Equals in second argument', () => { + const actual = formatter.formatLine('foo(x,y= \"a\",'); + assert.equal(actual, 'foo(x, y=\"a\",'); + }); + test('Equals in multiline arguments', () => { + const actual = formatter.formatLine('x = 1,y =-2)'); + assert.equal(actual, 'x=1, y=-2)'); + }); + test('Equals in multiline arguments starting comma', () => { + const actual = formatter.formatLine(',x = 1,y =m)'); + assert.equal(actual, ', x=1, y=m)'); + }); }); From 99e037c0a2533c37faa3e7da54125f7fe1d7ae27 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 28 Mar 2018 10:09:43 -0700 Subject: [PATCH 15/34] Add a couple missing periods [skip ci] --- src/client/formatters/lineFormatter.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 694af69ea5ff..fc347235a525 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -28,7 +28,7 @@ export class LineFormatter { const ws = this.text.substr(0, this.tokens.getItemAt(0).start); if (ws.length > 0) { - this.builder.append(ws); // Preserve leading indentation + this.builder.append(ws); // Preserve leading indentation. } for (let i = 0; i < this.tokens.count; i += 1) { @@ -56,16 +56,16 @@ export class LineFormatter { break; case TokenType.Colon: - // x: 1 if not in slice, x[1:y] if inside the slice + // x: 1 if not in slice, x[1:y] if inside the slice. this.builder.append(':'); if (!this.braceCounter.isOpened(TokenType.OpenBracket) && (next && next.type !== TokenType.Colon)) { - // Not inside opened [[ ... ] sequence + // Not inside opened [[ ... ] sequence. this.builder.softAppendSpace(); } break; case TokenType.Comment: - // add space before in-line comment + // Add space before in-line comment. if (prev) { this.builder.softAppendSpace(); } @@ -129,7 +129,7 @@ export class LineFormatter { } if (this.isEqualsInsideArguments(index - 1)) { - // Don't add space around = inside function arguments + // Don't add space around = inside function arguments. this.builder.append(this.text.substring(t.start, t.end)); return; } @@ -137,13 +137,13 @@ export class LineFormatter { if (index > 0) { const prev = this.tokens.getItemAt(index - 1); if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon) { - // Don't insert space after (, [ or { + // Don't insert space after (, [ or { . this.builder.append(this.text.substring(t.start, t.end)); return; } } - // In general, keep tokens separated + // In general, keep tokens separated. this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } From 3caeab70e0a0952c87d92166a776366851185c81 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 14:47:16 -0800 Subject: [PATCH 16/34] Undo changes --- pythonFiles/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index e530be32b367..99f8582c614c 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -88,7 +88,7 @@ def _generate_signature(self, completion): return '' return '%s(%s)' % ( completion.name, - ', '.join(p.description[6:] for p in completion.params if p)) + ', '.join(self._get_param_name(p.description) for p in completion.params if p)) def _get_call_signatures(self, script): """Extract call signatures from jedi.api.Script object in failsafe way. From eeb1f11f7132e3c9c3458651d4720370b48f06d1 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 15:44:21 -0800 Subject: [PATCH 17/34] Test fixes --- pythonFiles/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index 99f8582c614c..e530be32b367 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -88,7 +88,7 @@ def _generate_signature(self, completion): return '' return '%s(%s)' % ( completion.name, - ', '.join(self._get_param_name(p.description) for p in completion.params if p)) + ', '.join(p.description[6:] for p in completion.params if p)) def _get_call_signatures(self, script): """Extract call signatures from jedi.api.Script object in failsafe way. From f5f78c732e7a2f8639118d55ad25eec6d41aa729 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 1 Mar 2018 22:03:47 -0800 Subject: [PATCH 18/34] Increase timeout --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 135ca5d8b6c1..22ffd45a0675 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -21,7 +21,7 @@ const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, - timeout: 25000, + timeout: 35000, retries: 3, grep }; From 88744daf193d21c55c72de7acd0d162602152b9d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 16:26:53 -0800 Subject: [PATCH 19/34] Remove double event listening --- src/client/providers/linterProvider.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index fb66aab3971b..27aa85ffa61f 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -9,7 +9,6 @@ import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; -import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; @@ -17,7 +16,6 @@ export class LinterProvider implements vscode.Disposable { private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; private configMonitor: ConfigSettingMonitor; - private interpreterService: IInterpreterService; private documents: IDocumentManager; private configuration: IConfigurationService; private linterManager: ILinterManager; @@ -31,12 +29,9 @@ export class LinterProvider implements vscode.Disposable { this.fs = serviceContainer.get(IFileSystem); this.engine = serviceContainer.get(ILintingEngine); this.linterManager = serviceContainer.get(ILinterManager); - this.interpreterService = serviceContainer.get(IInterpreterService); this.documents = serviceContainer.get(IDocumentManager); this.configuration = serviceContainer.get(IConfigurationService); - this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles())); - this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); From 65dde44f59ccf2754aa8203e933c9d96ef3be338 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 16:35:39 -0800 Subject: [PATCH 20/34] Remove test --- src/test/linters/lint.provider.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 023ee86223be..51e49d3d35b9 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -113,16 +113,6 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'save'), TypeMoq.Times.never()); }); - test('Lint on change interpreters', () => { - const e = new vscode.EventEmitter(); - interpreterService.setup(x => x.onDidChangeInterpreter).returns(() => e.event); - - // tslint:disable-next-line:no-unused-variable - const provider = new LinterProvider(context.object, serviceContainer); - e.fire(); - engine.verify(x => x.lintOpenPythonFiles(), TypeMoq.Times.once()); - }); - test('Lint on save pylintrc', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('.pylintrc')); From c513f717892d167d0256e1465c3427f61aa65891 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 17:02:12 -0800 Subject: [PATCH 21/34] Revert "Remove test" This reverts commit e240c3fd117c38b9e6fdcbdd1ba2715789fefe48. --- src/test/linters/lint.provider.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 51e49d3d35b9..023ee86223be 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -113,6 +113,16 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'save'), TypeMoq.Times.never()); }); + test('Lint on change interpreters', () => { + const e = new vscode.EventEmitter(); + interpreterService.setup(x => x.onDidChangeInterpreter).returns(() => e.event); + + // tslint:disable-next-line:no-unused-variable + const provider = new LinterProvider(context.object, serviceContainer); + e.fire(); + engine.verify(x => x.lintOpenPythonFiles(), TypeMoq.Times.once()); + }); + test('Lint on save pylintrc', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('.pylintrc')); From ccb3886f22c08589c124c709372b46f3d5d54ee2 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 17:02:47 -0800 Subject: [PATCH 22/34] Revert "Remove double event listening" This reverts commit af573be27372a79d5589e2134002cc753bb54f2a. --- src/client/providers/linterProvider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index 27aa85ffa61f..fb66aab3971b 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -9,6 +9,7 @@ import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; +import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; @@ -16,6 +17,7 @@ export class LinterProvider implements vscode.Disposable { private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; private configMonitor: ConfigSettingMonitor; + private interpreterService: IInterpreterService; private documents: IDocumentManager; private configuration: IConfigurationService; private linterManager: ILinterManager; @@ -29,9 +31,12 @@ export class LinterProvider implements vscode.Disposable { this.fs = serviceContainer.get(IFileSystem); this.engine = serviceContainer.get(ILintingEngine); this.linterManager = serviceContainer.get(ILinterManager); + this.interpreterService = serviceContainer.get(IInterpreterService); this.documents = serviceContainer.get(IDocumentManager); this.configuration = serviceContainer.get(IConfigurationService); + this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles())); + this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); From 106f4dba19b160315ecb5b88509573dafefe8397 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 15:35:31 -0700 Subject: [PATCH 23/34] Merge fix --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 22ffd45a0675..135ca5d8b6c1 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -21,7 +21,7 @@ const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, - timeout: 35000, + timeout: 25000, retries: 3, grep }; From e1da6a66086a17e998c0d5485c0ea845e5029935 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 10:31:41 -0700 Subject: [PATCH 24/34] #1257 On type formatting errors for args and kwargs --- src/client/formatters/lineFormatter.ts | 13 ++++++++++--- src/test/index.ts | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index fc347235a525..046533952464 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -95,15 +95,22 @@ export class LineFormatter { } break; case Char.Period: - this.builder.append('.'); - return; case Char.At: - this.builder.append('@'); + case Char.ExclamationMark: + this.builder.append(this.text[t.start]); return; default: break; } } + // Do not append space if operator is preceded by '(' or ',' as in foo(**kwarg) + if (index > 0) { + const prev = this.tokens.getItemAt(index - 1); + if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma) { + this.builder.append(this.text.substring(t.start, t.end)); + return; + } + } this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); this.builder.softAppendSpace(); diff --git a/src/test/index.ts b/src/test/index.ts index 135ca5d8b6c1..848b18152792 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -13,7 +13,7 @@ process.env.IS_MULTI_ROOT_TEST = IS_MULTI_ROOT_TEST.toString(); // If running on CI server and we're running the debugger tests, then ensure we only run debug tests. // We do this to ensure we only run debugger test, as debugger tests are very flaky on CI. // So the solution is to run them separately and first on CI. -const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; +const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : 'line formatter'; // You can directly control Mocha options by uncommenting the following lines. // See https://github.com/mochajs/mocha/wiki/Using-mocha-programmatically#set-options for more info. From e78f0fba4412361f7144dad8bb7502478763d727 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 11:05:39 -0700 Subject: [PATCH 25/34] Handle f-strings --- src/client/language/tokenizer.ts | 12 ++++-- .../format/extension.lineFormatter.test.ts | 6 ++- src/test/language/tokenizer.test.ts | 37 +++++++++++++++++++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index c481c4201ac0..fcb29ed8b9a3 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -85,10 +85,16 @@ export class Tokenizer implements ITokenizer { } } + // tslint:disable-next-line:cyclomatic-complexity private handleCharacter(): boolean { + // f-strings + const fString = this.cs.currentChar === Char.f && (this.cs.nextChar === Char.SingleQuote || this.cs.nextChar === Char.DoubleQuote); + if (fString) { + this.cs.moveNext(); + } const quoteType = this.getQuoteType(); if (quoteType !== QuoteType.None) { - this.handleString(quoteType); + this.handleString(quoteType, fString); return true; } if (this.cs.currentChar === Char.Hash) { @@ -342,8 +348,8 @@ export class Tokenizer implements ITokenizer { return QuoteType.None; } - private handleString(quoteType: QuoteType): void { - const start = this.cs.position; + private handleString(quoteType: QuoteType, fString: boolean): void { + const start = fString ? this.cs.position - 1 : this.cs.position; if (quoteType === QuoteType.Single || quoteType === QuoteType.Double) { this.cs.moveNext(); this.skipToSingleEndQuote(quoteType === QuoteType.Single diff --git a/src/test/format/extension.lineFormatter.test.ts b/src/test/format/extension.lineFormatter.test.ts index 79de72c5774a..3325c19382a2 100644 --- a/src/test/format/extension.lineFormatter.test.ts +++ b/src/test/format/extension.lineFormatter.test.ts @@ -73,7 +73,7 @@ suite('Formatting - line formatter', () => { const actual = formatter.formatLine('foo(x,y= \"a\",'); assert.equal(actual, 'foo(x, y=\"a\",'); }); - test('Equals in multiline arguments', () => { + test('Equals in multiline arguments', () => { const actual = formatter.formatLine('x = 1,y =-2)'); assert.equal(actual, 'x=1, y=-2)'); }); @@ -81,4 +81,8 @@ suite('Formatting - line formatter', () => { const actual = formatter.formatLine(',x = 1,y =m)'); assert.equal(actual, ', x=1, y=m)'); }); + test('Operators without following space', () => { + const actual = formatter.formatLine('foo( *a, ** b, ! c)'); + assert.equal(actual, 'foo(*a, **b, !c)'); + }); }); diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index 1d2bf15d2b7b..8d37f49dd791 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -79,6 +79,43 @@ suite('Language.Tokenizer', () => { assert.equal(tokens.getItemAt(0).type, TokenType.String); assert.equal(tokens.getItemAt(0).length, 12); }); + test('Strings: single quoted f-string ', async () => { + const t = new Tokenizer(); + // tslint:disable-next-line:quotemark + const tokens = t.tokenize("a+f'quoted'"); + assert.equal(tokens.count, 3); + assert.equal(tokens.getItemAt(0).type, TokenType.Identifier); + assert.equal(tokens.getItemAt(1).type, TokenType.Operator); + assert.equal(tokens.getItemAt(2).type, TokenType.String); + assert.equal(tokens.getItemAt(2).length, 9); + }); + test('Strings: double quoted f-string ', async () => { + const t = new Tokenizer(); + const tokens = t.tokenize('x(1,f"quoted")'); + assert.equal(tokens.count, 6); + assert.equal(tokens.getItemAt(0).type, TokenType.Identifier); + assert.equal(tokens.getItemAt(1).type, TokenType.OpenBrace); + assert.equal(tokens.getItemAt(2).type, TokenType.Number); + assert.equal(tokens.getItemAt(3).type, TokenType.Comma); + assert.equal(tokens.getItemAt(4).type, TokenType.String); + assert.equal(tokens.getItemAt(4).length, 9); + assert.equal(tokens.getItemAt(5).type, TokenType.CloseBrace); + }); + test('Strings: single quoted multiline f-string ', async () => { + const t = new Tokenizer(); + // tslint:disable-next-line:quotemark + const tokens = t.tokenize("f'''quoted'''"); + assert.equal(tokens.count, 1); + assert.equal(tokens.getItemAt(0).type, TokenType.String); + assert.equal(tokens.getItemAt(0).length, 13); + }); + test('Strings: double quoted multiline f-string ', async () => { + const t = new Tokenizer(); + const tokens = t.tokenize('f"""quoted """'); + assert.equal(tokens.count, 1); + assert.equal(tokens.getItemAt(0).type, TokenType.String); + assert.equal(tokens.getItemAt(0).length, 14); + }); test('Comments', async () => { const t = new Tokenizer(); const tokens = t.tokenize(' #co"""mment1\n\t\n#comm\'ent2 '); From 725cf7199757073e927a3b874a02d1c43e92e2c2 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 11:33:50 -0700 Subject: [PATCH 26/34] Stop importing from test code --- src/client/extension.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index e86959d62255..1617b18cf44c 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -11,7 +11,6 @@ import { extensions, IndentAction, languages, Memento, OutputChannel, window } from 'vscode'; -import { IS_ANALYSIS_ENGINE_TEST } from '../test/constants'; import { AnalysisExtensionActivator } from './activation/analysis'; import { ClassicExtensionActivator } from './activation/classic'; import { IExtensionActivator } from './activation/types'; @@ -75,7 +74,8 @@ export async function activate(context: ExtensionContext) { const configuration = serviceManager.get(IConfigurationService); const pythonSettings = configuration.getSettings(); - const activator: IExtensionActivator = IS_ANALYSIS_ENGINE_TEST || !pythonSettings.jediEnabled + const analysisEngineTest = process.env.VSC_PYTHON_ANALYSIS === '1'; + const activator: IExtensionActivator = analysisEngineTest || !pythonSettings.jediEnabled ? new AnalysisExtensionActivator(serviceManager, pythonSettings) : new ClassicExtensionActivator(serviceManager, pythonSettings); From 5cd6d45931c2da9ee3685b0c61daa4bd9b9b9f39 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 11:54:47 -0700 Subject: [PATCH 27/34] #1308 Single line statements leading to an indentation on the next line --- src/client/extension.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/client/extension.ts b/src/client/extension.ts index 1617b18cf44c..10abaf6ef80d 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -107,6 +107,10 @@ export async function activate(context: ExtensionContext) { // tslint:disable-next-line:no-non-null-assertion languages.setLanguageConfiguration(PYTHON.language!, { onEnterRules: [ + { + beforeText: /:\s*pass\s*$/, + action: { indentAction: IndentAction.None } + }, { beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*/, action: { indentAction: IndentAction.Indent } From 27613db0db2efdbbc6468c7a0e4ff06e3c34cdc1 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 12:06:09 -0700 Subject: [PATCH 28/34] #726 editing python after inline if statement invalid indent --- src/client/extension.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 10abaf6ef80d..2328ff6ef58f 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -108,11 +108,11 @@ export async function activate(context: ExtensionContext) { languages.setLanguageConfiguration(PYTHON.language!, { onEnterRules: [ { - beforeText: /:\s*pass\s*$/, + beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except)\b.*:\s*\S+/, action: { indentAction: IndentAction.None } }, { - beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*/, + beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/, action: { indentAction: IndentAction.Indent } }, { From 8061a209b63c57faeb1c46271793db9e7010ae59 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 12:36:39 -0700 Subject: [PATCH 29/34] Undo change --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 848b18152792..135ca5d8b6c1 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -13,7 +13,7 @@ process.env.IS_MULTI_ROOT_TEST = IS_MULTI_ROOT_TEST.toString(); // If running on CI server and we're running the debugger tests, then ensure we only run debug tests. // We do this to ensure we only run debugger test, as debugger tests are very flaky on CI. // So the solution is to run them separately and first on CI. -const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : 'line formatter'; +const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; // You can directly control Mocha options by uncommenting the following lines. // See https://github.com/mochajs/mocha/wiki/Using-mocha-programmatically#set-options for more info. From 17dc292c0aa25696886906129764c3360c74bf8f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Apr 2018 15:59:29 -0700 Subject: [PATCH 30/34] Move constant --- src/client/common/constants.ts | 3 +++ src/client/extension.ts | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index 2995452cb2f7..de0c7d260a9f 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -69,5 +69,8 @@ export function isTestExecution(): boolean { // tslint:disable-next-line:interface-name no-string-literal return process.env['VSC_PYTHON_CI_TEST'] === '1'; } +export function isPythonAnalysisEngineTest(): boolean { + return process.env.VSC_PYTHON_ANALYSIS === '1'; +} export const EXTENSION_ROOT_DIR = path.join(__dirname, '..', '..', '..'); diff --git a/src/client/extension.ts b/src/client/extension.ts index 2328ff6ef58f..04457bb99a15 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -15,7 +15,7 @@ import { AnalysisExtensionActivator } from './activation/analysis'; import { ClassicExtensionActivator } from './activation/classic'; import { IExtensionActivator } from './activation/types'; import { PythonSettings } from './common/configSettings'; -import { STANDARD_OUTPUT_CHANNEL } from './common/constants'; +import { isPythonAnalysisEngineTest, STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { FeatureDeprecationManager } from './common/featureDeprecationManager'; import { createDeferred } from './common/helpers'; import { PythonInstaller } from './common/installer/pythonInstallation'; @@ -74,8 +74,7 @@ export async function activate(context: ExtensionContext) { const configuration = serviceManager.get(IConfigurationService); const pythonSettings = configuration.getSettings(); - const analysisEngineTest = process.env.VSC_PYTHON_ANALYSIS === '1'; - const activator: IExtensionActivator = analysisEngineTest || !pythonSettings.jediEnabled + const activator: IExtensionActivator = isPythonAnalysisEngineTest() || !pythonSettings.jediEnabled ? new AnalysisExtensionActivator(serviceManager, pythonSettings) : new ClassicExtensionActivator(serviceManager, pythonSettings); From 65964b9dc67d5c6a571b8e4ea8636825c464fa4f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 10 Apr 2018 14:16:11 -0700 Subject: [PATCH 31/34] Harden LS startup error checks --- src/client/activation/analysis.ts | 36 +++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/client/activation/analysis.ts b/src/client/activation/analysis.ts index 554daace8d96..cfc03a4d0d5d 100644 --- a/src/client/activation/analysis.ts +++ b/src/client/activation/analysis.ts @@ -3,9 +3,11 @@ import * as path from 'path'; import { ExtensionContext, OutputChannel } from 'vscode'; -import { Disposable, LanguageClient, LanguageClientOptions, ServerOptions } from 'vscode-languageclient'; +import { Message } from 'vscode-jsonrpc'; +import { CloseAction, Disposable, ErrorAction, ErrorHandler, LanguageClient, LanguageClientOptions, ServerOptions } from 'vscode-languageclient'; import { IApplicationShell } from '../common/application/types'; import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../common/constants'; +import { createDeferred, Deferred } from '../common/helpers'; import { IFileSystem, IPlatformService } from '../common/platform/types'; import { IProcessService } from '../common/process/types'; import { StopWatch } from '../common/stopWatch'; @@ -21,6 +23,18 @@ const dotNetCommand = 'dotnet'; const languageClientName = 'Python Tools'; const analysisEngineFolder = 'analysis'; +class LanguageServerStartupErrorHandler implements ErrorHandler { + constructor(private readonly deferred: Deferred) { } + public error(error: Error, message: Message, count: number): ErrorAction { + this.deferred.reject(); + return ErrorAction.Shutdown; + } + public closed(): CloseAction { + this.deferred.reject(); + return CloseAction.DoNotRestart; + } +} + export class AnalysisExtensionActivator implements IExtensionActivator { private readonly configuration: IConfigurationService; private readonly appShell: IApplicationShell; @@ -92,16 +106,23 @@ export class AnalysisExtensionActivator implements IExtensionActivator { private async tryStartLanguageClient(context: ExtensionContext, lc: LanguageClient): Promise { let disposable: Disposable | undefined; + const deferred = createDeferred(); try { + lc.clientOptions.errorHandler = new LanguageServerStartupErrorHandler(deferred); + disposable = lc.start(); - await lc.onReady(); + lc.onReady() + .then(() => deferred.resolve()) + .catch(ex => deferred.reject()); + await deferred.promise; + this.output.appendLine(`Language server ready: ${this.sw.elapsedTime} ms`); context.subscriptions.push(disposable); } catch (ex) { if (disposable) { disposable.dispose(); - throw ex; } + throw ex; } } @@ -157,12 +178,8 @@ export class AnalysisExtensionActivator implements IExtensionActivator { // tslint:disable-next-line:no-string-literal properties['SearchPaths'] = searchPaths; - if (isTestExecution()) { - // tslint:disable-next-line:no-string-literal - properties['TestEnvironment'] = true; - } - const selector: string[] = [PYTHON]; + // Options to control the language client return { // Register the server for Python documents @@ -181,7 +198,8 @@ export class AnalysisExtensionActivator implements IExtensionActivator { trimDocumentationText: false, maxDocumentationTextLength: 0 }, - asyncStartup: true + asyncStartup: true, + testEnvironment: isTestExecution() } }; } From 4bf5a4cd83173a18966e34ff76ed7ed0ccd98df0 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 10 Apr 2018 14:33:56 -0700 Subject: [PATCH 32/34] #1364 Intellisense doesn't work after specific const string --- src/client/language/tokenizer.ts | 3 +++ src/test/language/tokenizer.test.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index fcb29ed8b9a3..e1c8c4b03d9e 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -366,6 +366,9 @@ export class Tokenizer implements ITokenizer { private skipToSingleEndQuote(quote: number): void { while (!this.cs.isEndOfStream()) { + if (this.cs.currentChar === Char.LineFeed || this.cs.currentChar === Char.CarriageReturn) { + return; // Unterminated single-line string + } if (this.cs.currentChar === Char.Backslash && this.cs.nextChar === quote) { this.cs.advance(2); continue; diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index 8d37f49dd791..202f0c774297 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -116,6 +116,23 @@ suite('Language.Tokenizer', () => { assert.equal(tokens.getItemAt(0).type, TokenType.String); assert.equal(tokens.getItemAt(0).length, 14); }); + test('Strings: escape at the end of single quoted string ', async () => { + const t = new Tokenizer(); + // tslint:disable-next-line:quotemark + const tokens = t.tokenize("'quoted\\'\nx"); + assert.equal(tokens.count, 2); + assert.equal(tokens.getItemAt(0).type, TokenType.String); + assert.equal(tokens.getItemAt(0).length, 9); + assert.equal(tokens.getItemAt(1).type, TokenType.Identifier); + }); + test('Strings: escape at the end of double quoted string ', async () => { + const t = new Tokenizer(); + const tokens = t.tokenize('"quoted\\"\nx'); + assert.equal(tokens.count, 2); + assert.equal(tokens.getItemAt(0).type, TokenType.String); + assert.equal(tokens.getItemAt(0).length, 9); + assert.equal(tokens.getItemAt(1).type, TokenType.Identifier); + }); test('Comments', async () => { const t = new Tokenizer(); const tokens = t.tokenize(' #co"""mment1\n\t\n#comm\'ent2 '); From ddbd295e593a4b052b605103d82b95cbf1c7d1d1 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 12 Apr 2018 12:01:47 -0700 Subject: [PATCH 33/34] Telemetry for the analysis enging --- src/client/activation/analysis.ts | 19 ++++++++++++++++++- src/client/telemetry/constants.ts | 4 ++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/client/activation/analysis.ts b/src/client/activation/analysis.ts index cfc03a4d0d5d..9fcbb8ade27b 100644 --- a/src/client/activation/analysis.ts +++ b/src/client/activation/analysis.ts @@ -13,6 +13,13 @@ import { IProcessService } from '../common/process/types'; import { StopWatch } from '../common/stopWatch'; import { IConfigurationService, IOutputChannel, IPythonSettings } from '../common/types'; import { IServiceContainer } from '../ioc/types'; +import { + PYTHON_ANALYSIS_ENGINE_DOWNLOADED, + PYTHON_ANALYSIS_ENGINE_ENABLED, + PYTHON_ANALYSIS_ENGINE_ERROR, + PYTHON_ANALYSIS_ENGINE_STARTUP +} from '../telemetry/constants'; +import { getTelemetryReporter } from '../telemetry/telemetry'; import { AnalysisEngineDownloader } from './downloader'; import { InterpreterDataService } from './interpreterDataService'; import { PlatformData } from './platformData'; @@ -26,7 +33,7 @@ const analysisEngineFolder = 'analysis'; class LanguageServerStartupErrorHandler implements ErrorHandler { constructor(private readonly deferred: Deferred) { } public error(error: Error, message: Message, count: number): ErrorAction { - this.deferred.reject(); + this.deferred.reject(error); return ErrorAction.Shutdown; } public closed(): CloseAction { @@ -71,6 +78,9 @@ export class AnalysisExtensionActivator implements IExtensionActivator { const mscorlib = path.join(context.extensionPath, analysisEngineFolder, 'mscorlib.dll'); let downloadPackage = false; + const reporter = getTelemetryReporter(); + reporter.sendTelemetryEvent(PYTHON_ANALYSIS_ENGINE_ENABLED); + if (!await this.fs.fileExistsAsync(mscorlib)) { // Depends on .NET Runtime or SDK this.languageClient = this.createSimpleLanguageClient(context, clientOptions); @@ -80,6 +90,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator { } catch (ex) { if (await this.isDotNetInstalled()) { this.appShell.showErrorMessage(`.NET Runtime appears to be installed but the language server did not start. Error ${ex}`); + reporter.sendTelemetryEvent(PYTHON_ANALYSIS_ENGINE_ERROR, { error: 'Failed to start (MSIL)' }); return false; } // No .NET Runtime, no mscorlib - need to download self-contained package. @@ -90,6 +101,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator { if (downloadPackage) { const downloader = new AnalysisEngineDownloader(this.services, analysisEngineFolder); await downloader.downloadAnalysisEngine(context); + reporter.sendTelemetryEvent(PYTHON_ANALYSIS_ENGINE_DOWNLOADED); } const serverModule = path.join(context.extensionPath, analysisEngineFolder, this.platformData.getEngineExecutableName()); @@ -100,6 +112,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator { return true; } catch (ex) { this.appShell.showErrorMessage(`Language server failed to start. Error ${ex}`); + reporter.sendTelemetryEvent(PYTHON_ANALYSIS_ENGINE_ERROR, { error: 'Failed to start (platform)' }); return false; } } @@ -108,6 +121,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator { let disposable: Disposable | undefined; const deferred = createDeferred(); try { + const sw = new StopWatch(); lc.clientOptions.errorHandler = new LanguageServerStartupErrorHandler(deferred); disposable = lc.start(); @@ -118,6 +132,9 @@ export class AnalysisExtensionActivator implements IExtensionActivator { this.output.appendLine(`Language server ready: ${this.sw.elapsedTime} ms`); context.subscriptions.push(disposable); + + const reporter = getTelemetryReporter(); + reporter.sendTelemetryEvent(PYTHON_ANALYSIS_ENGINE_STARTUP, {}, { startup_time: sw.elapsedTime }); } catch (ex) { if (disposable) { disposable.dispose(); diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index bf02b07c63c7..be0cdc8a2c21 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -30,3 +30,7 @@ export const UNITTEST_STOP = 'UNITTEST.STOP'; export const UNITTEST_RUN = 'UNITTEST.RUN'; export const UNITTEST_DISCOVER = 'UNITTEST.DISCOVER'; export const UNITTEST_VIEW_OUTPUT = 'UNITTEST.VIEW_OUTPUT'; +export const PYTHON_ANALYSIS_ENGINE_ENABLED = 'PYTHON_ANALYSIS_ENGINE.ENABLED'; +export const PYTHON_ANALYSIS_ENGINE_DOWNLOADED = 'PYTHON_ANALYSIS_ENGINE.DOWNLOADED'; +export const PYTHON_ANALYSIS_ENGINE_ERROR = 'PYTHON_ANALYSIS_ENGINE.ERROR'; +export const PYTHON_ANALYSIS_ENGINE_STARTUP = 'PYTHON_ANALYSIS_ENGINE.STARTUP'; From d4afb6c26b2efe00584c8c4e056529080b771d14 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 13 Apr 2018 10:28:53 -0700 Subject: [PATCH 34/34] PR feedback --- src/client/activation/analysis.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/analysis.ts b/src/client/activation/analysis.ts index 9fcbb8ade27b..f6702f075ddc 100644 --- a/src/client/activation/analysis.ts +++ b/src/client/activation/analysis.ts @@ -127,7 +127,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator { disposable = lc.start(); lc.onReady() .then(() => deferred.resolve()) - .catch(ex => deferred.reject()); + .catch(deferred.reject); await deferred.promise; this.output.appendLine(`Language server ready: ${this.sw.elapsedTime} ms`);