diff --git a/packages/language-python/grammars/ts/indents.scm b/packages/language-python/grammars/ts/indents.scm index 9753893079..2a46fe1a38 100644 --- a/packages/language-python/grammars/ts/indents.scm +++ b/packages/language-python/grammars/ts/indents.scm @@ -1,4 +1,8 @@ -; Excluding dictionary key/value separators… + +; IGNORE NON-BLOCK-STARTING COLONS +; ================================ + +; First, exclude dictionary key/value separators… (dictionary (pair ":" @_IGNORE_ (#set! capture.final))) @@ -7,14 +11,164 @@ ((lambda ":" @_IGNORE_) (#set! capture.final)) -; …and type annotations on function parameters/class members… +; …list subscript syntax… +(slice ":" @_IGNORE_ + (#set! capture.final)) + +; …and type annotations on function parameters/class members. (":" @_IGNORE_ . (type) (#set! capture.final)) -; …all other colons we encounter hint at upcoming indents. +; IGNORE BLOCK-STARTING COLONS BEFORE ONE-LINERS +; ============================================== + +; Now that we've done that, all block-starting colons that have their +; consequence block start and end on the same line should be filtered out. +; +; We also test for `lastTextOnRow` to ensure we're not followed by an _empty_ +; consequence block, which is surprisingly common. Probably a bug, but it's got +; to be worked around in the meantime. +; +; We check for adjacency between the `:` and the `block` because otherwise we +; might incorrectly match cases like +; +; if 2 > 1: # some comment +; +; since those comments can also be followed by an empty `block` node on the same +; line. +; +(if_statement + ":" @_IGNORE_ + . + consequence: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(elif_clause + ":" @_IGNORE_ + . + consequence: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(else_clause + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(match_statement + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(case_clause + ":" @_IGNORE_ + . + consequence: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(while_statement + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(for_statement + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(try_statement + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(except_clause + ":" @_IGNORE_ + . + (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +; Special case for try/except statements, since they don't seem to be valid +; until they're fully intact. If we don't do this, `except` doesn't dedent. +; +; This is like the `elif`/`else` problem below, but it's trickier because an +; identifier could plausibly begin with the string `except` and we don't want +; to make an across-the-board assumption. +(ERROR + "try" + ":" @indent + (block + (expression_statement + (identifier) @dedent + (#match? @dedent "except") + ) + ) +) + +(function_definition + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + +(class_definition + ":" @_IGNORE_ + . + body: (block) + (#is-not? test.lastTextOnRow) + (#is? test.startsOnSameRowAs "nextSibling.endPosition") + (#set! capture.final) +) + + +; REMAINING COLONS +; ================ + +; Now that we've done this work, all other colons we encounter hint at upcoming +; indents. +; +; TODO: Based on the stuff we're doing above, it's arguable that the +; exclude-all-counterexamples approach is no longer useful and we should +; instead be opting into indentation. Revisit this! ":" @indent +; MISCELLANEOUS +; ============= + ; When typing out "else" after an "if" statement, tree-sitter-python won't -; acknowlege it as an `else` statement until it's indented properly, which is +; acknowledge it as an `else` statement until it's indented properly, which is ; quite the dilemma for us. Before that happens, it's an identifier named ; "else". This has a chance of spuriously dedenting if you're typing out a ; variable called `elsewhere` or something, but I'm OK with that. @@ -22,7 +176,21 @@ ; This also means that we _should not_ mark an actual `else` keyword with ; `@dedent`, because if it's recognized as such, that's a sign that it's ; already indented correctly and we shouldn't touch it. +; +; All this also applies to `elif`. ((identifier) @dedent (#match? @dedent "^(elif|else)$")) +; Likewise, typing `case` at the beginning of a line within a match block — in +; cases where it's interpreted as an identifier — strongly suggests that we +; should dedent one level so it's properly recognized as a new `case` keyword. +( + (identifier) @dedent + (#equals? @dedent "case") + (#is? test.descendantOfType "case_clause") +) + + +; All instances of brackets/braces should be indented if they span multiple +; lines. ["(" "[" "{"] @indent [")" "]" "}"] @dedent diff --git a/packages/language-python/spec/.eslintrc.js b/packages/language-python/spec/.eslintrc.js new file mode 100644 index 0000000000..8cc26374a1 --- /dev/null +++ b/packages/language-python/spec/.eslintrc.js @@ -0,0 +1,14 @@ +module.exports = { + env: { jasmine: true }, + globals: { + waitsForPromise: true, + runGrammarTests: true, + runFoldsTests: true + }, + rules: { + "node/no-unpublished-require": "off", + "node/no-extraneous-require": "off", + "no-unused-vars": "off", + "no-empty": "off" + } +}; diff --git a/packages/language-python/spec/indentation-spec.js b/packages/language-python/spec/indentation-spec.js new file mode 100644 index 0000000000..fa4d058704 --- /dev/null +++ b/packages/language-python/spec/indentation-spec.js @@ -0,0 +1,222 @@ +const dedent = require('dedent'); + +describe('Python indentation (modern Tree-sitter)', () => { + let editor; + let languageMode; + let grammar; + + async function insertNewline() { + editor.getLastSelection().insertText('\n', { autoIndent: true, autoIndentNewline: true }) + await languageMode.atTransactionEnd(); + } + + async function expectToAutoIndentAfter(text, assert = true) { + editor.setText(text); + await languageMode.atTransactionEnd(); + editor.setCursorBufferPosition([Infinity, Infinity]); + let currentRow = editor.getLastCursor().getBufferPosition().row; + insertNewline(); + if (assert) { + expect(editor.lineTextForBufferRow(currentRow + 1)).toEqual(' '); + } else { + expect(editor.lineTextForBufferRow(currentRow + 1)).toEqual(''); + } + } + + beforeEach(async () => { + atom.config.set('core.useTreeSitterParsers', true); + + editor = await atom.workspace.open(); + await atom.packages.activatePackage('language-python'); + grammar = atom.grammars.grammarForScopeName('source.python'); + editor.setGrammar(grammar); + languageMode = editor.languageMode; + await languageMode.ready; + }); + + it('indents blocks properly', async () => { + await expectToAutoIndentAfter(`if 1 > 2:`) + await expectToAutoIndentAfter(`if 1 > 2: # test`) + await expectToAutoIndentAfter(`if 1 > 2: pass`, false) + + await expectToAutoIndentAfter(`def f(x):`) + await expectToAutoIndentAfter(`def f(x): # test`) + await expectToAutoIndentAfter(`def f(x): pass`, false) + + await expectToAutoIndentAfter(`class Fx(object):`) + await expectToAutoIndentAfter(`class Fx(object): # test`) + await expectToAutoIndentAfter(`class Fx(object): pass`, false) + + await expectToAutoIndentAfter(`while True:`) + await expectToAutoIndentAfter(`while True: # test`) + await expectToAutoIndentAfter(`while True: pass`, false) + + await expectToAutoIndentAfter(`for _ in iter(x):`) + await expectToAutoIndentAfter(`for _ in iter(x): # test`) + await expectToAutoIndentAfter(`for _ in iter(x): pass`, false) + + await expectToAutoIndentAfter(dedent` + if 1 > 2: + pass + elif 2 > 3: + `) + + await expectToAutoIndentAfter(dedent` + if 1 > 2: + pass + elif 2 > 3: # test + `) + + await expectToAutoIndentAfter(dedent` + if 1 > 2: + pass + elif 2 > 3: pass + `, false) + + await expectToAutoIndentAfter(dedent` + if 1 > 2: + pass + elif 2 > 3: + pass + else: + `) + + await expectToAutoIndentAfter(dedent` + if 1 > 2: + pass + elif 2 > 3: + pass + else: # test + `) + + await expectToAutoIndentAfter(dedent` + if 1 > 2: + pass + elif 2 > 3: + pass + else: pass + `, false) + + await expectToAutoIndentAfter(`try:`) + + // The assertions below don't work because `tree-sitter-python` doesn't + // parse them correctly unless they occur within an already intact + // `try/except` block. This needs to be fixed upstream. + // await expectToAutoIndentAfter(`try: # test`) + // await expectToAutoIndentAfter(`try: pass`, false) + + await expectToAutoIndentAfter(dedent` + try: + do_something() + except: + `) + await expectToAutoIndentAfter(dedent` + try: + do_something() + except: # test + `) + await expectToAutoIndentAfter(dedent` + try: + do_something() + except: pass + `, false) + }); + + it('indents blocks properly (complex cases)', async () => { + editor.setText(dedent` + try: pass + except: pass + `); + await languageMode.atTransactionEnd(); + editor.setCursorBufferPosition([0, Infinity]); + await insertNewline(); + expect(editor.lineTextForBufferRow(1)).toBe('') + + editor.setText(dedent` + try: #foo + except: pass + `) + await languageMode.atTransactionEnd(); + editor.setCursorBufferPosition([0, Infinity]); + await insertNewline(); + expect(editor.lineTextForBufferRow(1)).toBe(' ') + }) + + it(`does not indent for other usages of colons`, async () => { + await expectToAutoIndentAfter(`x = lambda a : a + 10`, false) + await expectToAutoIndentAfter(`x = list[:2]`, false) + await expectToAutoIndentAfter(`x = { foo: 2 }`, false) + }); + + it('indents braces properly', async () => { + let pairs = [ + ['[', ']'], + ['{', '}'], + ['(', ')'] + ]; + for (let [a, b] of pairs) { + editor.setText(`x = ${a} + + ${b}`) + await languageMode.atTransactionEnd(); + editor.setCursorBufferPosition([0, Infinity]); + await insertNewline(); + expect(editor.lineTextForBufferRow(1)).toBe(' ') + } + + editor.setText(`x = < + + >`) + await languageMode.atTransactionEnd(); + editor.setCursorBufferPosition([0, Infinity]); + await insertNewline(); + expect(editor.lineTextForBufferRow(1)).toBe('') + }); + + it('dedents properly', async () => { + editor.setText(dedent` + if 1 > 2: + pass + eli + `); + editor.setCursorBufferPosition([Infinity, Infinity]); + await languageMode.atTransactionEnd(); + editor.getLastSelection().insertText('f', { + autoIndent: true, + autoDecreaseIndent: true + }); + await languageMode.atTransactionEnd(); + expect(editor.lineTextForBufferRow(2)).toBe('elif'); + + editor.setText(dedent` + if 1 > 2: + pass + els + `); + editor.setCursorBufferPosition([Infinity, Infinity]); + await languageMode.atTransactionEnd(); + editor.getLastSelection().insertText('e', { + autoIndent: true, + autoDecreaseIndent: true + }); + await languageMode.atTransactionEnd(); + expect(editor.lineTextForBufferRow(2)).toBe('else'); + + + editor.setText(dedent` + match x: + case "a": + pass + cas + `); + editor.setCursorBufferPosition([Infinity, Infinity]); + await languageMode.atTransactionEnd(); + editor.getLastSelection().insertText('e', { + autoIndent: true, + autoDecreaseIndent: true + }); + await languageMode.atTransactionEnd(); + expect(editor.lineTextForBufferRow(3)).toBe(' case'); + }); + +});