From b8ae8590170161c9009e4a0232aaa0d1a6287ada Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 15:23:49 -0600 Subject: [PATCH 01/21] Revert "Revert "Add regex to dedent `else` and friends (#6497)" (#6945)" This reverts commit 2402dd5fe184b5c38b9d22062f6a676a5d526e22. --- src/client/extension.ts | 6 +- src/client/language/languageConfiguration.ts | 34 +++++---- .../languageConfiguration.unit.test.ts | 73 ++++++++++++++++++- 3 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index db56a5b24c66..67a6f9dd9507 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -40,7 +40,7 @@ import { registerTypes as appRegisterTypes } from './application/serviceRegistry import { IApplicationDiagnostics } from './application/types'; import { DebugService } from './common/application/debugService'; import { IApplicationShell, ICommandManager, IWorkspaceService } from './common/application/types'; -import { Commands, isTestExecution, PYTHON, STANDARD_OUTPUT_CHANNEL } from './common/constants'; +import { Commands, isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { registerTypes as registerDotNetTypes } from './common/dotnet/serviceRegistry'; import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; import { traceError } from './common/logger'; @@ -85,7 +85,7 @@ import { registerTypes as interpretersRegisterTypes } from './interpreter/servic import { ServiceContainer } from './ioc/container'; import { ServiceManager } from './ioc/serviceManager'; import { IServiceContainer, IServiceManager } from './ioc/types'; -import { setLanguageConfiguration } from './language/languageConfiguration'; +import { getLanguageConfiguration } from './language/languageConfiguration'; import { LinterCommands } from './linters/linterCommands'; import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry'; import { ILintingEngine } from './linters/types'; @@ -176,7 +176,7 @@ async function activateUnsafe(context: ExtensionContext): Promise const linterProvider = new LinterProvider(context, serviceManager); context.subscriptions.push(linterProvider); - setLanguageConfiguration(); + languages.setLanguageConfiguration(PYTHON_LANGUAGE, getLanguageConfiguration()); if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') { const formatProvider = new PythonFormattingEditProvider(context, serviceContainer); diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 19eebd8d0131..fa8f86d966f3 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -2,19 +2,24 @@ // Licensed under the MIT License. 'use strict'; -import { IndentAction, languages } from 'vscode'; -import { PYTHON_LANGUAGE } from '../common/constants'; +import { IndentAction } from 'vscode'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; +/** + * This does not handle all cases. However, it does handle nearly all usage. + * Here's what it does not cover: + * - the statement is split over multiple lines (and hence the ":" is on a different line) + * - the code block is inlined (after the ":") + * - there are multiple statements on the line (separated by semicolons) + * Also note that `lambda` is purposefully excluded. + */ +export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/; +export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; +export const OUTDENT_ONENTER_REGEX = /^\s*(?:break|continue|pass|(?:raise|return)\b.*)\s*(#.*)?$/; -export function setLanguageConfiguration() { - // Enable indentAction - languages.setLanguageConfiguration(PYTHON_LANGUAGE, { +export function getLanguageConfiguration() { + return { onEnterRules: [ - { - beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/, - action: { indentAction: IndentAction.Indent } - }, { beforeText: MULTILINE_SEPARATOR_INDENT_REGEX, action: { indentAction: IndentAction.Indent } @@ -25,10 +30,13 @@ export function setLanguageConfiguration() { action: { indentAction: IndentAction.None, appendText: '# ' } }, { - beforeText: /^\s+(continue|break|return)\b.*/, - afterText: /\s+$/, + beforeText: OUTDENT_ONENTER_REGEX, action: { indentAction: IndentAction.Outdent } } - ] - }); + ], + indentationRules: { + increaseIndentPattern: INCREASE_INDENT_REGEX, + decreaseIndentPattern: DECREASE_INDENT_REGEX + } + }; } diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 1356995cfdf4..37433879daa8 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -5,19 +5,84 @@ import { expect } from 'chai'; -import { MULTILINE_SEPARATOR_INDENT_REGEX } from '../../client/language/languageConfiguration'; +import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX, MULTILINE_SEPARATOR_INDENT_REGEX, OUTDENT_ONENTER_REGEX } from '../../client/language/languageConfiguration'; suite('Language configuration regexes', () => { test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); - expect (result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); + expect(result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); }); + test('Multiline separator indent regex should not pick up strings with escaped characters', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'hello \\n\''); - expect (result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); + expect(result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); }); + test('Multiline separator indent regex should pick up strings ending with a multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'multiline \\'); - expect (result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); + expect(result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); + }); + + [ + 'async def test(self):', + 'class TestClass:', + 'def foo(self, node, namespace=""):', + 'for item in items:', + 'if foo is None:', + 'try:', + 'while \'::\' in macaddress:', + 'with self.test:' + ].forEach(example => { + const keyword = example.split(' ')[0]; + + test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = INCREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); + }); + + test(`Decrease indent regex should not pick up lines containing the ${keyword} keyword`, async () => { + const result = DECREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(false, `Decrease indent regex should not pick up lines containing the ${keyword} keyword`); + }); + }); + + ['elif x < 5:', 'else:', 'except TestError:', 'finally:'].forEach(example => { + const keyword = example.split(' ')[0]; + + test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = INCREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); + }); + + test(`Decrease indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = DECREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(true, `Decrease indent regex should pick up lines containing the ${keyword} keyword`); + }); + }); + + test('Increase indent regex should not pick up lines without keywords', async () => { + const result = INCREASE_INDENT_REGEX.test('a = \'hello \\n \''); + expect(result).to.be.equal(false, 'Increase indent regex should not pick up lines without keywords'); + }); + + test('Decrease indent regex should not pick up lines without keywords', async () => { + const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); + expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); + }); + + [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\'', ' return [ True, False, False ]'].forEach(example => { + const keyword = example.trim().split(' ')[0]; + + const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`; + test(testWithoutComments, () => { + const result = OUTDENT_ONENTER_REGEX.test(example); + expect(result).to.be.equal(true, testWithoutComments); + }); + + const testWithComments = `Outdent regex on enter should pick up lines containing the ${keyword} keyword and ending with comments`; + test(testWithComments, () => { + const result = OUTDENT_ONENTER_REGEX.test(`${example} # test comment`); + expect(result).to.be.equal(true, testWithComments); + }); }); }); From 9b94f35f84d77fb599e35af19d352a59bc2dff46 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 15:13:00 -0600 Subject: [PATCH 02/21] Copy IndentAction into the mock VSCode. --- src/test/mocks/vsc/index.ts | 6 ++++++ src/test/vscode-mock.ts | 1 + 2 files changed, 7 insertions(+) diff --git a/src/test/mocks/vsc/index.ts b/src/test/mocks/vsc/index.ts index d9586c4a4d35..64cbc6721eb9 100644 --- a/src/test/mocks/vsc/index.ts +++ b/src/test/mocks/vsc/index.ts @@ -172,6 +172,12 @@ export namespace vscMock { Operator = 24, TypeParameter = 25 } + export enum IndentAction { + None = 0, + Indent = 1, + IndentOutdent = 2, + Outdent = 3 + } export class CodeActionKind { public static readonly Empty: CodeActionKind = new CodeActionKind('empty'); diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index cd99771644b4..647142bdaace 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -54,6 +54,7 @@ mockedVSCode.EventEmitter = vscodeMocks.vscMock.EventEmitter; mockedVSCode.CancellationTokenSource = vscodeMocks.vscMock.CancellationTokenSource; mockedVSCode.CompletionItemKind = vscodeMocks.vscMock.CompletionItemKind; mockedVSCode.SymbolKind = vscodeMocks.vscMock.SymbolKind; +mockedVSCode.IndentAction = vscodeMocks.vscMock.IndentAction; mockedVSCode.Uri = vscodeMocks.vscMock.Uri as any; mockedVSCode.Range = vscodeMocks.vscMockExtHostedTypes.Range; mockedVSCode.Position = vscodeMocks.vscMockExtHostedTypes.Position; From 319dd87aa7d8efcfbe536f8ca260fc5f94a32c47 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 15:15:41 -0600 Subject: [PATCH 03/21] Test against getLanguageConfiguration(). --- src/test/language/languageConfiguration.unit.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 37433879daa8..0038dba5f397 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -5,9 +5,17 @@ import { expect } from 'chai'; -import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX, MULTILINE_SEPARATOR_INDENT_REGEX, OUTDENT_ONENTER_REGEX } from '../../client/language/languageConfiguration'; +import { + getLanguageConfiguration +} from '../../client/language/languageConfiguration'; suite('Language configuration regexes', () => { + const cfg = getLanguageConfiguration(); + const DECREASE_INDENT_REGEX = cfg.indentationRules.decreaseIndentPattern; + const INCREASE_INDENT_REGEX = cfg.indentationRules.increaseIndentPattern; + const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules[0].beforeText; + const OUTDENT_ONENTER_REGEX = cfg.onEnterRules[2].beforeText; + test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); expect(result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); From d1da2928d85a1c745e34cc90907801a2166eeaec Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 15:18:34 -0600 Subject: [PATCH 04/21] language config formatting --- src/client/language/languageConfiguration.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index fa8f86d966f3..36b70e263d51 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -22,16 +22,23 @@ export function getLanguageConfiguration() { onEnterRules: [ { beforeText: MULTILINE_SEPARATOR_INDENT_REGEX, - action: { indentAction: IndentAction.Indent } + action: { + indentAction: IndentAction.Indent + } }, { beforeText: /^\s*#.*/, afterText: /.+$/, - action: { indentAction: IndentAction.None, appendText: '# ' } + action: { + indentAction: IndentAction.None, + appendText: '# ' + } }, { beforeText: OUTDENT_ONENTER_REGEX, - action: { indentAction: IndentAction.Outdent } + action: { + indentAction: IndentAction.Outdent + } } ], indentationRules: { From 9add75cfdfa806b1fd8e903d29397b980db85392 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 15:21:26 -0600 Subject: [PATCH 05/21] Inline the regexes. --- src/client/language/languageConfiguration.ts | 32 +++++++++----------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 36b70e263d51..84d489dd4475 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -4,28 +4,17 @@ import { IndentAction } from 'vscode'; -export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; -/** - * This does not handle all cases. However, it does handle nearly all usage. - * Here's what it does not cover: - * - the statement is split over multiple lines (and hence the ":" is on a different line) - * - the code block is inlined (after the ":") - * - there are multiple statements on the line (separated by semicolons) - * Also note that `lambda` is purposefully excluded. - */ -export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/; -export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; -export const OUTDENT_ONENTER_REGEX = /^\s*(?:break|continue|pass|(?:raise|return)\b.*)\s*(#.*)?$/; - export function getLanguageConfiguration() { return { onEnterRules: [ + // multi-line separator { - beforeText: MULTILINE_SEPARATOR_INDENT_REGEX, + beforeText: /^(?!\s+\\)[^#\n]+\\$/, action: { indentAction: IndentAction.Indent } }, + // continue comments { beforeText: /^\s*#.*/, afterText: /.+$/, @@ -34,16 +23,25 @@ export function getLanguageConfiguration() { appendText: '# ' } }, + // outdent on enter { - beforeText: OUTDENT_ONENTER_REGEX, + beforeText: /^\s*(?:break|continue|pass|(?:raise|return)\b.*)\s*(#.*)?$/, action: { indentAction: IndentAction.Outdent } } ], indentationRules: { - increaseIndentPattern: INCREASE_INDENT_REGEX, - decreaseIndentPattern: DECREASE_INDENT_REGEX + /** + * This does not handle all cases. However, it does handle nearly all usage. + * Here's what it does not cover: + * - the statement is split over multiple lines (and hence the ":" is on a different line) + * - the code block is inlined (after the ":") + * - there are multiple statements on the line (separated by semicolons) + * Also note that `lambda` is purposefully excluded. + */ + increaseIndentPattern: /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/, + decreaseIndentPattern: /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/ } }; } From 465147f55adc7b7c7419eebf47b624d952acb817 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 17:00:37 -0600 Subject: [PATCH 06/21] Add a verboseRegExp() helper. --- src/client/common/utils/regexp.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/client/common/utils/regexp.ts diff --git a/src/client/common/utils/regexp.ts b/src/client/common/utils/regexp.ts new file mode 100644 index 000000000000..3a353c1fc006 --- /dev/null +++ b/src/client/common/utils/regexp.ts @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +export function verboseRegExp(pattern: string): RegExp { + pattern = pattern.replace(/\s+?/g, ''); + return RegExp(pattern); +} From e90bcbba3202985836c0f7b9afa4d028c4045e11 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 17:03:44 -0600 Subject: [PATCH 07/21] Use verbose regexes in the language config. --- src/client/language/languageConfiguration.ts | 77 +++++++++++++++++++- 1 file changed, 73 insertions(+), 4 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 84d489dd4475..13edc9046ad8 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -3,13 +3,23 @@ 'use strict'; import { IndentAction } from 'vscode'; +import { verboseRegExp } from '../common/utils/regexp'; +// tslint:disable:no-multiline-string + +// tslint:disable-next-line:max-func-body-length export function getLanguageConfiguration() { return { onEnterRules: [ // multi-line separator { - beforeText: /^(?!\s+\\)[^#\n]+\\$/, + beforeText: verboseRegExp(` + ^ + (?! \\s+ \\\\ ) + [^#\n]+ + \\\\ + $ + `), action: { indentAction: IndentAction.Indent } @@ -25,7 +35,20 @@ export function getLanguageConfiguration() { }, // outdent on enter { - beforeText: /^\s*(?:break|continue|pass|(?:raise|return)\b.*)\s*(#.*)?$/, + beforeText: verboseRegExp(` + ^ + \\s* + (?: + break | + continue | + pass | + raise \\b .* | + return \\b .* + ) + \\s* + ( [#] .* )? + $ + `), action: { indentAction: IndentAction.Outdent } @@ -40,8 +63,54 @@ export function getLanguageConfiguration() { * - there are multiple statements on the line (separated by semicolons) * Also note that `lambda` is purposefully excluded. */ - increaseIndentPattern: /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/, - decreaseIndentPattern: /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/ + increaseIndentPattern: verboseRegExp(` + ^ + \\s* + (?: + (?: + (?: + async | + class | + def | + elif | + except | + for | + if | + while | + with + ) + \\b .* + ) | + ( + else | + finally | + try + ) + ) + \\s* + [:] + \\s* + ( [#] .* )? + $ + `), + decreaseIndentPattern: verboseRegExp(` + ^ + \\s* + (?: + else | + finally | + (?: + elif | + except + ) + \\b .* + ) + \\s* + [:] + \\s* + ( [#] .* )? + $ + `) } }; } From 09f41102880505eddd5d725c262e9a7c22be6007 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 17:09:45 -0600 Subject: [PATCH 08/21] Make the regex tests a little easier to read/modify. --- .../language/languageConfiguration.unit.test.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 0038dba5f397..60ae3de8a551 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -54,7 +54,12 @@ suite('Language configuration regexes', () => { }); }); - ['elif x < 5:', 'else:', 'except TestError:', 'finally:'].forEach(example => { + [ + 'elif x < 5:', + 'else:', + 'except TestError:', + 'finally:' + ].forEach(example => { const keyword = example.split(' ')[0]; test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { @@ -78,7 +83,13 @@ suite('Language configuration regexes', () => { expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); }); - [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\'', ' return [ True, False, False ]'].forEach(example => { + [ + ' break', + '\t\t continue', + ' pass', + 'raise Exception(\'Unknown Exception\'', + ' return [ True, False, False ]' + ].forEach(example => { const keyword = example.trim().split(' ')[0]; const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`; From cbbb4194300de21ae767efcfbc995d9f324d3503 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 1 Aug 2019 17:38:03 -0600 Subject: [PATCH 09/21] Correct the break/continue/return regex. --- src/client/language/languageConfiguration.ts | 47 +++++++++++-------- .../languageConfiguration.unit.test.ts | 1 + 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 13edc9046ad8..ea4b53d5b255 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -37,13 +37,22 @@ export function getLanguageConfiguration() { { beforeText: verboseRegExp(` ^ - \\s* (?: - break | - continue | - pass | - raise \\b .* | - return \\b .* + (?: + \\s* + (?: + pass | + raise \\b .* | + ) + ) | + (?: + \\s+ + (?: + break | + continue | + return \\b .* + ) + ) ) \\s* ( [#] .* )? @@ -65,27 +74,25 @@ export function getLanguageConfiguration() { */ increaseIndentPattern: verboseRegExp(` ^ - \\s* (?: + \\s* (?: (?: async | class | def | - elif | except | for | if | + elif | while | with ) \\b .* ) | - ( - else | - finally | - try - ) + else | + try | + finally ) \\s* [:] @@ -97,13 +104,15 @@ export function getLanguageConfiguration() { ^ \\s* (?: - else | - finally | (?: - elif | - except - ) - \\b .* + (?: + elif | + except + ) + \\b .* + ) | + else | + finally ) \\s* [:] diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 60ae3de8a551..d9fb92ca97b9 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -88,6 +88,7 @@ suite('Language configuration regexes', () => { '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\'', + ' return', ' return [ True, False, False ]' ].forEach(example => { const keyword = example.trim().split(' ')[0]; From 0ccf9cda3040317e13eb05c473d8d2cb4aae47bc Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 10:05:06 -0600 Subject: [PATCH 10/21] Fill in language config regex test coverage. --- .../languageConfiguration.unit.test.ts | 251 +++++++++++++----- 1 file changed, 187 insertions(+), 64 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index d9fb92ca97b9..fbe97b5c4a66 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -3,18 +3,67 @@ 'use strict'; +// tslint:disable:max-func-body-length + import { expect } from 'chai'; import { getLanguageConfiguration } from '../../client/language/languageConfiguration'; +const NEEDS_INDENT = [ + /^break$/, + /^continue$/, + /^raise$/, // only re-raise + /^return\b/ +]; +const INDENT_ON_MATCH = [ + /^async\s+def\b/, + /^class\b/, + /^def\b/, + /^with\b/, + /^try\b/, + /^except\b/, + /^finally\b/, + /^while\b/, + /^for\b/, + /^if\b/, + /^elif\b/, + /^else\b/ +]; +const DEDENT_ON_MATCH = [ + /^elif\b/, + /^else\b/, + /^except\b/, + /^finally\b/ +]; +const DEDENT_ON_ENTER = [ + /^break$/, + /^continue$/, + /^raise\b/, + /^return\b/, + /^pass\b/ +]; + +function isMember(line: string, regexes: RegExp[]): boolean { + for (const regex of regexes) { + if (regex.test(line)) { + return true; + } + } + return false; +} + suite('Language configuration regexes', () => { const cfg = getLanguageConfiguration(); + const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules[0].beforeText; const DECREASE_INDENT_REGEX = cfg.indentationRules.decreaseIndentPattern; const INCREASE_INDENT_REGEX = cfg.indentationRules.increaseIndentPattern; - const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules[0].beforeText; const OUTDENT_ONENTER_REGEX = cfg.onEnterRules[2].beforeText; + // To see the actual (non-verbose) regex patterns, un-comment here: + //console.log(DECREASE_INDENT_REGEX.source); + //console.log(INCREASE_INDENT_REGEX.source); + //console.log(OUTDENT_ONENTER_REGEX.source); test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); @@ -32,77 +81,151 @@ suite('Language configuration regexes', () => { }); [ + // compound statements 'async def test(self):', - 'class TestClass:', - 'def foo(self, node, namespace=""):', + 'async def :', + 'async :', + 'class Test:', + 'class Test(object):', + 'class :', + 'def spam():', + 'def spam(self, node, namespace=""):', + 'def :', 'for item in items:', + 'for item in :', + 'for :', 'if foo is None:', + 'if :', 'try:', 'while \'::\' in macaddress:', - 'with self.test:' - ].forEach(example => { - const keyword = example.split(' ')[0]; - - test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { - const result = INCREASE_INDENT_REGEX.test(example); - expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); - }); - - test(`Decrease indent regex should not pick up lines containing the ${keyword} keyword`, async () => { - const result = DECREASE_INDENT_REGEX.test(example); - expect(result).to.be.equal(false, `Decrease indent regex should not pick up lines containing the ${keyword} keyword`); - }); - }); - - [ + 'while :', + 'with self.test:', + 'with :', 'elif x < 5:', + 'elif :', 'else:', + //'else if x < 5:', 'except TestError:', - 'finally:' - ].forEach(example => { - const keyword = example.split(' ')[0]; - - test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { - const result = INCREASE_INDENT_REGEX.test(example); - expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); - }); - - test(`Decrease indent regex should pick up lines containing the ${keyword} keyword`, async () => { - const result = DECREASE_INDENT_REGEX.test(example); - expect(result).to.be.equal(true, `Decrease indent regex should pick up lines containing the ${keyword} keyword`); - }); - }); - - test('Increase indent regex should not pick up lines without keywords', async () => { - const result = INCREASE_INDENT_REGEX.test('a = \'hello \\n \''); - expect(result).to.be.equal(false, 'Increase indent regex should not pick up lines without keywords'); - }); - - test('Decrease indent regex should not pick up lines without keywords', async () => { - const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); - expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); - }); - - [ - ' break', - '\t\t continue', - ' pass', - 'raise Exception(\'Unknown Exception\'', - ' return', - ' return [ True, False, False ]' - ].forEach(example => { - const keyword = example.trim().split(' ')[0]; - - const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`; - test(testWithoutComments, () => { - const result = OUTDENT_ONENTER_REGEX.test(example); - expect(result).to.be.equal(true, testWithoutComments); - }); - - const testWithComments = `Outdent regex on enter should pick up lines containing the ${keyword} keyword and ending with comments`; - test(testWithComments, () => { - const result = OUTDENT_ONENTER_REGEX.test(`${example} # test comment`); - expect(result).to.be.equal(true, testWithComments); + 'except :', + 'finally:', + // simple statemenhts + 'pass', + 'raise Exception(msg)', + 'raise Exception', + 'raise', // re-raise + 'break', + 'continue', + 'return True', + 'return (True, False, False)', + 'return [True, False, False]', + 'return {True, False, False}', + 'return (', + 'return [', + 'return {', + 'return', + // bogus + '', + ' ', + ' ' + ].forEach(base => { + [ + ['', '', '', ''], + // leading + [' ', '', '', ''], + [' ', '', '', ''], // unusual indent + ['\t\t', '', '', ''], + // pre-keyword + ['x', '', '', ''], + // post-keyword + ['', 'x', '', ''], + // pre-colon + ['', '', ' ', ''], + // trailing + ['', '', '', ' '], + ['', '', '', '# a comment'], + ['', '', '', ' # ...'] + ].forEach(whitespace => { + const [leading, postKeyword, preColon, trailing] = whitespace; + let invalid: string | undefined; + if (base.trim() === '') { + invalid = 'blank line'; + } else if (leading === '' && isMember(base, NEEDS_INDENT)) { + invalid = 'expected indent'; + } else if (leading.trim() !== '') { + invalid = 'look-alike - pre-keyword'; + } else if (postKeyword.trim() !== '') { + invalid = 'look-alike - post-keyword'; + } + + let resolvedBase = base; + if (postKeyword !== '') { + if (resolvedBase.includes(' ')) { + const kw = resolvedBase.split(' ', 1)[0]; + const remainder = resolvedBase.substring(kw.length); + resolvedBase = `${kw}${postKeyword} ${remainder}`; + } else { + if (resolvedBase.endsWith(':')) { + resolvedBase = `${resolvedBase.substring(0, resolvedBase.length - 1)}${postKeyword}:`; + } else { + resolvedBase = `${resolvedBase}${postKeyword}`; + } + } + } + if (preColon !== '') { + if (resolvedBase.endsWith(':')) { + resolvedBase = `${resolvedBase.substring(0, resolvedBase.length - 1)}${preColon}:`; + } else { + return; + } + } + const example = `${leading}${resolvedBase}${trailing}`; + + if (invalid) { + test(`Line "${example}" ignored (${invalid})`, () => { + let result = INCREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(false, 'unexpected match'); + + result = DECREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(false, 'unexpected match'); + + result = OUTDENT_ONENTER_REGEX.test(example); + expect(result).to.be.equal(false, 'unexpected match'); + }); + return; + } + + test(`Check indent-on-match for line "${example}"`, () => { + let expected = false; + if (isMember(base, INDENT_ON_MATCH)) { + expected = true; + } + + const result = INCREASE_INDENT_REGEX.test(example); + + expect(result).to.be.equal(expected, 'unexpected result'); + }); + + test(`Check dedent-on-match for line "${example}"`, () => { + let expected = false; + if (isMember(base, DEDENT_ON_MATCH)) { + expected = true; + } + + const result = DECREASE_INDENT_REGEX.test(example); + + expect(result).to.be.equal(expected, 'unexpected result'); + }); + + test(`Check dedent-on-enter for line "${example}"`, () => { + let expected = false; + if (isMember(base, DEDENT_ON_ENTER)) { + expected = true; + } + + const result = OUTDENT_ONENTER_REGEX.test(example); + + expect(result).to.be.equal(expected, 'unexpected result'); + }); }); }); }); From 02cee4bd324c0d4f823a56db5cf7cc5652ddd1c2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 10:06:10 -0600 Subject: [PATCH 11/21] Minor fixes to language config regex patterns. --- src/client/language/languageConfiguration.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index ea4b53d5b255..ab2af9c1cd21 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -42,12 +42,13 @@ export function getLanguageConfiguration() { \\s* (?: pass | - raise \\b .* | + raise \\s+ [^#\\s] [^#]* ) ) | (?: \\s+ (?: + raise | break | continue | return \\b .* @@ -55,7 +56,7 @@ export function getLanguageConfiguration() { ) ) \\s* - ( [#] .* )? + (?: [#] .* )? $ `), action: { @@ -74,11 +75,11 @@ export function getLanguageConfiguration() { */ increaseIndentPattern: verboseRegExp(` ^ + \\s* (?: - \\s* (?: (?: - async | + async \\s+ def | class | def | except | @@ -97,7 +98,7 @@ export function getLanguageConfiguration() { \\s* [:] \\s* - ( [#] .* )? + (?: [#] .* )? $ `), decreaseIndentPattern: verboseRegExp(` @@ -117,7 +118,7 @@ export function getLanguageConfiguration() { \\s* [:] \\s* - ( [#] .* )? + (?: [#] .* )? $ `) } From cc055fe9ad1740f634ad6bf50d14179715f45cc4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 10:12:47 -0600 Subject: [PATCH 12/21] Drop support for indent-on-enter for "return *". --- src/client/language/languageConfiguration.ts | 2 +- src/test/language/languageConfiguration.unit.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index ab2af9c1cd21..43ae6a9dc2d2 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -51,7 +51,7 @@ export function getLanguageConfiguration() { raise | break | continue | - return \\b .* + return ) ) ) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index fbe97b5c4a66..6da2a0fc9dff 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -41,7 +41,7 @@ const DEDENT_ON_ENTER = [ /^break$/, /^continue$/, /^raise\b/, - /^return\b/, + /^return$/, /^pass\b/ ]; @@ -115,6 +115,7 @@ suite('Language configuration regexes', () => { 'raise', // re-raise 'break', 'continue', + 'return', 'return True', 'return (True, False, False)', 'return [True, False, False]', From c20ea1c3ad60183800af0b05d54787c3288a69f8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 10:18:45 -0600 Subject: [PATCH 13/21] Drop indent-on-enter for "return" completely. --- src/client/language/languageConfiguration.ts | 3 +-- src/test/language/languageConfiguration.unit.test.ts | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 43ae6a9dc2d2..f9481e0d1fe9 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -50,8 +50,7 @@ export function getLanguageConfiguration() { (?: raise | break | - continue | - return + continue ) ) ) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 6da2a0fc9dff..1bd439738a3f 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -41,7 +41,8 @@ const DEDENT_ON_ENTER = [ /^break$/, /^continue$/, /^raise\b/, - /^return$/, + // For now we are ignoring "return" completely. See gh-6564. + ///^return\b/, /^pass\b/ ]; From 1ff16dcc77f87c489fe8acb8d59a539badb595dd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 10:23:23 -0600 Subject: [PATCH 14/21] Drop an erroneous test. --- src/test/language/languageConfiguration.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 1bd439738a3f..8d7fe9be51a1 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -105,7 +105,6 @@ suite('Language configuration regexes', () => { 'elif x < 5:', 'elif :', 'else:', - //'else if x < 5:', 'except TestError:', 'except :', 'finally:', From b630586a9f06e9ac39fc4ed192516ab6047079cd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 10:28:53 -0600 Subject: [PATCH 15/21] Add a NEWS entry. --- news/2 Fixes/6813.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 news/2 Fixes/6813.md diff --git a/news/2 Fixes/6813.md b/news/2 Fixes/6813.md new file mode 100644 index 000000000000..4c813edd73f6 --- /dev/null +++ b/news/2 Fixes/6813.md @@ -0,0 +1,2 @@ +Drop dedent-on-enter for "return" statements. It will be addressed in: + https://github.com/microsoft/vscode-python/issues/6564 From 395442fad98460c9595153a7daa76cded70e8e2f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 11:11:35 -0600 Subject: [PATCH 16/21] Add tests for verboseRegExp(). --- src/test/common/utils/regexp.unit.test.ts | 75 +++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 src/test/common/utils/regexp.unit.test.ts diff --git a/src/test/common/utils/regexp.unit.test.ts b/src/test/common/utils/regexp.unit.test.ts new file mode 100644 index 000000000000..9c9ccf459876 --- /dev/null +++ b/src/test/common/utils/regexp.unit.test.ts @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:no-multiline-string + +import { expect } from 'chai'; + +import { + verboseRegExp +} from '../../../client/common/utils/regexp'; + +suite('Utils for regular expressions - verboseRegExp()', () => { + test('typical usage', () => { + const regex = verboseRegExp(` + ^ + (?: + spam \\b .* + ) | + (?: + eggs \\b .* + ) + $ + `); + + expect(regex.source).to.equal('^(?:spam\\b.*)|(?:eggs\\b.*)$', 'mismatch'); + }); + + const whitespaceTests = [ + ['spam eggs', 'spameggs'], + [`spam + eggs`, 'spameggs'], + // empty + [' ', '(?:)'], + [` + `, '(?:)'] + ]; + for (const [pat, expected] of whitespaceTests) { + test(`whitespace removed ("${pat}")`, () => { + const regex = verboseRegExp(pat); + + expect(regex.source).to.equal(expected, 'mismatch'); + }); + } + + const noopPatterns = [ + '^(?:spam\\b.*)$', + 'spam', + '^spam$', + 'spam$', + '^spam' + ]; + for (const pat of noopPatterns) { + test(`pattern not changed ("${pat}")`, () => { + const regex = verboseRegExp(pat); + + expect(regex.source).to.equal(pat, 'mismatch'); + }); + } + + const emptyPatterns = [ + '', + ` + `, + ' ' + ]; + for (const pat of emptyPatterns) { + test(`no pattern ("${pat}")`, () => { + const regex = verboseRegExp(pat); + + expect(regex.source).to.equal('(?:)', 'mismatch'); + }); + } +}); From dea0c9aef76e150f17d321eb775f10a62807fe9b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 16:26:35 -0600 Subject: [PATCH 17/21] Move indentationRules back to onEnterRules. --- src/client/language/languageConfiguration.ts | 111 ++++++------- .../languageConfiguration.unit.test.ts | 153 ++++++++++-------- 2 files changed, 136 insertions(+), 128 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index f9481e0d1fe9..409371555210 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -2,13 +2,13 @@ // Licensed under the MIT License. 'use strict'; -import { IndentAction } from 'vscode'; +import { IndentAction, LanguageConfiguration } from 'vscode'; import { verboseRegExp } from '../common/utils/regexp'; // tslint:disable:no-multiline-string // tslint:disable-next-line:max-func-body-length -export function getLanguageConfiguration() { +export function getLanguageConfiguration(): LanguageConfiguration { return { onEnterRules: [ // multi-line separator @@ -33,7 +33,49 @@ export function getLanguageConfiguration() { appendText: '# ' } }, - // outdent on enter + // indent on enter (block-beginning statements) + { + /** + * This does not handle all cases. However, it does handle nearly all usage. + * Here's what it does not cover: + * - the statement is split over multiple lines (and hence the ":" is on a different line) + * - the code block is inlined (after the ":") + * - there are multiple statements on the line (separated by semicolons) + * Also note that `lambda` is purposefully excluded. + */ + beforeText: verboseRegExp(` + ^ + \\s* + (?: + (?: + (?: + class | + def | + async \\s+ def | + except | + for | + if | + elif | + while | + with + ) + \\b .* + ) | + else | + try | + finally + ) + \\s* + [:] + \\s* + (?: [#] .* )? + $ + `), + action: { + indentAction: IndentAction.Indent + } + }, + // outdent on enter (block-ending statements) { beforeText: verboseRegExp(` ^ @@ -62,64 +104,9 @@ export function getLanguageConfiguration() { indentAction: IndentAction.Outdent } } - ], - indentationRules: { - /** - * This does not handle all cases. However, it does handle nearly all usage. - * Here's what it does not cover: - * - the statement is split over multiple lines (and hence the ":" is on a different line) - * - the code block is inlined (after the ":") - * - there are multiple statements on the line (separated by semicolons) - * Also note that `lambda` is purposefully excluded. - */ - increaseIndentPattern: verboseRegExp(` - ^ - \\s* - (?: - (?: - (?: - async \\s+ def | - class | - def | - except | - for | - if | - elif | - while | - with - ) - \\b .* - ) | - else | - try | - finally - ) - \\s* - [:] - \\s* - (?: [#] .* )? - $ - `), - decreaseIndentPattern: verboseRegExp(` - ^ - \\s* - (?: - (?: - (?: - elif | - except - ) - \\b .* - ) | - else | - finally - ) - \\s* - [:] - \\s* - (?: [#] .* )? - $ - `) - } + // Note that we do not currently have an auto-dedent + // solution for "elif", "else", "except", and "finally". + // We had one but had to remove it (see issue #6886). + ] }; } diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 8d7fe9be51a1..df25c2cc9a77 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -17,7 +17,7 @@ const NEEDS_INDENT = [ /^raise$/, // only re-raise /^return\b/ ]; -const INDENT_ON_MATCH = [ +const INDENT_ON_ENTER = [ // block-beginning statements /^async\s+def\b/, /^class\b/, /^def\b/, @@ -31,13 +31,7 @@ const INDENT_ON_MATCH = [ /^elif\b/, /^else\b/ ]; -const DEDENT_ON_MATCH = [ - /^elif\b/, - /^else\b/, - /^except\b/, - /^finally\b/ -]; -const DEDENT_ON_ENTER = [ +const DEDENT_ON_ENTER = [ // block-ending statements /^break$/, /^continue$/, /^raise\b/, @@ -55,15 +49,76 @@ function isMember(line: string, regexes: RegExp[]): boolean { return false; } -suite('Language configuration regexes', () => { - const cfg = getLanguageConfiguration(); - const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules[0].beforeText; - const DECREASE_INDENT_REGEX = cfg.indentationRules.decreaseIndentPattern; - const INCREASE_INDENT_REGEX = cfg.indentationRules.increaseIndentPattern; - const OUTDENT_ONENTER_REGEX = cfg.onEnterRules[2].beforeText; - // To see the actual (non-verbose) regex patterns, un-comment here: - //console.log(DECREASE_INDENT_REGEX.source); - //console.log(INCREASE_INDENT_REGEX.source); +function resolveExample( + base: string, + leading: string, + postKeyword: string, + preColon: string, + trailing: string +): [string | undefined, string | undefined, boolean] { + let invalid: string | undefined; + if (base.trim() === '') { + invalid = 'blank line'; + } else if (leading === '' && isMember(base, NEEDS_INDENT)) { + invalid = 'expected indent'; + } else if (leading.trim() !== '') { + invalid = 'look-alike - pre-keyword'; + } else if (postKeyword.trim() !== '') { + invalid = 'look-alike - post-keyword'; + } + + let resolvedBase = base; + if (postKeyword !== '') { + if (resolvedBase.includes(' ')) { + const kw = resolvedBase.split(' ', 1)[0]; + const remainder = resolvedBase.substring(kw.length); + resolvedBase = `${kw}${postKeyword} ${remainder}`; + } else { + if (resolvedBase.endsWith(':')) { + resolvedBase = `${resolvedBase.substring(0, resolvedBase.length - 1)}${postKeyword}:`; + } else { + resolvedBase = `${resolvedBase}${postKeyword}`; + } + } + } + if (preColon !== '') { + if (resolvedBase.endsWith(':')) { + resolvedBase = `${resolvedBase.substring(0, resolvedBase.length - 1)}${preColon}:`; + } else { + return [undefined, undefined, true]; + } + } + const example = `${leading}${resolvedBase}${trailing}`; + return [example, invalid, false]; +} + +const cfg = getLanguageConfiguration(); + +suite('Language configuration - brackets', () => { + test('brackets is not defined', () => { + expect(cfg.brackets).to.be.equal(undefined, 'missing tests'); + }); +}); + +suite('Language configuration - comments', () => { + test('comments is not defined', () => { + expect(cfg.comments).to.be.equal(undefined, 'missing tests'); + }); +}); + +suite('Language configuration - indentationRules', () => { + test('indentationRules is not defined', () => { + expect(cfg.indentationRules).to.be.equal(undefined, 'missing tests'); + }); +}); + +suite('Language configuration - onEnterRules', () => { + const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules![0].beforeText; + const INDENT_ONENTER_REGEX = cfg.onEnterRules![2].beforeText; + const OUTDENT_ONENTER_REGEX = cfg.onEnterRules![3].beforeText; + // To see the actual (non-verbose) regex patterns, un-comment + // the following lines: + //console.log(INDENT_ONENTER_REGEX.source); //console.log(OUTDENT_ONENTER_REGEX.source); test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { @@ -147,46 +202,17 @@ suite('Language configuration regexes', () => { ['', '', '', ' # ...'] ].forEach(whitespace => { const [leading, postKeyword, preColon, trailing] = whitespace; - let invalid: string | undefined; - if (base.trim() === '') { - invalid = 'blank line'; - } else if (leading === '' && isMember(base, NEEDS_INDENT)) { - invalid = 'expected indent'; - } else if (leading.trim() !== '') { - invalid = 'look-alike - pre-keyword'; - } else if (postKeyword.trim() !== '') { - invalid = 'look-alike - post-keyword'; - } - - let resolvedBase = base; - if (postKeyword !== '') { - if (resolvedBase.includes(' ')) { - const kw = resolvedBase.split(' ', 1)[0]; - const remainder = resolvedBase.substring(kw.length); - resolvedBase = `${kw}${postKeyword} ${remainder}`; - } else { - if (resolvedBase.endsWith(':')) { - resolvedBase = `${resolvedBase.substring(0, resolvedBase.length - 1)}${postKeyword}:`; - } else { - resolvedBase = `${resolvedBase}${postKeyword}`; - } - } - } - if (preColon !== '') { - if (resolvedBase.endsWith(':')) { - resolvedBase = `${resolvedBase.substring(0, resolvedBase.length - 1)}${preColon}:`; - } else { - return; - } + const [_example, invalid, ignored] = resolveExample(base, leading, postKeyword, preColon, trailing); + if (ignored) { + return; } - const example = `${leading}${resolvedBase}${trailing}`; + const example = _example!; if (invalid) { test(`Line "${example}" ignored (${invalid})`, () => { - let result = INCREASE_INDENT_REGEX.test(example); - expect(result).to.be.equal(false, 'unexpected match'); + let result: boolean; - result = DECREASE_INDENT_REGEX.test(example); + result = INDENT_ONENTER_REGEX.test(example); expect(result).to.be.equal(false, 'unexpected match'); result = OUTDENT_ONENTER_REGEX.test(example); @@ -195,24 +221,13 @@ suite('Language configuration regexes', () => { return; } - test(`Check indent-on-match for line "${example}"`, () => { - let expected = false; - if (isMember(base, INDENT_ON_MATCH)) { - expected = true; - } - - const result = INCREASE_INDENT_REGEX.test(example); - - expect(result).to.be.equal(expected, 'unexpected result'); - }); - - test(`Check dedent-on-match for line "${example}"`, () => { + test(`Check indent-on-enter for line "${example}"`, () => { let expected = false; - if (isMember(base, DEDENT_ON_MATCH)) { + if (isMember(base, INDENT_ON_ENTER)) { expected = true; } - const result = DECREASE_INDENT_REGEX.test(example); + const result = INDENT_ONENTER_REGEX.test(example); expect(result).to.be.equal(expected, 'unexpected result'); }); @@ -230,3 +245,9 @@ suite('Language configuration regexes', () => { }); }); }); + +suite('Language configuration - wordPattern', () => { + test('wordPattern is not defined', () => { + expect(cfg.wordPattern).to.be.equal(undefined, 'missing tests'); + }); +}); From ad4a4be0d1b7d19c8638a33bd8e7425a46aeb955 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Aug 2019 16:45:32 -0600 Subject: [PATCH 18/21] Move a comment. --- src/test/language/languageConfiguration.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index df25c2cc9a77..c666f7c45a4e 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -32,11 +32,11 @@ const INDENT_ON_ENTER = [ // block-beginning statements /^else\b/ ]; const DEDENT_ON_ENTER = [ // block-ending statements + // For now we are ignoring "return" completely. See gh-6564. + ///^return\b/, /^break$/, /^continue$/, /^raise\b/, - // For now we are ignoring "return" completely. See gh-6564. - ///^return\b/, /^pass\b/ ]; From 15d1ff2d1fd9b856e6801c9cd6e2e2027142470e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 13 Aug 2019 10:34:20 -0600 Subject: [PATCH 19/21] Add a doc comment for verboseRegExp(). --- src/client/common/utils/regexp.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/client/common/utils/regexp.ts b/src/client/common/utils/regexp.ts index 3a353c1fc006..2d20b73e7f28 100644 --- a/src/client/common/utils/regexp.ts +++ b/src/client/common/utils/regexp.ts @@ -3,6 +3,19 @@ 'use strict'; +/* Generate a RegExp from a "verbose" pattern. + * + * All whitespace in the pattern is removed, including newlines. This + * allows the pattern to be much more readable by allowing it to span + * multiple lines and to separate tokens with insignificant whitespace. + * The functionality is similar to the VERBOSE ("x") flag in Python's + * regular expressions. + * + * Note that significant whitespace in the pattern must be explicitly + * indicated by "\s". Also, unlike with regular expression literals, + * backslashes must be escaped. Conversely, forward slashes do not + * need to be escaped. + */ export function verboseRegExp(pattern: string): RegExp { pattern = pattern.replace(/\s+?/g, ''); return RegExp(pattern); From 563201468b1904613d0a9d7225124be944ac697c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 13 Aug 2019 10:37:48 -0600 Subject: [PATCH 20/21] Clarify the objective of a test. --- src/test/common/utils/regexp.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/utils/regexp.unit.test.ts b/src/test/common/utils/regexp.unit.test.ts index 9c9ccf459876..9e8c8049bec6 100644 --- a/src/test/common/utils/regexp.unit.test.ts +++ b/src/test/common/utils/regexp.unit.test.ts @@ -12,7 +12,7 @@ import { } from '../../../client/common/utils/regexp'; suite('Utils for regular expressions - verboseRegExp()', () => { - test('typical usage', () => { + test('whitespace removed in multiline pattern (example of typical usage)', () => { const regex = verboseRegExp(` ^ (?: From 611b1266b97307029be7f3aed21c8b832c047e5d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 13 Aug 2019 10:42:19 -0600 Subject: [PATCH 21/21] Move the language configuration test suites into a parent suite. --- .../languageConfiguration.unit.test.ts | 268 +++++++++--------- 1 file changed, 135 insertions(+), 133 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index c666f7c45a4e..00ec4cd9a353 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -92,162 +92,164 @@ function resolveExample( return [example, invalid, false]; } -const cfg = getLanguageConfiguration(); +suite('Language Configuration', () => { + const cfg = getLanguageConfiguration(); -suite('Language configuration - brackets', () => { - test('brackets is not defined', () => { - expect(cfg.brackets).to.be.equal(undefined, 'missing tests'); + suite('"brackets"', () => { + test('brackets is not defined', () => { + expect(cfg.brackets).to.be.equal(undefined, 'missing tests'); + }); }); -}); -suite('Language configuration - comments', () => { - test('comments is not defined', () => { - expect(cfg.comments).to.be.equal(undefined, 'missing tests'); + suite('"comments"', () => { + test('comments is not defined', () => { + expect(cfg.comments).to.be.equal(undefined, 'missing tests'); + }); }); -}); -suite('Language configuration - indentationRules', () => { - test('indentationRules is not defined', () => { - expect(cfg.indentationRules).to.be.equal(undefined, 'missing tests'); + suite('"indentationRules"', () => { + test('indentationRules is not defined', () => { + expect(cfg.indentationRules).to.be.equal(undefined, 'missing tests'); + }); }); -}); -suite('Language configuration - onEnterRules', () => { - const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules![0].beforeText; - const INDENT_ONENTER_REGEX = cfg.onEnterRules![2].beforeText; - const OUTDENT_ONENTER_REGEX = cfg.onEnterRules![3].beforeText; - // To see the actual (non-verbose) regex patterns, un-comment - // the following lines: - //console.log(INDENT_ONENTER_REGEX.source); - //console.log(OUTDENT_ONENTER_REGEX.source); - - test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { - const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); - expect(result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); - }); + suite('"onEnterRules"', () => { + const MULTILINE_SEPARATOR_INDENT_REGEX = cfg.onEnterRules![0].beforeText; + const INDENT_ONENTER_REGEX = cfg.onEnterRules![2].beforeText; + const OUTDENT_ONENTER_REGEX = cfg.onEnterRules![3].beforeText; + // To see the actual (non-verbose) regex patterns, un-comment + // the following lines: + //console.log(INDENT_ONENTER_REGEX.source); + //console.log(OUTDENT_ONENTER_REGEX.source); + + test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { + const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); + expect(result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); + }); - test('Multiline separator indent regex should not pick up strings with escaped characters', async () => { - const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'hello \\n\''); - expect(result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); - }); + test('Multiline separator indent regex should not pick up strings with escaped characters', async () => { + const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'hello \\n\''); + expect(result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); + }); - test('Multiline separator indent regex should pick up strings ending with a multiline separator', async () => { - const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'multiline \\'); - expect(result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); - }); + test('Multiline separator indent regex should pick up strings ending with a multiline separator', async () => { + const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'multiline \\'); + expect(result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); + }); - [ - // compound statements - 'async def test(self):', - 'async def :', - 'async :', - 'class Test:', - 'class Test(object):', - 'class :', - 'def spam():', - 'def spam(self, node, namespace=""):', - 'def :', - 'for item in items:', - 'for item in :', - 'for :', - 'if foo is None:', - 'if :', - 'try:', - 'while \'::\' in macaddress:', - 'while :', - 'with self.test:', - 'with :', - 'elif x < 5:', - 'elif :', - 'else:', - 'except TestError:', - 'except :', - 'finally:', - // simple statemenhts - 'pass', - 'raise Exception(msg)', - 'raise Exception', - 'raise', // re-raise - 'break', - 'continue', - 'return', - 'return True', - 'return (True, False, False)', - 'return [True, False, False]', - 'return {True, False, False}', - 'return (', - 'return [', - 'return {', - 'return', - // bogus - '', - ' ', - ' ' - ].forEach(base => { [ - ['', '', '', ''], - // leading - [' ', '', '', ''], - [' ', '', '', ''], // unusual indent - ['\t\t', '', '', ''], - // pre-keyword - ['x', '', '', ''], - // post-keyword - ['', 'x', '', ''], - // pre-colon - ['', '', ' ', ''], - // trailing - ['', '', '', ' '], - ['', '', '', '# a comment'], - ['', '', '', ' # ...'] - ].forEach(whitespace => { - const [leading, postKeyword, preColon, trailing] = whitespace; - const [_example, invalid, ignored] = resolveExample(base, leading, postKeyword, preColon, trailing); - if (ignored) { - return; - } - const example = _example!; - - if (invalid) { - test(`Line "${example}" ignored (${invalid})`, () => { - let result: boolean; + // compound statements + 'async def test(self):', + 'async def :', + 'async :', + 'class Test:', + 'class Test(object):', + 'class :', + 'def spam():', + 'def spam(self, node, namespace=""):', + 'def :', + 'for item in items:', + 'for item in :', + 'for :', + 'if foo is None:', + 'if :', + 'try:', + 'while \'::\' in macaddress:', + 'while :', + 'with self.test:', + 'with :', + 'elif x < 5:', + 'elif :', + 'else:', + 'except TestError:', + 'except :', + 'finally:', + // simple statemenhts + 'pass', + 'raise Exception(msg)', + 'raise Exception', + 'raise', // re-raise + 'break', + 'continue', + 'return', + 'return True', + 'return (True, False, False)', + 'return [True, False, False]', + 'return {True, False, False}', + 'return (', + 'return [', + 'return {', + 'return', + // bogus + '', + ' ', + ' ' + ].forEach(base => { + [ + ['', '', '', ''], + // leading + [' ', '', '', ''], + [' ', '', '', ''], // unusual indent + ['\t\t', '', '', ''], + // pre-keyword + ['x', '', '', ''], + // post-keyword + ['', 'x', '', ''], + // pre-colon + ['', '', ' ', ''], + // trailing + ['', '', '', ' '], + ['', '', '', '# a comment'], + ['', '', '', ' # ...'] + ].forEach(whitespace => { + const [leading, postKeyword, preColon, trailing] = whitespace; + const [_example, invalid, ignored] = resolveExample(base, leading, postKeyword, preColon, trailing); + if (ignored) { + return; + } + const example = _example!; - result = INDENT_ONENTER_REGEX.test(example); - expect(result).to.be.equal(false, 'unexpected match'); + if (invalid) { + test(`Line "${example}" ignored (${invalid})`, () => { + let result: boolean; - result = OUTDENT_ONENTER_REGEX.test(example); - expect(result).to.be.equal(false, 'unexpected match'); - }); - return; - } + result = INDENT_ONENTER_REGEX.test(example); + expect(result).to.be.equal(false, 'unexpected match'); - test(`Check indent-on-enter for line "${example}"`, () => { - let expected = false; - if (isMember(base, INDENT_ON_ENTER)) { - expected = true; + result = OUTDENT_ONENTER_REGEX.test(example); + expect(result).to.be.equal(false, 'unexpected match'); + }); + return; } - const result = INDENT_ONENTER_REGEX.test(example); + test(`Check indent-on-enter for line "${example}"`, () => { + let expected = false; + if (isMember(base, INDENT_ON_ENTER)) { + expected = true; + } - expect(result).to.be.equal(expected, 'unexpected result'); - }); + const result = INDENT_ONENTER_REGEX.test(example); - test(`Check dedent-on-enter for line "${example}"`, () => { - let expected = false; - if (isMember(base, DEDENT_ON_ENTER)) { - expected = true; - } + expect(result).to.be.equal(expected, 'unexpected result'); + }); + + test(`Check dedent-on-enter for line "${example}"`, () => { + let expected = false; + if (isMember(base, DEDENT_ON_ENTER)) { + expected = true; + } - const result = OUTDENT_ONENTER_REGEX.test(example); + const result = OUTDENT_ONENTER_REGEX.test(example); - expect(result).to.be.equal(expected, 'unexpected result'); + expect(result).to.be.equal(expected, 'unexpected result'); + }); }); }); }); -}); -suite('Language configuration - wordPattern', () => { - test('wordPattern is not defined', () => { - expect(cfg.wordPattern).to.be.equal(undefined, 'missing tests'); + suite('"wordPattern"', () => { + test('wordPattern is not defined', () => { + expect(cfg.wordPattern).to.be.equal(undefined, 'missing tests'); + }); }); });