From a764bc9822e113a18c954286a62b26fde6514a64 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 14:47:16 -0800 Subject: [PATCH 01/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] #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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] #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/37] 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/37] 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/37] #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/37] #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/37] 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/37] 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/37] 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/37] #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/37] 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/37] 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`); From 12186b8a1e5779142270d67486ea4842bddd806f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 16 Apr 2018 10:24:35 -0700 Subject: [PATCH 35/37] Fix typo --- src/client/common/configSettings.ts | 11 ++++++----- src/client/common/types.ts | 8 ++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index b88b82ce65bf..62d75c0ec0ba 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import { ConfigurationTarget, DiagnosticSeverity, Disposable, Uri, workspace } from 'vscode'; import { isTestExecution } from './constants'; import { - IAutoCompeteSettings, + IAutoCompleteSettings, IFormattingSettings, ILintingSettings, IPythonSettings, @@ -35,7 +35,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { public devOptions: string[] = []; public linting?: ILintingSettings; public formatting?: IFormattingSettings; - public autoComplete?: IAutoCompeteSettings; + public autoComplete?: IAutoCompleteSettings; public unitTest?: IUnitTestSettings; public terminal?: ITerminalSettings; public sortImports?: ISortImportSettings; @@ -219,9 +219,9 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { this.formatting.yapfPath = getAbsolutePath(systemVariables.resolveAny(this.formatting.yapfPath), workspaceRoot); // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion - const autoCompleteSettings = systemVariables.resolveAny(pythonSettings.get('autoComplete'))!; + const autoCompleteSettings = systemVariables.resolveAny(pythonSettings.get('autoComplete'))!; if (this.autoComplete) { - Object.assign(this.autoComplete, autoCompleteSettings); + Object.assign(this.autoComplete, autoCompleteSettings); } else { this.autoComplete = autoCompleteSettings; } @@ -229,7 +229,8 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { this.autoComplete = this.autoComplete ? this.autoComplete : { extraPaths: [], addBrackets: false, - preloadModules: [] + preloadModules: [], + showAdvancedMembers: false }; // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion diff --git a/src/client/common/types.ts b/src/client/common/types.ts index f64617178288..5e16a4557786 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -105,7 +105,7 @@ export interface IPythonSettings { readonly linting?: ILintingSettings; readonly formatting?: IFormattingSettings; readonly unitTest?: IUnitTestSettings; - readonly autoComplete?: IAutoCompeteSettings; + readonly autoComplete?: IAutoCompleteSettings; readonly terminal?: ITerminalSettings; readonly sortImports?: ISortImportSettings; readonly workspaceSymbols?: IWorkspaceSymbolSettings; @@ -194,10 +194,11 @@ export interface IFormattingSettings { yapfPath: string; readonly yapfArgs: string[]; } -export interface IAutoCompeteSettings { +export interface IAutoCompleteSettings { readonly addBrackets: boolean; readonly extraPaths: string[]; readonly preloadModules: string[]; + readonly showAdvancedMembers: boolean; } export interface IWorkspaceSymbolSettings { readonly enabled: boolean; @@ -212,6 +213,9 @@ export interface ITerminalSettings { readonly launchArgs: string[]; readonly activateEnvironment: boolean; } +export interface IPythonAnalysisEngineSettings { + readonly showAdvancedMembers: boolean; +} export const IConfigurationService = Symbol('IConfigurationService'); From ca90529fbf3e5c4820d70c3549583ee1ff4b5b12 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 16 Apr 2018 11:17:53 -0700 Subject: [PATCH 36/37] Test baseline update --- src/test/definitions/hover.ptvs.test.ts | 126 +++++++++++++++------- src/test/signature/signature.ptvs.test.ts | 15 ++- 2 files changed, 95 insertions(+), 46 deletions(-) diff --git a/src/test/definitions/hover.ptvs.test.ts b/src/test/definitions/hover.ptvs.test.ts index 8c3b981ca4bb..d2a456efd4bd 100644 --- a/src/test/definitions/hover.ptvs.test.ts +++ b/src/test/definitions/hover.ptvs.test.ts @@ -49,10 +49,15 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '30,11', 'End position is incorrect'); assert.equal(def[0].contents.length, 1, 'Invalid content items'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 2, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'obj.method1: method method1 of one.Class1 objects', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'This is method1', 'function signature line #2 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'obj.method1:', + 'method method1 of one.Class1 objects', + '```html', + 'This is method1', + '```' + ]; + verifySignatureLines(actual, expected); }); test('Across files', async () => { @@ -61,10 +66,15 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '1,0', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '1,12', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 2, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'two.ct().fun: method fun of two.ct objects', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'This is fun', 'function signature line #2 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'two.ct().fun:', + 'method fun of two.ct objects', + '```html', + 'This is fun', + '```' + ]; + verifySignatureLines(actual, expected); }); test('With Unicode Characters', async () => { @@ -73,13 +83,18 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '25,0', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '25,7', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 5, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'Foo.bar: def four.Foo.bar()', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), '说明 - keep this line, it works', 'function signature line #2 is incorrect'); - assert.equal(lines[2].trim(), 'delete following line, it works', 'function signature line #3 is incorrect'); - assert.equal(lines[3].trim(), '如果存在需要等待审批或正在执行的任务，将不刷新页面', 'function signature line #4 is incorrect'); - assert.equal(lines[4].trim(), 'declared in Foo', 'function signature line #5 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'Foo.bar:', + 'four.Foo.bar() -> bool', + '```html', + '说明 - keep this line, it works', + 'delete following line, it works', + '如果存在需要等待审批或正在执行的任务，将不刷新页面', + '```', + 'declared in Foo' + ]; + verifySignatureLines(actual, expected); }); test('Across files with Unicode Characters', async () => { @@ -88,11 +103,16 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '1,0', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '1,16', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 3, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'four.showMessage: def four.showMessage()', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'Кюм ут жэмпэр пошжим льаборэж, коммюны янтэрэсщэт нам ед, декта игнота ныморэ жят эи.', 'function signature line #2 is incorrect'); - assert.equal(lines[2].trim(), 'Шэа декам экшырки эи, эи зыд эррэм докэндё, векж факэтэ пэрчыквюэрёж ку.', 'function signature line #3 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'four.showMessage:', + 'four.showMessage()', + '```html', + 'Кюм ут жэмпэр пошжим льаборэж, коммюны янтэрэсщэт нам ед, декта игнота ныморэ жят эи.', + 'Шэа декам экшырки эи, эи зыд эррэм докэндё, векж факэтэ пэрчыквюэрёж ку.', + '```' + ]; + verifySignatureLines(actual, expected); }); test('Nothing for keywords (class)', async () => { @@ -111,10 +131,22 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '11,7', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '11,18', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 9, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'misc.Random: class misc.Random(_random.Random)', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'Random number generator base class used by bound module functions.', 'function signature line #2 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'misc.Random:', + 'class misc.Random(_random.Random)', + 'Random number generator base class used by bound module functions.', + '```html', + 'Used to instantiate instances of Random to get generators that don\'t', + 'share state.', + 'Class Random can also be subclassed if you want to use a different basic', + 'generator of your own devising: in that case, override the following', + 'methods: random(), seed(), getstate(), and setstate().', + 'Optionally, implement a getrandbits() method so that randrange()', + 'can cover arbitrarily large ranges.', + '```' + ]; + verifySignatureLines(actual, expected); }); test('Highlight Method', async () => { @@ -123,10 +155,13 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '12,0', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '12,12', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 2, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'rnd2.randint: method randint of misc.Random objects -> int', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'Return random integer in range [a, b], including both end points.', 'function signature line #2 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'rnd2.randint:', + 'method randint of misc.Random objects -> int', + 'Return random integer in range [a, b], including both end points.' + ]; + verifySignatureLines(actual, expected); }); test('Highlight Function', async () => { @@ -135,11 +170,14 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '8,6', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '8,15', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 3, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'math.acos: built-in function acos(x)', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'acos(x)', 'function signature line #2 is incorrect'); - assert.equal(lines[2].trim(), 'Return the arc cosine (measured in radians) of x.', 'function signature line #3 is incorrect'); + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'math.acos:', + 'built-in function acos(x)', + 'acos(x)', + 'Return the arc cosine (measured in radians) of x.' + ]; + verifySignatureLines(actual, expected); }); test('Highlight Multiline Method Signature', async () => { @@ -148,11 +186,16 @@ suite('Hover Definition (Analysis Engine)', () => { assert.equal(`${def[0].range!.start.line},${def[0].range!.start.character}`, '14,4', 'Start position is incorrect'); assert.equal(`${def[0].range!.end.line},${def[0].range!.end.character}`, '14,15', 'End position is incorrect'); - const lines = normalizeMarkedString(def[0].contents[0]).splitLines(); - assert.equal(lines.length, 3, 'incorrect number of lines'); - assert.equal(lines[0].trim(), 'misc.Thread: class misc.Thread(_Verbose)', 'function signature line #1 is incorrect'); - assert.equal(lines[1].trim(), 'A class that represents a thread of control.', 'function signature line #2 is incorrect'); - + const actual = normalizeMarkedString(def[0].contents[0]).splitLines(); + const expected = [ + 'misc.Thread:', + 'class misc.Thread(_Verbose)', + 'A class that represents a thread of control.', + '```html', + 'This class can be safely subclassed in a limited fashion.', + '```' + ]; + verifySignatureLines(actual, expected); }); test('Variable', async () => { @@ -181,4 +224,11 @@ suite('Hover Definition (Analysis Engine)', () => { assert.fail(contents, '', '\'Return a capitalized version of S/Return a copy of the string S with only its first character\' message missing', 'compare'); } }); + + function verifySignatureLines(actual: string[], expected: string[]) { + assert.equal(actual.length, expected.length, 'incorrect number of lines'); + for (let i = 0; i < actual.length; i += 1) { + assert.equal(actual[i].trim(), expected[i], `signature line ${i + 1} is incorrect`); + } + } }); diff --git a/src/test/signature/signature.ptvs.test.ts b/src/test/signature/signature.ptvs.test.ts index 68720e33cde1..ad8e58508342 100644 --- a/src/test/signature/signature.ptvs.test.ts +++ b/src/test/signature/signature.ptvs.test.ts @@ -74,14 +74,13 @@ suite('Signatures (Analysis Engine)', () => { new SignatureHelpResult(0, 3, 1, -1, null), new SignatureHelpResult(0, 4, 1, -1, null), new SignatureHelpResult(0, 5, 1, -1, null), - new SignatureHelpResult(0, 6, 1, 0, 'stop'), - new SignatureHelpResult(0, 7, 1, 0, 'stop') - // https://github.com/Microsoft/PTVS/issues/3869 - // new SignatureHelpResult(0, 8, 1, 1, 'stop'), - // new SignatureHelpResult(0, 9, 1, 1, 'stop'), - // new SignatureHelpResult(0, 10, 1, 1, 'stop'), - // new SignatureHelpResult(0, 11, 1, 2, 'step'), - // new SignatureHelpResult(1, 0, 1, 2, 'step') + new SignatureHelpResult(0, 6, 1, 0, 'start'), + new SignatureHelpResult(0, 7, 1, 0, 'start'), + new SignatureHelpResult(0, 8, 1, 1, 'stop'), + new SignatureHelpResult(0, 9, 1, 1, 'stop'), + new SignatureHelpResult(0, 10, 1, 1, 'stop'), + new SignatureHelpResult(0, 11, 1, 2, 'step'), + new SignatureHelpResult(1, 0, 1, 2, 'step') ]; const document = await openDocument(path.join(autoCompPath, 'basicSig.py')); From ef7c5c75b8e38a9ef99c867bb0f8c74cfc5e4b60 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 26 Apr 2018 12:12:49 -0700 Subject: [PATCH 37/37] Improve function argument detection --- src/client/formatters/lineFormatter.ts | 57 ++++---- src/client/typeFormatters/onEnterFormatter.ts | 16 +-- .../format/extension.lineFormatter.test.ts | 136 ++++++++++-------- .../pythonFiles/formatting/pythonGrammar.py | 8 +- 4 files changed, 123 insertions(+), 94 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 2c7b37580f11..28a71f6ff08d 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -3,6 +3,7 @@ // tslint:disable-next-line:import-name import Char from 'typescript-char'; +import { TextDocument } from 'vscode'; import { BraceCounter } from '../language/braceCounter'; import { TextBuilder } from '../language/textBuilder'; import { TextRangeCollection } from '../language/textRangeCollection'; @@ -14,11 +15,15 @@ export class LineFormatter { private tokens: ITextRangeCollection = new TextRangeCollection([]); private braceCounter = new BraceCounter(); private text = ''; + private document?: TextDocument; + private lineNumber = 0; // tslint:disable-next-line:cyclomatic-complexity - public formatLine(text: string): string { - this.tokens = new Tokenizer().tokenize(text); - this.text = text; + public formatLine(document: TextDocument, lineNumber: number): string { + this.document = document; + this.lineNumber = lineNumber; + this.text = document.lineAt(lineNumber).text; + this.tokens = new Tokenizer().tokenize(this.text); this.builder = new TextBuilder(); this.braceCounter = new BraceCounter(); @@ -107,7 +112,7 @@ export class LineFormatter { this.builder.append(this.text[t.start]); return; case Char.Asterisk: - if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') { + if (prev && this.isKeyword(prev, 'lambda')) { this.builder.softAppendSpace(); this.builder.append('*'); return; @@ -122,7 +127,7 @@ export class LineFormatter { this.builder.append('**'); return; } - if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') { + if (prev && this.isKeyword(prev, 'lambda')) { this.builder.softAppendSpace(); this.builder.append('**'); return; @@ -194,6 +199,8 @@ export class LineFormatter { this.builder.softAppendSpace(); } } + + // tslint:disable-next-line:cyclomatic-complexity private isEqualsInsideArguments(index: number): boolean { // Since we don't have complete statement, this is mostly heuristics. // Therefore the code may not be handling all possible ways of the @@ -217,28 +224,31 @@ export class LineFormatter { return true; // Line ends in comma } - if (index >= 2) { - // (x=1 or ,x=1 - const prevPrev = this.tokens.getItemAt(index - 2); - return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace; + if (last.type === TokenType.Comment && this.tokens.count > 1 && this.tokens.getItemAt(this.tokens.count - 2).type === TokenType.Comma) { + return true; // Line ends in comma and then comment } - if (index >= this.tokens.count - 2) { - return false; + if (this.document) { + const prevLine = this.lineNumber > 0 ? this.document.lineAt(this.lineNumber - 1).text : ''; + const prevLineTokens = new Tokenizer().tokenize(prevLine); + if (prevLineTokens.count > 0) { + const lastOnPrevLine = prevLineTokens.getItemAt(prevLineTokens.count - 1); + if (lastOnPrevLine.type === TokenType.Comma) { + return true; // Previous line ends in comma + } + if (lastOnPrevLine.type === TokenType.Comment && prevLineTokens.count > 1 && prevLineTokens.getItemAt(prevLineTokens.count - 2).type === TokenType.Comma) { + return true; // Previous line ends in comma and then comment + } + } } - const next = this.tokens.getItemAt(index + 1); - const nextNext = this.tokens.getItemAt(index + 2); - // x=1, or x=1) - if (this.isValueType(next.type)) { - if (nextNext.type === TokenType.CloseBrace) { + for (let i = 0; i < index; i += 1) { + const t = this.tokens.getItemAt(i); + if (this.isKeyword(t, 'lambda')) { return true; } - if (nextNext.type === TokenType.Comma) { - return last.type === TokenType.CloseBrace; - } } - return false; + return this.braceCounter.isOpened(TokenType.OpenBrace); } private isOpenBraceType(type: TokenType): boolean { @@ -250,10 +260,6 @@ 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) { @@ -268,4 +274,7 @@ export class LineFormatter { s === 'import' || s === 'except' || s === 'for' || s === 'as' || s === 'is'; } + 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/typeFormatters/onEnterFormatter.ts b/src/client/typeFormatters/onEnterFormatter.ts index 013b2d2a85f9..3e17e714d6ee 100644 --- a/src/client/typeFormatters/onEnterFormatter.ts +++ b/src/client/typeFormatters/onEnterFormatter.ts @@ -1,20 +1,20 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as vscode from 'vscode'; +import { CancellationToken, FormattingOptions, OnTypeFormattingEditProvider, Position, TextDocument, TextEdit } from 'vscode'; import { LineFormatter } from '../formatters/lineFormatter'; import { TokenizerMode, TokenType } from '../language/types'; import { getDocumentTokens } from '../providers/providerUtilities'; -export class OnEnterFormatter implements vscode.OnTypeFormattingEditProvider { +export class OnEnterFormatter implements OnTypeFormattingEditProvider { private readonly formatter = new LineFormatter(); public provideOnTypeFormattingEdits( - document: vscode.TextDocument, - position: vscode.Position, + document: TextDocument, + position: Position, ch: string, - options: vscode.FormattingOptions, - cancellationToken: vscode.CancellationToken): vscode.TextEdit[] { + options: FormattingOptions, + cancellationToken: CancellationToken): TextEdit[] { if (position.line === 0) { return []; } @@ -30,10 +30,10 @@ export class OnEnterFormatter implements vscode.OnTypeFormattingEditProvider { return []; } } - const formatted = this.formatter.formatLine(prevLine.text); + const formatted = this.formatter.formatLine(document, prevLine.lineNumber); if (formatted === prevLine.text) { return []; } - return [new vscode.TextEdit(prevLine.range, formatted)]; + return [new TextEdit(prevLine.range, formatted)]; } } diff --git a/src/test/format/extension.lineFormatter.test.ts b/src/test/format/extension.lineFormatter.test.ts index a9cb0fa04447..46ca5e46f816 100644 --- a/src/test/format/extension.lineFormatter.test.ts +++ b/src/test/format/extension.lineFormatter.test.ts @@ -5,6 +5,8 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; +import * as TypeMoq from 'typemoq'; +import { TextDocument, TextLine } from 'vscode'; import '../../client/common/extensions'; import { LineFormatter } from '../../client/formatters/lineFormatter'; @@ -17,124 +19,142 @@ suite('Formatting - line formatter', () => { const formatter = new LineFormatter(); test('Operator spacing', () => { - const actual = formatter.formatLine('( x +1 )*y/ 3'); - assert.equal(actual, '(x + 1) * y / 3'); + testFormatLine('( x +1 )*y/ 3', '(x + 1) * y / 3'); }); test('Braces spacing', () => { - const actual = formatter.formatLine('foo =(0 ,)'); - assert.equal(actual, 'foo = (0,)'); + testFormatLine('foo =(0 ,)', 'foo = (0,)'); }); test('Function arguments', () => { - const actual = formatter.formatLine('foo (0 , x= 1, (3+7) , y , z )'); - assert.equal(actual, 'foo(0, x=1, (3 + 7), y, z)'); + testFormatLine('z=foo (0 , x= 1, (3+7) , y , z )', + 'z = foo(0, x=1, (3 + 7), y, z)'); }); test('Colon regular', () => { - const actual = formatter.formatLine('if x == 4 : print x,y; x,y= y, x'); - assert.equal(actual, 'if x == 4: print x, y; x, y = y, x'); + testFormatLine('if x == 4 : print x,y; x,y= y, x', + 'if x == 4: print x, y; x, y = y, x'); }); test('Colon slices', () => { - const actual = formatter.formatLine('x[1: 30]'); - assert.equal(actual, 'x[1:30]'); + testFormatLine('x[1: 30]', 'x[1:30]'); }); test('Colon slices in arguments', () => { - const actual = formatter.formatLine('spam ( ham[ 1 :3], {eggs : 2})'); - assert.equal(actual, 'spam(ham[1:3], {eggs: 2})'); + testFormatLine('spam ( ham[ 1 :3], {eggs : 2})', + 'spam(ham[1:3], {eggs: 2})'); }); test('Colon slices with double colon', () => { - const actual = formatter.formatLine('ham [1:9 ], ham[ 1: 9: 3], ham[: 9 :3], ham[1: :3], ham [ 1: 9:]'); - assert.equal(actual, 'ham[1:9], ham[1:9:3], ham[:9:3], ham[1::3], ham[1:9:]'); + testFormatLine('ham [1:9 ], ham[ 1: 9: 3], ham[: 9 :3], ham[1: :3], ham [ 1: 9:]', + 'ham[1:9], ham[1:9:3], ham[:9:3], ham[1::3], ham[1:9:]'); }); test('Colon slices with operators', () => { - const actual = formatter.formatLine('ham [lower+ offset :upper+offset]'); - assert.equal(actual, 'ham[lower + offset:upper + offset]'); + testFormatLine('ham [lower+ offset :upper+offset]', + 'ham[lower + offset:upper + offset]'); }); test('Colon slices with functions', () => { - const actual = formatter.formatLine('ham[ : upper_fn ( x) : step_fn(x )], ham[ :: step_fn(x)]'); - assert.equal(actual, 'ham[:upper_fn(x):step_fn(x)], ham[::step_fn(x)]'); + testFormatLine('ham[ : upper_fn ( x) : step_fn(x )], ham[ :: step_fn(x)]', + 'ham[:upper_fn(x):step_fn(x)], ham[::step_fn(x)]'); }); test('Colon in for loop', () => { - const actual = formatter.formatLine('for index in range( len(fruits) ): '); - assert.equal(actual, 'for index in range(len(fruits)):'); + testFormatLine('for index in range( len(fruits) ): ', + 'for index in range(len(fruits)):'); }); test('Nested braces', () => { - const actual = formatter.formatLine('[ 1 :[2: (x,),y]]{1}'); - assert.equal(actual, '[1:[2:(x,), y]]{1}'); + testFormatLine('[ 1 :[2: (x,),y]]{1}', '[1:[2:(x,), y]]{1}'); }); test('Trailing comment', () => { - const actual = formatter.formatLine('x=1 # comment'); - assert.equal(actual, 'x = 1 # comment'); + testFormatLine('x=1 # comment', 'x = 1 # comment'); }); test('Single comment', () => { - const actual = formatter.formatLine('# comment'); - assert.equal(actual, '# comment'); + testFormatLine('# comment', '# comment'); }); test('Comment with leading whitespace', () => { - const actual = formatter.formatLine(' # comment'); - assert.equal(actual, ' # comment'); + testFormatLine(' # comment', ' # comment'); }); test('Equals in first argument', () => { - const actual = formatter.formatLine('foo(x =0)'); - assert.equal(actual, 'foo(x=0)'); + testFormatLine('foo(x =0)', 'foo(x=0)'); }); test('Equals in second argument', () => { - const actual = formatter.formatLine('foo(x,y= \"a\",'); - assert.equal(actual, 'foo(x, y=\"a\",'); + testFormatLine('foo(x,y= \"a\",', 'foo(x, y=\"a\",'); }); test('Equals in multiline arguments', () => { - const actual = formatter.formatLine('x = 1,y =-2)'); - assert.equal(actual, 'x=1, y=-2)'); + testFormatLine2('foo(a,', 'x = 1,y =-2)', '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)'); + testFormatLine(',x = 1,y =m)', ', x=1, y=m)'); }); test('Equals in multiline arguments ending comma', () => { - const actual = formatter.formatLine('x = 1,y =m,'); - assert.equal(actual, 'x=1, y=m,'); + testFormatLine('x = 1,y =m,', 'x=1, y=m,'); }); test('Operators without following space', () => { - const actual = formatter.formatLine('foo( *a, ** b, ! c)'); - assert.equal(actual, 'foo(*a, **b, !c)'); + testFormatLine('foo( *a, ** b, ! c)', 'foo(*a, **b, !c)'); }); test('Brace after keyword', () => { - const actual = formatter.formatLine('for x in(1,2,3)'); - assert.equal(actual, 'for x in (1, 2, 3)'); + testFormatLine('for x in(1,2,3)', 'for x in (1, 2, 3)'); }); test('Dot operator', () => { - const actual = formatter.formatLine('x.y'); - assert.equal(actual, 'x.y'); + testFormatLine('x.y', 'x.y'); }); test('Unknown tokens no space', () => { - const actual = formatter.formatLine('abc\\n\\'); - assert.equal(actual, 'abc\\n\\'); + testFormatLine('abc\\n\\', 'abc\\n\\'); }); test('Unknown tokens with space', () => { - const actual = formatter.formatLine('abc \\n \\'); - assert.equal(actual, 'abc \\n \\'); + testFormatLine('abc \\n \\', 'abc \\n \\'); }); test('Double asterisk', () => { - const actual = formatter.formatLine('a**2, ** k'); - assert.equal(actual, 'a ** 2, **k'); + testFormatLine('a**2, ** k', 'a ** 2, **k'); }); test('Lambda', () => { - const actual = formatter.formatLine('lambda * args, :0'); - assert.equal(actual, 'lambda *args,: 0'); + testFormatLine('lambda * args, :0', 'lambda *args,: 0'); }); test('Comma expression', () => { - const actual = formatter.formatLine('x=1,2,3'); - assert.equal(actual, 'x = 1, 2, 3'); + testFormatLine('x=1,2,3', 'x = 1, 2, 3'); }); test('is exression', () => { - const actual = formatter.formatLine('a( (False is 2) is 3)'); - assert.equal(actual, 'a((False is 2) is 3)'); + testFormatLine('a( (False is 2) is 3)', 'a((False is 2) is 3)'); + }); + test('Function returning tuple', () => { + testFormatLine('x,y=f(a)', 'x, y = f(a)'); }); test('Grammar file', () => { const content = fs.readFileSync(grammarFile).toString('utf8'); const lines = content.splitLines({ trim: false, removeEmptyEntries: false }); + let prevLine = ''; for (let i = 0; i < lines.length; i += 1) { const line = lines[i]; - const actual = formatter.formatLine(line); - assert.equal(actual, line, `Line ${i + 1} changed: '${line}' to '${actual}'`); + const actual = formatLine2(prevLine, line); + assert.equal(actual, line, `Line ${i + 1} changed: '${line.trim()}' to '${actual.trim()}'`); + prevLine = line; } }); + + function testFormatLine(text: string, expected: string): void { + const actual = formatLine(text); + assert.equal(actual, expected); + } + + function formatLine(text: string): string { + const line = TypeMoq.Mock.ofType(); + line.setup(x => x.text).returns(() => text); + + const document = TypeMoq.Mock.ofType(); + document.setup(x => x.lineAt(TypeMoq.It.isAnyNumber())).returns(() => line.object); + + return formatter.formatLine(document.object, 0); + } + + function formatLine2(prevLineText: string, lineText: string): string { + const thisLine = TypeMoq.Mock.ofType(); + thisLine.setup(x => x.text).returns(() => lineText); + + const prevLine = TypeMoq.Mock.ofType(); + prevLine.setup(x => x.text).returns(() => prevLineText); + + const document = TypeMoq.Mock.ofType(); + document.setup(x => x.lineAt(0)).returns(() => prevLine.object); + document.setup(x => x.lineAt(1)).returns(() => thisLine.object); + + return formatter.formatLine(document.object, 1); + } + + function testFormatLine2(prevLineText: string, lineText: string, expected: string): void { + const actual = formatLine2(prevLineText, lineText); + assert.equal(actual, expected); + } }); diff --git a/src/test/pythonFiles/formatting/pythonGrammar.py b/src/test/pythonFiles/formatting/pythonGrammar.py index 32b82285c12f..1a17d94302b5 100644 --- a/src/test/pythonFiles/formatting/pythonGrammar.py +++ b/src/test/pythonFiles/formatting/pythonGrammar.py @@ -646,7 +646,7 @@ def test_lambdef(self): 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() + l4 = lambda x=lambda y=lambda z=1: z: y(): x() self.assertEqual(l4(), 1) l5 = lambda x, y, z=2: x + y + z self.assertEqual(l5(1, 2), 5) @@ -696,8 +696,8 @@ def test_expr_stmt(self): x = 1 x = 1, 2, 3 x = y = z = 1, 2, 3 - x, y, z=1, 2, 3 - abc=a, b, c=x, y, z=xyz = 1, 2, (3, 4) + x, y, z = 1, 2, 3 + abc = a, b, c = x, y, z = xyz = 1, 2, (3, 4) check_syntax_error(self, "x + 1 = 1") check_syntax_error(self, "a + 1 = b + 2") @@ -730,7 +730,7 @@ def test_former_statements_refer_to_builtins(self): def test_del_stmt(self): # 'del' exprlist abc = [1, 2, 3] - x, y, z=abc + x, y, z = abc xyz = x, y, z del abc