Skip to content

Commit d27f00c

Browse files
author
Mikhail Arkhipov
authored
Improve detection if '=' is in function or lambda arguments (#1824)
* Undo changes * Test fixes * Increase timeout * Remove double event listening * Remove test * Revert "Remove test" This reverts commit e240c3f. * Revert "Remove double event listening" This reverts commit af573be. * #1096 The if statement is automatically formatted incorrectly * Merge fix * Add more tests * More tests * Typo * Test * Also better handle multiline arguments * Add a couple missing periods [skip ci] * Undo changes * Test fixes * Increase timeout * Remove double event listening * Remove test * Revert "Remove test" This reverts commit e240c3f. * Revert "Remove double event listening" This reverts commit af573be. * Merge fix * #1257 On type formatting errors for args and kwargs * Handle f-strings * Stop importing from test code * #1308 Single line statements leading to an indentation on the next line * #726 editing python after inline if statement invalid indent * Undo change * Move constant * Harden LS startup error checks * #1364 Intellisense doesn't work after specific const string * Telemetry for the analysis enging * PR feedback * Fix typo * Test baseline update * Jedi 0.12 * Priority to goto_defition * News * Replace unzip * Linux flavors + test * Grammar check * Grammar test * Test baselines * Add news * Pin dependency [skip ci] * Specify markdown as preferable format * Improve function argument detection * Specify markdown * Pythia setting * Baseline updates * Baseline update * Improve startup * Handle missing interpreter better * Handle interpreter change * Delete old file * Fix LS startup time reporting * Remove Async suffix from IFileSystem * Remove Pythia * Remove pre-packaged MSIL * Exe name on Unix * Plain linux * Fix casing * Fix message * Update PTVS engine activation steps * Type formatter eats space in from . * fIX CASING * Remove flag * Don't wait for LS * Small test fixes * Update hover baselines * Rename the engine * Formatting 1 * Add support for 'rf' strings * Add two spaces before comment per PEP * Fix @ operator spacing * Handle module and unary ops * Type hints * Fix typo * Trailing comma * Require space after if * underscore numbers * Update list of keywords * Function arguments * Function arguments * PR feedback * Handle lambdas * News
1 parent d112ab8 commit d27f00c

File tree

3 files changed

+202
-100
lines changed

3 files changed

+202
-100
lines changed

‎news/2 Fixes/1796.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
`editor.formatOnType` now better handles multiline function arguments

‎src/client/formatters/lineFormatter.ts

Lines changed: 118 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
// tslint:disable-next-line:import-name
55
import Char from 'typescript-char';
6-
import { TextDocument } from 'vscode';
6+
import { Position, Range, TextDocument } from 'vscode';
77
import { BraceCounter } from '../language/braceCounter';
88
import { TextBuilder } from '../language/textBuilder';
99
import { TextRangeCollection } from '../language/textRangeCollection';
@@ -255,13 +255,11 @@ export class LineFormatter {
255255

256256
// tslint:disable-next-line:cyclomatic-complexity
257257
private isEqualsInsideArguments(index: number): boolean {
258-
// Since we don't have complete statement, this is mostly heuristics.
259-
// Therefore the code may not be handling all possible ways of the
260-
// argument list continuation.
261258
if (index < 1) {
262259
return false;
263260
}
264261

262+
// We are looking for IDENT = ?
265263
const prev = this.tokens.getItemAt(index - 1);
266264
if (prev.type !== TokenType.Identifier) {
267265
return false;
@@ -271,41 +269,7 @@ export class LineFormatter {
271269
return false; // Type hint should have spaces around like foo(x: int = 1) per PEP 8
272270
}
273271

274-
const first = this.tokens.getItemAt(0);
275-
if (first.type === TokenType.Comma) {
276-
return true; // Line starts with commma
277-
}
278-
279-
const last = this.tokens.getItemAt(this.tokens.count - 1);
280-
if (last.type === TokenType.Comma) {
281-
return true; // Line ends in comma
282-
}
283-
284-
if (last.type === TokenType.Comment && this.tokens.count > 1 && this.tokens.getItemAt(this.tokens.count - 2).type === TokenType.Comma) {
285-
return true; // Line ends in comma and then comment
286-
}
287-
288-
if (this.document) {
289-
const prevLine = this.lineNumber > 0 ? this.document.lineAt(this.lineNumber - 1).text : '';
290-
const prevLineTokens = new Tokenizer().tokenize(prevLine);
291-
if (prevLineTokens.count > 0) {
292-
const lastOnPrevLine = prevLineTokens.getItemAt(prevLineTokens.count - 1);
293-
if (lastOnPrevLine.type === TokenType.Comma) {
294-
return true; // Previous line ends in comma
295-
}
296-
if (lastOnPrevLine.type === TokenType.Comment && prevLineTokens.count > 1 && prevLineTokens.getItemAt(prevLineTokens.count - 2).type === TokenType.Comma) {
297-
return true; // Previous line ends in comma and then comment
298-
}
299-
}
300-
}
301-
302-
for (let i = 0; i < index; i += 1) {
303-
const t = this.tokens.getItemAt(i);
304-
if (this.isKeyword(t, 'lambda')) {
305-
return true;
306-
}
307-
}
308-
return this.braceCounter.isOpened(TokenType.OpenBrace);
272+
return this.isInsideFunctionArguments(this.tokens.getItemAt(index).start);
309273
}
310274

311275
private isOpenBraceType(type: TokenType): boolean {
@@ -317,6 +281,7 @@ export class LineFormatter {
317281
private isBraceType(type: TokenType): boolean {
318282
return this.isOpenBraceType(type) || this.isCloseBraceType(type);
319283
}
284+
320285
private isMultipleStatements(index: number): boolean {
321286
for (let i = index; i >= 0; i -= 1) {
322287
if (this.tokens.getItemAt(i).type === TokenType.Semicolon) {
@@ -332,4 +297,118 @@ export class LineFormatter {
332297
private isKeyword(t: IToken, keyword: string): boolean {
333298
return t.type === TokenType.Identifier && t.length === keyword.length && this.text.substr(t.start, t.length) === keyword;
334299
}
300+
301+
// tslint:disable-next-line:cyclomatic-complexity
302+
private isInsideFunctionArguments(position: number): boolean {
303+
if (!this.document) {
304+
return false; // unable to determine
305+
}
306+
307+
// Walk up until beginning of the document or line with 'def IDENT(' or line ending with :
308+
// IDENT( by itself is not reliable since they can be nested in IDENT(IDENT(a), x=1)
309+
let start = new Position(0, 0);
310+
for (let i = this.lineNumber; i >= 0; i -= 1) {
311+
const line = this.document.lineAt(i);
312+
const lineTokens = new Tokenizer().tokenize(line.text);
313+
if (lineTokens.count === 0) {
314+
continue;
315+
}
316+
// 'def IDENT('
317+
const first = lineTokens.getItemAt(0);
318+
if (lineTokens.count >= 3 &&
319+
first.length === 3 && line.text.substr(first.start, first.length) === 'def' &&
320+
lineTokens.getItemAt(1).type === TokenType.Identifier &&
321+
lineTokens.getItemAt(2).type === TokenType.OpenBrace) {
322+
start = line.range.start;
323+
break;
324+
}
325+
326+
if (lineTokens.count > 0 && i < this.lineNumber) {
327+
// One of previous lines ends with :
328+
const last = lineTokens.getItemAt(lineTokens.count - 1);
329+
if (last.type === TokenType.Colon) {
330+
start = this.document.lineAt(i + 1).range.start;
331+
break;
332+
} else if (lineTokens.count > 1) {
333+
const beforeLast = lineTokens.getItemAt(lineTokens.count - 2);
334+
if (beforeLast.type === TokenType.Colon && last.type === TokenType.Comment) {
335+
start = this.document.lineAt(i + 1).range.start;
336+
break;
337+
}
338+
}
339+
}
340+
}
341+
342+
// Now tokenize from the nearest reasonable point
343+
const currentLine = this.document.lineAt(this.lineNumber);
344+
const text = this.document.getText(new Range(start, currentLine.range.end));
345+
const tokens = new Tokenizer().tokenize(text);
346+
347+
// Translate position in the line being formatted to the position in the tokenized block
348+
position = this.document.offsetAt(currentLine.range.start) + position - this.document.offsetAt(start);
349+
350+
// Walk tokens locating narrowest function signature as in IDENT( | )
351+
let funcCallStartIndex = -1;
352+
let funcCallEndIndex = -1;
353+
for (let i = 0; i < tokens.count - 1; i += 1) {
354+
const t = tokens.getItemAt(i);
355+
if (t.type === TokenType.Identifier) {
356+
const next = tokens.getItemAt(i + 1);
357+
if (next.type === TokenType.OpenBrace && !this.isKeywordWithSpaceBeforeBrace(text.substr(t.start, t.length))) {
358+
// We are at IDENT(, try and locate the closing brace
359+
let closeBraceIndex = this.findClosingBrace(tokens, i + 1);
360+
// Closing brace is not required in case construct is not yet terminated
361+
closeBraceIndex = closeBraceIndex > 0 ? closeBraceIndex : tokens.count - 1;
362+
// Are we in range?
363+
if (position > next.start && position < tokens.getItemAt(closeBraceIndex).start) {
364+
funcCallStartIndex = i;
365+
funcCallEndIndex = closeBraceIndex;
366+
}
367+
}
368+
}
369+
}
370+
// Did we find anything?
371+
if (funcCallStartIndex < 0) {
372+
// No? See if we are between 'lambda' and ':'
373+
for (let i = 0; i < tokens.count; i += 1) {
374+
const t = tokens.getItemAt(i);
375+
if (t.type === TokenType.Identifier && text.substr(t.start, t.length) === 'lambda') {
376+
if (position < t.start) {
377+
break; // Position is before the nearest 'lambda'
378+
}
379+
let colonIndex = this.findNearestColon(tokens, i + 1);
380+
// Closing : is not required in case construct is not yet terminated
381+
colonIndex = colonIndex > 0 ? colonIndex : tokens.count - 1;
382+
if (position > t.start && position < tokens.getItemAt(colonIndex).start) {
383+
funcCallStartIndex = i;
384+
funcCallEndIndex = colonIndex;
385+
}
386+
}
387+
}
388+
}
389+
return funcCallStartIndex >= 0 && funcCallEndIndex > 0;
390+
}
391+
392+
private findNearestColon(tokens: ITextRangeCollection<IToken>, index: number): number {
393+
for (let i = index; i < tokens.count; i += 1) {
394+
if (tokens.getItemAt(i).type === TokenType.Colon) {
395+
return i;
396+
}
397+
}
398+
return -1;
399+
}
400+
401+
private findClosingBrace(tokens: ITextRangeCollection<IToken>, index: number): number {
402+
const braceCounter = new BraceCounter();
403+
for (let i = index; i < tokens.count; i += 1) {
404+
const t = tokens.getItemAt(i);
405+
if (t.type === TokenType.OpenBrace || t.type === TokenType.CloseBrace) {
406+
braceCounter.countBrace(t);
407+
}
408+
if (braceCounter.count === 0) {
409+
return i;
410+
}
411+
}
412+
return -1;
413+
}
335414
}

0 commit comments

Comments
 (0)