Skip to content

Commit

Permalink
Fixes multiple issues with formatting on type (#1450)
Browse files Browse the repository at this point in the history 
* Undo changes

* Test fixes

* Increase timeout

* Remove double event listening

* Remove test

* Revert "Remove test"

This reverts commit e240c3f.

* Revert "Remove double event listening"

This reverts commit af573be.

* #1096 The if statement is automatically formatted incorrectly

* Merge fix

* Add more tests

* More tests

* Typo

* Test

* Also better handle multiline arguments

* Add a couple missing periods

[skip ci]

* Undo changes

* Test fixes

* Increase timeout

* Remove double event listening

* Remove test

* Revert "Remove test"

This reverts commit e240c3f.

* Revert "Remove double event listening"

This reverts commit af573be.

* Merge fix

* #1257 On type formatting errors for args and kwargs

* Handle f-strings

* Stop importing from test code

* #1308 Single line statements leading to an indentation on the next line

* #726 editing python after inline if statement invalid indent

* Undo change

* Move constant

* Harden LS startup error checks

* #1364 Intellisense doesn't work after specific const string

* Telemetry for the analysis enging

* PR feedback

* Fix typo

* Test baseline update

* Jedi 0.12

* Priority to goto_defition

* News

* Replace unzip

* Linux flavors + test

* Grammar check

* Grammar test

* Test baselines

* Pin dependency

[skip ci]
  • Loading branch information
Mikhail Arkhipov authored Apr 23, 2018
1 parent 1465bda commit db9f6cf
Show file tree
Hide file tree
Showing 13 changed files with 2,089 additions and 152 deletions.
6 changes: 6 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,12 @@
"description": "Automatically add brackets for functions.",
"scope": "resource"
},
"python.autoComplete.showAdvancedMembers": {
"type": "boolean",
"default": false,
"description": "Controls appearance of methods with double underscores in the completion list.",
"scope": "resource"
},
"python.workspaceSymbols.tagFilePath": {
"type": "string",
"default": "${workspaceFolder}/.vscode/tags",
Expand Down
2 changes: 1 addition & 1 deletion src/client/activation/analysis.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
this.appShell = this.services.get<IApplicationShell>(IApplicationShell);
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.fs = this.services.get<IFileSystem>(IFileSystem);
this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService));
this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService), this.fs);
}

public async activate(context: ExtensionContext): Promise<boolean> {
Expand Down
15 changes: 11 additions & 4 deletions src/client/activation/analysisEngineHashes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@

// This file will be replaced by a generated one during the release build
// with actual hashes of the uploaded packages.
export const analysis_engine_win_x86_sha512 = '';
export const analysis_engine_win_x64_sha512 = '';
export const analysis_engine_osx_x64_sha512 = '';
export const analysis_engine_linux_x64_sha512 = '';
// Values are for test purposes only
export const analysis_engine_win_x86_sha512 = 'win-x86';
export const analysis_engine_win_x64_sha512 = 'win-x64';
export const analysis_engine_osx_x64_sha512 = 'osx-x64';
export const analysis_engine_centos_x64_sha512 = 'centos-x64';
export const analysis_engine_debian_x64_sha512 = 'debian-x64';
export const analysis_engine_fedora_x64_sha512 = 'fedora-x64';
export const analysis_engine_ol_x64_sha512 = 'ol-x64';
export const analysis_engine_opensuse_x64_sha512 = 'opensuse-x64';
export const analysis_engine_rhel_x64_sha512 = 'rhel-x64';
export const analysis_engine_ubuntu_x64_sha512 = 'ubuntu-x64';
8 changes: 4 additions & 4 deletions src/client/activation/downloader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ExtensionContext, OutputChannel, ProgressLocation, window } from 'vscod
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
import { noop } from '../common/core.utils';
import { createDeferred, createTemporaryFile } from '../common/helpers';
import { IPlatformService } from '../common/platform/types';
import { IFileSystem, IPlatformService } from '../common/platform/types';
import { IOutputChannel } from '../common/types';
import { IServiceContainer } from '../ioc/types';
import { HashVerifier } from './hashVerifier';
Expand All @@ -31,7 +31,7 @@ export class AnalysisEngineDownloader {
constructor(private readonly services: IServiceContainer, private engineFolder: string) {
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.platform = this.services.get<IPlatformService>(IPlatformService);
this.platformData = new PlatformData(this.platform);
this.platformData = new PlatformData(this.platform, this.services.get<IFileSystem>(IFileSystem));
}

public async downloadAnalysisEngine(context: ExtensionContext): Promise<void> {
Expand All @@ -49,7 +49,7 @@ export class AnalysisEngineDownloader {
}

private async downloadFile(): Promise<string> {
const platformString = this.platformData.getPlatformDesignator();
const platformString = await this.platformData.getPlatformName();
const remoteFileName = `${downloadBaseFileName}-${platformString}.${downloadVersion}${downloadFileExtension}`;
const uri = `${downloadUriPrefix}/${remoteFileName}`;
this.output.append(`Downloading ${uri}... `);
Expand Down Expand Up @@ -98,7 +98,7 @@ export class AnalysisEngineDownloader {
this.output.appendLine('');
this.output.append('Verifying download... ');
const verifier = new HashVerifier();
if (!await verifier.verifyHash(filePath, this.platformData.getExpectedHash())) {
if (!await verifier.verifyHash(filePath, await this.platformData.getExpectedHash())) {
throw new Error('Hash of the downloaded file does not match.');
}
this.output.append('valid.');
Expand Down
67 changes: 58 additions & 9 deletions src/client/activation/platformData.ts
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,54 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { IPlatformService } from '../common/platform/types';
import { IFileSystem, IPlatformService } from '../common/platform/types';
import {
analysis_engine_linux_x64_sha512,
analysis_engine_centos_x64_sha512,
analysis_engine_debian_x64_sha512,
analysis_engine_fedora_x64_sha512,
analysis_engine_ol_x64_sha512,
analysis_engine_opensuse_x64_sha512,
analysis_engine_osx_x64_sha512,
analysis_engine_rhel_x64_sha512,
analysis_engine_ubuntu_x64_sha512,
analysis_engine_win_x64_sha512,
analysis_engine_win_x86_sha512
} from './analysisEngineHashes';

// '/etc/os-release', ID=flavor
const supportedLinuxFlavors = [
'centos',
'debian',
'fedora',
'ol',
'opensuse',
'rhel',
'ubuntu'
];

export class PlatformData {
constructor(private platform: IPlatformService) { }
public getPlatformDesignator(): string {
constructor(private platform: IPlatformService, private fs: IFileSystem) { }
public async getPlatformName(): Promise<string> {
if (this.platform.isWindows) {
return this.platform.is64bit ? 'win-x64' : 'win-x86';
}
if (this.platform.isMac) {
return 'osx-x64';
}
if (this.platform.isLinux && this.platform.is64bit) {
return 'linux-x64';
if (this.platform.isLinux) {
if (!this.platform.is64bit) {
throw new Error('Python Analysis Engine does not support 32-bit Linux.');
}
const linuxFlavor = await this.getLinuxFlavor();
if (linuxFlavor.length === 0) {
throw new Error('Unable to determine Linux flavor from /etc/os-release.');
}
if (supportedLinuxFlavors.indexOf(linuxFlavor) < 0) {
throw new Error(`${linuxFlavor} is not supported.`);
}
return `${linuxFlavor}-x64`;
}
throw new Error('Python Analysis Engine does not support 32-bit Linux.');
throw new Error('Unknown OS platform.');
}

public getEngineDllName(): string {
Expand All @@ -34,16 +61,38 @@ export class PlatformData {
: 'Microsoft.PythonTools.VsCode';
}

public getExpectedHash(): string {
public async getExpectedHash(): Promise<string> {
if (this.platform.isWindows) {
return this.platform.is64bit ? analysis_engine_win_x64_sha512 : analysis_engine_win_x86_sha512;
}
if (this.platform.isMac) {
return analysis_engine_osx_x64_sha512;
}
if (this.platform.isLinux && this.platform.is64bit) {
return analysis_engine_linux_x64_sha512;
const linuxFlavor = await this.getLinuxFlavor();
// tslint:disable-next-line:switch-default
switch (linuxFlavor) {
case 'centos': return analysis_engine_centos_x64_sha512;
case 'debian': return analysis_engine_debian_x64_sha512;
case 'fedora': return analysis_engine_fedora_x64_sha512;
case 'ol': return analysis_engine_ol_x64_sha512;
case 'opensuse': return analysis_engine_opensuse_x64_sha512;
case 'rhel': return analysis_engine_rhel_x64_sha512;
case 'ubuntu': return analysis_engine_ubuntu_x64_sha512;
}
}
throw new Error('Unknown platform.');
}

private async getLinuxFlavor(): Promise<string> {
const verFile = '/etc/os-release';
const data = await this.fs.readFile(verFile);
if (data) {
const res = /ID=(.*)/.exec(data);
if (res && res.length > 1) {
return res[1];
}
}
return '';
}
}
132 changes: 101 additions & 31 deletions src/client/formatters/lineFormatter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class LineFormatter {

case TokenType.Comma:
this.builder.append(',');
if (next && !this.isCloseBraceType(next.type)) {
if (next && !this.isCloseBraceType(next.type) && next.type !== TokenType.Colon) {
this.builder.softAppendSpace();
}
break;
Expand All @@ -52,7 +52,12 @@ export class LineFormatter {
if (prev && !this.isOpenBraceType(prev.type) && prev.type !== TokenType.Colon && prev.type !== TokenType.Operator) {
this.builder.softAppendSpace();
}
this.builder.append(this.text.substring(t.start, t.end));
const id = this.text.substring(t.start, t.end);
this.builder.append(id);
if (this.keywordWithSpaceAfter(id) && next && this.isOpenBraceType(next.type)) {
// for x in ()
this.builder.softAppendSpace();
}
break;

case TokenType.Colon:
Expand Down Expand Up @@ -84,8 +89,10 @@ export class LineFormatter {
return this.builder.getText();
}

// tslint:disable-next-line:cyclomatic-complexity
private handleOperator(index: number): void {
const t = this.tokens.getItemAt(index);
const prev = index > 0 ? this.tokens.getItemAt(index - 1) : undefined;
if (t.length === 1) {
const opCode = this.text.charCodeAt(t.start);
switch (opCode) {
Expand All @@ -99,18 +106,36 @@ export class LineFormatter {
case Char.ExclamationMark:
this.builder.append(this.text[t.start]);
return;
case Char.Asterisk:
if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') {
this.builder.softAppendSpace();
this.builder.append('*');
return;
}
break;
default:
break;
}
} else if (t.length === 2) {
if (this.text.charCodeAt(t.start) === Char.Asterisk && this.text.charCodeAt(t.start + 1) === Char.Asterisk) {
if (!prev || (prev.type !== TokenType.Identifier && prev.type !== TokenType.Number)) {
this.builder.append('**');
return;
}
if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') {
this.builder.softAppendSpace();
this.builder.append('**');
return;
}
}
}

// Do not append space if operator is preceded by '(' or ',' as in foo(**kwarg)
if (index > 0) {
const prev = this.tokens.getItemAt(index - 1);
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma) {
this.builder.append(this.text.substring(t.start, t.end));
return;
}
if (prev && (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma)) {
this.builder.append(this.text.substring(t.start, t.end));
return;
}

this.builder.softAppendSpace();
this.builder.append(this.text.substring(t.start, t.end));
this.builder.softAppendSpace();
Expand All @@ -135,43 +160,82 @@ export class LineFormatter {
return;
}

if (this.isEqualsInsideArguments(index - 1)) {
const prev = index > 0 ? this.tokens.getItemAt(index - 1) : undefined;
if (prev && prev.length === 1 && this.text.charCodeAt(prev.start) === Char.Equal && this.isEqualsInsideArguments(index - 1)) {
// Don't add space around = inside function arguments.
this.builder.append(this.text.substring(t.start, t.end));
return;
}

if (index > 0) {
const prev = this.tokens.getItemAt(index - 1);
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon) {
// Don't insert space after (, [ or { .
this.builder.append(this.text.substring(t.start, t.end));
return;
}
if (prev && (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon)) {
// Don't insert space after (, [ or { .
this.builder.append(this.text.substring(t.start, t.end));
return;
}

// In general, keep tokens separated.
this.builder.softAppendSpace();
this.builder.append(this.text.substring(t.start, t.end));
if (t.type === TokenType.Unknown) {
this.handleUnknown(t);
} else {
// In general, keep tokens separated.
this.builder.softAppendSpace();
this.builder.append(this.text.substring(t.start, t.end));
}
}

private handleUnknown(t: IToken): void {
const prevChar = t.start > 0 ? this.text.charCodeAt(t.start - 1) : 0;
if (prevChar === Char.Space || prevChar === Char.Tab) {
this.builder.softAppendSpace();
}
this.builder.append(this.text.substring(t.start, t.end));

const nextChar = t.end < this.text.length - 1 ? this.text.charCodeAt(t.end) : 0;
if (nextChar === Char.Space || nextChar === Char.Tab) {
this.builder.softAppendSpace();
}
}
private isEqualsInsideArguments(index: number): boolean {
// Since we don't have complete statement, this is mostly heuristics.
// Therefore the code may not be handling all possible ways of the
// argument list continuation.
if (index < 1) {
return false;
}

const prev = this.tokens.getItemAt(index - 1);
if (prev.type === TokenType.Identifier) {
if (index >= 2) {
// (x=1 or ,x=1
const prevPrev = this.tokens.getItemAt(index - 2);
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
} else if (index < this.tokens.count - 2) {
const next = this.tokens.getItemAt(index + 1);
const nextNext = this.tokens.getItemAt(index + 2);
// x=1, or x=1)
if (this.isValueType(next.type)) {
return nextNext.type === TokenType.Comma || nextNext.type === TokenType.CloseBrace;
}
if (prev.type !== TokenType.Identifier) {
return false;
}

const first = this.tokens.getItemAt(0);
if (first.type === TokenType.Comma) {
return true; // Line starts with commma
}

const last = this.tokens.getItemAt(this.tokens.count - 1);
if (last.type === TokenType.Comma) {
return true; // Line ends in comma
}

if (index >= 2) {
// (x=1 or ,x=1
const prevPrev = this.tokens.getItemAt(index - 2);
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
}

if (index >= this.tokens.count - 2) {
return false;
}

const next = this.tokens.getItemAt(index + 1);
const nextNext = this.tokens.getItemAt(index + 2);
// x=1, or x=1)
if (this.isValueType(next.type)) {
if (nextNext.type === TokenType.CloseBrace) {
return true;
}
if (nextNext.type === TokenType.Comma) {
return last.type === TokenType.CloseBrace;
}
}
return false;
Expand All @@ -198,4 +262,10 @@ export class LineFormatter {
}
return false;
}
private keywordWithSpaceAfter(s: string): boolean {
return s === 'in' || s === 'return' || s === 'and' ||
s === 'or' || s === 'not' || s === 'from' ||
s === 'import' || s === 'except' || s === 'for' ||
s === 'as' || s === 'is';
}
}
Loading

0 comments on commit db9f6cf

Please sign in to comment.