From 767590151728baa77f2b3297a2b802c32121c8c2 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Dec 2017 10:18:47 -0800 Subject: [PATCH 01/51] Basic tokenizer --- .vscode/launch.json | 11 +- .vscode/tasks.json | 6 +- gulpfile.js | 89 +++++++++++--- package.json | 9 +- src/client/language/characterStream.ts | 134 +++++++++++++++++++++ src/client/language/definitions.ts | 81 +++++++++++++ src/client/language/textIterator.ts | 53 ++++++++ src/client/language/textRangeCollection.ts | 103 ++++++++++++++++ src/client/language/tokenizer.ts | 119 ++++++++++++++++++ src/client/providers/completionProvider.ts | 11 +- 10 files changed, 584 insertions(+), 32 deletions(-) create mode 100644 src/client/language/characterStream.ts create mode 100644 src/client/language/definitions.ts create mode 100644 src/client/language/textIterator.ts create mode 100644 src/client/language/textRangeCollection.ts create mode 100644 src/client/language/tokenizer.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index 3fbf982ae0b3..a69c3396ff4e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -15,7 +15,7 @@ "outFiles": [ "${workspaceFolder}/out/**/*.js" ], - "preLaunchTask": "compile" + "preLaunchTask": "Compile" }, { "name": "Launch Extension as debugServer", // https://code.visualstudio.com/docs/extensions/example-debuggers @@ -30,7 +30,8 @@ "outFiles": [ "${workspaceFolder}/out/client/**/*.js" ], - "cwd": "${workspaceFolder}" + "cwd": "${workspaceFolder}", + "preLaunchTask": "Compile" }, { "name": "Launch Tests", @@ -47,7 +48,7 @@ "outFiles": [ "${workspaceFolder}/out/**/*.js" ], - "preLaunchTask": "compile" + "preLaunchTask": "Compile" }, { "name": "Launch Multiroot Tests", @@ -64,7 +65,7 @@ "outFiles": [ "${workspaceFolder}/out/**/*.js" ], - "preLaunchTask": "compile" + "preLaunchTask": "Compile" } ], "compounds": [ @@ -76,4 +77,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 155b94220ae6..ccf99a2c6f20 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -13,7 +13,7 @@ "script": "compile", "isBackground": true, "problemMatcher": [ - "$tsc", + "$tsc-watch", { "base": "$tslint5", "fileLocation": "relative" @@ -36,7 +36,7 @@ "panel": "shared" }, "problemMatcher": [ - "$tsc", + "$tsc-watch", { "base": "$tslint5", "fileLocation": "relative" @@ -72,4 +72,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/gulpfile.js b/gulpfile.js index 868a7281cc41..6c3f7819d003 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -22,7 +22,7 @@ const colors = require('colors/safe'); * named according to the checks performed on them. Each subset contains * the following one, as described in mathematical notation: * - * all ⊃ eol ⊇ indentation ⊃ copyright ⊃ typescript + * all ⊃ eol ⊇ indentation ⊃ typescript */ const all = [ @@ -115,12 +115,12 @@ const hygiene = (some, options) => { .toString('utf8') .split(/\r\n|\r|\n/) .forEach((line, i) => { - if (/^\s*$/.test(line)) { + if (/^\s*$/.test(line) || /^\S+.*$/.test(line)) { // Empty or whitespace lines are OK. } else if (/^(\s\s\s\s)+.*/.test(line)) { // Good indent. } else if (/^[\t]+.*/.test(line)) { - console.error(file.relative + '(' + (i + 1) + ',1): Bad whitespace indentation'); + console.error(file.relative + '(' + (i + 1) + ',1): Bad whitespace indentation (use 4 spaces instead of tabs or other)'); errorCount++; } }); @@ -137,7 +137,7 @@ const hygiene = (some, options) => { tsfmt: true }).then(result => { if (result.error) { - console.error(result.message); + console.error(result.message.trim()); errorCount++; } cb(null, file); @@ -147,14 +147,14 @@ const hygiene = (some, options) => { }); }); + const program = require('tslint').Linter.createProgram("./tsconfig.json"); + const linter = new tslint.Linter(options, program); const tsl = es.through(function (file) { const configuration = tslint.Configuration.findConfiguration(null, '.'); const options = { formatter: 'json' }; const contents = file.contents.toString('utf8'); - const program = require('tslint').Linter.createProgram("./tsconfig.json"); - const linter = new tslint.Linter(options, program); linter.lint(file.relative, contents, configuration.results); const result = linter.getResult(); if (result.failureCount > 0 || result.errorCount > 0) { @@ -206,22 +206,16 @@ const hygiene = (some, options) => { .pipe(filter(f => !f.stat.isDirectory())) .pipe(filter(eolFilter)) .pipe(options.skipEOL ? es.through() : eol) - .pipe(filter(indentationFilter)); - - if (!options.skipIndentationCheck) { - result = result - .pipe(indentation); - } + .pipe(filter(indentationFilter)) + .pipe(indentation); // Type script checks. let typescript = result - .pipe(filter(tslintFilter)); + .pipe(filter(tslintFilter)) + .pipe(formatting); - if (!options.skipFormatCheck) { - typescript = typescript - .pipe(formatting); - } - typescript = typescript.pipe(tsl) + typescript = typescript + .pipe(tsl) .pipe(tscFilesTracker) .pipe(tsc()); @@ -244,16 +238,32 @@ gulp.task('hygiene-staged', () => run({ mode: 'changes' })); gulp.task('hygiene-watch', ['hygiene-staged', 'hygiene-watch-runner']); gulp.task('hygiene-watch-runner', function () { + /** + * @type {Deferred} + */ + let runPromise; + return watch(all, { events: ['add', 'change'] }, function (event) { + // Damn bounce does not work, do our own checks. const start = new Date(); + if (runPromise && !runPromise.completed) { + console.log(`[${start.toLocaleTimeString()}] Already running`); + return; + } console.log(`[${start.toLocaleTimeString()}] Starting '${colors.cyan('hygiene-watch-runner')}'...`); + + runPromise = new Deferred(); // Skip indentation and formatting checks to speed up linting. - return run({ mode: 'watch', skipFormatCheck: true, skipIndentationCheck: true }) + run({ mode: 'watch', skipFormatCheck: true, skipIndentationCheck: true }) .then(() => { const end = new Date(); const time = (end.getTime() - start.getTime()) / 1000; console.log(`[${end.toLocaleTimeString()}] Finished '${colors.cyan('hygiene-watch-runner')}' after ${time} seconds`); - }); + runPromise.resolve(); + }) + .catch(runPromise.reject.bind); + + return runPromise.promise; }); }); @@ -402,7 +412,46 @@ function getModifiedFiles() { }); }); } + // this allows us to run hygiene as a git pre-commit hook. if (require.main === module) { run({ exitOnError: true, mode: 'staged' }); } + +class Deferred { + constructor(scope) { + this.scope = scope; + this._resolved = false; + this._rejected = false; + + this._promise = new Promise((resolve, reject) => { + this._resolve = resolve; + this._reject = reject; + }); + } + resolve(value) { + this._resolve.apply(this.scope ? this.scope : this, arguments); + this._resolved = true; + } + /** + * Rejects the promise + * @param {any} reason + * @memberof Deferred + */ + reject(reason) { + this._reject.apply(this.scope ? this.scope : this, arguments); + this._rejected = true; + } + get promise() { + return this._promise; + } + get resolved() { + return this._resolved === true; + } + get rejected() { + return this._rejected === true; + } + get completed() { + return this._rejected || this._resolved; + } +} \ No newline at end of file diff --git a/package.json b/package.json index 0155540cb629..17145aa47ed6 100644 --- a/package.json +++ b/package.json @@ -1529,7 +1529,7 @@ "fuzzy": "^0.1.3", "get-port": "^3.2.0", "iconv-lite": "^0.4.19", - "inversify": "^4.5.2", + "inversify": "^4.5.1", "line-by-line": "^0.1.5", "lodash": "^4.17.4", "minimatch": "^3.0.3", @@ -1583,5 +1583,10 @@ "typescript": "^2.5.2", "typescript-formatter": "^6.0.0", "vscode": "^1.1.5" + }, + "__metadata": { + "id": "f1f59ae4-9318-4f3c-a9b5-81b2eaa5f8a5", + "publisherDisplayName": "Microsoft", + "publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8" } -} +} \ No newline at end of file diff --git a/src/client/language/characterStream.ts b/src/client/language/characterStream.ts new file mode 100644 index 000000000000..fbb65f903644 --- /dev/null +++ b/src/client/language/characterStream.ts @@ -0,0 +1,134 @@ +'use strict'; + +// tslint:disable-next-line:import-name +import Char from 'typescript-char'; +import { ICharacterStream, ITextIterator } from './definitions'; +import { TextIterator } from './textIterator'; + +export class CharacterStream implements ICharacterStream { + private text: ITextIterator; + private pos: number; + private curChar: number; + private endOfStream: boolean; + + constructor(text: string | ITextIterator) { + const iter = text as ITextIterator; + const s = text as string; + + this.text = iter !== null ? iter : new TextIterator(s); + this.pos = 0; + this.curChar = text.length > 0 ? text.charCodeAt(0) : 0; + this.endOfStream = text.length === 0; + } + + public getText(): string { + return this.text.getText(); + } + + public get position(): number { + return this.pos; + } + + public set position(value: number) { + this.pos = value; + this.checkBounds(); + } + + public get currentChar(): number { + return this.curChar; + } + + public get nextChar(): number { + return this.position + 1 < this.text.length ? this.text.charCodeAt(this.position + 1) : 0; + } + + public get prevChar(): number { + return this.position - 1 >= 0 ? this.text.charCodeAt(this.position - 1) : 0; + } + + public isEndOfStream(): boolean { + return this.endOfStream; + } + + public lookAhead(offset: number): number { + const pos = this.position + offset; + return pos < 0 || pos >= this.text.length ? 0 : this.text.charCodeAt(pos); + } + + public advance(offset: number) { + this.position += offset; + } + + public moveNext(): boolean { + if (this.pos < this.text.length - 1) { + // Most common case, no need to check bounds extensively + this.pos += 1; + this.curChar = this.text.charCodeAt(this.pos); + return true; + } + this.advance(1); + return !this.endOfStream; + } + + public isAtWhiteSpace(): boolean { + return this.curChar <= Char.Space || this.curChar === 0x200B; + } + + public isAtLineBreak(): boolean { + return this.curChar === Char.CarriageReturn || this.curChar === Char.DataLineEscape; + } + + public skipLineBreak(): void { + if (this.curChar === Char.CarriageReturn) { + this.moveNext(); + if (this.currentChar === Char.LineFeed) { + this.moveNext(); + } + } else if (this.curChar === Char.LineFeed) { + this.moveNext(); + } + } + + public skipWhitespace(): void { + while (!this.endOfStream && this.isAtWhiteSpace()) { + this.moveNext(); + } + } + + public skipToEol(): void { + while (!this.endOfStream && !this.isAtLineBreak()) { + this.moveNext(); + } + } + + public skipToWhitespace(): void { + while (!this.endOfStream && !this.isAtWhiteSpace()) { + this.moveNext(); + } + } + + public isAtString(): boolean { + return this.curChar === 0x22 || this.curChar === 0x27; + } + + public charCodeAt(index: number): number { + return this.text.charCodeAt(index); + } + + public get length(): number { + return this.text.length; + } + + private checkBounds(): void { + if (this.pos < 0) { + this.pos = 0; + } + + this.endOfStream = this.pos >= this.text.length; + if (this.endOfStream) { + this.pos = this.text.length; + } + + this.curChar = this.endOfStream ? 0 : this.text.charCodeAt(this.pos); + } +} diff --git a/src/client/language/definitions.ts b/src/client/language/definitions.ts new file mode 100644 index 000000000000..a4f7a22b4da7 --- /dev/null +++ b/src/client/language/definitions.ts @@ -0,0 +1,81 @@ +'use strict'; + +export interface ITextRange { + readonly start: number; + readonly end: number; + readonly length: number; + contains(position: number): boolean; +} + +export class TextRange implements ITextRange { + public readonly start: number; + public readonly length: number; + + constructor(start: number, length: number) { + if (start < 0) { + throw new Error('start must be non-negative'); + } + if (length < 0) { + throw new Error('length must be non-negative'); + } + this.start = start; + this.length = length; + } + + public static fromBounds(start: number, end: number) { + return new TextRange(start, end - start); + } + + public get end(): number { + return this.start + this.length; + } + + public contains(position: number): boolean { + return position >= this.start && position < this.end; + } +} + +export interface ITextRangeCollection extends ITextRange { + count: number; + getItemAt(index: number): T; + getItemAtPosition(position: number): number; + getItemContaining(position: number): number; +} + +export interface ITextIterator { + readonly length: number; + charCodeAt(index: number): number; + getText(): string; +} + +export interface ICharacterStream extends ITextIterator { + position: number; + readonly currentChar: number; + readonly nextChar: number; + readonly prevChar: number; + getText(): string; + isEndOfStream(): boolean; + lookAhead(offset: number): number; + advance(offset: number); + moveNext(): boolean; + isAtWhiteSpace(): boolean; + isAtLineBreak(): boolean; + isAtString(): boolean; + skipLineBreak(): void; + skipWhitespace(): void; + skipToEol(): void; + skipToWhitespace(): void; +} + +export enum TokenType { + String +} + +export interface IToken extends ITextRange { + readonly type: TokenType; +} + +export interface ITokenizer { + Tokenize(text: string): ITextRangeCollection; + Tokenize(text: string, start: number, length: number): ITextRangeCollection; +} diff --git a/src/client/language/textIterator.ts b/src/client/language/textIterator.ts new file mode 100644 index 000000000000..8af0e1caefda --- /dev/null +++ b/src/client/language/textIterator.ts @@ -0,0 +1,53 @@ +'use strict'; + +import { Position, Range, TextDocument } from 'vscode'; +import { ITextIterator } from './definitions'; + +export class TextIterator implements ITextIterator { + private text: string; + + constructor(text: string) { + this.text = text; + } + + public charCodeAt(index: number): number { + if (index >= 0 && index < this.length) { + return this.text.charCodeAt[index]; + } + return 0; + } + + public get length(): number { + return this.text.length; + } + + public getText(): string { + return this.text; + } +} + +export class DocumentTextIterator implements ITextIterator { + public readonly length: number; + + private document: TextDocument; + + constructor(document: TextDocument) { + this.document = document; + + const lastIndex = this.document.lineCount - 1; + const lastLine = this.document.lineAt(lastIndex); + const end = new Position(lastIndex, lastLine.range.end.character); + this.length = this.document.offsetAt(end); + } + + public charCodeAt(index: number): number { + const position = this.document.positionAt(index); + return this.document + .getText(new Range(position, position.translate(0, 1))) + .charCodeAt(position.character); + } + + public getText(): string { + return this.document.getText(); + } +} diff --git a/src/client/language/textRangeCollection.ts b/src/client/language/textRangeCollection.ts new file mode 100644 index 000000000000..5448445c3092 --- /dev/null +++ b/src/client/language/textRangeCollection.ts @@ -0,0 +1,103 @@ +'use strict'; + +import { ITextRange, ITextRangeCollection } from './definitions'; + +export class TextRangeCollection implements ITextRangeCollection { + private items: T[]; + + constructor(items: T[]) { + this.items = items; + } + + public get start(): number { + return this.items.length > 0 ? this.items[0].start : 0; + } + + public get end(): number { + return this.items.length > 0 ? this.items[this.items.length - 1].end : 0; + } + + public get length(): number { + return this.end - this.start; + } + + public get count(): number { + return this.items.length; + } + + public contains(position: number) { + return position >= this.start && position < this.end; + } + + public getItemAt(index: number): T { + if (index < 0 || index >= this.items.length) { + throw new Error('index is out of range'); + } + return this.items[index] as T; + } + + public getItemAtPosition(position: number): number { + if (this.count === 0) { + return -1; + } + if (position < this.start) { + return -1; + } + if (position >= this.end) { + return -1; + } + + let min = 0; + let max = this.count - 1; + + while (min <= max) { + const mid = min + (max - min) / 2; + const item = this.items[mid]; + + if (item.start === position) { + return mid; + } + + if (position < item.start) { + max = mid - 1; + } else { + min = mid + 1; + } + } + return -1; + } + + public getItemContaining(position: number): number { + if (this.count === 0) { + return -1; + } + if (position < this.start) { + return -1; + } + if (position > this.end) { + return -1; + } + + let min = 0; + let max = this.count - 1; + + while (min <= max) { + const mid = min + (max - min) / 2; + const item = this[mid]; + + if (item.Contains(position)) { + return mid; + } + if (mid < this.count - 1 && item.end <= position && position < this.items[mid + 1].start) { + return -1; + } + + if (position < item.Start) { + max = mid - 1; + } else { + min = mid + 1; + } + } + return -1; + } +} diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts new file mode 100644 index 000000000000..25af381dfc26 --- /dev/null +++ b/src/client/language/tokenizer.ts @@ -0,0 +1,119 @@ +'use strict'; + +import Char from 'typescript-char'; +import { CharacterStream } from './characterStream'; +import { ICharacterStream, ITextRangeCollection, IToken, ITokenizer, TextRange, TokenType } from './definitions'; +import { TextRangeCollection } from './textRangeCollection'; + +enum QuoteType { + None, + Single, + Double, + TripleSingle, + TripleDouble +} + +class Token extends TextRange implements IToken { + public readonly type: TokenType; + + constructor(type: TokenType, start: number, length: number) { + super(start, length); + this.type = type; + } +} + +export class Tokenizer implements ITokenizer { + private cs: ICharacterStream; + private tokens: IToken[] = []; + + public Tokenize(text: string): ITextRangeCollection; + public Tokenize(text: string, start: number, length: number): ITextRangeCollection; + + public Tokenize(text: string, start?: number, length?: number): ITextRangeCollection { + if (start === undefined) { + start = 0; + } else if (start < 0 || start >= text.length) { + throw new Error('Invalid range start'); + } + + if (length === undefined) { + length = text.length; + } else if (length < 0 || start + length >= text.length) { + throw new Error('Invalid range length'); + } + + this.cs = new CharacterStream(text); + this.cs.position = start; + + const end = start + length; + while (!this.cs.isEndOfStream()) { + this.AddNextToken(); + if (this.cs.position >= end) { + break; + } + } + return new TextRangeCollection(this.tokens); + } + + private AddNextToken(): void { + this.cs.skipWhitespace(); + if (this.cs.isEndOfStream()) { + return; + } + + if (!this.handleCharacter()) { + this.cs.moveNext(); + } + } + + private handleCharacter(): boolean { + const quoteType = this.getQuoteType(); + if (quoteType !== QuoteType.None) { + this.handleString(quoteType); + return true; + } + return false; + } + + private getQuoteType(): QuoteType { + if (this.cs.currentChar === Char.SingleQuote) { + return this.cs.nextChar === Char.SingleQuote && this.cs.lookAhead(2) === Char.SingleQuote + ? QuoteType.TripleSingle + : QuoteType.Single; + } + if (this.cs.currentChar === Char.DoubleQuote) { + return this.cs.nextChar === Char.DoubleQuote && this.cs.lookAhead(2) === Char.DoubleQuote + ? QuoteType.TripleDouble + : QuoteType.Double; + } + return QuoteType.None; + } + + private handleString(quoteType: QuoteType): void { + const start = this.cs.position; + if (quoteType === QuoteType.Single || quoteType === QuoteType.Double) { + this.cs.moveNext(); + this.skipToSingleEndQuote(quoteType === QuoteType.Single + ? Char.SingleQuote + : Char.DoubleQuote); + } else { + this.cs.advance(3); + this.skipToTripleEndQuote(quoteType === QuoteType.TripleSingle + ? Char.SingleQuote + : Char.DoubleQuote); + } + this.tokens.push(new Token(TokenType.String, start, this.cs.position - start)); + } + + private skipToSingleEndQuote(quote: number): void { + while (!this.cs.isEndOfStream() && this.cs.currentChar !== quote) { + this.cs.moveNext(); + } + } + + private skipToTripleEndQuote(quote: number): void { + while (!this.cs.isEndOfStream() && (this.cs.currentChar !== quote || this.cs.nextChar !== quote || this.cs.lookAhead(2) !== quote)) { + this.cs.moveNext(); + } + } +} diff --git a/src/client/providers/completionProvider.ts b/src/client/providers/completionProvider.ts index b453d19a7f19..020fb71daffc 100644 --- a/src/client/providers/completionProvider.ts +++ b/src/client/providers/completionProvider.ts @@ -1,8 +1,9 @@ 'use strict'; import * as vscode from 'vscode'; -import { ProviderResult, SnippetString, Uri } from 'vscode'; +import { Position, ProviderResult, SnippetString, Uri } from 'vscode'; import { PythonSettings } from '../common/configSettings'; +import { Tokenizer } from '../language/tokenizer'; import { JediFactory } from '../languageServices/jediProxyFactory'; import { captureTelemetry } from '../telemetry'; import { COMPLETION } from '../telemetry/constants'; @@ -47,7 +48,7 @@ export class PythonCompletionItemProvider implements vscode.CompletionItemProvid return Promise.resolve([]); } // If starts with a """ (possible doc string), then return - if (lineText.trim().startsWith('"""')) { + if (this.isPositionInsideString(document, position)) { return Promise.resolve([]); } const type = proxy.CommandType.Completions; @@ -66,4 +67,10 @@ export class PythonCompletionItemProvider implements vscode.CompletionItemProvid return PythonCompletionItemProvider.parseData(data, document.uri); }); } + + private isPositionInsideString(document: vscode.TextDocument, position: vscode.Position): boolean { + const text = document.getText(new vscode.Range(new Position(0, 0), position)); + const t = new Tokenizer(); + return t.Tokenize(text).getItemContaining(document.offsetAt(position)) >= 0; + } } From eb4266980d1308fbae7a0017a9da673b788a9e45 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Dec 2017 12:19:00 -0800 Subject: [PATCH 02/51] Fixed property names --- src/client/language/textRangeCollection.ts | 6 +++--- src/client/language/tokenizer.ts | 3 +++ src/client/providers/completionProvider.ts | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/client/language/textRangeCollection.ts b/src/client/language/textRangeCollection.ts index 5448445c3092..0464dc945382 100644 --- a/src/client/language/textRangeCollection.ts +++ b/src/client/language/textRangeCollection.ts @@ -83,16 +83,16 @@ export class TextRangeCollection implements ITextRangeColl while (min <= max) { const mid = min + (max - min) / 2; - const item = this[mid]; + const item = this.items[mid]; - if (item.Contains(position)) { + if (item.contains(position)) { return mid; } if (mid < this.count - 1 && item.end <= position && position < this.items[mid + 1].start) { return -1; } - if (position < item.Start) { + if (position < item.start) { max = mid - 1; } else { min = mid + 1; diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index 25af381dfc26..5cb0d4e3e474 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -1,5 +1,6 @@ 'use strict'; +// tslint:disable-next-line:import-name import Char from 'typescript-char'; import { CharacterStream } from './characterStream'; import { ICharacterStream, ITextRangeCollection, IToken, ITokenizer, TextRange, TokenType } from './definitions'; @@ -109,11 +110,13 @@ export class Tokenizer implements ITokenizer { while (!this.cs.isEndOfStream() && this.cs.currentChar !== quote) { this.cs.moveNext(); } + this.cs.moveNext(); } private skipToTripleEndQuote(quote: number): void { while (!this.cs.isEndOfStream() && (this.cs.currentChar !== quote || this.cs.nextChar !== quote || this.cs.lookAhead(2) !== quote)) { this.cs.moveNext(); } + this.cs.advance(3); } } diff --git a/src/client/providers/completionProvider.ts b/src/client/providers/completionProvider.ts index 020fb71daffc..b097b1633a9e 100644 --- a/src/client/providers/completionProvider.ts +++ b/src/client/providers/completionProvider.ts @@ -69,7 +69,8 @@ export class PythonCompletionItemProvider implements vscode.CompletionItemProvid } private isPositionInsideString(document: vscode.TextDocument, position: vscode.Position): boolean { - const text = document.getText(new vscode.Range(new Position(0, 0), position)); + const tokenizeTo = position.translate(1, 0); + const text = document.getText(new vscode.Range(new Position(0, 0), tokenizeTo)); const t = new Tokenizer(); return t.Tokenize(text).getItemContaining(document.offsetAt(position)) >= 0; } From 275697428d0b725083b2054b24e9aaa08b0db7b1 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Dec 2017 15:10:20 -0800 Subject: [PATCH 03/51] Tests, round I --- src/client/language/characterStream.ts | 63 ++++++------- src/client/language/definitions.ts | 2 + src/client/language/textIterator.ts | 4 +- src/test/.vscode/settings.json | 3 +- src/test/index.ts | 3 +- src/test/language/characterStream.test.ts | 108 ++++++++++++++++++++++ src/test/language/textIterator.test.ts | 24 +++++ src/test/language/textRange.test.ts | 52 +++++++++++ 8 files changed, 222 insertions(+), 37 deletions(-) create mode 100644 src/test/language/characterStream.test.ts create mode 100644 src/test/language/textIterator.test.ts create mode 100644 src/test/language/textRange.test.ts diff --git a/src/client/language/characterStream.ts b/src/client/language/characterStream.ts index fbb65f903644..e90ef7f2e6b7 100644 --- a/src/client/language/characterStream.ts +++ b/src/client/language/characterStream.ts @@ -7,18 +7,15 @@ import { TextIterator } from './textIterator'; export class CharacterStream implements ICharacterStream { private text: ITextIterator; - private pos: number; - private curChar: number; - private endOfStream: boolean; + private _position: number; + private _currentChar: number; + private _isEndOfStream: boolean; constructor(text: string | ITextIterator) { - const iter = text as ITextIterator; - const s = text as string; - - this.text = iter !== null ? iter : new TextIterator(s); - this.pos = 0; - this.curChar = text.length > 0 ? text.charCodeAt(0) : 0; - this.endOfStream = text.length === 0; + this.text = typeof text === 'string' ? new TextIterator(text) : text; + this._position = 0; + this._currentChar = text.length > 0 ? text.charCodeAt(0) : 0; + this._isEndOfStream = text.length === 0; } public getText(): string { @@ -26,16 +23,16 @@ export class CharacterStream implements ICharacterStream { } public get position(): number { - return this.pos; + return this._position; } public set position(value: number) { - this.pos = value; + this._position = value; this.checkBounds(); } public get currentChar(): number { - return this.curChar; + return this._currentChar; } public get nextChar(): number { @@ -47,11 +44,11 @@ export class CharacterStream implements ICharacterStream { } public isEndOfStream(): boolean { - return this.endOfStream; + return this._isEndOfStream; } public lookAhead(offset: number): number { - const pos = this.position + offset; + const pos = this._position + offset; return pos < 0 || pos >= this.text.length ? 0 : this.text.charCodeAt(pos); } @@ -60,55 +57,55 @@ export class CharacterStream implements ICharacterStream { } public moveNext(): boolean { - if (this.pos < this.text.length - 1) { + if (this._position < this.text.length - 1) { // Most common case, no need to check bounds extensively - this.pos += 1; - this.curChar = this.text.charCodeAt(this.pos); + this._position += 1; + this._currentChar = this.text.charCodeAt(this._position); return true; } this.advance(1); - return !this.endOfStream; + return !this.isEndOfStream(); } public isAtWhiteSpace(): boolean { - return this.curChar <= Char.Space || this.curChar === 0x200B; + return this.currentChar <= Char.Space || this.currentChar === 0x200B; // Unicode whitespace } public isAtLineBreak(): boolean { - return this.curChar === Char.CarriageReturn || this.curChar === Char.DataLineEscape; + return this.currentChar === Char.CarriageReturn || this.currentChar === Char.LineFeed; } public skipLineBreak(): void { - if (this.curChar === Char.CarriageReturn) { + if (this._currentChar === Char.CarriageReturn) { this.moveNext(); if (this.currentChar === Char.LineFeed) { this.moveNext(); } - } else if (this.curChar === Char.LineFeed) { + } else if (this._currentChar === Char.LineFeed) { this.moveNext(); } } public skipWhitespace(): void { - while (!this.endOfStream && this.isAtWhiteSpace()) { + while (!this.isEndOfStream() && this.isAtWhiteSpace()) { this.moveNext(); } } public skipToEol(): void { - while (!this.endOfStream && !this.isAtLineBreak()) { + while (!this.isEndOfStream() && !this.isAtLineBreak()) { this.moveNext(); } } public skipToWhitespace(): void { - while (!this.endOfStream && !this.isAtWhiteSpace()) { + while (!this.isEndOfStream() && !this.isAtWhiteSpace()) { this.moveNext(); } } public isAtString(): boolean { - return this.curChar === 0x22 || this.curChar === 0x27; + return this.currentChar === Char.SingleQuote || this.currentChar === Char.DoubleQuote; } public charCodeAt(index: number): number { @@ -120,15 +117,15 @@ export class CharacterStream implements ICharacterStream { } private checkBounds(): void { - if (this.pos < 0) { - this.pos = 0; + if (this._position < 0) { + this._position = 0; } - this.endOfStream = this.pos >= this.text.length; - if (this.endOfStream) { - this.pos = this.text.length; + this._isEndOfStream = this._position >= this.text.length; + if (this._isEndOfStream) { + this._position = this.text.length; } - this.curChar = this.endOfStream ? 0 : this.text.charCodeAt(this.pos); + this._currentChar = this._isEndOfStream ? 0 : this.text.charCodeAt(this._position); } } diff --git a/src/client/language/definitions.ts b/src/client/language/definitions.ts index a4f7a22b4da7..3e965b0b91bf 100644 --- a/src/client/language/definitions.ts +++ b/src/client/language/definitions.ts @@ -8,6 +8,8 @@ export interface ITextRange { } export class TextRange implements ITextRange { + public static readonly empty = TextRange.fromBounds(0, 0); + public readonly start: number; public readonly length: number; diff --git a/src/client/language/textIterator.ts b/src/client/language/textIterator.ts index 8af0e1caefda..927078da3939 100644 --- a/src/client/language/textIterator.ts +++ b/src/client/language/textIterator.ts @@ -11,8 +11,8 @@ export class TextIterator implements ITextIterator { } public charCodeAt(index: number): number { - if (index >= 0 && index < this.length) { - return this.text.charCodeAt[index]; + if (index >= 0 && index < this.text.length) { + return this.text.charCodeAt(index); } return 0; } diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index 2218e2cecd87..12cde5b9dc53 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -21,5 +21,6 @@ "python.linting.pydocstyleEnabled": false, "python.linting.pylamaEnabled": false, "python.linting.mypyEnabled": false, - "python.formatting.provider": "yapf" + "python.formatting.provider": "yapf", + "python.pythonPath": "python" } \ No newline at end of file diff --git a/src/test/index.ts b/src/test/index.ts index 0202b4e2dc43..acce5db2392a 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -8,6 +8,7 @@ testRunner.configure({ ui: 'tdd', useColors: true, timeout: 25000, - retries: 3 + retries: 3, + grep: 'Language.CharacterStream' } as {}); module.exports = testRunner; diff --git a/src/test/language/characterStream.test.ts b/src/test/language/characterStream.test.ts new file mode 100644 index 000000000000..d1c003d6b2d9 --- /dev/null +++ b/src/test/language/characterStream.test.ts @@ -0,0 +1,108 @@ +import * as assert from 'assert'; +// tslint:disable-next-line:import-name +import Char from 'typescript-char'; +import { CharacterStream } from '../../client/language/characterStream'; +import { ICharacterStream, TextRange } from '../../client/language/definitions'; +import { TextIterator } from '../../client/language/textIterator'; + +// tslint:disable-next-line:max-func-body-length +suite('Language.CharacterStream', () => { + test('Iteration (string)', async () => { + const content = 'some text'; + const cs = new CharacterStream(content); + testIteration(cs, content); + }); + test('Iteration (iterator)', async () => { + const content = 'some text'; + const cs = new CharacterStream(new TextIterator(content)); + testIteration(cs, content); + }); + test('Positioning', async () => { + const content = 'some text'; + const cs = new CharacterStream(content); + assert.equal(cs.position, 0); + cs.advance(1); + assert.equal(cs.position, 1); + cs.advance(1); + assert.equal(cs.position, 2); + cs.advance(2); + assert.equal(cs.position, 4); + cs.advance(-3); + assert.equal(cs.position, 1); + cs.advance(-3); + assert.equal(cs.position, 0); + cs.advance(100); + assert.equal(cs.position, content.length); + }); + test('Characters', async () => { + const content = 'some \ttext "" \' \' \n text \r\n more text'; + const cs = new CharacterStream(content); + for (let i = 0; i < content.length; i += 1) { + assert.equal(cs.currentChar, content.charCodeAt(i)); + + assert.equal(cs.nextChar, i < content.length - 1 ? content.charCodeAt(i + 1) : 0); + assert.equal(cs.prevChar, i > 0 ? content.charCodeAt(i - 1) : 0); + + assert.equal(cs.lookAhead(2), i < content.length - 2 ? content.charCodeAt(i + 2) : 0); + assert.equal(cs.lookAhead(-2), i > 1 ? content.charCodeAt(i - 2) : 0); + + const ch = content.charCodeAt(i); + const isLineBreak = ch === Char.LineFeed || ch === Char.CarriageReturn; + assert.equal(cs.isAtWhiteSpace(), ch === Char.Tab || ch === Char.Space || isLineBreak); + assert.equal(cs.isAtLineBreak(), isLineBreak); + assert.equal(cs.isAtString(), ch === Char.SingleQuote || ch === Char.DoubleQuote); + + cs.moveNext(); + } + }); + test('Skip', async () => { + const content = 'some \ttext "" \' \' \n text \r\n more text'; + const cs = new CharacterStream(content); + + cs.skipWhitespace(); + assert.equal(cs.position, 0); + + cs.skipToWhitespace(); + assert.equal(cs.position, 4); + + cs.skipToWhitespace(); + assert.equal(cs.position, 4); + + cs.skipWhitespace(); + assert.equal(cs.position, 6); + + cs.skipLineBreak(); + assert.equal(cs.position, 6); + + cs.skipToEol(); + assert.equal(cs.position, 18); + + cs.skipLineBreak(); + assert.equal(cs.position, 19); + }); +}); + +function testIteration(cs: ICharacterStream, content: string) { + assert.equal(cs.position, 0); + assert.equal(cs.length, content.length); + assert.equal(cs.isEndOfStream(), false); + + for (let i = -2; i < content.length + 2; i += 1) { + const ch = cs.charCodeAt(i); + if (i < 0 || i >= content.length) { + assert.equal(ch, 0); + } else { + assert.equal(ch, content.charCodeAt(i)); + } + } + + for (let i = 0; i < content.length; i += 1) { + assert.equal(cs.isEndOfStream(), false); + assert.equal(cs.position, i); + assert.equal(cs.currentChar, content.charCodeAt(i)); + cs.moveNext(); + } + + assert.equal(cs.isEndOfStream(), true); + assert.equal(cs.position, content.length); +} diff --git a/src/test/language/textIterator.test.ts b/src/test/language/textIterator.test.ts new file mode 100644 index 000000000000..dddfe3478ec5 --- /dev/null +++ b/src/test/language/textIterator.test.ts @@ -0,0 +1,24 @@ +import * as assert from 'assert'; +import { TextIterator } from '../../client/language/textIterator'; + +// tslint:disable-next-line:max-func-body-length +suite('Language.TextIterator', () => { + test('Construction', async () => { + const content = 'some text'; + const ti = new TextIterator(content); + assert.equal(ti.length, content.length); + assert.equal(ti.getText(), content); + }); + test('Iteration', async () => { + const content = 'some text'; + const ti = new TextIterator(content); + for (let i = -2; i < content.length + 2; i += 1) { + const ch = ti.charCodeAt(i); + if (i < 0 || i >= content.length) { + assert.equal(ch, 0); + } else { + assert.equal(ch, content.charCodeAt(i)); + } + } + }); +}); diff --git a/src/test/language/textRange.test.ts b/src/test/language/textRange.test.ts new file mode 100644 index 000000000000..4e0da8feb06c --- /dev/null +++ b/src/test/language/textRange.test.ts @@ -0,0 +1,52 @@ +import * as assert from 'assert'; +import { TextRange } from '../../client/language/definitions'; + +// tslint:disable-next-line:max-func-body-length +suite('Language.TextRange', () => { + test('Empty static', async () => { + const e = TextRange.empty; + assert.equal(e.start, 0); + assert.equal(e.end, 0); + assert.equal(e.length, 0); + }); + test('Construction', async () => { + let r = new TextRange(10, 20); + assert.equal(r.start, 10); + assert.equal(r.end, 30); + assert.equal(r.length, 20); + r = new TextRange(10, 0); + assert.equal(r.start, 10); + assert.equal(r.end, 10); + assert.equal(r.length, 0); + }); + test('From bounds', async () => { + let r = TextRange.fromBounds(7, 9); + assert.equal(r.start, 7); + assert.equal(r.end, 9); + assert.equal(r.length, 2); + + r = TextRange.fromBounds(5, 5); + assert.equal(r.start, 5); + assert.equal(r.end, 5); + assert.equal(r.length, 0); + }); + test('Contains', async () => { + const r = TextRange.fromBounds(7, 9); + assert.equal(r.contains(-1), false); + assert.equal(r.contains(6), false); + assert.equal(r.contains(7), true); + assert.equal(r.contains(8), true); + assert.equal(r.contains(9), false); + assert.equal(r.contains(10), false); + }); + test('Exceptions', async () => { + assert.throws( + () => { const e = new TextRange(0, -1); }, + Error + ); + assert.throws( + () => { const e = TextRange.fromBounds(3, 1); }, + Error + ); + }); +}); From c2c1ced6cf64be60ca808c269291af191df3b3b2 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 1 Dec 2017 16:07:28 -0800 Subject: [PATCH 04/51] Tests, round II --- src/client/language/definitions.ts | 3 +- src/client/language/tokenizer.ts | 13 +++++ src/client/providers/completionProvider.ts | 15 +++--- src/test/index.ts | 2 +- src/test/language/textRangeCollection.test.ts | 53 +++++++++++++++++++ src/test/language/tokenizer.test.ts | 39 ++++++++++++++ 6 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 src/test/language/textRangeCollection.test.ts create mode 100644 src/test/language/tokenizer.test.ts diff --git a/src/client/language/definitions.ts b/src/client/language/definitions.ts index 3e965b0b91bf..d001adccbd88 100644 --- a/src/client/language/definitions.ts +++ b/src/client/language/definitions.ts @@ -70,7 +70,8 @@ export interface ICharacterStream extends ITextIterator { } export enum TokenType { - String + String, + Comment } export interface IToken extends ITextRange { diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index 5cb0d4e3e474..6d63f4168414 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -73,9 +73,22 @@ export class Tokenizer implements ITokenizer { this.handleString(quoteType); return true; } + switch (this.cs.currentChar) { + case Char.Hash: + this.handleComment(); + break; + default: + break; + } return false; } + private handleComment(): void { + const start = this.cs.position; + this.cs.skipToEol(); + this.tokens.push(new Token(TokenType.Comment, start, this.cs.position - start)); + } + private getQuoteType(): QuoteType { if (this.cs.currentChar === Char.SingleQuote) { return this.cs.nextChar === Char.SingleQuote && this.cs.lookAhead(2) === Char.SingleQuote diff --git a/src/client/providers/completionProvider.ts b/src/client/providers/completionProvider.ts index b097b1633a9e..10c930348e33 100644 --- a/src/client/providers/completionProvider.ts +++ b/src/client/providers/completionProvider.ts @@ -3,6 +3,7 @@ import * as vscode from 'vscode'; import { Position, ProviderResult, SnippetString, Uri } from 'vscode'; import { PythonSettings } from '../common/configSettings'; +import { TokenType } from '../language/definitions'; import { Tokenizer } from '../language/tokenizer'; import { JediFactory } from '../languageServices/jediProxyFactory'; import { captureTelemetry } from '../telemetry'; @@ -43,12 +44,8 @@ export class PythonCompletionItemProvider implements vscode.CompletionItemProvid if (lineText.match(/^\s*\/\//)) { return Promise.resolve([]); } - // If starts with a comment, then return - if (lineText.trim().startsWith('#')) { - return Promise.resolve([]); - } - // If starts with a """ (possible doc string), then return - if (this.isPositionInsideString(document, position)) { + // Suppress completion inside string and comments + if (this.isPositionInsideStringOrComment(document, position)) { return Promise.resolve([]); } const type = proxy.CommandType.Completions; @@ -68,10 +65,12 @@ export class PythonCompletionItemProvider implements vscode.CompletionItemProvid }); } - private isPositionInsideString(document: vscode.TextDocument, position: vscode.Position): boolean { + private isPositionInsideStringOrComment(document: vscode.TextDocument, position: vscode.Position): boolean { const tokenizeTo = position.translate(1, 0); const text = document.getText(new vscode.Range(new Position(0, 0), tokenizeTo)); const t = new Tokenizer(); - return t.Tokenize(text).getItemContaining(document.offsetAt(position)) >= 0; + const tokens = t.Tokenize(text); + const index = tokens.getItemContaining(document.offsetAt(position)); + return index >= 0 && (tokens[index].TokenType === TokenType.String || tokens[index].TokenType === TokenType.Comment); } } diff --git a/src/test/index.ts b/src/test/index.ts index acce5db2392a..6480c37439b6 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -9,6 +9,6 @@ testRunner.configure({ useColors: true, timeout: 25000, retries: 3, - grep: 'Language.CharacterStream' + grep: 'Language.Tokenizer' } as {}); module.exports = testRunner; diff --git a/src/test/language/textRangeCollection.test.ts b/src/test/language/textRangeCollection.test.ts new file mode 100644 index 000000000000..5b5dfad8c8b6 --- /dev/null +++ b/src/test/language/textRangeCollection.test.ts @@ -0,0 +1,53 @@ +import * as assert from 'assert'; +import { TextRange } from '../../client/language/definitions'; +import { TextRangeCollection } from '../../client/language/textRangeCollection'; + +// tslint:disable-next-line:max-func-body-length +suite('Language.TextRangeCollection', () => { + test('Empty', async () => { + const items: TextRange[] = []; + const c = new TextRangeCollection(items); + assert.equal(c.start, 0); + assert.equal(c.end, 0); + assert.equal(c.length, 0); + assert.equal(c.count, 0); + }); + test('Basic', async () => { + const items: TextRange[] = []; + items.push(new TextRange(2, 1)); + items.push(new TextRange(4, 2)); + const c = new TextRangeCollection(items); + assert.equal(c.start, 2); + assert.equal(c.end, 6); + assert.equal(c.length, 4); + assert.equal(c.count, 2); + + assert.equal(c.getItemAt(0).start, 2); + assert.equal(c.getItemAt(0).length, 1); + + assert.equal(c.getItemAt(1).start, 4); + assert.equal(c.getItemAt(1).length, 2); + }); + test('Contains position', async () => { + const items: TextRange[] = []; + items.push(new TextRange(2, 1)); + items.push(new TextRange(4, 2)); + const c = new TextRangeCollection(items); + const results = [-1, -1, 0, -1, 1, 1, -1]; + for (let i = 0; i < results.length; i += 1) { + const index = c.getItemContaining(i); + assert.equal(index, results[i]); + } + }); + test('Item at position', async () => { + const items: TextRange[] = []; + items.push(new TextRange(2, 1)); + items.push(new TextRange(4, 2)); + const c = new TextRangeCollection(items); + const results = [-1, -1, 0, -1, 1, 1, -1]; + for (let i = 0; i < results.length; i += 1) { + const index = c.getItemAtPosition(i); + assert.equal(index, results[i]); + } + }); +}); diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts new file mode 100644 index 000000000000..29874f75c26a --- /dev/null +++ b/src/test/language/tokenizer.test.ts @@ -0,0 +1,39 @@ +import * as assert from 'assert'; +import { TextRange, TokenType } from '../../client/language/definitions'; +import { TextRangeCollection } from '../../client/language/textRangeCollection'; +import { Tokenizer } from '../../client/language/tokenizer'; + +// tslint:disable-next-line:max-func-body-length +suite('Language.Tokenizer', () => { + test('Empty', async () => { + const t = new Tokenizer(); + const tokens = t.Tokenize(''); + assert.equal(tokens instanceof TextRangeCollection, true); + assert.equal(tokens.count, 0); + assert.equal(tokens.length, 0); + }); + test('Strings', async () => { + const t = new Tokenizer(); + const tokens = t.Tokenize(' "string" """line1\n#line2"""\t\'un#closed'); + assert.equal(tokens.count, 3); + + const ranges = [1, 8, 10, 18, 29, 10]; + for (let i = 0; i < ranges.length / 2; i += 2) { + assert.equal(tokens.getItemAt(i).start, ranges[i]); + assert.equal(tokens.getItemAt(i).length, ranges[i + 1]); + assert.equal(tokens.getItemAt(i).type, TokenType.String); + } + }); + test('Comments', async () => { + const t = new Tokenizer(); + const tokens = t.Tokenize(' #co"""mment1\n\t\n#comm\'ent2 '); + assert.equal(tokens.count, 2); + + const ranges = [1, 12, 15, 11]; + for (let i = 0; i < ranges.length / 2; i += 2) { + assert.equal(tokens.getItemAt(i).start, ranges[i]); + assert.equal(tokens.getItemAt(i).length, ranges[i + 1]); + assert.equal(tokens.getItemAt(i).type, TokenType.Comment); + } + }); +}); From 14864a580d0affdc9f8fda616fe20e2dbdbb2df9 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Sun, 3 Dec 2017 16:42:46 -0800 Subject: [PATCH 05/51] tokenizer test --- src/test/language/tokenizer.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index 29874f75c26a..1ce0165e4241 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -18,9 +18,9 @@ suite('Language.Tokenizer', () => { assert.equal(tokens.count, 3); const ranges = [1, 8, 10, 18, 29, 10]; - for (let i = 0; i < ranges.length / 2; i += 2) { - assert.equal(tokens.getItemAt(i).start, ranges[i]); - assert.equal(tokens.getItemAt(i).length, ranges[i + 1]); + for (let i = 0; i < tokens.count; i += 1) { + assert.equal(tokens.getItemAt(i).start, ranges[2 * i]); + assert.equal(tokens.getItemAt(i).length, ranges[2 * i + 1]); assert.equal(tokens.getItemAt(i).type, TokenType.String); } }); From 0ed51d64ee215356c7e5665c3b642c386e997f2d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Sun, 3 Dec 2017 16:46:19 -0800 Subject: [PATCH 06/51] Remove temorary change --- src/test/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/index.ts b/src/test/index.ts index 6480c37439b6..0202b4e2dc43 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -8,7 +8,6 @@ testRunner.configure({ ui: 'tdd', useColors: true, timeout: 25000, - retries: 3, - grep: 'Language.Tokenizer' + retries: 3 } as {}); module.exports = testRunner; From 51b544ca9df3515857f6d07411b7bf36bbb415d0 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Sun, 3 Dec 2017 17:08:35 -0800 Subject: [PATCH 07/51] Fix merge issue --- package.json | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/package.json b/package.json index 17145aa47ed6..e910867b6620 100644 --- a/package.json +++ b/package.json @@ -1529,7 +1529,7 @@ "fuzzy": "^0.1.3", "get-port": "^3.2.0", "iconv-lite": "^0.4.19", - "inversify": "^4.5.1", + "inversify": "^4.5.2", "line-by-line": "^0.1.5", "lodash": "^4.17.4", "minimatch": "^3.0.3", @@ -1583,10 +1583,5 @@ "typescript": "^2.5.2", "typescript-formatter": "^6.0.0", "vscode": "^1.1.5" - }, - "__metadata": { - "id": "f1f59ae4-9318-4f3c-a9b5-81b2eaa5f8a5", - "publisherDisplayName": "Microsoft", - "publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8" } } \ No newline at end of file From 3cd11e649febd2cc750f9c8bd238a7f9e0a222d5 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Sun, 3 Dec 2017 17:18:09 -0800 Subject: [PATCH 08/51] Merge conflict --- .vscode/settings.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 1884ba9a0255..de5f2d58fde1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -17,6 +17,5 @@ "python.linting.enabled": false, "python.formatting.formatOnSave": false, "python.unitTest.promptToConfigure": false, - "python.workspaceSymbols.enabled": false, - "python.formatting.provider": "yapf" + "python.workspaceSymbols.enabled": false } From 82e0ad16d200cfdb2ba4be1270305a882015ac9b Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Sun, 3 Dec 2017 17:25:50 -0800 Subject: [PATCH 09/51] Merge conflict --- .vscode/settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index de5f2d58fde1..1884ba9a0255 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -17,5 +17,6 @@ "python.linting.enabled": false, "python.formatting.formatOnSave": false, "python.unitTest.promptToConfigure": false, - "python.workspaceSymbols.enabled": false + "python.workspaceSymbols.enabled": false, + "python.formatting.provider": "yapf" } From 9295c1ab90d1a328351b771d35e5c99103d32ecb Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 4 Dec 2017 11:23:18 -0800 Subject: [PATCH 10/51] Completion test --- package-lock.json | 5 +++ package.json | 3 +- src/test/autocomplete/base.test.ts | 53 +++++++++++++++++++---- src/test/pythonFiles/autocomp/suppress.py | 6 +++ 4 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 src/test/pythonFiles/autocomp/suppress.py diff --git a/package-lock.json b/package-lock.json index 10e608b1634d..50cf5f0c7820 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5457,6 +5457,11 @@ "integrity": "sha1-PFtv1/beCRQmkCfwPAlGdY92c6Q=", "dev": true }, + "typescript-char": { + "version": "0.0.0", + "resolved": "https://registry.npmjs.org/typescript-char/-/typescript-char-0.0.0.tgz", + "integrity": "sha1-VY/tpzfHZaYQtzfu+7F3Xum8jas=" + }, "typescript-formatter": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/typescript-formatter/-/typescript-formatter-6.1.0.tgz", diff --git a/package.json b/package.json index e910867b6620..23dc208e4ccf 100644 --- a/package.json +++ b/package.json @@ -1539,6 +1539,7 @@ "semver": "^5.4.1", "tmp": "0.0.29", "tree-kill": "^1.1.0", + "typescript-char": "^0.0.0", "uint64be": "^1.0.1", "untildify": "^3.0.2", "vscode-debugadapter": "^1.0.1", @@ -1584,4 +1585,4 @@ "typescript-formatter": "^6.0.0", "vscode": "^1.1.5" } -} \ No newline at end of file +} diff --git a/src/test/autocomplete/base.test.ts b/src/test/autocomplete/base.test.ts index a5398b7beb43..59a7e51c14f4 100644 --- a/src/test/autocomplete/base.test.ts +++ b/src/test/autocomplete/base.test.ts @@ -1,19 +1,18 @@ // Note: This example test is leveraging the Mocha test framework. // Please refer to their documentation on https://mochajs.org/ for help. - // The module 'assert' provides assertion methods from node import * as assert from 'assert'; import { EOL } from 'os'; +import * as path from 'path'; // You can import and use all API from the 'vscode' module // as well as import your extension to test it import * as vscode from 'vscode'; -import * as path from 'path'; import * as settings from '../../client/common/configSettings'; -import { initialize, closeActiveWindows, initializeTest } from '../initialize'; -import { execPythonFile } from '../../client/common/utils'; import { PythonSettings } from '../../client/common/configSettings'; +import { execPythonFile } from '../../client/common/utils'; import { rootWorkspaceUri } from '../common'; +import { closeActiveWindows, initialize, initializeTest } from '../initialize'; const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'autocomp'); const fileOne = path.join(autoCompPath, 'one.py'); @@ -23,7 +22,9 @@ const fileLambda = path.join(autoCompPath, 'lamb.py'); const fileDecorator = path.join(autoCompPath, 'deco.py'); const fileEncoding = path.join(autoCompPath, 'four.py'); const fileEncodingUsed = path.join(autoCompPath, 'five.py'); +const fileSuppress = path.join(autoCompPath, 'suppress.py'); +// tslint:disable-next-line:max-func-body-length suite('Autocomplete', () => { let isPython3: Promise; suiteSetup(async () => { @@ -31,9 +32,9 @@ suite('Autocomplete', () => { const version = await execPythonFile(rootWorkspaceUri, PythonSettings.getInstance(rootWorkspaceUri).pythonPath, ['--version'], __dirname, true); isPython3 = Promise.resolve(version.indexOf('3.') >= 0); }); - setup(() => initializeTest()); - suiteTeardown(() => closeActiveWindows()); - teardown(() => closeActiveWindows()); + setup(initializeTest); + suiteTeardown(closeActiveWindows); + teardown(closeActiveWindows); test('For "sys."', done => { let textEditor: vscode.TextEditor; @@ -115,7 +116,7 @@ suite('Autocomplete', () => { const position = new vscode.Position(10, 9); const list = await vscode.commands.executeCommand('vscode.executeCompletionItemProvider', textDocument.uri, position); assert.notEqual(list.items.filter(item => item.label === 'sleep').length, 0, 'sleep not found'); - assert.notEqual(list.items.filter(item => item.documentation.toString().startsWith("Delay execution for a given number of seconds. The argument may be")).length, 0, 'Documentation incorrect'); + assert.notEqual(list.items.filter(item => item.documentation.toString().startsWith('Delay execution for a given number of seconds. The argument may be')).length, 0, 'Documentation incorrect'); }); test('For custom class', done => { @@ -173,4 +174,40 @@ suite('Autocomplete', () => { assert.equal(list.items.filter(item => item.label === 'showMessage')[0].documentation, documentation, 'showMessage unicode documentation is incorrect'); }).then(done, done); }); + + // https://github.com/Microsoft/vscode-python/issues/110 + test('Suppress in strings/comments', done => { + let textEditor: vscode.TextEditor; + let textDocument: vscode.TextDocument; + const positions = [ + new vscode.Position(0, 1), // false + new vscode.Position(0, 9), // true + new vscode.Position(0, 12), // false + new vscode.Position(1, 1), // false + new vscode.Position(1, 3), // false + new vscode.Position(2, 7), // false + new vscode.Position(3, 0), // false + new vscode.Position(4, 2), // false + new vscode.Position(4, 8), // false + new vscode.Position(5, 4) // false + ]; + const expected = [ + false, true, false, false, false, false, false, false, false, false + ]; + vscode.workspace.openTextDocument(fileSuppress).then(document => { + textDocument = document; + return vscode.window.showTextDocument(textDocument); + }).then(editor => { + assert(vscode.window.activeTextEditor, 'No active editor'); + textEditor = editor; + for (let i = 0; i < positions.length; i += 1) { + vscode.commands.executeCommand('vscode.executeCompletionItemProvider', + textDocument.uri, positions[i]).then(list => { + const result = list.items.filter(item => item.label === 'abs').length; + assert.equal(result > 0, expected[i], + `Expected ${expected[i]} at position ${positions[i].line}:${positions[i].character} but got ${result}`); + }); + } + }).then(done, done); + }); }); diff --git a/src/test/pythonFiles/autocomp/suppress.py b/src/test/pythonFiles/autocomp/suppress.py new file mode 100644 index 000000000000..9f74959ef14b --- /dev/null +++ b/src/test/pythonFiles/autocomp/suppress.py @@ -0,0 +1,6 @@ +"string" #comment +""" +content +""" +#comment +'un#closed From 06eb1a56049bdbcb9db71c550d9cbd869a4fce63 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 4 Dec 2017 11:26:07 -0800 Subject: [PATCH 11/51] Fix last line --- .vscode/launch.json | 2 +- .vscode/tasks.json | 2 +- gulpfile.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index a69c3396ff4e..51590d047be8 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -77,4 +77,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/.vscode/tasks.json b/.vscode/tasks.json index ccf99a2c6f20..8a17b7da905f 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -72,4 +72,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/gulpfile.js b/gulpfile.js index 6c3f7819d003..ecf4dd1d5bca 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -454,4 +454,4 @@ class Deferred { get completed() { return this._rejected || this._resolved; } -} \ No newline at end of file +} From e9db8e0de99936daef098000181c44db517c0d8c Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 4 Dec 2017 12:47:05 -0800 Subject: [PATCH 12/51] Fix javascript math --- src/client/language/textRangeCollection.ts | 4 ++-- src/test/index.ts | 3 ++- src/test/language/textRangeCollection.test.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/client/language/textRangeCollection.ts b/src/client/language/textRangeCollection.ts index 0464dc945382..d09becea902f 100644 --- a/src/client/language/textRangeCollection.ts +++ b/src/client/language/textRangeCollection.ts @@ -51,7 +51,7 @@ export class TextRangeCollection implements ITextRangeColl let max = this.count - 1; while (min <= max) { - const mid = min + (max - min) / 2; + const mid = Math.floor(min + (max - min) / 2); const item = this.items[mid]; if (item.start === position) { @@ -82,7 +82,7 @@ export class TextRangeCollection implements ITextRangeColl let max = this.count - 1; while (min <= max) { - const mid = min + (max - min) / 2; + const mid = Math.floor(min + (max - min) / 2); const item = this.items[mid]; if (item.contains(position)) { diff --git a/src/test/index.ts b/src/test/index.ts index 0202b4e2dc43..9eb083463586 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -8,6 +8,7 @@ testRunner.configure({ ui: 'tdd', useColors: true, timeout: 25000, - retries: 3 + retries: 3, + grep: "Language.TextRangeCollection" } as {}); module.exports = testRunner; diff --git a/src/test/language/textRangeCollection.test.ts b/src/test/language/textRangeCollection.test.ts index 5b5dfad8c8b6..44666dd811ce 100644 --- a/src/test/language/textRangeCollection.test.ts +++ b/src/test/language/textRangeCollection.test.ts @@ -44,7 +44,7 @@ suite('Language.TextRangeCollection', () => { items.push(new TextRange(2, 1)); items.push(new TextRange(4, 2)); const c = new TextRangeCollection(items); - const results = [-1, -1, 0, -1, 1, 1, -1]; + const results = [-1, -1, 0, -1, 1, -1, -1]; for (let i = 0; i < results.length; i += 1) { const index = c.getItemAtPosition(i); assert.equal(index, results[i]); From d8ab041f3f8fb459a37bd3adc10de82fadfd3c7b Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 5 Dec 2017 10:07:55 -0800 Subject: [PATCH 13/51] Make test await for results --- src/test/autocomplete/base.test.ts | 27 +++++++++------------------ src/test/index.ts | 3 +-- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/test/autocomplete/base.test.ts b/src/test/autocomplete/base.test.ts index 59a7e51c14f4..e4b8b854570d 100644 --- a/src/test/autocomplete/base.test.ts +++ b/src/test/autocomplete/base.test.ts @@ -176,9 +176,7 @@ suite('Autocomplete', () => { }); // https://github.com/Microsoft/vscode-python/issues/110 - test('Suppress in strings/comments', done => { - let textEditor: vscode.TextEditor; - let textDocument: vscode.TextDocument; + test('Suppress in strings/comments', async () => { const positions = [ new vscode.Position(0, 1), // false new vscode.Position(0, 9), // true @@ -194,20 +192,13 @@ suite('Autocomplete', () => { const expected = [ false, true, false, false, false, false, false, false, false, false ]; - vscode.workspace.openTextDocument(fileSuppress).then(document => { - textDocument = document; - return vscode.window.showTextDocument(textDocument); - }).then(editor => { - assert(vscode.window.activeTextEditor, 'No active editor'); - textEditor = editor; - for (let i = 0; i < positions.length; i += 1) { - vscode.commands.executeCommand('vscode.executeCompletionItemProvider', - textDocument.uri, positions[i]).then(list => { - const result = list.items.filter(item => item.label === 'abs').length; - assert.equal(result > 0, expected[i], - `Expected ${expected[i]} at position ${positions[i].line}:${positions[i].character} but got ${result}`); - }); - } - }).then(done, done); + const textDocument = await vscode.workspace.openTextDocument(fileSuppress); + await vscode.window.showTextDocument(textDocument); + for (let i = 0; i < positions.length; i += 1) { + const list = await vscode.commands.executeCommand('vscode.executeCompletionItemProvider', textDocument.uri, positions[i]); + const result = list.items.filter(item => item.label === 'abs').length; + assert.equal(result > 0, expected[i], + `Expected ${expected[i]} at position ${positions[i].line}:${positions[i].character} but got ${result}`); + } }); }); diff --git a/src/test/index.ts b/src/test/index.ts index 9eb083463586..0202b4e2dc43 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -8,7 +8,6 @@ testRunner.configure({ ui: 'tdd', useColors: true, timeout: 25000, - retries: 3, - grep: "Language.TextRangeCollection" + retries: 3 } as {}); module.exports = testRunner; From db75cd00e892d8721c92df14b7b77ca92d5c2a0a Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 5 Dec 2017 10:22:37 -0800 Subject: [PATCH 14/51] Add license headers --- src/client/language/characterStream.ts | 2 ++ src/test/autocomplete/base.test.ts | 3 +++ src/test/language/characterStream.test.ts | 4 ++++ src/test/language/textIterator.test.ts | 4 ++++ src/test/language/textRange.test.ts | 4 ++++ src/test/language/textRangeCollection.test.ts | 4 ++++ src/test/language/tokenizer.test.ts | 4 ++++ 7 files changed, 25 insertions(+) diff --git a/src/client/language/characterStream.ts b/src/client/language/characterStream.ts index e90ef7f2e6b7..a4da08659a9d 100644 --- a/src/client/language/characterStream.ts +++ b/src/client/language/characterStream.ts @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. 'use strict'; // tslint:disable-next-line:import-name diff --git a/src/test/autocomplete/base.test.ts b/src/test/autocomplete/base.test.ts index e4b8b854570d..1873f86f0776 100644 --- a/src/test/autocomplete/base.test.ts +++ b/src/test/autocomplete/base.test.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + // Note: This example test is leveraging the Mocha test framework. // Please refer to their documentation on https://mochajs.org/ for help. diff --git a/src/test/language/characterStream.test.ts b/src/test/language/characterStream.test.ts index d1c003d6b2d9..165fdde51ae9 100644 --- a/src/test/language/characterStream.test.ts +++ b/src/test/language/characterStream.test.ts @@ -1,3 +1,7 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + import * as assert from 'assert'; // tslint:disable-next-line:import-name import Char from 'typescript-char'; diff --git a/src/test/language/textIterator.test.ts b/src/test/language/textIterator.test.ts index dddfe3478ec5..34daa81534cd 100644 --- a/src/test/language/textIterator.test.ts +++ b/src/test/language/textIterator.test.ts @@ -1,3 +1,7 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + import * as assert from 'assert'; import { TextIterator } from '../../client/language/textIterator'; diff --git a/src/test/language/textRange.test.ts b/src/test/language/textRange.test.ts index 4e0da8feb06c..fecf287032a0 100644 --- a/src/test/language/textRange.test.ts +++ b/src/test/language/textRange.test.ts @@ -1,3 +1,7 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + import * as assert from 'assert'; import { TextRange } from '../../client/language/definitions'; diff --git a/src/test/language/textRangeCollection.test.ts b/src/test/language/textRangeCollection.test.ts index 44666dd811ce..5c56c3f139c7 100644 --- a/src/test/language/textRangeCollection.test.ts +++ b/src/test/language/textRangeCollection.test.ts @@ -1,3 +1,7 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + import * as assert from 'assert'; import { TextRange } from '../../client/language/definitions'; import { TextRangeCollection } from '../../client/language/textRangeCollection'; diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index 1ce0165e4241..139441aeea81 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -1,3 +1,7 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + import * as assert from 'assert'; import { TextRange, TokenType } from '../../client/language/definitions'; import { TextRangeCollection } from '../../client/language/textRangeCollection'; From 9ab2c47a609629570fc387e3b3697cd60e8397e4 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 5 Dec 2017 10:35:58 -0800 Subject: [PATCH 15/51] Rename definitions to types --- src/client/language/characterStream.ts | 2 +- src/client/language/textIterator.ts | 2 +- src/client/language/textRangeCollection.ts | 2 +- src/client/language/tokenizer.ts | 2 +- src/client/language/{definitions.ts => types.ts} | 0 src/client/providers/completionProvider.ts | 2 +- src/test/language/characterStream.test.ts | 2 +- src/test/language/textRange.test.ts | 2 +- src/test/language/textRangeCollection.test.ts | 2 +- src/test/language/tokenizer.test.ts | 2 +- 10 files changed, 9 insertions(+), 9 deletions(-) rename src/client/language/{definitions.ts => types.ts} (100%) diff --git a/src/client/language/characterStream.ts b/src/client/language/characterStream.ts index a4da08659a9d..a95af7ede457 100644 --- a/src/client/language/characterStream.ts +++ b/src/client/language/characterStream.ts @@ -4,8 +4,8 @@ // tslint:disable-next-line:import-name import Char from 'typescript-char'; -import { ICharacterStream, ITextIterator } from './definitions'; import { TextIterator } from './textIterator'; +import { ICharacterStream, ITextIterator } from './types'; export class CharacterStream implements ICharacterStream { private text: ITextIterator; diff --git a/src/client/language/textIterator.ts b/src/client/language/textIterator.ts index 927078da3939..3984dfbe3458 100644 --- a/src/client/language/textIterator.ts +++ b/src/client/language/textIterator.ts @@ -1,7 +1,7 @@ 'use strict'; import { Position, Range, TextDocument } from 'vscode'; -import { ITextIterator } from './definitions'; +import { ITextIterator } from './types'; export class TextIterator implements ITextIterator { private text: string; diff --git a/src/client/language/textRangeCollection.ts b/src/client/language/textRangeCollection.ts index d09becea902f..47983f8fe0cb 100644 --- a/src/client/language/textRangeCollection.ts +++ b/src/client/language/textRangeCollection.ts @@ -1,6 +1,6 @@ 'use strict'; -import { ITextRange, ITextRangeCollection } from './definitions'; +import { ITextRange, ITextRangeCollection } from './types'; export class TextRangeCollection implements ITextRangeCollection { private items: T[]; diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index 6d63f4168414..8b122da7c346 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -3,8 +3,8 @@ // tslint:disable-next-line:import-name import Char from 'typescript-char'; import { CharacterStream } from './characterStream'; -import { ICharacterStream, ITextRangeCollection, IToken, ITokenizer, TextRange, TokenType } from './definitions'; import { TextRangeCollection } from './textRangeCollection'; +import { ICharacterStream, ITextRangeCollection, IToken, ITokenizer, TextRange, TokenType } from './types'; enum QuoteType { None, diff --git a/src/client/language/definitions.ts b/src/client/language/types.ts similarity index 100% rename from src/client/language/definitions.ts rename to src/client/language/types.ts diff --git a/src/client/providers/completionProvider.ts b/src/client/providers/completionProvider.ts index 10c930348e33..b6d62c340dd7 100644 --- a/src/client/providers/completionProvider.ts +++ b/src/client/providers/completionProvider.ts @@ -3,8 +3,8 @@ import * as vscode from 'vscode'; import { Position, ProviderResult, SnippetString, Uri } from 'vscode'; import { PythonSettings } from '../common/configSettings'; -import { TokenType } from '../language/definitions'; import { Tokenizer } from '../language/tokenizer'; +import { TokenType } from '../language/types'; import { JediFactory } from '../languageServices/jediProxyFactory'; import { captureTelemetry } from '../telemetry'; import { COMPLETION } from '../telemetry/constants'; diff --git a/src/test/language/characterStream.test.ts b/src/test/language/characterStream.test.ts index 165fdde51ae9..63ea71f01746 100644 --- a/src/test/language/characterStream.test.ts +++ b/src/test/language/characterStream.test.ts @@ -6,8 +6,8 @@ import * as assert from 'assert'; // tslint:disable-next-line:import-name import Char from 'typescript-char'; import { CharacterStream } from '../../client/language/characterStream'; -import { ICharacterStream, TextRange } from '../../client/language/definitions'; import { TextIterator } from '../../client/language/textIterator'; +import { ICharacterStream, TextRange } from '../../client/language/types'; // tslint:disable-next-line:max-func-body-length suite('Language.CharacterStream', () => { diff --git a/src/test/language/textRange.test.ts b/src/test/language/textRange.test.ts index fecf287032a0..02cad753c16f 100644 --- a/src/test/language/textRange.test.ts +++ b/src/test/language/textRange.test.ts @@ -3,7 +3,7 @@ 'use strict'; import * as assert from 'assert'; -import { TextRange } from '../../client/language/definitions'; +import { TextRange } from '../../client/language/types'; // tslint:disable-next-line:max-func-body-length suite('Language.TextRange', () => { diff --git a/src/test/language/textRangeCollection.test.ts b/src/test/language/textRangeCollection.test.ts index 5c56c3f139c7..32522e63c778 100644 --- a/src/test/language/textRangeCollection.test.ts +++ b/src/test/language/textRangeCollection.test.ts @@ -3,8 +3,8 @@ 'use strict'; import * as assert from 'assert'; -import { TextRange } from '../../client/language/definitions'; import { TextRangeCollection } from '../../client/language/textRangeCollection'; +import { TextRange } from '../../client/language/types'; // tslint:disable-next-line:max-func-body-length suite('Language.TextRangeCollection', () => { diff --git a/src/test/language/tokenizer.test.ts b/src/test/language/tokenizer.test.ts index 139441aeea81..7642b88acfaa 100644 --- a/src/test/language/tokenizer.test.ts +++ b/src/test/language/tokenizer.test.ts @@ -3,9 +3,9 @@ 'use strict'; import * as assert from 'assert'; -import { TextRange, TokenType } from '../../client/language/definitions'; import { TextRangeCollection } from '../../client/language/textRangeCollection'; import { Tokenizer } from '../../client/language/tokenizer'; +import { TextRange, TokenType } from '../../client/language/types'; // tslint:disable-next-line:max-func-body-length suite('Language.Tokenizer', () => { From d587485696c15d2a5dec35fdc9f317dfce3b9a39 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 5 Dec 2017 12:02:27 -0800 Subject: [PATCH 16/51] License headers --- src/client/language/textIterator.ts | 2 ++ src/client/language/textRangeCollection.ts | 2 ++ src/client/language/tokenizer.ts | 2 ++ src/client/language/types.ts | 2 ++ 4 files changed, 8 insertions(+) diff --git a/src/client/language/textIterator.ts b/src/client/language/textIterator.ts index 3984dfbe3458..d5eda4783e2c 100644 --- a/src/client/language/textIterator.ts +++ b/src/client/language/textIterator.ts @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. 'use strict'; import { Position, Range, TextDocument } from 'vscode'; diff --git a/src/client/language/textRangeCollection.ts b/src/client/language/textRangeCollection.ts index 47983f8fe0cb..8ce5a744c9a6 100644 --- a/src/client/language/textRangeCollection.ts +++ b/src/client/language/textRangeCollection.ts @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. 'use strict'; import { ITextRange, ITextRangeCollection } from './types'; diff --git a/src/client/language/tokenizer.ts b/src/client/language/tokenizer.ts index 8b122da7c346..60d9fadc7e2e 100644 --- a/src/client/language/tokenizer.ts +++ b/src/client/language/tokenizer.ts @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. 'use strict'; // tslint:disable-next-line:import-name diff --git a/src/client/language/types.ts b/src/client/language/types.ts index d001adccbd88..121ee682c085 100644 --- a/src/client/language/types.ts +++ b/src/client/language/types.ts @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. 'use strict'; export interface ITextRange { From 1ac4932c51f35f95aaf4067b40155d5b18ad3f49 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 11 Dec 2017 14:57:37 -0800 Subject: [PATCH 17/51] Fix typo in completion details (typo) --- src/client/providers/itemInfoSource.ts | 2 +- src/test/index.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/client/providers/itemInfoSource.ts b/src/client/providers/itemInfoSource.ts index 9dc906e23580..152bf10cc48f 100644 --- a/src/client/providers/itemInfoSource.ts +++ b/src/client/providers/itemInfoSource.ts @@ -117,7 +117,7 @@ export class ItemInfoSource { } const descriptionWithHighlightedCode = this.highlightCode(dnd[1]); - const tooltip = new vscode.MarkdownString(['y```python', signature, '```', descriptionWithHighlightedCode].join(EOL)); + const tooltip = new vscode.MarkdownString(['```python', signature, '```', descriptionWithHighlightedCode].join(EOL)); infos.push(new LanguageItemInfo(tooltip, dnd[0], new vscode.MarkdownString(dnd[1]))); const key = signature + lines.join(''); diff --git a/src/test/index.ts b/src/test/index.ts index eebaa9b59c8d..4d3b12a351ca 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -12,8 +12,7 @@ const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, timeout: 25000, - retries: 3, - grep: 'Autocomplete' + retries: 3 }; testRunner.configure(options); module.exports = testRunner; From 2aa5a6c32b0e1c8df853f65acb7ceb27bd519f78 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Mon, 11 Dec 2017 17:06:59 -0800 Subject: [PATCH 18/51] Fix hover test --- src/client/providers/itemInfoSource.ts | 2 +- src/test/definitions/hover.test.ts | 70 ++++++++++++++------------ 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/client/providers/itemInfoSource.ts b/src/client/providers/itemInfoSource.ts index 152bf10cc48f..3cb471959bac 100644 --- a/src/client/providers/itemInfoSource.ts +++ b/src/client/providers/itemInfoSource.ts @@ -116,7 +116,7 @@ export class ItemInfoSource { lines.shift(); } - const descriptionWithHighlightedCode = this.highlightCode(dnd[1]); + const descriptionWithHighlightedCode = this.highlightCode(lines.join(EOL)); const tooltip = new vscode.MarkdownString(['```python', signature, '```', descriptionWithHighlightedCode].join(EOL)); infos.push(new LanguageItemInfo(tooltip, dnd[0], new vscode.MarkdownString(dnd[1]))); diff --git a/src/test/definitions/hover.test.ts b/src/test/definitions/hover.test.ts index 07f06b8843ca..adc7832b80fc 100644 --- a/src/test/definitions/hover.test.ts +++ b/src/test/definitions/hover.test.ts @@ -1,15 +1,14 @@ // Note: This example test is leveraging the Mocha test framework. // Please refer to their documentation on https://mochajs.org/ for help. - // The module 'assert' provides assertion methods from node import * as assert from 'assert'; import { EOL } from 'os'; // You can import and use all API from the 'vscode' module // as well as import your extension to test it -import * as vscode from 'vscode'; import * as path from 'path'; -import { initialize, closeActiveWindows, initializeTest } from '../initialize'; +import * as vscode from 'vscode'; +import { closeActiveWindows, initialize, initializeTest } from '../initialize'; import { normalizeMarkedString } from '../textUtils'; const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'autocomp'); @@ -21,11 +20,12 @@ const fileEncodingUsed = path.join(autoCompPath, 'five.py'); const fileHover = path.join(autoCompPath, 'hoverTest.py'); const fileStringFormat = path.join(hoverPath, 'stringFormat.py'); +// tslint:disable-next-line:max-func-body-length suite('Hover Definition', () => { - suiteSetup(() => initialize()); - setup(() => initializeTest()); - suiteTeardown(() => closeActiveWindows()); - teardown(() => closeActiveWindows()); + suiteSetup(initialize); + setup(initializeTest); + suiteTeardown(closeActiveWindows); + teardown(closeActiveWindows); test('Method', done => { let textEditor: vscode.TextEditor; @@ -43,6 +43,7 @@ suite('Hover Definition', () => { assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '30,4', 'Start position is incorrect'); 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'); + // tslint:disable-next-line:prefer-template const expectedContent = '```python' + EOL + 'def method1()' + EOL + '```' + EOL + 'This is method1'; assert.equal(normalizeMarkedString(def[0].contents[0]), expectedContent, 'function signature incorrect'); }).then(done, done); @@ -63,6 +64,7 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '1,9', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '1,12', 'End position is incorrect'); + // tslint:disable-next-line:prefer-template assert.equal(normalizeMarkedString(def[0].contents[0]), '```python' + EOL + 'def fun()' + EOL + '```' + EOL + 'This is fun', 'Invalid conents'); }).then(done, done); }); @@ -82,6 +84,7 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '25,4', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '25,7', 'End position is incorrect'); + // tslint:disable-next-line:prefer-template assert.equal(normalizeMarkedString(def[0].contents[0]), '```python' + EOL + 'def bar()' + EOL + '```' + EOL + '说明 - keep this line, it works' + EOL + 'delete following line, it works' + EOL + '如果存在需要等待审批或正在执行的任务，将不刷新页面', 'Invalid conents'); @@ -103,6 +106,7 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '1,5', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '1,16', 'End position is incorrect'); + // tslint:disable-next-line:prefer-template assert.equal(normalizeMarkedString(def[0].contents[0]), '```python' + EOL + 'def showMessage()' + EOL + '```' + EOL + @@ -158,19 +162,20 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '11,12', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '11,18', 'End position is incorrect'); - let documentation = "```python" + EOL + - "class Random(x=None)" + EOL + - "```" + EOL + - "Random number generator base class used by bound module functions." + EOL + - "" + EOL + - "Used to instantiate instances of Random to get generators that don't" + EOL + - "share state." + EOL + - "" + EOL + - "Class Random can also be subclassed if you want to use a different basic" + EOL + - "generator of your own devising: in that case, override the following" + EOL + EOL + - "`methods` random(), seed(), getstate(), and setstate()." + EOL + EOL + - "Optionally, implement a getrandbits() method so that randrange()" + EOL + - "can cover arbitrarily large ranges."; + // tslint:disable-next-line:prefer-template + const documentation = '```python' + EOL + + 'class Random(x=None)' + EOL + + '```' + EOL + + 'Random number generator base class used by bound module functions.' + EOL + + '' + EOL + + 'Used to instantiate instances of Random to get generators that don\'t' + EOL + + 'share state.' + EOL + + '' + EOL + + 'Class Random can also be subclassed if you want to use a different basic' + EOL + + 'generator of your own devising: in that case, override the following' + EOL + EOL + + '`methods` random(), seed(), getstate(), and setstate().' + EOL + EOL + + 'Optionally, implement a getrandbits() method so that randrange()' + EOL + + 'can cover arbitrarily large ranges.'; assert.equal(normalizeMarkedString(def[0].contents[0]), documentation, 'Invalid conents'); }).then(done, done); @@ -191,6 +196,7 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '12,5', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '12,12', 'End position is incorrect'); + // tslint:disable-next-line:prefer-template assert.equal(normalizeMarkedString(def[0].contents[0]), '```python' + EOL + 'def randint(a, b)' + EOL + '```' + EOL + @@ -213,6 +219,7 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '8,11', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '8,15', 'End position is incorrect'); + // tslint:disable-next-line:prefer-template assert.equal(normalizeMarkedString(def[0].contents[0]), '```python' + EOL + 'def acos(x)' + EOL + '```' + EOL + @@ -235,6 +242,7 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(`${def[0].range.start.line},${def[0].range.start.character}`, '14,9', 'Start position is incorrect'); assert.equal(`${def[0].range.end.line},${def[0].range.end.character}`, '14,15', 'End position is incorrect'); + // tslint:disable-next-line:prefer-template assert.equal(normalizeMarkedString(def[0].contents[0]), '```python' + EOL + 'class Thread(group=None, target=None, name=None, args=(), kwargs=None, verbose=None)' + EOL + '```' + EOL + @@ -262,14 +270,14 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(def[0].contents.length, 1, 'Only expected one result'); const contents = normalizeMarkedString(def[0].contents[0]); - if (contents.indexOf("```python") === -1) { - assert.fail(contents, "", "First line is incorrect", "compare"); + if (contents.indexOf('```python') === -1) { + assert.fail(contents, '', 'First line is incorrect', 'compare'); } - if (contents.indexOf("Random number generator base class used by bound module functions.") === -1) { - assert.fail(contents, "", "'Random number generator' message missing", "compare"); + if (contents.indexOf('Random number generator base class used by bound module functions.') === -1) { + assert.fail(contents, '', '\'Random number generator\' message missing', 'compare'); } - if (contents.indexOf("Class Random can also be subclassed if you want to use a different basic") === -1) { - assert.fail(contents, "", "'Class Random message' missing", "compare"); + if (contents.indexOf('Class Random can also be subclassed if you want to use a different basic') === -1) { + assert.fail(contents, '', '\'Class Random message\' missing', 'compare'); } }).then(done, done); }); @@ -282,12 +290,12 @@ suite('Hover Definition', () => { assert.equal(def.length, 1, 'Definition length is incorrect'); assert.equal(def[0].contents.length, 1, 'Only expected one result'); const contents = normalizeMarkedString(def[0].contents[0]); - if (contents.indexOf("def capitalize") === -1) { - assert.fail(contents, "", "'def capitalize' is missing", "compare"); + if (contents.indexOf('def capitalize') === -1) { + assert.fail(contents, '', '\'def capitalize\' is missing', 'compare'); } - if (contents.indexOf("Return a capitalized version of S") === -1 && - contents.indexOf("Return a copy of the string S with only its first character") === -1) { - assert.fail(contents, "", "'Return a capitalized version of S/Return a copy of the string S with only its first character' message missing", "compare"); + if (contents.indexOf('Return a capitalized version of S') === -1 && + contents.indexOf('Return a copy of the string S with only its first character') === -1) { + assert.fail(contents, '', '\'Return a capitalized version of S/Return a copy of the string S with only its first character\' message missing', 'compare'); } }); }); From 560d2af430b6188a78b086b51d4d2d669f685c61 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 12 Dec 2017 16:10:28 -0800 Subject: [PATCH 19/51] Russian translations --- package.nls.ru.json | 50 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 package.nls.ru.json diff --git a/package.nls.ru.json b/package.nls.ru.json new file mode 100644 index 000000000000..4b082501ca78 --- /dev/null +++ b/package.nls.ru.json @@ -0,0 +1,50 @@ +{ + "python.command.python.sortImports.title": "Отсортировать Imports", + "python.command.python.startREPL.title": "Открыть REPL", + "python.command.python.buildWorkspaceSymbols.title": "Собрать символы рабочего пространства", + "python.command.python.runtests.title": "Запустить все тесты", + "python.command.python.debugtests.title": "Запустить все тесты под отладчиком", + "python.command.python.execInTerminal.title": "Выполнить файл в консоли", + "python.command.python.setInterpreter.title": "Выбрать интерпретатор", + "python.command.python.updateSparkLibrary.title": "Обновить библиотеки PySpark", + "python.command.python.refactorExtractVariable.title": "Создать переменную", + "python.command.python.refactorExtractMethod.title": "Создать метод", + "python.command.python.viewTestOutput.title": "Показать вывод теста", + "python.command.python.selectAndRunTestMethod.title": "Запусть тестовый метод...", + "python.command.python.selectAndDebugTestMethod.title": "Отладить тестовый метод...", + "python.command.python.selectAndRunTestFile.title": "Запустить тестовый файл...", + "python.command.python.runCurrentTestFile.title": "Запустить текущий тестовый файл", + "python.command.python.runFailedTests.title": "Запустить непрошедшие тесты", + "python.command.python.execSelectionInTerminal.title": "Выполнить выбранный текст или текущую строку в консоли", + "python.command.python.execSelectionInDjangoShell.title": "Выполнить выбранный текст или текущую строку в оболочке Django", + "python.command.jupyter.runSelectionLine.title": "Выполнить выбранный текст или текущую строку", + "python.command.jupyter.execCurrentCell.title": "Выполнить ячейку", + "python.command.jupyter.execCurrentCellAndAdvance.title": "Выполнить ячейку и перейти к следующей", + "python.command.jupyter.gotToPreviousCell.title": "Перейти к предыдущей ячейке", + "python.command.jupyter.gotToNextCell.title": "Перейти к следующей ячейке", + "python.command.python.goToPythonObject.title": "Перейти к объекту Python", + "python.snippet.launch.standard.label": "Python", + "python.snippet.launch.standard.description": "Отладить программу Python со стандартным выводом", + "python.snippet.launch.pyspark.label": "Python: PySpark", + "python.snippet.launch.pyspark.description": "Отладка PySpark", + "python.snippet.launch.module.label": "Python: Модуль", + "python.snippet.launch.module.description": "Отладка модуля", + "python.snippet.launch.terminal.label": "Python: Интегрированная консоль", + "python.snippet.launch.terminal.description": "Отладка программы Python в интегрированной консоли", + "python.snippet.launch.externalTerminal.label": "Python: Внешний терминал", + "python.snippet.launch.externalTerminal.description": "Отладка программы Python во внешней консоли", + "python.snippet.launch.django.label": "Python: Django", + "python.snippet.launch.django.description": "Отладка приложения Django", + "python.snippet.launch.flask.label": "Python: Flask (0.11.x или новее)", + "python.snippet.launch.flask.description": "Отладка приложения Flask", + "python.snippet.launch.flaskOld.label": "Python: Flask (0.10.x или старее)", + "python.snippet.launch.flaskOld.description": "Отладка приложения Flask (старый стиль)", + "python.snippet.launch.pyramid.label": "Python: Приложение Pyramid", + "python.snippet.launch.pyramid.description": "Отладка приложения Pyramid", + "python.snippet.launch.watson.label": "Python: Приложение Watson", + "python.snippet.launch.watson.description": "Отладка приложения Watson", + "python.snippet.launch.attach.label": "Python: Подключить отладчик", + "python.snippet.launch.attach.description": "Подключить отладчик для удаленной отладки", + "python.snippet.launch.scrapy.label": "Python: Scrapy", + "python.snippet.launch.scrapy.description": "Scrapy в интергрированной консоли" +} From 31aa087843c9bb84ca29c322110828909256ee15 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 13 Dec 2017 10:32:54 -0800 Subject: [PATCH 20/51] Update to better translation --- package.nls.ru.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.nls.ru.json b/package.nls.ru.json index 4b082501ca78..1ab5894793da 100644 --- a/package.nls.ru.json +++ b/package.nls.ru.json @@ -7,8 +7,8 @@ "python.command.python.execInTerminal.title": "Выполнить файл в консоли", "python.command.python.setInterpreter.title": "Выбрать интерпретатор", "python.command.python.updateSparkLibrary.title": "Обновить библиотеки PySpark", - "python.command.python.refactorExtractVariable.title": "Создать переменную", - "python.command.python.refactorExtractMethod.title": "Создать метод", + "python.command.python.refactorExtractVariable.title": "Извлечь в переменную", + "python.command.python.refactorExtractMethod.title": "Извлечь в метод", "python.command.python.viewTestOutput.title": "Показать вывод теста", "python.command.python.selectAndRunTestMethod.title": "Запусть тестовый метод...", "python.command.python.selectAndDebugTestMethod.title": "Отладить тестовый метод...", From 593ae0558c05398ca818ad033f06a4d10587ec2c Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 13 Dec 2017 10:34:13 -0800 Subject: [PATCH 21/51] Fix typo --- package.nls.ru.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.nls.ru.json b/package.nls.ru.json index 1ab5894793da..e0e22abbc06d 100644 --- a/package.nls.ru.json +++ b/package.nls.ru.json @@ -46,5 +46,5 @@ "python.snippet.launch.attach.label": "Python: Подключить отладчик", "python.snippet.launch.attach.description": "Подключить отладчик для удаленной отладки", "python.snippet.launch.scrapy.label": "Python: Scrapy", - "python.snippet.launch.scrapy.description": "Scrapy в интергрированной консоли" + "python.snippet.launch.scrapy.description": "Scrapy в интегрированной консоли" } From e6d69bb7a88eacae85b09a9ebab0026c707fb239 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 13 Dec 2017 14:45:16 -0800 Subject: [PATCH 22/51] #70 How to get all parameter info when filling in a function param list --- src/client/providers/signatureProvider.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/client/providers/signatureProvider.ts b/src/client/providers/signatureProvider.ts index bf6480a4d3a2..af2ba64d9692 100644 --- a/src/client/providers/signatureProvider.ts +++ b/src/client/providers/signatureProvider.ts @@ -1,5 +1,6 @@ 'use strict'; +import { EOL } from 'os'; import * as vscode from 'vscode'; import { CancellationToken, Position, SignatureHelp, TextDocument } from 'vscode'; import { JediFactory } from '../languageServices/jediProxyFactory'; @@ -55,8 +56,13 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { signature.activeParameter = def.paramindex; // Don't display the documentation, as vs code doesn't format the docmentation. // i.e. line feeds are not respected, long content is stripped. + const docLines = def.docstring.splitLines(); + const label = docLines[0].trim(); + const documentation = docLines.length > 1 ? docLines.filter((line, index) => index > 0).join(EOL) : ''; + const sig = { - label: def.description, + label: label, + documentation: documentation, parameters: [] }; sig.parameters = def.params.map(arg => { @@ -65,7 +71,7 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { } return { documentation: arg.docstring.length > 0 ? arg.docstring : arg.description, - label: arg.description.length > 0 ? arg.description : arg.name + label: arg.name }; }); signature.signatures.push(sig); @@ -85,7 +91,7 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { source: document.getText() }; return this.jediFactory.getJediProxyHandler(document.uri).sendCommand(cmd, token).then(data => { - return PythonSignatureProvider.parseData(data); + return data ? PythonSignatureProvider.parseData(data) : undefined; }); } } From b5a23d3f24b5e55edff0838376a962c029d7a4cc Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 13 Dec 2017 16:03:31 -0800 Subject: [PATCH 23/51] Fix #70 How to get all parameter info when filling in a function param list --- src/test/pythonFiles/signature/one.py | 6 ++ src/test/pythonFiles/signature/two.py | 1 + src/test/signature/signature.test.ts | 95 +++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 src/test/pythonFiles/signature/one.py create mode 100644 src/test/pythonFiles/signature/two.py create mode 100644 src/test/signature/signature.test.ts diff --git a/src/test/pythonFiles/signature/one.py b/src/test/pythonFiles/signature/one.py new file mode 100644 index 000000000000..baa4045489e7 --- /dev/null +++ b/src/test/pythonFiles/signature/one.py @@ -0,0 +1,6 @@ +class Person: + def __init__(self, name, age = 23): + self.name = name + self.age = age + +p1 = Person('Bob', ) diff --git a/src/test/pythonFiles/signature/two.py b/src/test/pythonFiles/signature/two.py new file mode 100644 index 000000000000..beaa970c7eb5 --- /dev/null +++ b/src/test/pythonFiles/signature/two.py @@ -0,0 +1 @@ +pow(c, 1, diff --git a/src/test/signature/signature.test.ts b/src/test/signature/signature.test.ts new file mode 100644 index 000000000000..d3578b62b8b3 --- /dev/null +++ b/src/test/signature/signature.test.ts @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import * as assert from 'assert'; +import { EOL } from 'os'; +import * as path from 'path'; +import * as vscode from 'vscode'; +import { PythonSettings } from '../../client/common/configSettings'; +import { execPythonFile } from '../../client/common/utils'; +import { rootWorkspaceUri } from '../common'; +import { closeActiveWindows, initialize, initializeTest } from '../initialize'; + +const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'signature'); +const fileOne = path.join(autoCompPath, 'one.py'); +const fileTwo = path.join(autoCompPath, 'two.py'); + +class SignatureHelpResult { + constructor( + public line: number, + public index: number, + public signaturesCount: number, + public activeParameter: number, + public parameterName: string | null) { } +} + +// tslint:disable-next-line:max-func-body-length +suite('Signatures', () => { + suiteSetup(async () => { + await initialize(); + const version = await execPythonFile(rootWorkspaceUri, PythonSettings.getInstance(rootWorkspaceUri).pythonPath, ['--version'], __dirname, true); + }); + setup(initializeTest); + suiteTeardown(closeActiveWindows); + teardown(closeActiveWindows); + + test('For ctor', async () => { + const expected = [ + new SignatureHelpResult(5, 11, 0, 0, null), + new SignatureHelpResult(5, 12, 1, 0, 'name'), + new SignatureHelpResult(5, 13, 1, 0, 'name'), + new SignatureHelpResult(5, 14, 1, 0, 'name'), + new SignatureHelpResult(5, 15, 1, 0, 'name'), + new SignatureHelpResult(5, 16, 1, 0, 'name'), + new SignatureHelpResult(5, 17, 1, 0, 'name'), + new SignatureHelpResult(5, 18, 1, 1, 'age'), + new SignatureHelpResult(5, 19, 1, 1, 'age'), + new SignatureHelpResult(5, 20, 0, 0, null) + ]; + + const document = await openDocument(fileOne); + for (const e of expected) { + await checkSignature(e, document!.uri); + } + }); + + test('For intrinsic', async () => { + const expected = [ + new SignatureHelpResult(0, 0, 0, 0, null), + new SignatureHelpResult(0, 1, 0, 0, null), + new SignatureHelpResult(0, 2, 0, 0, null), + new SignatureHelpResult(0, 3, 0, 0, null), + new SignatureHelpResult(0, 4, 1, 0, 'x'), + new SignatureHelpResult(0, 5, 1, 0, 'x'), + new SignatureHelpResult(0, 6, 1, 1, 'y'), + new SignatureHelpResult(0, 7, 1, 1, 'y'), + new SignatureHelpResult(0, 8, 1, 1, 'y'), + new SignatureHelpResult(0, 9, 1, 2, 'z'), + new SignatureHelpResult(0, 10, 1, 2, 'z'), + new SignatureHelpResult(1, 0, 1, 2, 'z') + ]; + + const document = await openDocument(fileTwo); + for (const e of expected) { + await checkSignature(e, document!.uri); + } + }); +}); + +async function openDocument(documentPath: string): Promise { + const document = await vscode.workspace.openTextDocument(documentPath); + await vscode.window.showTextDocument(document!); + return document; +} + +async function checkSignature(expected: SignatureHelpResult, uri: vscode.Uri) { + const position = new vscode.Position(expected.line, expected.index); + const actual = await vscode.commands.executeCommand('vscode.executeSignatureHelpProvider', uri, position); + assert.equal(actual!.signatures.length, expected.signaturesCount); + if (expected.signaturesCount > 0) { + assert.equal(actual!.activeParameter, expected.activeParameter); + const parameter = actual!.signatures[0].parameters[expected.activeParameter]; + assert.equal(parameter.label, expected.parameterName); + } +} From cd200f7913a959c472ebfe1e194edc584f4f9249 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 13 Dec 2017 16:05:04 -0800 Subject: [PATCH 24/51] Clean up --- src/test/signature/signature.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/signature/signature.test.ts b/src/test/signature/signature.test.ts index d3578b62b8b3..0295a49352f1 100644 --- a/src/test/signature/signature.test.ts +++ b/src/test/signature/signature.test.ts @@ -3,7 +3,6 @@ 'use strict'; import * as assert from 'assert'; -import { EOL } from 'os'; import * as path from 'path'; import * as vscode from 'vscode'; import { PythonSettings } from '../../client/common/configSettings'; @@ -28,7 +27,6 @@ class SignatureHelpResult { suite('Signatures', () => { suiteSetup(async () => { await initialize(); - const version = await execPythonFile(rootWorkspaceUri, PythonSettings.getInstance(rootWorkspaceUri).pythonPath, ['--version'], __dirname, true); }); setup(initializeTest); suiteTeardown(closeActiveWindows); From 7c33228d2dd127f485ac7afde0c2776f9e293411 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 13 Dec 2017 16:05:48 -0800 Subject: [PATCH 25/51] Clean imports --- src/test/signature/signature.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/signature/signature.test.ts b/src/test/signature/signature.test.ts index 0295a49352f1..6cac87049e45 100644 --- a/src/test/signature/signature.test.ts +++ b/src/test/signature/signature.test.ts @@ -5,9 +5,6 @@ import * as assert from 'assert'; import * as path from 'path'; import * as vscode from 'vscode'; -import { PythonSettings } from '../../client/common/configSettings'; -import { execPythonFile } from '../../client/common/utils'; -import { rootWorkspaceUri } from '../common'; import { closeActiveWindows, initialize, initializeTest } from '../initialize'; const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'signature'); From c4a6b90d7d0e47c3dc3259a5001abd3d1fea4ab5 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Dec 2017 09:39:20 -0800 Subject: [PATCH 26/51] CR feedback --- src/client/providers/signatureProvider.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/providers/signatureProvider.ts b/src/client/providers/signatureProvider.ts index af2ba64d9692..d86afae0df7e 100644 --- a/src/client/providers/signatureProvider.ts +++ b/src/client/providers/signatureProvider.ts @@ -57,12 +57,12 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { // Don't display the documentation, as vs code doesn't format the docmentation. // i.e. line feeds are not respected, long content is stripped. const docLines = def.docstring.splitLines(); - const label = docLines[0].trim(); - const documentation = docLines.length > 1 ? docLines.filter((line, index) => index > 0).join(EOL) : ''; + const label = docLines.shift().trim(); + const documentation = docLines.join(EOL).trim(); const sig = { - label: label, - documentation: documentation, + label, + documentation, parameters: [] }; sig.parameters = def.params.map(arg => { @@ -91,7 +91,7 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { source: document.getText() }; return this.jediFactory.getJediProxyHandler(document.uri).sendCommand(cmd, token).then(data => { - return data ? PythonSignatureProvider.parseData(data) : undefined; + return data ? PythonSignatureProvider.parseData(data) : new SignatureHelp(); }); } } From f85b848a82c4e522831f060bd7b2c241ca8e5f0b Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Dec 2017 14:55:13 -0800 Subject: [PATCH 27/51] Trim whitespace for test stability --- src/client/providers/signatureProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/providers/signatureProvider.ts b/src/client/providers/signatureProvider.ts index d86afae0df7e..d6f24484cbb7 100644 --- a/src/client/providers/signatureProvider.ts +++ b/src/client/providers/signatureProvider.ts @@ -71,7 +71,7 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { } return { documentation: arg.docstring.length > 0 ? arg.docstring : arg.description, - label: arg.name + label: arg.name.trim() }; }); signature.signatures.push(sig); From 37c210ba5fa5f2cfe3cdace9e749ffbe1cb2365d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Dec 2017 16:53:04 -0800 Subject: [PATCH 28/51] More tests --- pythonFiles/completion.py | 5 --- src/client/providers/signatureProvider.ts | 18 ++++++-- src/test/pythonFiles/signature/three.py | 1 + src/test/pythonFiles/signature/two.py | 2 +- src/test/signature/signature.test.ts | 53 ++++++++++++++++------- 5 files changed, 53 insertions(+), 26 deletions(-) create mode 100644 src/test/pythonFiles/signature/three.py diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index f072349d0999..ce798d246bab 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -6,7 +6,6 @@ import traceback import platform -WORD_RE = re.compile(r'\w') jediPreview = False class RedirectStdout(object): @@ -111,8 +110,6 @@ def _get_call_signatures(self, script): continue if param.name == 'self' and pos == 0: continue - if WORD_RE.match(param.name) is None: - continue try: name, value = param.description.split('=') except ValueError: @@ -155,8 +152,6 @@ def _get_call_signatures_with_args(self, script): continue if param.name == 'self' and pos == 0: continue - if WORD_RE.match(param.name) is None: - continue try: name, value = param.description.split('=') except ValueError: diff --git a/src/client/providers/signatureProvider.ts b/src/client/providers/signatureProvider.ts index d6f24484cbb7..43caa67cd9cf 100644 --- a/src/client/providers/signatureProvider.ts +++ b/src/client/providers/signatureProvider.ts @@ -54,11 +54,21 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { data.definitions.forEach(def => { signature.activeParameter = def.paramindex; - // Don't display the documentation, as vs code doesn't format the docmentation. + // Don't display the documentation, as vs code doesn't format the documentation. // i.e. line feeds are not respected, long content is stripped. - const docLines = def.docstring.splitLines(); - const label = docLines.shift().trim(); - const documentation = docLines.join(EOL).trim(); + + // Some functions do not come with parameter docs + let label: string; + let documentation: string; + + if (def.params && def.params.length > 0) { + const docLines = def.docstring.splitLines(); + label = docLines.shift().trim(); + documentation = docLines.join(EOL).trim(); + } else { + label = ''; + documentation = def.docstring; + } const sig = { label, diff --git a/src/test/pythonFiles/signature/three.py b/src/test/pythonFiles/signature/three.py new file mode 100644 index 000000000000..fe666b9ff4c8 --- /dev/null +++ b/src/test/pythonFiles/signature/three.py @@ -0,0 +1 @@ +print(a, b, z) diff --git a/src/test/pythonFiles/signature/two.py b/src/test/pythonFiles/signature/two.py index beaa970c7eb5..ae7a551707b8 100644 --- a/src/test/pythonFiles/signature/two.py +++ b/src/test/pythonFiles/signature/two.py @@ -1 +1 @@ -pow(c, 1, +range(c, 1, diff --git a/src/test/signature/signature.test.ts b/src/test/signature/signature.test.ts index 6cac87049e45..5fa42aee2ea0 100644 --- a/src/test/signature/signature.test.ts +++ b/src/test/signature/signature.test.ts @@ -10,6 +10,7 @@ import { closeActiveWindows, initialize, initializeTest } from '../initialize'; const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'signature'); const fileOne = path.join(autoCompPath, 'one.py'); const fileTwo = path.join(autoCompPath, 'two.py'); +const fileThree = path.join(autoCompPath, 'three.py'); class SignatureHelpResult { constructor( @@ -44,8 +45,8 @@ suite('Signatures', () => { ]; const document = await openDocument(fileOne); - for (const e of expected) { - await checkSignature(e, document!.uri); + for (let i = 0; i < expected.length; i += 1) { + await checkSignature(expected[i], document!.uri, i); } }); @@ -55,19 +56,39 @@ suite('Signatures', () => { new SignatureHelpResult(0, 1, 0, 0, null), new SignatureHelpResult(0, 2, 0, 0, null), new SignatureHelpResult(0, 3, 0, 0, null), - new SignatureHelpResult(0, 4, 1, 0, 'x'), - new SignatureHelpResult(0, 5, 1, 0, 'x'), - new SignatureHelpResult(0, 6, 1, 1, 'y'), - new SignatureHelpResult(0, 7, 1, 1, 'y'), - new SignatureHelpResult(0, 8, 1, 1, 'y'), - new SignatureHelpResult(0, 9, 1, 2, 'z'), - new SignatureHelpResult(0, 10, 1, 2, 'z'), - new SignatureHelpResult(1, 0, 1, 2, 'z') + new SignatureHelpResult(0, 4, 0, 0, null), + new SignatureHelpResult(0, 5, 0, 0, null), + 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(fileTwo); - for (const e of expected) { - await checkSignature(e, document!.uri); + for (let i = 0; i < expected.length; i += 1) { + await checkSignature(expected[i], document!.uri, i); + } + }); + + test('For ellipsis', async () => { + const expected = [ + new SignatureHelpResult(0, 4, 0, 0, null), + new SignatureHelpResult(0, 5, 0, 0, null), + new SignatureHelpResult(0, 6, 1, 0, 'value'), + new SignatureHelpResult(0, 7, 1, 0, 'value'), + new SignatureHelpResult(0, 8, 1, 1, '...'), + new SignatureHelpResult(0, 9, 1, 1, '...'), + new SignatureHelpResult(0, 10, 1, 1, '...'), + new SignatureHelpResult(0, 11, 1, 2, 'sep'), + new SignatureHelpResult(0, 12, 1, 2, 'sep') + ]; + + const document = await openDocument(fileThree); + for (let i = 0; i < expected.length; i += 1) { + await checkSignature(expected[i], document!.uri, i); } }); }); @@ -78,13 +99,13 @@ async function openDocument(documentPath: string): Promise('vscode.executeSignatureHelpProvider', uri, position); - assert.equal(actual!.signatures.length, expected.signaturesCount); + assert.equal(actual!.signatures.length, expected.signaturesCount, `Signature count does not match, case ${caseIndex}`); if (expected.signaturesCount > 0) { - assert.equal(actual!.activeParameter, expected.activeParameter); + assert.equal(actual!.activeParameter, expected.activeParameter, `Parameter index does not match, case ${caseIndex}`); const parameter = actual!.signatures[0].parameters[expected.activeParameter]; - assert.equal(parameter.label, expected.parameterName); + assert.equal(parameter.label, expected.parameterName, `Parameter name is incorrect, case ${caseIndex}`); } } From 61a56504912b81b163eb1de4547032f243d0920f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 14 Dec 2017 17:02:09 -0800 Subject: [PATCH 29/51] Better handle no-parameters documentation --- src/client/providers/signatureProvider.ts | 26 +++++++++++++---------- src/test/index.ts | 3 ++- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/client/providers/signatureProvider.ts b/src/client/providers/signatureProvider.ts index 43caa67cd9cf..12dad261c39b 100644 --- a/src/client/providers/signatureProvider.ts +++ b/src/client/providers/signatureProvider.ts @@ -60,13 +60,14 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { // Some functions do not come with parameter docs let label: string; let documentation: string; + const validParamInfo = def.params && def.params.length > 0 && def.docstring.startsWith(`${def.name}(`); - if (def.params && def.params.length > 0) { + if (validParamInfo) { const docLines = def.docstring.splitLines(); label = docLines.shift().trim(); documentation = docLines.join(EOL).trim(); } else { - label = ''; + label = def.description; documentation = def.docstring; } @@ -75,15 +76,18 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider { documentation, parameters: [] }; - sig.parameters = def.params.map(arg => { - if (arg.docstring.length === 0) { - arg.docstring = extractParamDocString(arg.name, def.docstring); - } - return { - documentation: arg.docstring.length > 0 ? arg.docstring : arg.description, - label: arg.name.trim() - }; - }); + + if (validParamInfo) { + sig.parameters = def.params.map(arg => { + if (arg.docstring.length === 0) { + arg.docstring = extractParamDocString(arg.name, def.docstring); + } + return { + documentation: arg.docstring.length > 0 ? arg.docstring : arg.description, + label: arg.name.trim() + }; + }); + } signature.signatures.push(sig); }); return signature; diff --git a/src/test/index.ts b/src/test/index.ts index 4d3b12a351ca..3a5cdd7b0602 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -12,7 +12,8 @@ const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, timeout: 25000, - retries: 3 + retries: 3, + grep: 'Signatures' }; testRunner.configure(options); module.exports = testRunner; From a10305e115dff47eaf5a00764709a004076341c7 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 15 Dec 2017 09:40:02 -0800 Subject: [PATCH 30/51] Better handle ellipsis and Python3 --- src/client/common/utils.ts | 4 +- .../signature/{two.py => basicSig.py} | 1 + .../signature/{one.py => classCtor.py} | 0 src/test/pythonFiles/signature/ellipsis.py | 1 + src/test/pythonFiles/signature/noSigPy3.py | 1 + src/test/pythonFiles/signature/three.py | 1 - src/test/signature/signature.test.ts | 37 ++++++++++++++----- 7 files changed, 33 insertions(+), 12 deletions(-) rename src/test/pythonFiles/signature/{two.py => basicSig.py} (92%) rename src/test/pythonFiles/signature/{one.py => classCtor.py} (100%) create mode 100644 src/test/pythonFiles/signature/ellipsis.py create mode 100644 src/test/pythonFiles/signature/noSigPy3.py delete mode 100644 src/test/pythonFiles/signature/three.py diff --git a/src/client/common/utils.ts b/src/client/common/utils.ts index 34cefb118342..25e9a720ca01 100644 --- a/src/client/common/utils.ts +++ b/src/client/common/utils.ts @@ -340,8 +340,8 @@ export function getSubDirectories(rootDir: string): Promise { subDirs.push(fullPath); } } - catch (ex) { - } + // tslint:disable-next-line:no-empty + catch (ex) {} }); resolve(subDirs); }); diff --git a/src/test/pythonFiles/signature/two.py b/src/test/pythonFiles/signature/basicSig.py similarity index 92% rename from src/test/pythonFiles/signature/two.py rename to src/test/pythonFiles/signature/basicSig.py index ae7a551707b8..66ad4cbd0483 100644 --- a/src/test/pythonFiles/signature/two.py +++ b/src/test/pythonFiles/signature/basicSig.py @@ -1 +1,2 @@ range(c, 1, + diff --git a/src/test/pythonFiles/signature/one.py b/src/test/pythonFiles/signature/classCtor.py similarity index 100% rename from src/test/pythonFiles/signature/one.py rename to src/test/pythonFiles/signature/classCtor.py diff --git a/src/test/pythonFiles/signature/ellipsis.py b/src/test/pythonFiles/signature/ellipsis.py new file mode 100644 index 000000000000..c34faa6d231a --- /dev/null +++ b/src/test/pythonFiles/signature/ellipsis.py @@ -0,0 +1 @@ +print(a, b, c) diff --git a/src/test/pythonFiles/signature/noSigPy3.py b/src/test/pythonFiles/signature/noSigPy3.py new file mode 100644 index 000000000000..3d814698b7fe --- /dev/null +++ b/src/test/pythonFiles/signature/noSigPy3.py @@ -0,0 +1 @@ +pow() diff --git a/src/test/pythonFiles/signature/three.py b/src/test/pythonFiles/signature/three.py deleted file mode 100644 index fe666b9ff4c8..000000000000 --- a/src/test/pythonFiles/signature/three.py +++ /dev/null @@ -1 +0,0 @@ -print(a, b, z) diff --git a/src/test/signature/signature.test.ts b/src/test/signature/signature.test.ts index 5fa42aee2ea0..b42ecd115b16 100644 --- a/src/test/signature/signature.test.ts +++ b/src/test/signature/signature.test.ts @@ -5,12 +5,12 @@ import * as assert from 'assert'; import * as path from 'path'; import * as vscode from 'vscode'; +import { PythonSettings } from '../../client/common/configSettings'; +import { execPythonFile } from '../../client/common/utils'; +import { rootWorkspaceUri } from '../common'; import { closeActiveWindows, initialize, initializeTest } from '../initialize'; const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'signature'); -const fileOne = path.join(autoCompPath, 'one.py'); -const fileTwo = path.join(autoCompPath, 'two.py'); -const fileThree = path.join(autoCompPath, 'three.py'); class SignatureHelpResult { constructor( @@ -23,8 +23,11 @@ class SignatureHelpResult { // tslint:disable-next-line:max-func-body-length suite('Signatures', () => { + let isPython3: Promise; suiteSetup(async () => { await initialize(); + const version = await execPythonFile(rootWorkspaceUri, PythonSettings.getInstance(rootWorkspaceUri).pythonPath, ['--version'], __dirname, true); + isPython3 = Promise.resolve(version.indexOf('3.') >= 0); }); setup(initializeTest); suiteTeardown(closeActiveWindows); @@ -44,7 +47,7 @@ suite('Signatures', () => { new SignatureHelpResult(5, 20, 0, 0, null) ]; - const document = await openDocument(fileOne); + const document = await openDocument(path.join(autoCompPath, 'classCtor.py')); for (let i = 0; i < expected.length; i += 1) { await checkSignature(expected[i], document!.uri, i); } @@ -67,15 +70,17 @@ suite('Signatures', () => { new SignatureHelpResult(1, 0, 1, 2, 'step') ]; - const document = await openDocument(fileTwo); + const document = await openDocument(path.join(autoCompPath, 'basicSig.py')); for (let i = 0; i < expected.length; i += 1) { await checkSignature(expected[i], document!.uri, i); } }); test('For ellipsis', async () => { + if (!await isPython3) { + return; + } const expected = [ - new SignatureHelpResult(0, 4, 0, 0, null), new SignatureHelpResult(0, 5, 0, 0, null), new SignatureHelpResult(0, 6, 1, 0, 'value'), new SignatureHelpResult(0, 7, 1, 0, 'value'), @@ -86,11 +91,23 @@ suite('Signatures', () => { new SignatureHelpResult(0, 12, 1, 2, 'sep') ]; - const document = await openDocument(fileThree); + const document = await openDocument(path.join(autoCompPath, 'ellipsis.py')); for (let i = 0; i < expected.length; i += 1) { await checkSignature(expected[i], document!.uri, i); } }); + + test('For pow', async () => { + let expected: SignatureHelpResult; + if (await isPython3) { + expected = new SignatureHelpResult(0, 4, 1, 0, null); + } else { + expected = new SignatureHelpResult(0, 4, 1, 0, 'x'); + } + + const document = await openDocument(path.join(autoCompPath, 'noSigPy3.py')); + await checkSignature(expected, document!.uri, 0); + }); }); async function openDocument(documentPath: string): Promise { @@ -105,7 +122,9 @@ async function checkSignature(expected: SignatureHelpResult, uri: vscode.Uri, ca assert.equal(actual!.signatures.length, expected.signaturesCount, `Signature count does not match, case ${caseIndex}`); if (expected.signaturesCount > 0) { assert.equal(actual!.activeParameter, expected.activeParameter, `Parameter index does not match, case ${caseIndex}`); - const parameter = actual!.signatures[0].parameters[expected.activeParameter]; - assert.equal(parameter.label, expected.parameterName, `Parameter name is incorrect, case ${caseIndex}`); + if (expected.parameterName) { + const parameter = actual!.signatures[0].parameters[expected.activeParameter]; + assert.equal(parameter.label, expected.parameterName, `Parameter name is incorrect, case ${caseIndex}`); + } } } From 699c434dc6598cbccfb6bc54886a3a656d44278b Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 20 Dec 2017 15:02:47 -0800 Subject: [PATCH 31/51] Basic services --- .../common/application/applicationShell.ts | 54 +++++ src/client/common/application/types.ts | 198 ++++++++++++++++++ src/client/common/installer/installer.ts | 6 +- src/client/common/platform/constants.ts | 1 + src/client/common/platform/fileSystem.ts | 55 +++++ src/client/common/platform/pathUtils.ts | 1 + src/client/common/platform/platformService.ts | 31 +++ src/client/common/platform/serviceRegistry.ts | 17 ++ src/client/common/platform/types.ts | 19 +- src/client/common/serviceRegistry.ts | 7 +- src/client/extension.ts | 2 + 11 files changed, 383 insertions(+), 8 deletions(-) create mode 100644 src/client/common/application/applicationShell.ts create mode 100644 src/client/common/application/types.ts create mode 100644 src/client/common/platform/fileSystem.ts create mode 100644 src/client/common/platform/platformService.ts create mode 100644 src/client/common/platform/serviceRegistry.ts diff --git a/src/client/common/application/applicationShell.ts b/src/client/common/application/applicationShell.ts new file mode 100644 index 000000000000..c2312326ca97 --- /dev/null +++ b/src/client/common/application/applicationShell.ts @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import { injectable } from 'inversify'; +import * as vscode from 'vscode'; +import { IApplicationShell } from './types'; + +@injectable() +export class ApplicationShell implements IApplicationShell { + public showInformationMessage(message: string, ...items: string[]): Thenable ; + public showInformationMessage(message: string, options: vscode.MessageOptions, ...items: string[]): Thenable ; + public showInformationMessage(message: string, ...items: T[]): Thenable ; + public showInformationMessage(message: string, options: vscode.MessageOptions, ...items: T[]): Thenable ; + // tslint:disable-next-line:no-any + public showInformationMessage(message: string, options?: any, ...items: any[]): Thenable { + return vscode.window.showInformationMessage(message, options, ...items); + } + + public showWarningMessage(message: string, ...items: string[]): Thenable; + public showWarningMessage(message: string, options: vscode.MessageOptions, ...items: string[]): Thenable; + public showWarningMessage(message: string, ...items: T[]): Thenable; + public showWarningMessage(message: string, options: vscode.MessageOptions, ...items: T[]): Thenable; + // tslint:disable-next-line:no-any + public showWarningMessage(message: any, options?: any, ...items: any[]) { + return vscode.window.showWarningMessage(message, options, ...items); + } + + public showErrorMessage(message: string, ...items: string[]): Thenable; + public showErrorMessage(message: string, options: vscode.MessageOptions, ...items: string[]): Thenable; + public showErrorMessage(message: string, ...items: T[]): Thenable; + public showErrorMessage(message: string, options: vscode.MessageOptions, ...items: T[]): Thenable; + // tslint:disable-next-line:no-any + public showErrorMessage(message: any, options?: any, ...items: any[]) { + return vscode.window.showErrorMessage(message, options, ...items); + } + + public showQuickPick(items: string[] | Thenable, options?: vscode.QuickPickOptions, token?: vscode.CancellationToken): Thenable; + public showQuickPick(items: T[] | Thenable, options?: vscode.QuickPickOptions, token?: vscode.CancellationToken): Thenable; + // tslint:disable-next-line:no-any + public showQuickPick(items: any, options?: any, token?: any) { + return vscode.window.showQuickPick(items, options, token); + } + + public showOpenDialog(options: vscode.OpenDialogOptions): Thenable { + return vscode.window.showOpenDialog(options); + } + public showSaveDialog(options: vscode.SaveDialogOptions): Thenable { + return vscode.window.showSaveDialog(options); + } + public showInputBox(options?: vscode.InputBoxOptions, token?: vscode.CancellationToken): Thenable { + return vscode.window.showInputBox(options, token); + } +} diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts new file mode 100644 index 000000000000..e6f64a15af07 --- /dev/null +++ b/src/client/common/application/types.ts @@ -0,0 +1,198 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import * as vscode from 'vscode'; + +export const ITerminalService = Symbol('ITerminalService'); +export interface ITerminalService { + sendCommand(command: string, args: string[]): Promise; +} + +export const ILogger = Symbol('ILogger'); +export interface ILogger { + logError(message: string, error?: Error); + logWarning(message: string, error?: Error); +} + +export const IApplicationShell = Symbol('IApplicationShell'); +export interface IApplicationShell { + showInformationMessage(message: string, ...items: string[]): Thenable; + + /** + * Show an information message to users. Optionally provide an array of items which will be presented as + * clickable buttons. + * + * @param message The message to show. + * @param options Configures the behaviour of the message. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showInformationMessage(message: string, options: vscode.MessageOptions, ...items: string[]): Thenable; + + /** + * Show an information message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showInformationMessage(message: string, ...items: T[]): Thenable; + + /** + * Show an information message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param options Configures the behaviour of the message. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showInformationMessage(message: string, options: vscode.MessageOptions, ...items: T[]): Thenable; + + /** + * Show a warning message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showWarningMessage(message: string, ...items: string[]): Thenable; + + /** + * Show a warning message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param options Configures the behaviour of the message. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showWarningMessage(message: string, options: vscode.MessageOptions, ...items: string[]): Thenable; + + /** + * Show a warning message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showWarningMessage(message: string, ...items: T[]): Thenable; + + /** + * Show a warning message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param options Configures the behaviour of the message. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showWarningMessage(message: string, options: vscode.MessageOptions, ...items: T[]): Thenable; + + /** + * Show an error message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showErrorMessage(message: string, ...items: string[]): Thenable; + + /** + * Show an error message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param options Configures the behaviour of the message. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showErrorMessage(message: string, options: vscode.MessageOptions, ...items: string[]): Thenable; + + /** + * Show an error message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showErrorMessage(message: string, ...items: T[]): Thenable; + + /** + * Show an error message. + * + * @see [showInformationMessage](#window.showInformationMessage) + * + * @param message The message to show. + * @param options Configures the behaviour of the message. + * @param items A set of items that will be rendered as actions in the message. + * @return A thenable that resolves to the selected item or `undefined` when being dismissed. + */ + showErrorMessage(message: string, options: vscode.MessageOptions, ...items: T[]): Thenable; + + /** + * Shows a selection list. + * + * @param items An array of strings, or a promise that resolves to an array of strings. + * @param options Configures the behavior of the selection list. + * @param token A token that can be used to signal cancellation. + * @return A promise that resolves to the selection or `undefined`. + */ + showQuickPick(items: string[] | Thenable, options?: vscode.QuickPickOptions, token?: vscode.CancellationToken): Thenable; + + /** + * Shows a selection list. + * + * @param items An array of items, or a promise that resolves to an array of items. + * @param options Configures the behavior of the selection list. + * @param token A token that can be used to signal cancellation. + * @return A promise that resolves to the selected item or `undefined`. + */ + showQuickPick(items: T[] | Thenable, options?: vscode.QuickPickOptions, token?: vscode.CancellationToken): Thenable; + + /** + * Shows a file open dialog to the user which allows to select a file + * for opening-purposes. + * + * @param options Options that control the dialog. + * @returns A promise that resolves to the selected resources or `undefined`. + */ + showOpenDialog(options: vscode.OpenDialogOptions): Thenable; + + /** + * Shows a file save dialog to the user which allows to select a file + * for saving-purposes. + * + * @param options Options that control the dialog. + * @returns A promise that resolves to the selected resource or `undefined`. + */ + showSaveDialog(options: vscode.SaveDialogOptions): Thenable; + + /** + * Opens an input box to ask the user for input. + * + * The returned value will be `undefined` if the input box was canceled (e.g. pressing ESC). Otherwise the + * returned value will be the string typed by the user or an empty string if the user did not type + * anything but dismissed the input box with OK. + * + * @param options Configures the behavior of the input box. + * @param token A token that can be used to signal cancellation. + * @return A promise that resolves to a string the user provided or to `undefined` in case of dismissal. + */ + showInputBox(options?: vscode.InputBoxOptions, token?: vscode.CancellationToken): Thenable; +} diff --git a/src/client/common/installer/installer.ts b/src/client/common/installer/installer.ts index a78405d0670a..6d4af888abd9 100644 --- a/src/client/common/installer/installer.ts +++ b/src/client/common/installer/installer.ts @@ -10,6 +10,7 @@ import { ILinterHelper } from '../../linters/types'; import { ITestsHelper } from '../../unittests/common/types'; import { PythonSettings } from '../configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; +import { IPlatformService } from '../platform/types'; import { IProcessService, IPythonExecutionFactory } from '../process/types'; import { ITerminalService } from '../terminal/types'; import { IInstaller, ILogger, InstallerResponse, IOutputChannel, IsWindows, ModuleNamePurpose, Product } from '../types'; @@ -83,8 +84,7 @@ ProductTypes.set(Product.rope, ProductType.RefactoringLibrary); @injectable() export class Installer implements IInstaller { constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel: vscode.OutputChannel, - @inject(IsWindows) private isWindows: boolean) { + @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel: vscode.OutputChannel) { } // tslint:disable-next-line:no-empty public dispose() { } @@ -230,7 +230,7 @@ export class Installer implements IInstaller { return disablePromptForFeatures.indexOf(productName) === -1; } private installCTags() { - if (this.isWindows) { + if (this.serviceContainer.get(IPlatformService).isWindows) { this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols'); this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.'); this.outputChannel.appendLine('Option 1: Extract ctags.exe from the downloaded zip to any folder within your PATH so that Visual Studio Code can run it.'); diff --git a/src/client/common/platform/constants.ts b/src/client/common/platform/constants.ts index 26b3800134b6..3109c18110c5 100644 --- a/src/client/common/platform/constants.ts +++ b/src/client/common/platform/constants.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as arch from 'arch'; +// TO DO: Deprecate in favor of IPlatformService export const WINDOWS_PATH_VARIABLE_NAME = 'Path'; export const NON_WINDOWS_PATH_VARIABLE_NAME = 'PATH'; export const IS_WINDOWS = /^win/.test(process.platform); diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts new file mode 100644 index 000000000000..bcd9c17b095c --- /dev/null +++ b/src/client/common/platform/fileSystem.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import * as fs from 'fs'; +import { inject } from 'inversify'; +import * as path from 'path'; +import { IServiceContainer } from '../../ioc/types'; +import { IFileSystem, IPlatformService } from './types'; + +export class FileSystem implements IFileSystem { + + constructor(@inject(IServiceContainer) private platformService: IPlatformService) {} + + public get directorySeparatorChar(): string { + return this.platformService.isWindows ? '\\' : '/'; + } + + public existsAsync(filePath: string): Promise { + return new Promise(resolve => { + fs.exists(filePath, exists => { + return resolve(exists); + }); + }); + } + + public createDirectoryAsync(directoryPath: string): Promise { + return new Promise(resolve => { + fs.mkdir(directoryPath, error => { + return resolve(!error); + }); + }); + } + + public getSubDirectoriesAsync(rootDir: string): Promise { + return new Promise(resolve => { + fs.readdir(rootDir, (error, files) => { + if (error) { + return resolve([]); + } + const subDirs = []; + files.forEach(name => { + const fullPath = path.join(rootDir, name); + try { + if (fs.statSync(fullPath).isDirectory()) { + subDirs.push(fullPath); + } + // tslint:disable-next-line:no-empty + } catch (ex) {} + }); + resolve(subDirs); + }); + }); + } +} diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index b9f1779d8c63..b45c21a8bec8 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -3,6 +3,7 @@ import 'reflect-metadata'; import { IPathUtils, IsWindows } from '../types'; import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants'; +// TO DO: Deprecate in favor of IPlatformService @injectable() export class PathUtils implements IPathUtils { constructor( @inject(IsWindows) private isWindows: boolean) { } diff --git a/src/client/common/platform/platformService.ts b/src/client/common/platform/platformService.ts new file mode 100644 index 000000000000..8afc2588bd17 --- /dev/null +++ b/src/client/common/platform/platformService.ts @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import { arch } from 'os'; +import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants'; +import { IPlatformService } from './types'; + +export class PlatformService implements IPlatformService { + private _isWindows: boolean; + private _isMac: boolean; + + constructor() { + this._isWindows = /^win/.test(process.platform); + this._isMac = /^darwin/.test(process.platform); + } + public get isWindows(): boolean { + return this._isWindows; + } + public get isMac(): boolean { + return this._isMac; + } + public get isLinux(): boolean { + return !(this.isWindows || this.isLinux); + } + public get is64bit(): boolean { + return arch() === 'x64'; + } + public get pathVariableName() { + return this.isWindows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME; + }} diff --git a/src/client/common/platform/serviceRegistry.ts b/src/client/common/platform/serviceRegistry.ts new file mode 100644 index 000000000000..dce3511f9f37 --- /dev/null +++ b/src/client/common/platform/serviceRegistry.ts @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import { IServiceManager } from '../../ioc/types'; +import { FileSystem } from './fileSystem'; +import { PlatformService } from './platformService'; +import { RegistryImplementation } from './registry'; +import { IFileSystem, IPlatformService, IRegistry } from './types'; + +export function registerTypes(serviceManager: IServiceManager) { + serviceManager.addSingleton(IPlatformService, PlatformService); + serviceManager.addSingleton(IFileSystem, FileSystem); + if (serviceManager.get(IPlatformService).isWindows) { + serviceManager.addSingleton(IRegistry, RegistryImplementation); + } +} diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index b84cc7d9727f..a676cca89ff8 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -11,8 +11,25 @@ export enum RegistryHive { } export const IRegistry = Symbol('IRegistry'); - export interface IRegistry { getKeys(key: string, hive: RegistryHive, arch?: Architecture): Promise; getValue(key: string, hive: RegistryHive, arch?: Architecture, name?: string): Promise; } + +export const IPlatformService = Symbol('IPlatformService'); +export interface IPlatformService { + isWindows: boolean; + isMac: boolean; + isLinux: boolean; + is64bit: boolean; + pathVariableName: 'Path' | 'PATH'; +} + +export const IFileSystem = Symbol('IFileSystem'); +export interface IFileSystem { + directorySeparatorChar: string; + existsAsync(path: string): Promise; + createDirectoryAsync(path: string): Promise; + getSubDirectoriesAsync(rootDir: string): Promise; + arePathsSame(path1: string, path2: string): boolean; +} diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 3368f4df0ff7..2f70337f9b88 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -3,6 +3,8 @@ import 'reflect-metadata'; import { IServiceManager } from '../ioc/types'; +import { ApplicationShell } from './application/applicationShell'; +import { IApplicationShell } from './application/types'; import { CondaInstaller } from './installer/condaInstaller'; import { Installer } from './installer/installer'; import { PipInstaller } from './installer/pipInstaller'; @@ -28,8 +30,5 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ILogger, Logger); serviceManager.addSingleton(ITerminalService, TerminalService); serviceManager.addSingleton(IPathUtils, PathUtils); - - if (IS_WINDOWS) { - serviceManager.addSingleton(IRegistry, RegistryImplementation); - } + serviceManager.addSingleton(IApplicationShell, ApplicationShell); } diff --git a/src/client/extension.ts b/src/client/extension.ts index 80714ecf4713..a5a2014574bd 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -8,6 +8,7 @@ import * as settings from './common/configSettings'; import { STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { FeatureDeprecationManager } from './common/featureDeprecationManager'; import { createDeferred } from './common/helpers'; +import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry'; import { registerTypes as commonRegisterTypes } from './common/serviceRegistry'; import { GLOBAL_MEMENTO, IDisposableRegistry, ILogger, IMemento, IOutputChannel, IPersistentStateFactory, WORKSPACE_MEMENTO } from './common/types'; @@ -76,6 +77,7 @@ export async function activate(context: vscode.ExtensionContext) { lintersRegisterTypes(serviceManager); interpretersRegisterTypes(serviceManager); formattersRegisterTypes(serviceManager); + platformRegisterTypes(serviceManager); const persistentStateFactory = serviceManager.get(IPersistentStateFactory); const pythonSettings = settings.PythonSettings.getInstance(); From dd9ba0a44ca25dcf5069ae4ab7e67c4934d83995 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 20 Dec 2017 19:40:23 -0800 Subject: [PATCH 32/51] Install check --- src/client/common/application/types.ts | 11 ---- .../common/installer/pythonInstallation.ts | 58 +++++++++++++++++++ .../common/installer/serviceRegistry.ts | 16 +++++ src/client/common/installer/types.ts | 6 +- src/client/common/platform/fileSystem.ts | 18 ++++-- src/client/common/platform/platformService.ts | 2 + src/client/common/serviceRegistry.ts | 10 +--- src/client/extension.ts | 5 ++ 8 files changed, 102 insertions(+), 24 deletions(-) create mode 100644 src/client/common/installer/pythonInstallation.ts create mode 100644 src/client/common/installer/serviceRegistry.ts diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index e6f64a15af07..30d6085e956b 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -4,17 +4,6 @@ import * as vscode from 'vscode'; -export const ITerminalService = Symbol('ITerminalService'); -export interface ITerminalService { - sendCommand(command: string, args: string[]): Promise; -} - -export const ILogger = Symbol('ILogger'); -export interface ILogger { - logError(message: string, error?: Error); - logWarning(message: string, error?: Error); -} - export const IApplicationShell = Symbol('IApplicationShell'); export interface IApplicationShell { showInformationMessage(message: string, ...items: string[]): Thenable; diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts new file mode 100644 index 000000000000..81a516444bf4 --- /dev/null +++ b/src/client/common/installer/pythonInstallation.ts @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import { inject } from 'inversify'; +import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE } from '../../interpreter/contracts'; +import { IServiceContainer } from '../../ioc/types'; +import { IApplicationShell } from '../application/types'; +import { IFileSystem, IPlatformService } from '../platform/types'; +import { IProcessService, IPythonExecutionService } from '../process/types'; +import { ITerminalService } from '../terminal/types'; +import { IPythonInstallation } from './types'; + +export async function checkPythonInstallation(serviceContainer: IServiceContainer, interpreterManager): Promise { + const shell = serviceContainer.get(IApplicationShell); + const process = serviceContainer.get(IProcessService); + const platform = serviceContainer.get(IPlatformService); + const fs = serviceContainer.get(IFileSystem); + const locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); + + let interpreters = await locator.getInterpreters(); + if (interpreters.length > 0) { + return true; + } + + if (platform.isWindows) { + await shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.'); + await process.exec('https://www.python.org/downloads', [], {}); + return false; + } + + if (platform.isMac) { + if (await shell.showErrorMessage('Python that comes with Mac OS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') { + const terminal = serviceContainer.get(ITerminalService); + const brewInstalled = await ensureBrew(fs, terminal); + if (!brewInstalled) { + await shell.showErrorMessage('Unable to install Brew package manager'); + return false; + } + await terminal.sendCommand('brew', ['python']); + interpreterManager.refresh(); + } + } + + interpreters = await locator.getInterpreters(); + return interpreters.length > 0; +} + +function isBrewInstalled(fs: IFileSystem): Promise { + return fs.existsAsync('/usr/local/bin/brew'); +} + +async function ensureBrew(fs: IFileSystem, terminal: ITerminalService) { + if (!isBrewInstalled(fs)) { + await terminal.sendCommand('/usr/bin/ruby', ['-e', '$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)']); + } + return isBrewInstalled(fs); +} diff --git a/src/client/common/installer/serviceRegistry.ts b/src/client/common/installer/serviceRegistry.ts new file mode 100644 index 000000000000..85d73db373c6 --- /dev/null +++ b/src/client/common/installer/serviceRegistry.ts @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import { IServiceManager } from '../../ioc/types'; +import { IInstaller } from '../types'; +import { CondaInstaller } from './condaInstaller'; +import { Installer } from './installer'; +import { PipInstaller } from './pipInstaller'; +import { IModuleInstaller, IPythonInstallation } from './types'; + +export function registerTypes(serviceManager: IServiceManager) { + serviceManager.addSingleton(IInstaller, Installer); + serviceManager.addSingleton(IModuleInstaller, CondaInstaller); + serviceManager.addSingleton(IModuleInstaller, PipInstaller); +} diff --git a/src/client/common/installer/types.ts b/src/client/common/installer/types.ts index 0d95992254fa..a1759606de8b 100644 --- a/src/client/common/installer/types.ts +++ b/src/client/common/installer/types.ts @@ -4,9 +4,13 @@ import { Uri } from 'vscode'; export const IModuleInstaller = Symbol('IModuleInstaller'); - export interface IModuleInstaller { readonly displayName: string; installModule(name: string): Promise; isSupported(resource?: Uri): Promise; } + +export const IPythonInstallation = Symbol('IPythonInstallation'); +export interface IPythonInstallation { + checkInstallation(): Promise; +} diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index bcd9c17b095c..1ba4d705cb64 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -3,14 +3,14 @@ 'use strict'; import * as fs from 'fs'; -import { inject } from 'inversify'; +import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IServiceContainer } from '../../ioc/types'; import { IFileSystem, IPlatformService } from './types'; +@injectable() export class FileSystem implements IFileSystem { - - constructor(@inject(IServiceContainer) private platformService: IPlatformService) {} + constructor( @inject(IServiceContainer) private platformService: IPlatformService) { } public get directorySeparatorChar(): string { return this.platformService.isWindows ? '\\' : '/'; @@ -46,10 +46,20 @@ export class FileSystem implements IFileSystem { subDirs.push(fullPath); } // tslint:disable-next-line:no-empty - } catch (ex) {} + } catch (ex) { } }); resolve(subDirs); }); }); } + + public arePathsSame(path1: string, path2: string): boolean { + path1 = path.normalize(path1); + path2 = path.normalize(path2); + if (this.platformService.isWindows) { + return path1.toUpperCase() === path2.toUpperCase(); + } else { + return path1 === path2; + } + } } diff --git a/src/client/common/platform/platformService.ts b/src/client/common/platform/platformService.ts index 8afc2588bd17..2264513991ac 100644 --- a/src/client/common/platform/platformService.ts +++ b/src/client/common/platform/platformService.ts @@ -2,10 +2,12 @@ // Licensed under the MIT License. 'use strict'; +import { injectable } from 'inversify'; import { arch } from 'os'; import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants'; import { IPlatformService } from './types'; +@injectable() export class PlatformService implements IPlatformService { private _isWindows: boolean; private _isMac: boolean; diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 2f70337f9b88..6ef15161d3be 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -5,10 +5,6 @@ import 'reflect-metadata'; import { IServiceManager } from '../ioc/types'; import { ApplicationShell } from './application/applicationShell'; import { IApplicationShell } from './application/types'; -import { CondaInstaller } from './installer/condaInstaller'; -import { Installer } from './installer/installer'; -import { PipInstaller } from './installer/pipInstaller'; -import { IModuleInstaller } from './installer/types'; import { Logger } from './logger'; import { PersistentStateFactory } from './persistentState'; import { IS_64_BIT, IS_WINDOWS } from './platform/constants'; @@ -17,18 +13,16 @@ import { RegistryImplementation } from './platform/registry'; import { IRegistry } from './platform/types'; import { TerminalService } from './terminal/service'; import { ITerminalService } from './terminal/types'; -import { IInstaller, ILogger, IPathUtils, IPersistentStateFactory, Is64Bit, IsWindows } from './types'; +import { ILogger, IPathUtils, IPersistentStateFactory, Is64Bit, IsWindows } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingletonInstance(IsWindows, IS_WINDOWS); serviceManager.addSingletonInstance(Is64Bit, IS_64_BIT); serviceManager.addSingleton(IPersistentStateFactory, PersistentStateFactory); - serviceManager.addSingleton(IInstaller, Installer); - serviceManager.addSingleton(IModuleInstaller, CondaInstaller); - serviceManager.addSingleton(IModuleInstaller, PipInstaller); serviceManager.addSingleton(ILogger, Logger); serviceManager.addSingleton(ITerminalService, TerminalService); serviceManager.addSingleton(IPathUtils, PathUtils); serviceManager.addSingleton(IApplicationShell, ApplicationShell); + } diff --git a/src/client/extension.ts b/src/client/extension.ts index a5a2014574bd..61c96644ad9b 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -8,6 +8,9 @@ import * as settings from './common/configSettings'; import { STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { FeatureDeprecationManager } from './common/featureDeprecationManager'; import { createDeferred } from './common/helpers'; +import { checkPythonInstallation } from './common/installer/pythonInstallation'; +import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; +import { IPythonInstallation } from './common/installer/types'; import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry'; import { registerTypes as commonRegisterTypes } from './common/serviceRegistry'; @@ -78,6 +81,7 @@ export async function activate(context: vscode.ExtensionContext) { interpretersRegisterTypes(serviceManager); formattersRegisterTypes(serviceManager); platformRegisterTypes(serviceManager); + installerRegisterTypes(serviceManager); const persistentStateFactory = serviceManager.get(IPersistentStateFactory); const pythonSettings = settings.PythonSettings.getInstance(); @@ -85,6 +89,7 @@ export async function activate(context: vscode.ExtensionContext) { sortImports.activate(context, standardOutputChannel); const interpreterManager = new InterpreterManager(serviceContainer); + const passed = await checkPythonInstallation(serviceContainer, interpreterManager); // This must be completed before we can continue. await interpreterManager.autoSetInterpreter(); From 443ad65b7b9d7424818a2b79f33523a206eab574 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 21 Dec 2017 12:39:04 -0800 Subject: [PATCH 33/51] Output installer messages --- package.json | 1 + .../common/installer/pythonInstallation.ts | 109 ++++++++++++------ src/client/common/terminal/service.ts | 7 +- src/client/extension.ts | 7 +- src/client/interpreter/index.ts | 5 +- src/client/interpreter/locators/index.ts | 14 ++- 6 files changed, 98 insertions(+), 45 deletions(-) diff --git a/package.json b/package.json index a5850541fe92..fd158a1eb9d1 100644 --- a/package.json +++ b/package.json @@ -1533,6 +1533,7 @@ "lodash": "^4.17.4", "minimatch": "^3.0.3", "named-js-regexp": "^1.3.1", + "opn": "^5.1.0", "reflect-metadata": "^0.1.10", "rxjs": "^5.5.2", "semver": "^5.4.1", diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index 81a516444bf4..d400f6b31197 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -2,57 +2,98 @@ // Licensed under the MIT License. 'use strict'; +// tslint:disable-next-line:no-require-imports no-var-requires +const opn = require('opn'); + import { inject } from 'inversify'; +import { OutputChannel } from 'vscode'; import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { IApplicationShell } from '../application/types'; +import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IFileSystem, IPlatformService } from '../platform/types'; import { IProcessService, IPythonExecutionService } from '../process/types'; -import { ITerminalService } from '../terminal/types'; +import { IOutputChannel } from '../types'; import { IPythonInstallation } from './types'; -export async function checkPythonInstallation(serviceContainer: IServiceContainer, interpreterManager): Promise { - const shell = serviceContainer.get(IApplicationShell); - const process = serviceContainer.get(IProcessService); - const platform = serviceContainer.get(IPlatformService); - const fs = serviceContainer.get(IFileSystem); - const locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); +export class PythonInstaller { + private locator: IInterpreterLocatorService; + private platform: IPlatformService; + private process: IProcessService; + private fs: IFileSystem; + private outputChannel: OutputChannel; + private shell: IApplicationShell; - let interpreters = await locator.getInterpreters(); - if (interpreters.length > 0) { - return true; + constructor(private serviceContainer: IServiceContainer) { + this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); } - if (platform.isWindows) { - await shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.'); - await process.exec('https://www.python.org/downloads', [], {}); - return false; - } + public async checkPythonInstallation(): Promise { + let interpreters = await this.locator.getInterpreters(); + if (interpreters.length > 0) { + return true; + } + + this.platform = this.serviceContainer.get(IPlatformService); + this.process = this.serviceContainer.get(IProcessService); + this.fs = this.serviceContainer.get(IFileSystem); + this.outputChannel = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + this.shell = this.serviceContainer.get(IApplicationShell); - if (platform.isMac) { - if (await shell.showErrorMessage('Python that comes with Mac OS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') { - const terminal = serviceContainer.get(ITerminalService); - const brewInstalled = await ensureBrew(fs, terminal); - if (!brewInstalled) { - await shell.showErrorMessage('Unable to install Brew package manager'); - return false; + if (this.platform.isWindows) { + await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.'); + opn('https://www.python.org/downloads'); + return false; + } + + if (this.platform.isMac) { + if (await this.shell.showErrorMessage('Python that comes with Mac OS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') { + const brewInstalled = await this.ensureBrew(); + if (!brewInstalled) { + await this.shell.showErrorMessage('Unable to install Brew package manager'); + return false; + } + await this.executeAndOutput('brew', ['install', 'python']); } - await terminal.sendCommand('brew', ['python']); - interpreterManager.refresh(); } + + interpreters = await this.locator.getInterpreters(); + return interpreters.length > 0; } - interpreters = await locator.getInterpreters(); - return interpreters.length > 0; -} + private isBrewInstalled(): Promise { + return this.fs.existsAsync('/usr/local/bin/brew'); + } -function isBrewInstalled(fs: IFileSystem): Promise { - return fs.existsAsync('/usr/local/bin/brew'); -} + private async ensureBrew(): Promise { + if (await this.isBrewInstalled()) { + return true; + } + const result = await this.executeAndOutput( + '/usr/bin/ruby', + ['-e', '"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"']); + return result && await this.isBrewInstalled(); + } -async function ensureBrew(fs: IFileSystem, terminal: ITerminalService) { - if (!isBrewInstalled(fs)) { - await terminal.sendCommand('/usr/bin/ruby', ['-e', '$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)']); + private executeAndOutput(command: string, args: string[]): Promise { + let failed = false; + this.outputChannel.show(); + + const result = this.process.execObservable(command, args, { mergeStdOutErr: true, throwOnStdErr: false }); + result.out.subscribe(output => { + this.outputChannel.append(output.out); + }, error => { + failed = true; + this.shell.showErrorMessage(`Unable to execute '${command}', error: ${error}`); + }); + + return new Promise((resolve, reject) => { + if (failed) { + resolve(false); + } + result.proc.on('exit', (code, signal) => { + resolve(!signal); + }); + }); } - return isBrewInstalled(fs); } diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 116632d6b8ec..074557d0301a 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -5,7 +5,8 @@ import { inject, injectable } from 'inversify'; import 'reflect-metadata'; import { Disposable, Terminal, Uri, window, workspace } from 'vscode'; import { IServiceContainer } from '../../ioc/types'; -import { IDisposableRegistry, IsWindows } from '../types'; +import { IPlatformService } from '../platform/types'; +import { IDisposableRegistry } from '../types'; import { ITerminalService } from './types'; const IS_POWERSHELL = /powershell.exe$/i; @@ -56,8 +57,8 @@ export class TerminalService implements ITerminalService { } private terminalIsPowershell(resource?: Uri) { - const isWindows = this.serviceContainer.get(IsWindows); - if (!isWindows) { + const platform = this.serviceContainer.get(IPlatformService); + if (!platform.isWindows) { return false; } // tslint:disable-next-line:no-backbone-get-set-outside-model diff --git a/src/client/extension.ts b/src/client/extension.ts index 61c96644ad9b..6b88a6f26d17 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -8,7 +8,7 @@ import * as settings from './common/configSettings'; import { STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { FeatureDeprecationManager } from './common/featureDeprecationManager'; import { createDeferred } from './common/helpers'; -import { checkPythonInstallation } from './common/installer/pythonInstallation'; +import { PythonInstaller } from './common/installer/pythonInstallation'; import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; import { IPythonInstallation } from './common/installer/types'; import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; @@ -89,7 +89,10 @@ export async function activate(context: vscode.ExtensionContext) { sortImports.activate(context, standardOutputChannel); const interpreterManager = new InterpreterManager(serviceContainer); - const passed = await checkPythonInstallation(serviceContainer, interpreterManager); + + const pythonInstaller = new PythonInstaller(serviceContainer); + const passed = await pythonInstaller.checkPythonInstallation(); + // This must be completed before we can continue. await interpreterManager.autoSetInterpreter(); diff --git a/src/client/interpreter/index.ts b/src/client/interpreter/index.ts index c346dc1dfee7..2240322c4265 100644 --- a/src/client/interpreter/index.ts +++ b/src/client/interpreter/index.ts @@ -47,16 +47,17 @@ export class InterpreterManager implements Disposable { const virtualEnvInterpreterProvider = new VirtualEnvService([activeWorkspace.folderUri.fsPath], virtualEnvMgr, versionService); const interpreters = await virtualEnvInterpreterProvider.getInterpreters(activeWorkspace.folderUri); const workspacePathUpper = activeWorkspace.folderUri.fsPath.toUpperCase(); + const interpretersInWorkspace = interpreters.filter(interpreter => interpreter.path.toUpperCase().startsWith(workspacePathUpper)); - // Always pick the first available one. if (interpretersInWorkspace.length === 0) { return; } + // Always pick the highest version by default. // Ensure this new environment is at the same level as the current workspace. // In windows the interpreter is under scripts/python.exe on linux it is under bin/python. // Meaning the sub directory must be either scripts, bin or other (but only one level deep). - const pythonPath = interpretersInWorkspace[0].path; + const pythonPath = interpretersInWorkspace.sort((a, b) => a.version > b.version ? 1 : -1)[0].path; const relativePath = path.dirname(pythonPath).substring(activeWorkspace.folderUri.fsPath.length); if (relativePath.split(path.sep).filter(l => l.length > 0).length === 2) { await this.pythonPathUpdaterService.updatePythonPath(pythonPath, activeWorkspace.configTarget, 'load', activeWorkspace.folderUri); diff --git a/src/client/interpreter/locators/index.ts b/src/client/interpreter/locators/index.ts index de90e042a01f..f742647bfdb0 100644 --- a/src/client/interpreter/locators/index.ts +++ b/src/client/interpreter/locators/index.ts @@ -2,6 +2,7 @@ import { inject, injectable } from 'inversify'; import * as _ from 'lodash'; import * as path from 'path'; import { Disposable, Uri, workspace } from 'vscode'; +import { IPlatformService } from '../../common/platform/types'; import { IDisposableRegistry, IsWindows } from '../../common/types'; import { arePathsSame } from '../../common/utils'; import { IServiceContainer } from '../../ioc/types'; @@ -22,11 +23,13 @@ import { fixInterpreterDisplayName } from './helpers'; export class PythonInterpreterLocatorService implements IInterpreterLocatorService { private interpretersPerResource: Map>; private disposables: Disposable[] = []; - constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IsWindows) private isWindows: boolean) { + private platform: IPlatformService; + + constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.interpretersPerResource = new Map>(); this.disposables.push(workspace.onDidChangeConfiguration(this.onConfigChanged, this)); serviceContainer.get(IDisposableRegistry).push(this); + this.platform = serviceContainer.get(IPlatformService); } public async getInterpreters(resource?: Uri) { const resourceKey = this.getResourceKey(resource); @@ -59,6 +62,9 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi .map(fixInterpreterDisplayName) .map(item => { item.path = path.normalize(item.path); return item; }) .reduce((accumulator, current) => { + if (this.platform.isMac && current.path === '/usr/bin/python') { + return accumulator; + } const existingItem = accumulator.find(item => arePathsSame(item.path, current.path)); if (!existingItem) { accumulator.push(current); @@ -74,14 +80,14 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi private getLocators(resource?: Uri) { const locators: IInterpreterLocatorService[] = []; // The order of the services is important. - if (this.isWindows) { + if (this.platform.isWindows) { locators.push(this.serviceContainer.get(IInterpreterLocatorService, WINDOWS_REGISTRY_SERVICE)); } locators.push(this.serviceContainer.get(IInterpreterLocatorService, CONDA_ENV_SERVICE)); locators.push(this.serviceContainer.get(IInterpreterLocatorService, CONDA_ENV_FILE_SERVICE)); locators.push(this.serviceContainer.get(IInterpreterLocatorService, VIRTUAL_ENV_SERVICE)); - if (!this.isWindows) { + if (!this.platform.isWindows) { locators.push(this.serviceContainer.get(IInterpreterLocatorService, KNOWN_PATH_SERVICE)); } locators.push(this.serviceContainer.get(IInterpreterLocatorService, CURRENT_PATH_SERVICE)); From fdde6b8ca3f7d546846825d53caebdd923c8da19 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 23 Dec 2017 09:44:39 -0800 Subject: [PATCH 34/51] Warn default Mac OS interpreter --- .../common/installer/pythonInstallation.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index d400f6b31197..a74b0c3a8938 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -10,6 +10,7 @@ import { OutputChannel } from 'vscode'; import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { IApplicationShell } from '../application/types'; +import { PythonSettings } from '../configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IFileSystem, IPlatformService } from '../platform/types'; import { IProcessService, IPythonExecutionService } from '../process/types'; @@ -18,27 +19,28 @@ import { IPythonInstallation } from './types'; export class PythonInstaller { private locator: IInterpreterLocatorService; - private platform: IPlatformService; private process: IProcessService; private fs: IFileSystem; private outputChannel: OutputChannel; - private shell: IApplicationShell; + private _platform: IPlatformService; + private _shell: IApplicationShell; constructor(private serviceContainer: IServiceContainer) { this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); - } + } public async checkPythonInstallation(): Promise { let interpreters = await this.locator.getInterpreters(); if (interpreters.length > 0) { + if (this.platform.isMac && PythonSettings.getInstance().pythonPath === 'python') { + await this.shell.showWarningMessage('Selected interpreter is Mac OS system Python which is not recommended. Please select different interpreter'); + } return true; } - this.platform = this.serviceContainer.get(IPlatformService); this.process = this.serviceContainer.get(IProcessService); this.fs = this.serviceContainer.get(IFileSystem); this.outputChannel = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - this.shell = this.serviceContainer.get(IApplicationShell); if (this.platform.isWindows) { await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.'); @@ -96,4 +98,18 @@ export class PythonInstaller { }); }); } + + private get shell(): IApplicationShell { + if (!this._shell) { + this._shell = this.serviceContainer.get(IApplicationShell); + } + return this._shell; + } + + private get platform(): IPlatformService { + if (!this._platform) { + this._platform = this.serviceContainer.get(IPlatformService); + } + return this._platform; + } } From 8b7c920bea17db122e7d95f2c3dfb6c2eec375d9 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 2 Jan 2018 09:24:57 -0800 Subject: [PATCH 35/51] Remove test change --- src/test/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/index.ts b/src/test/index.ts index 300a874076fe..b4c43bfc6c0b 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -13,8 +13,7 @@ const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, timeout: 25000, - retries: 3, - grep: 'Signatures' + retries: 3 }; testRunner.configure(options); module.exports = testRunner; From 253df9e71558925813f31d9f131e9c1117e05861 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 2 Jan 2018 15:17:42 -0800 Subject: [PATCH 36/51] Add tests --- package.json | 1 + .../common/application/applicationShell.ts | 6 + src/client/common/application/types.ts | 7 + .../common/installer/pythonInstallation.ts | 26 +-- src/client/extension.ts | 3 +- src/test/index.ts | 3 +- src/test/install/pythonInstallation.test.ts | 190 ++++++++++++++++++ 7 files changed, 221 insertions(+), 15 deletions(-) create mode 100644 src/test/install/pythonInstallation.test.ts diff --git a/package.json b/package.json index fd158a1eb9d1..062e8fcdd5ea 100644 --- a/package.json +++ b/package.json @@ -1583,6 +1583,7 @@ "tslint": "^5.7.0", "tslint-eslint-rules": "^4.1.1", "tslint-microsoft-contrib": "^5.0.1", + "typemoq": "^2.1.0", "typescript": "^2.6.2", "typescript-formatter": "^6.0.0", "vscode": "^1.1.5", diff --git a/src/client/common/application/applicationShell.ts b/src/client/common/application/applicationShell.ts index c2312326ca97..d905d1085741 100644 --- a/src/client/common/application/applicationShell.ts +++ b/src/client/common/application/applicationShell.ts @@ -2,6 +2,9 @@ // Licensed under the MIT License. 'use strict'; +// tslint:disable-next-line:no-require-imports no-var-requires +const opn = require('opn'); + import { injectable } from 'inversify'; import * as vscode from 'vscode'; import { IApplicationShell } from './types'; @@ -51,4 +54,7 @@ export class ApplicationShell implements IApplicationShell { public showInputBox(options?: vscode.InputBoxOptions, token?: vscode.CancellationToken): Thenable { return vscode.window.showInputBox(options, token); } + public openUrl(url: string): void { + opn(url); + } } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index 30d6085e956b..4977256d0af3 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -184,4 +184,11 @@ export interface IApplicationShell { * @return A promise that resolves to a string the user provided or to `undefined` in case of dismissal. */ showInputBox(options?: vscode.InputBoxOptions, token?: vscode.CancellationToken): Thenable; + + /** + * Opens URL in a default browser. + * + * @param url Url to open. + */ + openUrl(url: string): void; } diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index a74b0c3a8938..ffdb19cb64a3 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -2,15 +2,12 @@ // Licensed under the MIT License. 'use strict'; -// tslint:disable-next-line:no-require-imports no-var-requires -const opn = require('opn'); - import { inject } from 'inversify'; import { OutputChannel } from 'vscode'; import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { IApplicationShell } from '../application/types'; -import { PythonSettings } from '../configSettings'; +import { IPythonSettings, isTestExecution } from '../configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IFileSystem, IPlatformService } from '../platform/types'; import { IProcessService, IPythonExecutionService } from '../process/types'; @@ -29,27 +26,27 @@ export class PythonInstaller { this.locator = serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); } - public async checkPythonInstallation(): Promise { + public async checkPythonInstallation(settings: IPythonSettings): Promise { let interpreters = await this.locator.getInterpreters(); if (interpreters.length > 0) { - if (this.platform.isMac && PythonSettings.getInstance().pythonPath === 'python') { - await this.shell.showWarningMessage('Selected interpreter is Mac OS system Python which is not recommended. Please select different interpreter'); + if (this.platform.isMac && settings.pythonPath === 'python') { + await this.shell.showWarningMessage('Selected interpreter is MacOS system Python which is not recommended. Please select different interpreter'); } return true; } - this.process = this.serviceContainer.get(IProcessService); - this.fs = this.serviceContainer.get(IFileSystem); - this.outputChannel = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - if (this.platform.isWindows) { await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.'); - opn('https://www.python.org/downloads'); + this.shell.openUrl('https://www.python.org/downloads'); return false; } + this.process = this.serviceContainer.get(IProcessService); + this.fs = this.serviceContainer.get(IFileSystem); + this.outputChannel = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + if (this.platform.isMac) { - if (await this.shell.showErrorMessage('Python that comes with Mac OS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') { + if (await this.shell.showErrorMessage('Python that comes with MacOS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') { const brewInstalled = await this.ensureBrew(); if (!brewInstalled) { await this.shell.showErrorMessage('Unable to install Brew package manager'); @@ -93,6 +90,9 @@ export class PythonInstaller { if (failed) { resolve(false); } + if (isTestExecution()) { + resolve(true); + } result.proc.on('exit', (code, signal) => { resolve(!signal); }); diff --git a/src/client/extension.ts b/src/client/extension.ts index 31a69b0a0a53..8b5f68f7391d 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -10,6 +10,7 @@ import * as vscode from 'vscode'; import { Disposable, Memento, OutputChannel, window } from 'vscode'; import { BannerService } from './banner'; import * as settings from './common/configSettings'; +import { PythonSettings } from './common/configSettings'; import { STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { FeatureDeprecationManager } from './common/featureDeprecationManager'; import { createDeferred } from './common/helpers'; @@ -96,7 +97,7 @@ export async function activate(context: vscode.ExtensionContext) { const interpreterManager = new InterpreterManager(serviceContainer); const pythonInstaller = new PythonInstaller(serviceContainer); - const passed = await pythonInstaller.checkPythonInstallation(); + const passed = await pythonInstaller.checkPythonInstallation(PythonSettings.getInstance()); // This must be completed before we can continue. await interpreterManager.autoSetInterpreter(); diff --git a/src/test/index.ts b/src/test/index.ts index b4c43bfc6c0b..aa86c0587ef1 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -13,7 +13,8 @@ const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, timeout: 25000, - retries: 3 + retries: 3, + grep: 'Installation' }; testRunner.configure(options); module.exports = testRunner; diff --git a/src/test/install/pythonInstallation.test.ts b/src/test/install/pythonInstallation.test.ts new file mode 100644 index 000000000000..a1882864817d --- /dev/null +++ b/src/test/install/pythonInstallation.test.ts @@ -0,0 +1,190 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import * as assert from 'assert'; +import { ChildProcess, SpawnOptions } from 'child_process'; +import { Container, interfaces } from 'inversify'; +import * as Rx from 'rxjs'; +import * as TypeMoq from 'typemoq'; +import * as vscode from 'vscode'; +import { IApplicationShell } from '../../client/common/application/types'; +import { IPythonSettings, PythonSettings } from '../../client/common/configSettings'; +import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants'; +import { PythonInstaller } from '../../client/common/installer/pythonInstallation'; +import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; +import { IProcessService, ObservableExecutionResult, Output } from '../../client/common/process/types'; +import { IOutputChannel } from '../../client/common/types'; +import { IInterpreterLocatorService } from '../../client/interpreter/contracts'; +import { InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts'; +import { PythonInterpreterLocatorService } from '../../client/interpreter/locators/index'; +import { ServiceContainer } from '../../client/ioc/container'; +import { ServiceManager } from '../../client/ioc/serviceManager'; +import { IServiceContainer } from '../../client/ioc/types'; +import { closeActiveWindows, initialize, initializeTest } from '../initialize'; + +class TestContext { + public serviceManager: ServiceManager; + public serviceContainer: IServiceContainer; + public platform: TypeMoq.IMock; + public fileSystem: TypeMoq.IMock; + public appShell: TypeMoq.IMock; + public locator: TypeMoq.IMock; + public settings: TypeMoq.IMock; + public process: TypeMoq.IMock; + public output: TypeMoq.IMock; + public pythonInstaller: PythonInstaller; + + constructor(isMac: boolean) { + const cont = new Container(); + this.serviceManager = new ServiceManager(cont); + this.serviceContainer = new ServiceContainer(cont); + + this.platform = TypeMoq.Mock.ofType(); + this.fileSystem = TypeMoq.Mock.ofType(); + this.appShell = TypeMoq.Mock.ofType(); + this.locator = TypeMoq.Mock.ofType(); + this.settings = TypeMoq.Mock.ofType(); + this.process = TypeMoq.Mock.ofType(); + this.output = TypeMoq.Mock.ofType(); + + this.serviceManager.addSingletonInstance(IPlatformService, this.platform.object); + this.serviceManager.addSingletonInstance(IFileSystem, this.fileSystem.object); + this.serviceManager.addSingletonInstance(IApplicationShell, this.appShell.object); + this.serviceManager.addSingletonInstance(IInterpreterLocatorService, this.locator.object); + this.serviceManager.addSingletonInstance(IProcessService, this.process.object); + this.serviceManager.addSingletonInstance(IOutputChannel, this.output.object, STANDARD_OUTPUT_CHANNEL); + this.pythonInstaller = new PythonInstaller(this.serviceContainer); + + this.platform.setup(x => x.isMac).returns(() => isMac); + this.platform.setup(x => x.isWindows).returns(() => !isMac); + } +} + +// tslint:disable-next-line:max-func-body-length +suite('Installation', () => { + suiteSetup(async () => { + await initialize(); + }); + setup(initializeTest); + suiteTeardown(closeActiveWindows); + teardown(closeActiveWindows); + + test('Windows: Python missing', async () => { + const c = new TestContext(false); + let showErrorMessageCalled = false; + let openUrlCalled = false; + let url; + + c.appShell.setup(x => x.showErrorMessage(TypeMoq.It.isAnyString())).callback(() => showErrorMessageCalled = true); + c.appShell.setup(x => x.openUrl(TypeMoq.It.isAnyString())).callback((s: string) => { + openUrlCalled = true; + url = s; + }); + c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([])); + + const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + assert.equal(passed, false, 'Python reported as present'); + assert.equal(showErrorMessageCalled, true, 'Error message not shown'); + assert.equal(openUrlCalled, true, 'Python download page not opened'); + assert.equal(url, 'https://www.python.org/downloads', 'Python download page is incorrect'); + }); + + test('Mac: Python missing', async () => { + const c = new TestContext(true); + let called = false; + c.appShell.setup(x => x.showWarningMessage(TypeMoq.It.isAnyString())).callback(() => called = true); + c.settings.setup(x => x.pythonPath).returns(() => 'python'); + const interpreter: PythonInterpreter = { + path: 'python', + type: InterpreterType.Unknown + }; + c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([interpreter])); + + const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + assert.equal(passed, true, 'Default MacOS Python not accepted'); + assert.equal(called, true, 'Warning not shown'); + }); + + test('Mac: Default Python, user refused install', async () => { + const c = new TestContext(true); + let errorMessage: string; + + c.appShell + .setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) + .callback((m: string, a1: string, a2: string) => errorMessage = m) + .returns(() => Promise.resolve('No')); + c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([])); + + const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + assert.equal(passed, false, 'Default MacOS Python accepted'); + assert.equal(errorMessage.startsWith('Python that comes with MacOS is not supported'), true, 'Error message that MacOS Python not supported not shown'); + }); + + test('Mac: Default Python, Brew installation', async () => { + const c = new TestContext(true); + let errorMessage: string; + let processName: string; + let args; + let brewPath; + let outputShown = false; + + c.appShell + .setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) + .returns(() => Promise.resolve('Yes')); + c.appShell + .setup(x => x.showErrorMessage(TypeMoq.It.isAnyString())) + .callback((m: string) => errorMessage = m); + c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([])); + c.fileSystem + .setup(x => x.existsAsync(TypeMoq.It.isAnyString())) + .returns((p: string) => { + brewPath = p; + return Promise.resolve(false); + }); + + const childProcess = TypeMoq.Mock.ofType(); + const processOutput: Output = { + source: 'stdout', + out: 'started' + }; + const observable = new Rx.Observable>(subscriber => { + subscriber.next(processOutput); + }); + const brewInstallProcess: ObservableExecutionResult = { + proc: childProcess.object, + out: observable + }; + + c.output.setup(x => x.show()).callback(() => outputShown = true); + c.process + .setup(x => x.execObservable(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .callback((p: string, a: string[], o: SpawnOptions) => { + processName = p; + args = a; + }) + .returns(() => brewInstallProcess); + + let passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + + assert.notEqual(brewPath, undefined, 'Brew installer location not checked'); + assert.equal(brewPath, '/usr/local/bin/brew', 'Brew installer location is incorrect'); + assert.notEqual(processName, undefined, 'Brew installer not invoked'); + assert.equal(processName, '/usr/bin/ruby', 'Brew installer name is incorrect'); + assert.equal(args[0], '-e', 'Brew installer argument is incorrect'); + assert.equal(args[1], '"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"', 'Brew installer argument is incorrect'); + assert.equal(outputShown, true, 'Output panel not shown'); + assert.equal(errorMessage.startsWith('Unable to install Brew'), true, 'Brew install failed message no shown'); + + c.fileSystem + .setup(x => x.existsAsync(TypeMoq.It.isAnyString())) + .returns(() => Promise.resolve(true)); + errorMessage = undefined; + + passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + assert.equal(errorMessage, undefined, `Unexpected error message ${errorMessage}`); + assert.equal(processName, 'brew', 'Brew installer name is incorrect'); + assert.equal(args[0], 'install', 'Brew "install" argument is incorrect'); + assert.equal(args[1], 'python', 'Brew "python" argument is incorrect'); + }); +}); From 97ed0a8ad84fa20921f0bb33e17fb454cac4e756 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 2 Jan 2018 15:20:38 -0800 Subject: [PATCH 37/51] PR feedback --- src/client/common/installer/pythonInstallation.ts | 3 ++- src/client/common/platform/platformService.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index ffdb19cb64a3..30c5353cf8b2 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -35,7 +35,8 @@ export class PythonInstaller { return true; } - if (this.platform.isWindows) { + if (!this.platform.isMac) { + // Windows or Linux await this.shell.showErrorMessage('Python is not installed. Please download and install Python before using the extension.'); this.shell.openUrl('https://www.python.org/downloads'); return false; diff --git a/src/client/common/platform/platformService.ts b/src/client/common/platform/platformService.ts index 2264513991ac..f3684372ce19 100644 --- a/src/client/common/platform/platformService.ts +++ b/src/client/common/platform/platformService.ts @@ -23,7 +23,7 @@ export class PlatformService implements IPlatformService { return this._isMac; } public get isLinux(): boolean { - return !(this.isWindows || this.isLinux); + return !(this.isWindows || this.isMac); } public get is64bit(): boolean { return arch() === 'x64'; From dcfc939cdf573ca8ff50b67bbafbe6c9d808abce Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 2 Jan 2018 16:47:32 -0800 Subject: [PATCH 38/51] CR feedback --- .../common/installer/pythonInstallation.ts | 9 +++--- src/client/common/platform/fileSystem.ts | 28 ++++++++++++------- src/client/common/platform/types.ts | 8 ++++-- src/test/index.ts | 3 +- src/test/install/pythonInstallation.test.ts | 21 +++++++------- 5 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index 30c5353cf8b2..81dd39b1065d 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. 'use strict'; -import { inject } from 'inversify'; import { OutputChannel } from 'vscode'; import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; @@ -10,9 +9,8 @@ import { IApplicationShell } from '../application/types'; import { IPythonSettings, isTestExecution } from '../configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IFileSystem, IPlatformService } from '../platform/types'; -import { IProcessService, IPythonExecutionService } from '../process/types'; +import { IProcessService } from '../process/types'; import { IOutputChannel } from '../types'; -import { IPythonInstallation } from './types'; export class PythonInstaller { private locator: IInterpreterLocatorService; @@ -50,7 +48,8 @@ export class PythonInstaller { if (await this.shell.showErrorMessage('Python that comes with MacOS is not supported. Would you like to install regular Python now?', 'Yes', 'No') === 'Yes') { const brewInstalled = await this.ensureBrew(); if (!brewInstalled) { - await this.shell.showErrorMessage('Unable to install Brew package manager'); + await this.shell.showErrorMessage('Unable to install Homebrew package manager. Try installing it manually.'); + this.shell.openUrl('https://brew.sh'); return false; } await this.executeAndOutput('brew', ['install', 'python']); @@ -62,7 +61,7 @@ export class PythonInstaller { } private isBrewInstalled(): Promise { - return this.fs.existsAsync('/usr/local/bin/brew'); + return this.fs.directoryExistsAsync('/usr/local/bin/brew'); } private async ensureBrew(): Promise { diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 1ba4d705cb64..bfe38a2ab836 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -3,6 +3,7 @@ 'use strict'; import * as fs from 'fs'; +import * as fse from 'fs-extra'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IServiceContainer } from '../../ioc/types'; @@ -13,23 +14,30 @@ export class FileSystem implements IFileSystem { constructor( @inject(IServiceContainer) private platformService: IPlatformService) { } public get directorySeparatorChar(): string { - return this.platformService.isWindows ? '\\' : '/'; + return path.sep; } - public existsAsync(filePath: string): Promise { + public objectExistsAsync(filePath: string, statCheck: (s: fs.Stats) => boolean): Promise { return new Promise(resolve => { - fs.exists(filePath, exists => { - return resolve(exists); + fse.stat(filePath, (error, stats) => { + if (error) { + return resolve(false); + } + return resolve(statCheck(stats)); }); }); } - public createDirectoryAsync(directoryPath: string): Promise { - return new Promise(resolve => { - fs.mkdir(directoryPath, error => { - return resolve(!error); - }); - }); + public fileExistsAsync(filePath: string): Promise { + return this.objectExistsAsync(filePath, (stats) => stats.isFile()); + } + + public directoryExistsAsync(filePath: string): Promise { + return this.objectExistsAsync(filePath, (stats) => stats.isDirectory()); + } + + public createDirectoryAsync(directoryPath: string): Promise { + return fse.mkdirp(directoryPath); } public getSubDirectoriesAsync(rootDir: string): Promise { diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index a676cca89ff8..58f940afdc53 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as fs from 'fs'; + export enum Architecture { Unknown = 1, x86 = 2, @@ -28,8 +30,10 @@ export interface IPlatformService { export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { directorySeparatorChar: string; - existsAsync(path: string): Promise; - createDirectoryAsync(path: string): Promise; + objectExistsAsync(path: string, statCheck: (s: fs.Stats) => boolean): Promise; + fileExistsAsync(path: string): Promise; + directoryExistsAsync(path: string): Promise; + createDirectoryAsync(path: string): Promise; getSubDirectoriesAsync(rootDir: string): Promise; arePathsSame(path1: string, path2: string): boolean; } diff --git a/src/test/index.ts b/src/test/index.ts index aa86c0587ef1..b4c43bfc6c0b 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -13,8 +13,7 @@ const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, timeout: 25000, - retries: 3, - grep: 'Installation' + retries: 3 }; testRunner.configure(options); module.exports = testRunner; diff --git a/src/test/install/pythonInstallation.test.ts b/src/test/install/pythonInstallation.test.ts index a1882864817d..36bdf513ea32 100644 --- a/src/test/install/pythonInstallation.test.ts +++ b/src/test/install/pythonInstallation.test.ts @@ -4,12 +4,12 @@ import * as assert from 'assert'; import { ChildProcess, SpawnOptions } from 'child_process'; -import { Container, interfaces } from 'inversify'; +import { Container } from 'inversify'; import * as Rx from 'rxjs'; import * as TypeMoq from 'typemoq'; import * as vscode from 'vscode'; import { IApplicationShell } from '../../client/common/application/types'; -import { IPythonSettings, PythonSettings } from '../../client/common/configSettings'; +import { IPythonSettings } from '../../client/common/configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants'; import { PythonInstaller } from '../../client/common/installer/pythonInstallation'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; @@ -17,7 +17,6 @@ import { IProcessService, ObservableExecutionResult, Output } from '../../client import { IOutputChannel } from '../../client/common/types'; import { IInterpreterLocatorService } from '../../client/interpreter/contracts'; import { InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts'; -import { PythonInterpreterLocatorService } from '../../client/interpreter/locators/index'; import { ServiceContainer } from '../../client/ioc/container'; import { ServiceManager } from '../../client/ioc/serviceManager'; import { IServiceContainer } from '../../client/ioc/types'; @@ -108,7 +107,7 @@ suite('Installation', () => { test('Mac: Default Python, user refused install', async () => { const c = new TestContext(true); - let errorMessage: string; + let errorMessage = ''; c.appShell .setup(x => x.showErrorMessage(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) @@ -123,8 +122,8 @@ suite('Installation', () => { test('Mac: Default Python, Brew installation', async () => { const c = new TestContext(true); - let errorMessage: string; - let processName: string; + let errorMessage = ''; + let processName = ''; let args; let brewPath; let outputShown = false; @@ -137,7 +136,7 @@ suite('Installation', () => { .callback((m: string) => errorMessage = m); c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([])); c.fileSystem - .setup(x => x.existsAsync(TypeMoq.It.isAnyString())) + .setup(x => x.directoryExistsAsync(TypeMoq.It.isAnyString())) .returns((p: string) => { brewPath = p; return Promise.resolve(false); @@ -165,7 +164,7 @@ suite('Installation', () => { }) .returns(() => brewInstallProcess); - let passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + await c.pythonInstaller.checkPythonInstallation(c.settings.object); assert.notEqual(brewPath, undefined, 'Brew installer location not checked'); assert.equal(brewPath, '/usr/local/bin/brew', 'Brew installer location is incorrect'); @@ -177,11 +176,11 @@ suite('Installation', () => { assert.equal(errorMessage.startsWith('Unable to install Brew'), true, 'Brew install failed message no shown'); c.fileSystem - .setup(x => x.existsAsync(TypeMoq.It.isAnyString())) + .setup(x => x.directoryExistsAsync(TypeMoq.It.isAnyString())) .returns(() => Promise.resolve(true)); - errorMessage = undefined; + errorMessage = ''; - passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + await c.pythonInstaller.checkPythonInstallation(c.settings.object); assert.equal(errorMessage, undefined, `Unexpected error message ${errorMessage}`); assert.equal(processName, 'brew', 'Brew installer name is incorrect'); assert.equal(args[0], 'install', 'Brew "install" argument is incorrect'); From 34790bb8aff268950b349e4140dd93207047a944 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 2 Jan 2018 17:05:37 -0800 Subject: [PATCH 39/51] Mock process instead --- src/client/common/installer/pythonInstallation.ts | 3 --- src/test/install/pythonInstallation.test.ts | 11 ++++++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index 81dd39b1065d..ce7ee112c5a9 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -90,9 +90,6 @@ export class PythonInstaller { if (failed) { resolve(false); } - if (isTestExecution()) { - resolve(true); - } result.proc.on('exit', (code, signal) => { resolve(!signal); }); diff --git a/src/test/install/pythonInstallation.test.ts b/src/test/install/pythonInstallation.test.ts index 36bdf513ea32..5a7b85f4ba04 100644 --- a/src/test/install/pythonInstallation.test.ts +++ b/src/test/install/pythonInstallation.test.ts @@ -143,6 +143,11 @@ suite('Installation', () => { }); const childProcess = TypeMoq.Mock.ofType(); + childProcess + .setup(p => p.on('exit', TypeMoq.It.isAny())) + .callback((e: string, listener: (code, signal) => void) => { + listener.call(0, undefined); + }); const processOutput: Output = { source: 'stdout', out: 'started' @@ -171,9 +176,9 @@ suite('Installation', () => { assert.notEqual(processName, undefined, 'Brew installer not invoked'); assert.equal(processName, '/usr/bin/ruby', 'Brew installer name is incorrect'); assert.equal(args[0], '-e', 'Brew installer argument is incorrect'); - assert.equal(args[1], '"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"', 'Brew installer argument is incorrect'); + assert.equal(args[1], '"$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"', 'Homebrew installer argument is incorrect'); assert.equal(outputShown, true, 'Output panel not shown'); - assert.equal(errorMessage.startsWith('Unable to install Brew'), true, 'Brew install failed message no shown'); + assert.equal(errorMessage.startsWith('Unable to install Homebrew'), true, 'Homebrew install failed message no shown'); c.fileSystem .setup(x => x.directoryExistsAsync(TypeMoq.It.isAnyString())) @@ -181,7 +186,7 @@ suite('Installation', () => { errorMessage = ''; await c.pythonInstaller.checkPythonInstallation(c.settings.object); - assert.equal(errorMessage, undefined, `Unexpected error message ${errorMessage}`); + assert.equal(errorMessage, '', `Unexpected error message ${errorMessage}`); assert.equal(processName, 'brew', 'Brew installer name is incorrect'); assert.equal(args[0], 'install', 'Brew "install" argument is incorrect'); assert.equal(args[1], 'python', 'Brew "python" argument is incorrect'); From b6caacc63d2b15445085a8dc1af9a0edbf584a91 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 3 Jan 2018 09:41:26 -0800 Subject: [PATCH 40/51] Fix Brew detection --- src/client/common/installer/pythonInstallation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index ce7ee112c5a9..e50043aac5d5 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -61,7 +61,7 @@ export class PythonInstaller { } private isBrewInstalled(): Promise { - return this.fs.directoryExistsAsync('/usr/local/bin/brew'); + return this.fs.fileExistsAsync('/usr/local/bin/brew'); } private async ensureBrew(): Promise { From bb648d71eaf53660153a29d7bdfcd53e4c85e7e8 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 09:56:16 -0800 Subject: [PATCH 41/51] Update test --- src/test/install/pythonInstallation.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/install/pythonInstallation.test.ts b/src/test/install/pythonInstallation.test.ts index 5a7b85f4ba04..8a4b08349796 100644 --- a/src/test/install/pythonInstallation.test.ts +++ b/src/test/install/pythonInstallation.test.ts @@ -136,7 +136,7 @@ suite('Installation', () => { .callback((m: string) => errorMessage = m); c.locator.setup(x => x.getInterpreters()).returns(() => Promise.resolve([])); c.fileSystem - .setup(x => x.directoryExistsAsync(TypeMoq.It.isAnyString())) + .setup(x => x.fileExistsAsync(TypeMoq.It.isAnyString())) .returns((p: string) => { brewPath = p; return Promise.resolve(false); @@ -152,9 +152,7 @@ suite('Installation', () => { source: 'stdout', out: 'started' }; - const observable = new Rx.Observable>(subscriber => { - subscriber.next(processOutput); - }); + const observable = new Rx.Observable>(subscriber => subscriber.next(processOutput)); const brewInstallProcess: ObservableExecutionResult = { proc: childProcess.object, out: observable @@ -181,7 +179,7 @@ suite('Installation', () => { assert.equal(errorMessage.startsWith('Unable to install Homebrew'), true, 'Homebrew install failed message no shown'); c.fileSystem - .setup(x => x.directoryExistsAsync(TypeMoq.It.isAnyString())) + .setup(x => x.fileExistsAsync(TypeMoq.It.isAnyString())) .returns(() => Promise.resolve(true)); errorMessage = ''; From a7bca77b693a0f9fdb1ac094969f1de338d51621 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 12:17:27 -0800 Subject: [PATCH 42/51] Elevated module install --- package.json | 1 + pythonFiles/completion.py | 2 +- src/client/common/helpers.ts | 2 +- .../common/installer/moduleInstaller.ts | 60 +++++++++++++++++-- src/client/formatters/baseFormatter.ts | 2 +- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 062e8fcdd5ea..cda5c21bfe79 100644 --- a/package.json +++ b/package.json @@ -1537,6 +1537,7 @@ "reflect-metadata": "^0.1.10", "rxjs": "^5.5.2", "semver": "^5.4.1", + "sudo-prompt": "^8.0.0", "tmp": "0.0.29", "tree-kill": "^1.1.0", "typescript-char": "^0.0.0", diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index ce798d246bab..4a0f915fd145 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -198,7 +198,7 @@ def _serialize_completions(self, script, identifier=None, prefix=''): # we pass 'text' here only for fuzzy matcher if value: _completion['snippet'] = '%s=${1:%s}$0' % (name, value) - _completion['text'] = '%s=%s' % (name, value) + _completion['text'] = '%s=' % (name) else: _completion['snippet'] = '%s=$1$0' % name _completion['text'] = name diff --git a/src/client/common/helpers.ts b/src/client/common/helpers.ts index fb8163a4087c..82d59e9308c4 100644 --- a/src/client/common/helpers.ts +++ b/src/client/common/helpers.ts @@ -13,7 +13,7 @@ export function isNotInstalledError(error: Error): boolean { return true; } - const isModuleNoInstalledError = errorObj.code === 1 && error.message.indexOf('No module named') >= 0; + const isModuleNoInstalledError = error.message.indexOf('No module named') >= 0; return errorObj.code === 'ENOENT' || errorObj.code === 127 || isModuleNoInstalledError; } diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index 25bbd88d55cb..79ff674184c9 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -1,27 +1,75 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// tslint:disable-next-line:no-require-imports no-var-requires +const sudo = require('sudo-prompt'); + +import * as fs from 'fs'; import { injectable } from 'inversify'; -import { Uri } from 'vscode'; +import * as path from 'path'; +import * as vscode from 'vscode'; import { IServiceContainer } from '../../ioc/types'; import { PythonSettings } from '../configSettings'; +import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { ITerminalService } from '../terminal/types'; -import { ExecutionInfo } from '../types'; +import { ExecutionInfo, IOutputChannel } from '../types'; @injectable() export abstract class ModuleInstaller { constructor(protected serviceContainer: IServiceContainer) { } - public async installModule(name: string, resource?: Uri): Promise { + public async installModule(name: string, resource?: vscode.Uri): Promise { const executionInfo = await this.getExecutionInfo(name, resource); const terminalService = this.serviceContainer.get(ITerminalService); if (executionInfo.moduleName) { const pythonPath = PythonSettings.getInstance(resource).pythonPath; - await terminalService.sendCommand(pythonPath, ['-m', 'pip'].concat(executionInfo.args)); + const args = ['-m', 'pip'].concat(executionInfo.args); + if (await this.isPathWritableAsync(path.dirname(pythonPath))) { + await terminalService.sendCommand(pythonPath, args); + } else { + this.elevatedInstall(pythonPath, args); + } } else { await terminalService.sendCommand(executionInfo.execPath!, executionInfo.args); } } - public abstract isSupported(resource?: Uri): Promise; - protected abstract getExecutionInfo(moduleName: string, resource?: Uri): Promise; + public abstract isSupported(resource?: vscode.Uri): Promise; + protected abstract getExecutionInfo(moduleName: string, resource?: vscode.Uri): Promise; + + private async isPathWritableAsync(directoryPath: string): Promise { + const filePath = `${directoryPath}${path.sep}___vscpTest___`; + return new Promise(resolve => { + fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, stats) => { + fs.unlink(filePath); + return resolve(!error); + }); + }); + } + + private elevatedInstall(execPath: string, args: string[]) { + const options = { + name: 'VS Code Python' + }; + const outputChannel = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + const command = `"${execPath.replace(/\\/g, '/')}" ${args.join(' ')}`; + + outputChannel.appendLine(''); + outputChannel.appendLine(`[Elevated] ${command}`); + + sudo.exec(command, options, (error, stdout, stderr) => { + if (error) { + vscode.window.showErrorMessage(error); + } else { + outputChannel.show(); + if (stdout) { + outputChannel.appendLine(''); + outputChannel.append(stdout); + } + if (stderr) { + outputChannel.appendLine(''); + outputChannel.append(`Warning: ${stderr}`); + } + } + }); + } } diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index 6e7a9322cbdb..d38f71a1e898 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -99,7 +99,7 @@ export abstract class BaseFormatter { if (isNotInstalledError(error)) { const installer = this.serviceContainer.get(IInstaller); const isInstalled = await installer.isInstalled(this.product, resource); - if (isInstalled) { + if (!isInstalled) { customError += `\nYou could either install the '${this.Id}' formatter, turn it off or use another formatter.`; installer.promptToInstall(this.product, resource).catch(ex => console.error('Python Extension: promptToInstall', ex)); } From b1f7fba06f6d9d15afa04f836ca8039eaaac2f31 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 12:41:29 -0800 Subject: [PATCH 43/51] Fix path check --- src/client/common/installer/moduleInstaller.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index 79ff674184c9..9c423c3e9fd5 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -39,8 +39,12 @@ export abstract class ModuleInstaller { private async isPathWritableAsync(directoryPath: string): Promise { const filePath = `${directoryPath}${path.sep}___vscpTest___`; return new Promise(resolve => { - fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, stats) => { - fs.unlink(filePath); + fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, fd) => { + if (!error) { + fs.close(fd, (e) => { + fs.unlink(filePath); + }); + } return resolve(!error); }); }); From d223410b981cc61ae6ffa81328e488dce407af76 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 13:41:43 -0800 Subject: [PATCH 44/51] Add check suppression option & suppress vor VE by default --- package.json | 6 ++++++ src/client/common/configSettings.ts | 3 +++ src/client/common/installer/pythonInstallation.ts | 11 ++++++++--- src/test/install/pythonInstallation.test.ts | 11 +++++++++++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 062e8fcdd5ea..0b22268b2542 100644 --- a/package.json +++ b/package.json @@ -919,6 +919,12 @@ }, "scope": "resource" }, + "python.disableInstallationCheck": { + "type": "boolean", + "default": false, + "description": "Whether to check if Python is installed.", + "scope": "resource" + }, "python.linting.enabled": { "type": "boolean", "default": true, diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 6899e248da85..15d34e711676 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -27,6 +27,7 @@ export interface IPythonSettings { workspaceSymbols: IWorkspaceSymbolSettings; envFile: string; disablePromptForFeatures: string[]; + disableInstallationChecks: boolean; } export interface ISortImportSettings { path: string; @@ -146,6 +147,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { public terminal: ITerminalSettings; public sortImports: ISortImportSettings; public workspaceSymbols: IWorkspaceSymbolSettings; + public disableInstallationChecks: boolean; private workspaceRoot: vscode.Uri; private disposables: vscode.Disposable[] = []; @@ -225,6 +227,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { } else { this.linting = lintingSettings; } + this.disableInstallationChecks = pythonSettings.get('disableInstallationCheck') === true; // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion const sortImportSettings = systemVariables.resolveAny(pythonSettings.get('sortImports'))!; if (this.sortImports) { diff --git a/src/client/common/installer/pythonInstallation.ts b/src/client/common/installer/pythonInstallation.ts index e50043aac5d5..6f96a62bdb41 100644 --- a/src/client/common/installer/pythonInstallation.ts +++ b/src/client/common/installer/pythonInstallation.ts @@ -3,10 +3,10 @@ 'use strict'; import { OutputChannel } from 'vscode'; -import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE } from '../../interpreter/contracts'; +import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { IApplicationShell } from '../application/types'; -import { IPythonSettings, isTestExecution } from '../configSettings'; +import { IPythonSettings } from '../configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IFileSystem, IPlatformService } from '../platform/types'; import { IProcessService } from '../process/types'; @@ -25,9 +25,14 @@ export class PythonInstaller { } public async checkPythonInstallation(settings: IPythonSettings): Promise { + if (settings.disableInstallationChecks === true) { + return true; + } let interpreters = await this.locator.getInterpreters(); if (interpreters.length > 0) { - if (this.platform.isMac && settings.pythonPath === 'python') { + if (this.platform.isMac && + settings.pythonPath === 'python' && + interpreters[0].type === InterpreterType.Unknown) { await this.shell.showWarningMessage('Selected interpreter is MacOS system Python which is not recommended. Please select different interpreter'); } return true; diff --git a/src/test/install/pythonInstallation.test.ts b/src/test/install/pythonInstallation.test.ts index 8a4b08349796..412e19ae3723 100644 --- a/src/test/install/pythonInstallation.test.ts +++ b/src/test/install/pythonInstallation.test.ts @@ -69,6 +69,17 @@ suite('Installation', () => { suiteTeardown(closeActiveWindows); teardown(closeActiveWindows); + test('Disable checks', async () => { + const c = new TestContext(false); + let showErrorMessageCalled = false; + + c.settings.setup(s => s.disableInstallationChecks).returns(() => true); + c.appShell.setup(x => x.showErrorMessage(TypeMoq.It.isAnyString())).callback(() => showErrorMessageCalled = true); + const passed = await c.pythonInstaller.checkPythonInstallation(c.settings.object); + assert.equal(passed, true, 'Disabling checks has no effect'); + assert.equal(showErrorMessageCalled, false, 'Disabling checks has no effect'); + }); + test('Windows: Python missing', async () => { const c = new TestContext(false); let showErrorMessageCalled = false; From 6873309cda410a12d12ce9f499338c9c37d6deb9 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 15:13:41 -0800 Subject: [PATCH 45/51] Fix most linter tests --- src/test/linters/lint.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index ac67620d300c..c0aeca87940d 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -4,7 +4,7 @@ import * as path from 'path'; import { OutputChannel, Uri } from 'vscode'; import * as vscode from 'vscode'; import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants'; -import { Product, SettingToDisableProduct } from '../../client/common/installer/installer'; +import { Installer, Product, SettingToDisableProduct } from '../../client/common/installer/installer'; import { IInstaller, ILogger, IOutputChannel } from '../../client/common/types'; import { IServiceContainer } from '../../client/ioc/types'; import { BaseLinter } from '../../client/linters/baseLinter'; @@ -122,6 +122,8 @@ suite('Linting', () => { ioc.registerProcessTypes(); ioc.registerLinterTypes(); ioc.registerVariableTypes(); + + ioc.serviceManager.addSingleton(IInstaller, Installer); } type LinterCtor = { new(outputChannel: OutputChannel, installer: IInstaller, helper: ILinterHelper, logger: ILogger, serviceContainer: IServiceContainer): BaseLinter }; From 9bd8948e16e8c0c73a996fc71d1bf962c711c0a0 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 15:21:43 -0800 Subject: [PATCH 46/51] Merge conflict --- src/client/common/installer/serviceRegistry.ts | 1 - src/client/common/serviceRegistry.ts | 4 +++- src/test/linters/lint.test.ts | 2 -- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client/common/installer/serviceRegistry.ts b/src/client/common/installer/serviceRegistry.ts index 85d73db373c6..d89a81c7ad3f 100644 --- a/src/client/common/installer/serviceRegistry.ts +++ b/src/client/common/installer/serviceRegistry.ts @@ -10,7 +10,6 @@ import { PipInstaller } from './pipInstaller'; import { IModuleInstaller, IPythonInstallation } from './types'; export function registerTypes(serviceManager: IServiceManager) { - serviceManager.addSingleton(IInstaller, Installer); serviceManager.addSingleton(IModuleInstaller, CondaInstaller); serviceManager.addSingleton(IModuleInstaller, PipInstaller); } diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index ceee3cbbf2d2..c9bee5e8aba2 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -4,6 +4,7 @@ import { IServiceManager } from '../ioc/types'; import { ApplicationShell } from './application/applicationShell'; import { IApplicationShell } from './application/types'; +import { Installer } from './installer/installer'; import { Logger } from './logger'; import { PersistentStateFactory } from './persistentState'; import { IS_64_BIT, IS_WINDOWS } from './platform/constants'; @@ -11,7 +12,7 @@ import { PathUtils } from './platform/pathUtils'; import { CurrentProcess } from './process/currentProcess'; import { TerminalService } from './terminal/service'; import { ITerminalService } from './terminal/types'; -import { ICurrentProcess, ILogger, IPathUtils, IPersistentStateFactory, Is64Bit, IsWindows } from './types'; +import { ICurrentProcess, IInstaller, ILogger, IPathUtils, IPersistentStateFactory, Is64Bit, IsWindows } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingletonInstance(IsWindows, IS_WINDOWS); @@ -23,4 +24,5 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IPathUtils, PathUtils); serviceManager.addSingleton(IApplicationShell, ApplicationShell); serviceManager.addSingleton(ICurrentProcess, CurrentProcess); + serviceManager.addSingleton(IInstaller, Installer); } diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index c0aeca87940d..860369a2f90e 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -122,8 +122,6 @@ suite('Linting', () => { ioc.registerProcessTypes(); ioc.registerLinterTypes(); ioc.registerVariableTypes(); - - ioc.serviceManager.addSingleton(IInstaller, Installer); } type LinterCtor = { new(outputChannel: OutputChannel, installer: IInstaller, helper: ILinterHelper, logger: ILogger, serviceContainer: IServiceContainer): BaseLinter }; From 29fadba141619a6e0be8b9698a17c3458ae5453c Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Wed, 3 Jan 2018 16:39:50 -0800 Subject: [PATCH 47/51] Per-user install --- package.json | 6 ++++++ src/client/common/configSettings.ts | 5 +++++ src/client/common/installer/moduleInstaller.ts | 13 +++++++++---- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 47c441ab8bea..59fb161be76a 100644 --- a/package.json +++ b/package.json @@ -925,6 +925,12 @@ "description": "Whether to check if Python is installed.", "scope": "resource" }, + "python.globalModuleInstallation": { + "type": "boolean", + "default": false, + "description": "Whether to install Python modules globally.", + "scope": "resource" + }, "python.linting.enabled": { "type": "boolean", "default": true, diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 1aa7b007ac43..7a8df51c1912 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -27,6 +27,7 @@ export interface IPythonSettings { envFile: string; disablePromptForFeatures: string[]; disableInstallationChecks: boolean; + globalModuleInstallation: boolean; } export interface ISortImportSettings { path: string; @@ -147,6 +148,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { public sortImports: ISortImportSettings; public workspaceSymbols: IWorkspaceSymbolSettings; public disableInstallationChecks: boolean; + public globalModuleInstallation: boolean; private workspaceRoot: vscode.Uri; private disposables: vscode.Disposable[] = []; @@ -224,7 +226,10 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { } else { this.linting = lintingSettings; } + this.disableInstallationChecks = pythonSettings.get('disableInstallationCheck') === true; + this.globalModuleInstallation = pythonSettings.get('globalModuleInstallation') === true; + // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion const sortImportSettings = systemVariables.resolveAny(pythonSettings.get('sortImports'))!; if (this.sortImports) { diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index 9c423c3e9fd5..357645ff63af 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -22,12 +22,17 @@ export abstract class ModuleInstaller { const terminalService = this.serviceContainer.get(ITerminalService); if (executionInfo.moduleName) { - const pythonPath = PythonSettings.getInstance(resource).pythonPath; + const settings = PythonSettings.getInstance(resource); const args = ['-m', 'pip'].concat(executionInfo.args); - if (await this.isPathWritableAsync(path.dirname(pythonPath))) { - await terminalService.sendCommand(pythonPath, args); + + if (settings.globalModuleInstallation) { + if (await this.isPathWritableAsync(path.dirname(settings.pythonPath))) { + await terminalService.sendCommand(settings.pythonPath, args); + } else { + this.elevatedInstall(settings.pythonPath, args); + } } else { - this.elevatedInstall(pythonPath, args); + await terminalService.sendCommand(settings.pythonPath, args.concat(['--user'])); } } else { await terminalService.sendCommand(executionInfo.execPath!, executionInfo.args); From 542c1fc2383aa5ba45504c5f4a0737192012848c Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 4 Jan 2018 09:38:20 -0800 Subject: [PATCH 48/51] Handle VE/Conda --- .../common/installer/moduleInstaller.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index 357645ff63af..0a2a513eab43 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -8,9 +8,11 @@ import * as fs from 'fs'; import { injectable } from 'inversify'; import * as path from 'path'; import * as vscode from 'vscode'; +import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { PythonSettings } from '../configSettings'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; +import { IFileSystem } from '../platform/types'; import { ITerminalService } from '../terminal/types'; import { ExecutionInfo, IOutputChannel } from '../types'; @@ -24,15 +26,22 @@ export abstract class ModuleInstaller { if (executionInfo.moduleName) { const settings = PythonSettings.getInstance(resource); const args = ['-m', 'pip'].concat(executionInfo.args); + const pythonPath = settings.pythonPath; - if (settings.globalModuleInstallation) { - if (await this.isPathWritableAsync(path.dirname(settings.pythonPath))) { - await terminalService.sendCommand(settings.pythonPath, args); + const locator = this.serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); + const fileSystem = this.serviceContainer.get(IFileSystem); + const currentInterpreter = (await locator.getInterpreters(resource)).filter(x => fileSystem.arePathsSame(x.path, pythonPath)); + + if (currentInterpreter[0].type !== InterpreterType.Unknown) { + await terminalService.sendCommand(pythonPath, args); + } else if (settings.globalModuleInstallation) { + if (await this.isPathWritableAsync(path.dirname(pythonPath))) { + await terminalService.sendCommand(pythonPath, args); } else { - this.elevatedInstall(settings.pythonPath, args); + this.elevatedInstall(pythonPath, args); } } else { - await terminalService.sendCommand(settings.pythonPath, args.concat(['--user'])); + await terminalService.sendCommand(pythonPath, args.concat(['--user'])); } } else { await terminalService.sendCommand(executionInfo.execPath!, executionInfo.args); From 0c87c20173a0708d315f1e3b078991d69b3c59b7 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 4 Jan 2018 13:24:40 -0800 Subject: [PATCH 49/51] Fix tests --- src/client/common/serviceRegistry.ts | 5 +++++ src/client/common/terminal/types.ts | 2 +- src/test/common/moduleInstaller.test.ts | 15 +++++++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index c9bee5e8aba2..842ea55c455e 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -8,7 +8,10 @@ import { Installer } from './installer/installer'; import { Logger } from './logger'; import { PersistentStateFactory } from './persistentState'; import { IS_64_BIT, IS_WINDOWS } from './platform/constants'; +import { FileSystem } from './platform/fileSystem'; import { PathUtils } from './platform/pathUtils'; +import { PlatformService } from './platform/platformService'; +import { IFileSystem, IPlatformService } from './platform/types'; import { CurrentProcess } from './process/currentProcess'; import { TerminalService } from './terminal/service'; import { ITerminalService } from './terminal/types'; @@ -25,4 +28,6 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IApplicationShell, ApplicationShell); serviceManager.addSingleton(ICurrentProcess, CurrentProcess); serviceManager.addSingleton(IInstaller, Installer); + serviceManager.addSingleton(IFileSystem, FileSystem); + serviceManager.addSingleton(IPlatformService, PlatformService); } diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index 1f03fa45a1dd..0bd7ac3fbc72 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -export const ITerminalService = Symbol('ITerminalCommandService'); +export const ITerminalService = Symbol('ITerminalService'); export interface ITerminalService { sendCommand(command: string, args: string[]): Promise; diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index aebee3cfacb5..430639c7109f 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -8,13 +8,16 @@ import { PipInstaller } from '../../client/common/installer/pipInstaller'; import { IModuleInstaller } from '../../client/common/installer/types'; import { Logger } from '../../client/common/logger'; import { PersistentStateFactory } from '../../client/common/persistentState'; +import { FileSystem } from '../../client/common/platform/fileSystem'; import { PathUtils } from '../../client/common/platform/pathUtils'; -import { Architecture } from '../../client/common/platform/types'; +import { PlatformService } from '../../client/common/platform/platformService'; +import { Architecture, IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { CurrentProcess } from '../../client/common/process/currentProcess'; import { IProcessService, IPythonExecutionFactory } from '../../client/common/process/types'; import { ITerminalService } from '../../client/common/terminal/types'; import { ICurrentProcess, IInstaller, ILogger, IPathUtils, IPersistentStateFactory, IsWindows } from '../../client/common/types'; import { ICondaLocatorService, IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../client/interpreter/contracts'; +import { PythonInterpreterLocatorService } from '../../client/interpreter/locators/index'; import { updateSetting } from '../common'; import { rootWorkspaceUri } from '../common'; import { MockProvider } from '../interpreters/mocks'; @@ -60,6 +63,8 @@ suite('Module Installer', () => { ioc.serviceManager.addSingleton(ICondaLocatorService, MockCondaLocator); ioc.serviceManager.addSingleton(IPathUtils, PathUtils); ioc.serviceManager.addSingleton(ICurrentProcess, CurrentProcess); + ioc.serviceManager.addSingleton(IFileSystem, FileSystem); + ioc.serviceManager.addSingleton(IPlatformService, PlatformService); ioc.registerMockProcessTypes(); ioc.serviceManager.addSingleton(ITerminalService, MockTerminalService); @@ -136,6 +141,9 @@ suite('Module Installer', () => { }); test('Validate pip install arguments', async () => { + const mockInterpreterLocator = new MockProvider([{ path: 'python', type: InterpreterType.Unknown }]); + ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, mockInterpreterLocator, INTERPRETER_LOCATOR_SERVICE); + const moduleName = 'xyz'; const terminalService = ioc.serviceContainer.get(ITerminalService); @@ -148,10 +156,13 @@ suite('Module Installer', () => { const commandSent = await terminalService.commandSent; const commandParts = commandSent.split(' '); commandParts.shift(); - expect(commandParts.join(' ')).equal(`-m pip install -U ${moduleName}`, 'Invalid command sent to terminal for installation.'); + expect(commandParts.join(' ')).equal(`-m pip install -U ${moduleName} --user`, 'Invalid command sent to terminal for installation.'); }); test('Validate Conda install arguments', async () => { + const mockInterpreterLocator = new MockProvider([{ path: 'python', type: InterpreterType.Conda }]); + ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, mockInterpreterLocator, INTERPRETER_LOCATOR_SERVICE); + const moduleName = 'xyz'; const terminalService = ioc.serviceContainer.get(ITerminalService); From b2612828bed641f6f57601de9664ced1aea471c2 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 4 Jan 2018 14:06:55 -0800 Subject: [PATCH 50/51] Remove double service --- src/client/common/serviceRegistry.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 842ea55c455e..8ac2689adcba 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -29,5 +29,4 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ICurrentProcess, CurrentProcess); serviceManager.addSingleton(IInstaller, Installer); serviceManager.addSingleton(IFileSystem, FileSystem); - serviceManager.addSingleton(IPlatformService, PlatformService); } From 57afa7f0a32efa7a6b85c61e75520a16d9211a4d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 4 Jan 2018 16:00:35 -0800 Subject: [PATCH 51/51] Better mocking --- src/client/common/installer/moduleInstaller.ts | 8 ++++++-- src/test/common/moduleInstaller.test.ts | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index 0a2a513eab43..2fb09a7216ec 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -30,9 +30,13 @@ export abstract class ModuleInstaller { const locator = this.serviceContainer.get(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE); const fileSystem = this.serviceContainer.get(IFileSystem); - const currentInterpreter = (await locator.getInterpreters(resource)).filter(x => fileSystem.arePathsSame(x.path, pythonPath)); + const interpreters = await locator.getInterpreters(resource); - if (currentInterpreter[0].type !== InterpreterType.Unknown) { + const currentInterpreter = interpreters.length > 1 + ? interpreters.filter(x => fileSystem.arePathsSame(x.path, pythonPath))[0] + : interpreters[0]; + + if (!currentInterpreter || currentInterpreter.type !== InterpreterType.Unknown) { await terminalService.sendCommand(pythonPath, args); } else if (settings.globalModuleInstallation) { if (await this.isPathWritableAsync(path.dirname(pythonPath))) { diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 430639c7109f..b769dc5dd618 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -141,7 +141,7 @@ suite('Module Installer', () => { }); test('Validate pip install arguments', async () => { - const mockInterpreterLocator = new MockProvider([{ path: 'python', type: InterpreterType.Unknown }]); + const mockInterpreterLocator = new MockProvider([{ path: await getCurrentPythonPath(), type: InterpreterType.Unknown }]); ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, mockInterpreterLocator, INTERPRETER_LOCATOR_SERVICE); const moduleName = 'xyz'; @@ -160,7 +160,7 @@ suite('Module Installer', () => { }); test('Validate Conda install arguments', async () => { - const mockInterpreterLocator = new MockProvider([{ path: 'python', type: InterpreterType.Conda }]); + const mockInterpreterLocator = new MockProvider([{ path: await getCurrentPythonPath(), type: InterpreterType.Conda }]); ioc.serviceManager.addSingletonInstance(IInterpreterLocatorService, mockInterpreterLocator, INTERPRETER_LOCATOR_SERVICE); const moduleName = 'xyz';