Fixes multiple issues with formatting on type (#1450)

* 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

* DonJayamanne#1257 On type formatting errors for args and kwargs

* Handle f-strings

* Stop importing from test code

* DonJayamanne#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]

package.json

@@ -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",

src/client/activation/analysis.ts

@@ -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> {

src/client/activation/analysisEngineHashes.ts

@@ -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';

src/client/activation/downloader.ts

@@ -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';
@@ -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> {
@@ -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}... `);
@@ -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.');

src/client/activation/platformData.ts

@@ -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 {
@@ -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 '';
+ }
 }

src/client/formatters/lineFormatter.ts

@@ -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;
@@ -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:
@@ -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) {
@@ -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();
@@ -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;
@@ -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';
+ }
 }