Skip to content

Commit d83ccda

Browse files
authored
Merge pull request #1 from GPTI314/claude/review-enhance-optimization-017dXcB4PEKc8xx7PtGYq5nm
Claude/review enhance optimization 017d xc b4 pe kc8xx7 pt g yq5nm
2 parents 56b6e0b + c2fa461 commit d83ccda

22 files changed

+271
-177
lines changed

extension/src/debugAdapter/goDebug.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ export function normalizeSeparators(filePath: string): string {
406406
// casing.
407407
// This is a workaround for issue in https://github.com/Microsoft/vscode/issues/9448#issuecomment-244804026
408408
if (filePath.indexOf(':') === 1) {
409-
filePath = filePath.substr(0, 1).toUpperCase() + filePath.substr(1);
409+
filePath = filePath.substring(0, 1).toUpperCase() + filePath.substring(1);
410410
}
411411
return filePath.replace(/\/|\\/g, '/');
412412
}
@@ -634,7 +634,7 @@ export class Delve {
634634
if (mode === 'exec' || (mode === 'debug' && !isProgramDirectory)) {
635635
dlvArgs.push(program);
636636
} else if (currentGOWorkspace && !launchArgs.packagePathToGoModPathMap[dirname]) {
637-
dlvArgs.push(dirname.substr(currentGOWorkspace.length + 1));
637+
dlvArgs.push(dirname.substring(currentGOWorkspace.length + 1));
638638
}
639639
// add user-specified dlv flags first. When duplicate flags are specified,
640640
// dlv doesn't mind but accepts the last flag value.
@@ -1212,7 +1212,7 @@ export class GoDebugSession extends LoggingDebugSession {
12121212
}
12131213

12141214
const relativeRemotePath = remotePath
1215-
.substr(importPathIndex)
1215+
.substring(importPathIndex)
12161216
.split(this.remotePathSeparator)
12171217
.join(this.localPathSeparator);
12181218
const pathToConvertWithLocalSeparator = remotePath
@@ -1253,7 +1253,7 @@ export class GoDebugSession extends LoggingDebugSession {
12531253
const goroot = this.getGOROOT();
12541254
const localGoRootImportPath = path.join(
12551255
goroot,
1256-
srcIndex >= 0 ? remotePathWithLocalSeparator.substr(srcIndex) : path.join('src', relativeRemotePath)
1256+
srcIndex >= 0 ? remotePathWithLocalSeparator.substring(srcIndex) : path.join('src', relativeRemotePath)
12571257
);
12581258
if (this.fileSystem.existsSync(localGoRootImportPath)) {
12591259
return localGoRootImportPath;
@@ -1281,7 +1281,7 @@ export class GoDebugSession extends LoggingDebugSession {
12811281
const localGoPathImportPath = path.join(
12821282
gopath,
12831283
indexGoModCache >= 0
1284-
? remotePathWithLocalSeparator.substr(indexGoModCache)
1284+
? remotePathWithLocalSeparator.substring(indexGoModCache)
12851285
: path.join('pkg', 'mod', relativeRemotePath)
12861286
);
12871287
if (this.fileSystem.existsSync(localGoPathImportPath)) {
@@ -1340,7 +1340,7 @@ export class GoDebugSession extends LoggingDebugSession {
13401340
const index = pathToConvert.indexOf(`${this.remotePathSeparator}src${this.remotePathSeparator}`);
13411341
const goroot = this.getGOROOT();
13421342
if (goroot && index > 0) {
1343-
return path.join(goroot, pathToConvert.substr(index));
1343+
return path.join(goroot, pathToConvert.substring(index));
13441344
}
13451345

13461346
const indexGoModCache = pathToConvert.indexOf(
@@ -1352,7 +1352,7 @@ export class GoDebugSession extends LoggingDebugSession {
13521352
return path.join(
13531353
gopath,
13541354
pathToConvert
1355-
.substr(indexGoModCache)
1355+
.substring(indexGoModCache)
13561356
.split(this.remotePathSeparator ?? '')
13571357
.join(this.localPathSeparator)
13581358
);
@@ -1647,7 +1647,7 @@ export class GoDebugSession extends LoggingDebugSession {
16471647
: (<ListVarsOut>listPkgVarsOut).Variables;
16481648
let initdoneIndex = -1;
16491649
for (let i = 0; i < globals.length; i++) {
1650-
globals[i].name = globals[i].name.substr(packageName.length + 1);
1650+
globals[i].name = globals[i].name.substring(packageName.length + 1);
16511651
if (initdoneIndex === -1 && globals[i].name === this.initdone) {
16521652
initdoneIndex = i;
16531653
}
@@ -2308,7 +2308,7 @@ export class GoDebugSession extends LoggingDebugSession {
23082308
return resolve();
23092309
}
23102310
const spaceIndex = stdout.indexOf(' ');
2311-
const result = stdout.substr(0, spaceIndex) === 'main' ? 'main' : stdout.substr(spaceIndex).trim();
2311+
const result = stdout.substring(0, spaceIndex) === 'main' ? 'main' : stdout.substring(spaceIndex).trim();
23122312
this.packageInfo.set(dir, result);
23132313
resolve(result);
23142314
}

extension/src/diffUtils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ function parseUniDiffs(diffOutput: jsDiff.IUniDiff[]): FilePatch[] {
9595
uniDiff.hunks.forEach((hunk: jsDiff.IHunk) => {
9696
let startLine = hunk.oldStart;
9797
hunk.lines.forEach((line) => {
98-
switch (line.substr(0, 1)) {
98+
switch (line.substring(0, 1)) {
9999
case '-':
100100
edit = new Edit(EditTypes.EDIT_DELETE, new Position(startLine - 1, 0));
101101
edit.end = new Position(startLine, 0);
@@ -104,7 +104,7 @@ function parseUniDiffs(diffOutput: jsDiff.IUniDiff[]): FilePatch[] {
104104
break;
105105
case '+':
106106
edit = new Edit(EditTypes.EDIT_INSERT, new Position(startLine - 1, 0));
107-
edit.text += line.substr(1) + '\n';
107+
edit.text += line.substring(1) + '\n';
108108
edits.push(edit);
109109
break;
110110
case ' ':

extension/src/goBuild.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export async function goBuild(
153153
if (!isMod) {
154154
// Find the right importPath instead of directly using `.`. Fixes https://github.com/Microsoft/vscode-go/issues/846
155155
if (currentGoWorkspace && !isMod) {
156-
importPath = cwd.substr(currentGoWorkspace.length + 1);
156+
importPath = cwd.substring(currentGoWorkspace.length + 1);
157157
} else {
158158
outputChannel.error(
159159
`Not able to determine import path of current package by using cwd: ${cwd} and Go workspace: ${currentGoWorkspace}`

extension/src/goCover.ts

Lines changed: 82 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -214,95 +214,91 @@ function clearCoverage() {
214214
* @param packageDirPath Absolute path of the package for which the coverage was calculated
215215
* @param dir Directory to execute go list in
216216
*/
217-
export function applyCodeCoverageToAllEditors(coverProfilePath: string, dir?: string): Promise<void> {
218-
const v = new Promise<void>((resolve, reject) => {
219-
try {
220-
const showCounts = getGoConfig().get('coverShowCounts') as boolean;
221-
const coveragePath = new Map<string, CoverageData>(); // <filename> from the cover profile to the coverage data.
222-
223-
// Clear existing coverage files
224-
clearCoverage();
225-
226-
// collect the packages named in the coverage file
227-
const seenPaths = new Set<string>();
228-
// for now read synchronously and hope for no errors
229-
const contents = fs.readFileSync(coverProfilePath).toString();
230-
contents.split('\n').forEach((line) => {
231-
// go test coverageprofile generates output:
232-
// filename:StartLine.StartColumn,EndLine.EndColumn Hits CoverCount
233-
// where the filename is either the import path + '/' + base file name, or
234-
// the actual file path (either absolute or starting with .)
235-
// See https://golang.org/issues/40251.
236-
//
237-
// The first line will be like "mode: set" which we will ignore.
238-
// TODO: port https://golang.org/cl/179377 for faster parsing.
239-
240-
const parse = line.match(/^(\S+)\:(\d+)\.(\d+)\,(\d+)\.(\d+)\s(\d+)\s(\d+)/);
241-
if (!parse) {
242-
return;
243-
}
217+
export async function applyCodeCoverageToAllEditors(coverProfilePath: string, dir?: string): Promise<void> {
218+
try {
219+
const showCounts = getGoConfig().get('coverShowCounts') as boolean;
220+
const coveragePath = new Map<string, CoverageData>(); // <filename> from the cover profile to the coverage data.
244221

245-
let filename = parse[1];
246-
if (filename.startsWith('.' + path.sep)) {
247-
// If it's a relative file path, convert it to an absolute path.
248-
// From now on, we can assume that it's a real file name if it is
249-
// an absolute path.
250-
filename = path.resolve(filename);
251-
}
252-
// If this is not a real file name, that's package_path + file name,
253-
// Record it in seenPaths for `go list` call to resolve package path ->
254-
// directory mapping.
255-
if (!path.isAbsolute(filename)) {
256-
const lastSlash = filename.lastIndexOf('/');
257-
if (lastSlash !== -1) {
258-
seenPaths.add(filename.slice(0, lastSlash));
259-
}
260-
}
222+
// Clear existing coverage files
223+
clearCoverage();
261224

262-
// and fill in coveragePath
263-
const coverage = coveragePath.get(parse[1]) || emptyCoverageData();
264-
// When line directive is used this information is artificial and
265-
// the source code file can be non-existent or wrong (go.dev/issues/41222).
266-
// There is no perfect way to guess whether the line/col in coverage profile
267-
// is bogus. At least, we know that 0 or negative values are not true line/col.
268-
const startLine = parseInt(parse[2], 10);
269-
const startCol = parseInt(parse[3], 10);
270-
const endLine = parseInt(parse[4], 10);
271-
const endCol = parseInt(parse[5], 10);
272-
if (startLine < 1 || startCol < 1 || endLine < 1 || endCol < 1) {
273-
return;
274-
}
275-
const range = new vscode.Range(
276-
// Convert lines and columns to 0-based
277-
startLine - 1,
278-
startCol - 1,
279-
endLine - 1,
280-
endCol - 1
281-
);
282-
283-
const counts = parseInt(parse[7], 10);
284-
// If is Covered (CoverCount > 0)
285-
if (counts > 0) {
286-
coverage.coveredOptions.push(...elaborate(range, counts, showCounts));
287-
} else {
288-
coverage.uncoveredOptions.push(...elaborate(range, counts, showCounts));
225+
// collect the packages named in the coverage file
226+
const seenPaths = new Set<string>();
227+
// Read coverage file asynchronously to avoid blocking the UI
228+
const contents = await fs.promises.readFile(coverProfilePath, 'utf8');
229+
contents.split('\n').forEach((line) => {
230+
// go test coverageprofile generates output:
231+
// filename:StartLine.StartColumn,EndLine.EndColumn Hits CoverCount
232+
// where the filename is either the import path + '/' + base file name, or
233+
// the actual file path (either absolute or starting with .)
234+
// See https://golang.org/issues/40251.
235+
//
236+
// The first line will be like "mode: set" which we will ignore.
237+
// TODO: port https://golang.org/cl/179377 for faster parsing.
238+
239+
const parse = line.match(/^(\S+)\:(\d+)\.(\d+)\,(\d+)\.(\d+)\s(\d+)\s(\d+)/);
240+
if (!parse) {
241+
return;
242+
}
243+
244+
let filename = parse[1];
245+
if (filename.startsWith('.' + path.sep)) {
246+
// If it's a relative file path, convert it to an absolute path.
247+
// From now on, we can assume that it's a real file name if it is
248+
// an absolute path.
249+
filename = path.resolve(filename);
250+
}
251+
// If this is not a real file name, that's package_path + file name,
252+
// Record it in seenPaths for `go list` call to resolve package path ->
253+
// directory mapping.
254+
if (!path.isAbsolute(filename)) {
255+
const lastSlash = filename.lastIndexOf('/');
256+
if (lastSlash !== -1) {
257+
seenPaths.add(filename.slice(0, lastSlash));
289258
}
259+
}
290260

291-
coveragePath.set(filename, coverage);
292-
});
293-
294-
getImportPathToFolder([...seenPaths], dir).then((pathsToDirs) => {
295-
createCoverageData(pathsToDirs, coveragePath);
296-
setDecorators();
297-
vscode.window.visibleTextEditors.forEach(applyCodeCoverage);
298-
resolve();
299-
});
300-
} catch (e) {
301-
vscode.window.showInformationMessage((e as any).msg);
302-
reject(e);
303-
}
304-
});
305-
return v;
261+
// and fill in coveragePath
262+
const coverage = coveragePath.get(parse[1]) || emptyCoverageData();
263+
// When line directive is used this information is artificial and
264+
// the source code file can be non-existent or wrong (go.dev/issues/41222).
265+
// There is no perfect way to guess whether the line/col in coverage profile
266+
// is bogus. At least, we know that 0 or negative values are not true line/col.
267+
const startLine = parseInt(parse[2], 10);
268+
const startCol = parseInt(parse[3], 10);
269+
const endLine = parseInt(parse[4], 10);
270+
const endCol = parseInt(parse[5], 10);
271+
if (startLine < 1 || startCol < 1 || endLine < 1 || endCol < 1) {
272+
return;
273+
}
274+
const range = new vscode.Range(
275+
// Convert lines and columns to 0-based
276+
startLine - 1,
277+
startCol - 1,
278+
endLine - 1,
279+
endCol - 1
280+
);
281+
282+
const counts = parseInt(parse[7], 10);
283+
// If is Covered (CoverCount > 0)
284+
if (counts > 0) {
285+
coverage.coveredOptions.push(...elaborate(range, counts, showCounts));
286+
} else {
287+
coverage.uncoveredOptions.push(...elaborate(range, counts, showCounts));
288+
}
289+
290+
coveragePath.set(filename, coverage);
291+
});
292+
293+
const pathsToDirs = await getImportPathToFolder([...seenPaths], dir);
294+
createCoverageData(pathsToDirs, coveragePath);
295+
setDecorators();
296+
vscode.window.visibleTextEditors.forEach(applyCodeCoverage);
297+
} catch (e) {
298+
const errorMsg = (e as any).msg || String(e);
299+
vscode.window.showInformationMessage(errorMsg);
300+
throw e;
301+
}
306302
}
307303

308304
// add decorations to the range
@@ -356,7 +352,7 @@ function createCoverageData(pathsToDirs: Map<string, string>, coveragePath: Map<
356352
*/
357353
function setCoverageDataByFilePath(filePath: string, data: CoverageData) {
358354
if (filePath.startsWith('_')) {
359-
filePath = filePath.substr(1);
355+
filePath = filePath.substring(1);
360356
}
361357
if (process.platform === 'win32') {
362358
const parts = filePath.split('/');

extension/src/goDebugConfiguration.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -394,14 +394,27 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
394394
const child = spawn(getBinPath('dlv'), ['substitute-path-guess-helper']);
395395
let stdoutData = '';
396396
let stderrData = '';
397-
child.stdout.on('data', (data) => {
397+
398+
const stdoutHandler = (data: Buffer) => {
398399
stdoutData += data;
399-
});
400-
child.stderr.on('data', (data) => {
400+
};
401+
const stderrHandler = (data: Buffer) => {
401402
stderrData += data;
402-
});
403+
};
404+
405+
// Cleanup function to remove all listeners
406+
const cleanup = () => {
407+
child.stdout?.removeListener('data', stdoutHandler);
408+
child.stderr?.removeListener('data', stderrHandler);
409+
child.removeAllListeners('close');
410+
child.removeAllListeners('error');
411+
};
412+
413+
child.stdout.on('data', stdoutHandler);
414+
child.stderr.on('data', stderrHandler);
403415

404416
child.on('close', (code) => {
417+
cleanup();
405418
if (code !== 0) {
406419
resolve(null);
407420
} else {
@@ -414,6 +427,7 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
414427
});
415428

416429
child.on('error', (error) => {
430+
cleanup();
417431
resolve(null);
418432
});
419433
});

extension/src/goGenerateTests.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ export const toggleTestFile: CommandFactory = () => () => {
6161
}
6262
let targetFilePath = '';
6363
if (currentFilePath.endsWith('_test.go')) {
64-
targetFilePath = currentFilePath.substr(0, currentFilePath.lastIndexOf('_test.go')) + '.go';
64+
targetFilePath = currentFilePath.substring(0, currentFilePath.lastIndexOf('_test.go')) + '.go';
6565
} else {
66-
targetFilePath = currentFilePath.substr(0, currentFilePath.lastIndexOf('.go')) + '_test.go';
66+
targetFilePath = currentFilePath.substring(0, currentFilePath.lastIndexOf('.go')) + '_test.go';
6767
}
6868
for (const doc of vscode.window.visibleTextEditors) {
6969
if (doc.document.fileName === targetFilePath) {
@@ -269,7 +269,7 @@ function generateTests(
269269
return element.startsWith(generatedWord);
270270
})
271271
.map((element) => {
272-
return element.substr(generatedWord.length);
272+
return element.substring(generatedWord.length);
273273
});
274274
message = `Generated ${lines.join(', ')}`;
275275
testsGenerated = true;

extension/src/goImport.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ async function askUserForImport(goCtx: GoExtensionContext): Promise<string | und
5555
return vscode.window.showQuickPick(packages);
5656
} catch (err) {
5757
if (typeof err === 'string' && err.startsWith(missingToolMsg)) {
58-
promptForMissingTool(err.substr(missingToolMsg.length));
58+
promptForMissingTool(err.substring(missingToolMsg.length));
5959
}
6060
}
6161
}

extension/src/goInstall.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export const installCurrentPackage: CommandFactory = () => async () => {
5555

5656
// Find the right importPath instead of directly using `.`. Fixes https://github.com/Microsoft/vscode-go/issues/846
5757
const currentGoWorkspace = getCurrentGoWorkspaceFromGOPATH(getCurrentGoPath(), cwd);
58-
const importPath = currentGoWorkspace && !isMod ? cwd.substr(currentGoWorkspace.length + 1) : '.';
58+
const importPath = currentGoWorkspace && !isMod ? cwd.substring(currentGoWorkspace.length + 1) : '.';
5959
args.push(importPath);
6060

6161
outputChannel.info(`Installing ${importPath === '.' ? 'current package' : importPath}`);

extension/src/goLint.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ export async function goLint(
101101
return;
102102
}
103103
if (flag.startsWith('--config=') || flag.startsWith('-config=')) {
104-
let configFilePath = flag.substr(flag.indexOf('=') + 1).trim();
104+
let configFilePath = flag.substring(flag.indexOf('=') + 1).trim();
105105
if (!configFilePath) {
106106
return;
107107
}
108108
configFilePath = resolvePath(configFilePath);
109-
args.push(`${flag.substr(0, flag.indexOf('=') + 1)}${configFilePath}`);
109+
args.push(`${flag.substring(0, flag.indexOf('=') + 1)}${configFilePath}`);
110110
return;
111111
}
112112
args.push(flag);

0 commit comments

Comments
 (0)