Skip to content

Commit db9f6cf

Browse files
author
Mikhail Arkhipov
authored
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]
1 parent 1465bda commit db9f6cf

13 files changed

Lines changed: 2089 additions & 152 deletions

File tree

‎package.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,12 @@
15811581
"description": "Automatically add brackets for functions.",
15821582
"scope": "resource"
15831583
},
1584+
"python.autoComplete.showAdvancedMembers": {
1585+
"type": "boolean",
1586+
"default": false,
1587+
"description": "Controls appearance of methods with double underscores in the completion list.",
1588+
"scope": "resource"
1589+
},
15841590
"python.workspaceSymbols.tagFilePath": {
15851591
"type": "string",
15861592
"default": "${workspaceFolder}/.vscode/tags",

‎src/client/activation/analysis.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
5757
this.appShell = this.services.get<IApplicationShell>(IApplicationShell);
5858
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
5959
this.fs = this.services.get<IFileSystem>(IFileSystem);
60-
this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService));
60+
this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService), this.fs);
6161
}
6262

6363
public async activate(context: ExtensionContext): Promise<boolean> {

‎src/client/activation/analysisEngineHashes.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@
33

44
// This file will be replaced by a generated one during the release build
55
// with actual hashes of the uploaded packages.
6-
export const analysis_engine_win_x86_sha512 = '';
7-
export const analysis_engine_win_x64_sha512 = '';
8-
export const analysis_engine_osx_x64_sha512 = '';
9-
export const analysis_engine_linux_x64_sha512 = '';
6+
// Values are for test purposes only
7+
export const analysis_engine_win_x86_sha512 = 'win-x86';
8+
export const analysis_engine_win_x64_sha512 = 'win-x64';
9+
export const analysis_engine_osx_x64_sha512 = 'osx-x64';
10+
export const analysis_engine_centos_x64_sha512 = 'centos-x64';
11+
export const analysis_engine_debian_x64_sha512 = 'debian-x64';
12+
export const analysis_engine_fedora_x64_sha512 = 'fedora-x64';
13+
export const analysis_engine_ol_x64_sha512 = 'ol-x64';
14+
export const analysis_engine_opensuse_x64_sha512 = 'opensuse-x64';
15+
export const analysis_engine_rhel_x64_sha512 = 'rhel-x64';
16+
export const analysis_engine_ubuntu_x64_sha512 = 'ubuntu-x64';

‎src/client/activation/downloader.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { ExtensionContext, OutputChannel, ProgressLocation, window } from 'vscod
99
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
1010
import { noop } from '../common/core.utils';
1111
import { createDeferred, createTemporaryFile } from '../common/helpers';
12-
import { IPlatformService } from '../common/platform/types';
12+
import { IFileSystem, IPlatformService } from '../common/platform/types';
1313
import { IOutputChannel } from '../common/types';
1414
import { IServiceContainer } from '../ioc/types';
1515
import { HashVerifier } from './hashVerifier';
@@ -31,7 +31,7 @@ export class AnalysisEngineDownloader {
3131
constructor(private readonly services: IServiceContainer, private engineFolder: string) {
3232
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
3333
this.platform = this.services.get<IPlatformService>(IPlatformService);
34-
this.platformData = new PlatformData(this.platform);
34+
this.platformData = new PlatformData(this.platform, this.services.get<IFileSystem>(IFileSystem));
3535
}
3636

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

5151
private async downloadFile(): Promise<string> {
52-
const platformString = this.platformData.getPlatformDesignator();
52+
const platformString = await this.platformData.getPlatformName();
5353
const remoteFileName = `${downloadBaseFileName}-${platformString}.${downloadVersion}${downloadFileExtension}`;
5454
const uri = `${downloadUriPrefix}/${remoteFileName}`;
5555
this.output.append(`Downloading ${uri}... `);
@@ -98,7 +98,7 @@ export class AnalysisEngineDownloader {
9898
this.output.appendLine('');
9999
this.output.append('Verifying download... ');
100100
const verifier = new HashVerifier();
101-
if (!await verifier.verifyHash(filePath, this.platformData.getExpectedHash())) {
101+
if (!await verifier.verifyHash(filePath, await this.platformData.getExpectedHash())) {
102102
throw new Error('Hash of the downloaded file does not match.');
103103
}
104104
this.output.append('valid.');

‎src/client/activation/platformData.ts‎

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,54 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { IPlatformService } from '../common/platform/types';
4+
import { IFileSystem, IPlatformService } from '../common/platform/types';
55
import {
6-
analysis_engine_linux_x64_sha512,
6+
analysis_engine_centos_x64_sha512,
7+
analysis_engine_debian_x64_sha512,
8+
analysis_engine_fedora_x64_sha512,
9+
analysis_engine_ol_x64_sha512,
10+
analysis_engine_opensuse_x64_sha512,
711
analysis_engine_osx_x64_sha512,
12+
analysis_engine_rhel_x64_sha512,
13+
analysis_engine_ubuntu_x64_sha512,
814
analysis_engine_win_x64_sha512,
915
analysis_engine_win_x86_sha512
1016
} from './analysisEngineHashes';
1117

18+
// '/etc/os-release', ID=flavor
19+
const supportedLinuxFlavors = [
20+
'centos',
21+
'debian',
22+
'fedora',
23+
'ol',
24+
'opensuse',
25+
'rhel',
26+
'ubuntu'
27+
];
28+
1229
export class PlatformData {
13-
constructor(private platform: IPlatformService) { }
14-
public getPlatformDesignator(): string {
30+
constructor(private platform: IPlatformService, private fs: IFileSystem) { }
31+
public async getPlatformName(): Promise<string> {
1532
if (this.platform.isWindows) {
1633
return this.platform.is64bit ? 'win-x64' : 'win-x86';
1734
}
1835
if (this.platform.isMac) {
1936
return 'osx-x64';
2037
}
21-
if (this.platform.isLinux && this.platform.is64bit) {
22-
return 'linux-x64';
38+
if (this.platform.isLinux) {
39+
if (!this.platform.is64bit) {
40+
throw new Error('Python Analysis Engine does not support 32-bit Linux.');
41+
}
42+
const linuxFlavor = await this.getLinuxFlavor();
43+
if (linuxFlavor.length === 0) {
44+
throw new Error('Unable to determine Linux flavor from /etc/os-release.');
45+
}
46+
if (supportedLinuxFlavors.indexOf(linuxFlavor) < 0) {
47+
throw new Error(`${linuxFlavor} is not supported.`);
48+
}
49+
return `${linuxFlavor}-x64`;
2350
}
24-
throw new Error('Python Analysis Engine does not support 32-bit Linux.');
51+
throw new Error('Unknown OS platform.');
2552
}
2653

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

37-
public getExpectedHash(): string {
64+
public async getExpectedHash(): Promise<string> {
3865
if (this.platform.isWindows) {
3966
return this.platform.is64bit ? analysis_engine_win_x64_sha512 : analysis_engine_win_x86_sha512;
4067
}
4168
if (this.platform.isMac) {
4269
return analysis_engine_osx_x64_sha512;
4370
}
4471
if (this.platform.isLinux && this.platform.is64bit) {
45-
return analysis_engine_linux_x64_sha512;
72+
const linuxFlavor = await this.getLinuxFlavor();
73+
// tslint:disable-next-line:switch-default
74+
switch (linuxFlavor) {
75+
case 'centos': return analysis_engine_centos_x64_sha512;
76+
case 'debian': return analysis_engine_debian_x64_sha512;
77+
case 'fedora': return analysis_engine_fedora_x64_sha512;
78+
case 'ol': return analysis_engine_ol_x64_sha512;
79+
case 'opensuse': return analysis_engine_opensuse_x64_sha512;
80+
case 'rhel': return analysis_engine_rhel_x64_sha512;
81+
case 'ubuntu': return analysis_engine_ubuntu_x64_sha512;
82+
}
4683
}
4784
throw new Error('Unknown platform.');
4885
}
86+
87+
private async getLinuxFlavor(): Promise<string> {
88+
const verFile = '/etc/os-release';
89+
const data = await this.fs.readFile(verFile);
90+
if (data) {
91+
const res = /ID=(.*)/.exec(data);
92+
if (res && res.length > 1) {
93+
return res[1];
94+
}
95+
}
96+
return '';
97+
}
4998
}

‎src/client/formatters/lineFormatter.ts‎

Lines changed: 101 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export class LineFormatter {
4343

4444
case TokenType.Comma:
4545
this.builder.append(',');
46-
if (next && !this.isCloseBraceType(next.type)) {
46+
if (next && !this.isCloseBraceType(next.type) && next.type !== TokenType.Colon) {
4747
this.builder.softAppendSpace();
4848
}
4949
break;
@@ -52,7 +52,12 @@ export class LineFormatter {
5252
if (prev && !this.isOpenBraceType(prev.type) && prev.type !== TokenType.Colon && prev.type !== TokenType.Operator) {
5353
this.builder.softAppendSpace();
5454
}
55-
this.builder.append(this.text.substring(t.start, t.end));
55+
const id = this.text.substring(t.start, t.end);
56+
this.builder.append(id);
57+
if (this.keywordWithSpaceAfter(id) && next && this.isOpenBraceType(next.type)) {
58+
// for x in ()
59+
this.builder.softAppendSpace();
60+
}
5661
break;
5762

5863
case TokenType.Colon:
@@ -84,8 +89,10 @@ export class LineFormatter {
8489
return this.builder.getText();
8590
}
8691

92+
// tslint:disable-next-line:cyclomatic-complexity
8793
private handleOperator(index: number): void {
8894
const t = this.tokens.getItemAt(index);
95+
const prev = index > 0 ? this.tokens.getItemAt(index - 1) : undefined;
8996
if (t.length === 1) {
9097
const opCode = this.text.charCodeAt(t.start);
9198
switch (opCode) {
@@ -99,18 +106,36 @@ export class LineFormatter {
99106
case Char.ExclamationMark:
100107
this.builder.append(this.text[t.start]);
101108
return;
109+
case Char.Asterisk:
110+
if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') {
111+
this.builder.softAppendSpace();
112+
this.builder.append('*');
113+
return;
114+
}
115+
break;
102116
default:
103117
break;
104118
}
119+
} else if (t.length === 2) {
120+
if (this.text.charCodeAt(t.start) === Char.Asterisk && this.text.charCodeAt(t.start + 1) === Char.Asterisk) {
121+
if (!prev || (prev.type !== TokenType.Identifier && prev.type !== TokenType.Number)) {
122+
this.builder.append('**');
123+
return;
124+
}
125+
if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') {
126+
this.builder.softAppendSpace();
127+
this.builder.append('**');
128+
return;
129+
}
130+
}
105131
}
132+
106133
// Do not append space if operator is preceded by '(' or ',' as in foo(**kwarg)
107-
if (index > 0) {
108-
const prev = this.tokens.getItemAt(index - 1);
109-
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma) {
110-
this.builder.append(this.text.substring(t.start, t.end));
111-
return;
112-
}
134+
if (prev && (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma)) {
135+
this.builder.append(this.text.substring(t.start, t.end));
136+
return;
113137
}
138+
114139
this.builder.softAppendSpace();
115140
this.builder.append(this.text.substring(t.start, t.end));
116141
this.builder.softAppendSpace();
@@ -135,43 +160,82 @@ export class LineFormatter {
135160
return;
136161
}
137162

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

144-
if (index > 0) {
145-
const prev = this.tokens.getItemAt(index - 1);
146-
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon) {
147-
// Don't insert space after (, [ or { .
148-
this.builder.append(this.text.substring(t.start, t.end));
149-
return;
150-
}
170+
if (prev && (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon)) {
171+
// Don't insert space after (, [ or { .
172+
this.builder.append(this.text.substring(t.start, t.end));
173+
return;
151174
}
152175

153-
// In general, keep tokens separated.
154-
this.builder.softAppendSpace();
155-
this.builder.append(this.text.substring(t.start, t.end));
176+
if (t.type === TokenType.Unknown) {
177+
this.handleUnknown(t);
178+
} else {
179+
// In general, keep tokens separated.
180+
this.builder.softAppendSpace();
181+
this.builder.append(this.text.substring(t.start, t.end));
182+
}
156183
}
157184

185+
private handleUnknown(t: IToken): void {
186+
const prevChar = t.start > 0 ? this.text.charCodeAt(t.start - 1) : 0;
187+
if (prevChar === Char.Space || prevChar === Char.Tab) {
188+
this.builder.softAppendSpace();
189+
}
190+
this.builder.append(this.text.substring(t.start, t.end));
191+
192+
const nextChar = t.end < this.text.length - 1 ? this.text.charCodeAt(t.end) : 0;
193+
if (nextChar === Char.Space || nextChar === Char.Tab) {
194+
this.builder.softAppendSpace();
195+
}
196+
}
158197
private isEqualsInsideArguments(index: number): boolean {
198+
// Since we don't have complete statement, this is mostly heuristics.
199+
// Therefore the code may not be handling all possible ways of the
200+
// argument list continuation.
159201
if (index < 1) {
160202
return false;
161203
}
204+
162205
const prev = this.tokens.getItemAt(index - 1);
163-
if (prev.type === TokenType.Identifier) {
164-
if (index >= 2) {
165-
// (x=1 or ,x=1
166-
const prevPrev = this.tokens.getItemAt(index - 2);
167-
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
168-
} else if (index < this.tokens.count - 2) {
169-
const next = this.tokens.getItemAt(index + 1);
170-
const nextNext = this.tokens.getItemAt(index + 2);
171-
// x=1, or x=1)
172-
if (this.isValueType(next.type)) {
173-
return nextNext.type === TokenType.Comma || nextNext.type === TokenType.CloseBrace;
174-
}
206+
if (prev.type !== TokenType.Identifier) {
207+
return false;
208+
}
209+
210+
const first = this.tokens.getItemAt(0);
211+
if (first.type === TokenType.Comma) {
212+
return true; // Line starts with commma
213+
}
214+
215+
const last = this.tokens.getItemAt(this.tokens.count - 1);
216+
if (last.type === TokenType.Comma) {
217+
return true; // Line ends in comma
218+
}
219+
220+
if (index >= 2) {
221+
// (x=1 or ,x=1
222+
const prevPrev = this.tokens.getItemAt(index - 2);
223+
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
224+
}
225+
226+
if (index >= this.tokens.count - 2) {
227+
return false;
228+
}
229+
230+
const next = this.tokens.getItemAt(index + 1);
231+
const nextNext = this.tokens.getItemAt(index + 2);
232+
// x=1, or x=1)
233+
if (this.isValueType(next.type)) {
234+
if (nextNext.type === TokenType.CloseBrace) {
235+
return true;
236+
}
237+
if (nextNext.type === TokenType.Comma) {
238+
return last.type === TokenType.CloseBrace;
175239
}
176240
}
177241
return false;
@@ -198,4 +262,10 @@ export class LineFormatter {
198262
}
199263
return false;
200264
}
265+
private keywordWithSpaceAfter(s: string): boolean {
266+
return s === 'in' || s === 'return' || s === 'and' ||
267+
s === 'or' || s === 'not' || s === 'from' ||
268+
s === 'import' || s === 'except' || s === 'for' ||
269+
s === 'as' || s === 'is';
270+
}
201271
}

0 commit comments

Comments
 (0)