Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Commit

Permalink
goImports: avoid collapsing new imports into pseudo import line (#3045)
Browse files Browse the repository at this point in the history
* goImports: avoid collapsing new imports into pseudo import line

getTextEditForAddImport tries to insert a newly added package
into an existing import group. If no import group exists, it
tries to merge single line import statements and create a group.

In this process, import statements like

  import "C"

are pseudo imports and shouldn't be grouped with other imports.
This CL changes parseFilePrelude to detect such pseudo imports
and excludes such imports from the grouping logic.

Fixes #1701

* correct the file name

mac and vsccode ignored casing, but linux and git were unhappy.
  • Loading branch information
hyangah authored Apr 27, 2020
1 parent f3dd04b commit c565b21
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 7 deletions.
10 changes: 6 additions & 4 deletions src/goImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ export function getTextEditForAddImport(arg: string): vscode.TextEdit[] {
}

const multis = imports.filter((x) => x.kind === 'multi');
const minusCgo = imports.filter((x) => x.kind !== 'pseudo');

if (multis.length > 0) {
// There is a multiple import declaration, add to the last one
const lastImportSection = multis[multis.length - 1];
Expand All @@ -92,12 +94,12 @@ export function getTextEditForAddImport(arg: string): vscode.TextEdit[] {
}
// Add import at the start of the block so that goimports/goreturns can order them correctly
return [vscode.TextEdit.insert(new vscode.Position(lastImportSection.start + 1, 0), '\t"' + arg + '"\n')];
} else if (imports.length > 0) {
} else if (minusCgo.length > 0) {
// There are some number of single line imports, which can just be collapsed into a block import.
const edits = [];

edits.push(vscode.TextEdit.insert(new vscode.Position(imports[0].start, 0), 'import (\n\t"' + arg + '"\n'));
imports.forEach((element) => {
edits.push(vscode.TextEdit.insert(new vscode.Position(minusCgo[0].start, 0), 'import (\n\t"' + arg + '"\n'));
minusCgo.forEach((element) => {
const currentLine = vscode.window.activeTextEditor.document.lineAt(element.start).text;
const updatedLine = currentLine.replace(/^\s*import\s*/, '\t');
edits.push(
Expand All @@ -107,7 +109,7 @@ export function getTextEditForAddImport(arg: string): vscode.TextEdit[] {
)
);
});
edits.push(vscode.TextEdit.insert(new vscode.Position(imports[imports.length - 1].end + 1, 0), ')\n'));
edits.push(vscode.TextEdit.insert(new vscode.Position(minusCgo[minusCgo.length - 1].end + 1, 0), ')\n'));

return edits;
} else if (pkg && pkg.start >= 0) {
Expand Down
7 changes: 4 additions & 3 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,12 @@ export function parseFilePrelude(text: string): Prelude {
}
if (line.match(/^(\s)*import(\s)+\(/)) {
ret.imports.push({ kind: 'multi', start: i, end: -1, pkgs: [] });
}
if (line.match(/^(\s)*import(\s)+[^\(]/)) {
} else if (line.match(/^\s*import\s+"C"/)) {
ret.imports.push({ kind: 'pseudo', start: i, end: i, pkgs: [] });
} else if (line.match(/^(\s)*import(\s)+[^\(]/)) {
ret.imports.push({ kind: 'single', start: i, end: i, pkgs: [] });
}
if (line.match(/^(\s)*(\/\*.*\*\/)*\s*\)/)) {
if (line.match(/^(\s)*(\/\*.*\*\/)*\s*\)/)) { // /* comments */
if (ret.imports[ret.imports.length - 1].end === -1) {
ret.imports[ret.imports.length - 1].end = i;
}
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/importTest/cgoImports.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package main

//#include <stdlib.h>
import "C"

import "math"

func main() {
_ = math.Max(1, 2)
}
22 changes: 22 additions & 0 deletions test/integration/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ suite('Go Extension Tests', function () {
path.join(fixtureSourcePath, 'importTest', 'singleImports.go'),
path.join(fixturePath, 'importTest', 'singleImports.go')
);
fs.copySync(
path.join(fixtureSourcePath, 'importTest', 'cgoImports.go'),
path.join(fixturePath, 'importTest', 'cgoImports.go')
);
fs.copySync(
path.join(fixtureSourcePath, 'fillStruct', 'input_1.go'),
path.join(fixturePath, 'fillStruct', 'input_1.go')
Expand Down Expand Up @@ -1590,6 +1594,24 @@ encountered.
assert.equal(vscode.window.activeTextEditor && vscode.window.activeTextEditor.document.getText(), expectedText);
});

test('Add imports and avoid pseudo package imports for cgo', async () => {
const uri = vscode.Uri.file(path.join(fixturePath, 'importTest', 'cgoImports.go'));
const document = await vscode.workspace.openTextDocument(uri);
await vscode.window.showTextDocument(document);

const expectedText = document
.getText()
.replace(
'import "math"',
'import (\n\t"bytes"\n\t"math"\n)'
);
const edits = getTextEditForAddImport('bytes');
const edit = new vscode.WorkspaceEdit();
edit.set(document.uri, edits);
await vscode.workspace.applyEdit(edit);
assert.equal(vscode.window.activeTextEditor && vscode.window.activeTextEditor.document.getText(), expectedText);
});

test('Fill struct', async () => {
const uri = vscode.Uri.file(path.join(fixturePath, 'fillStruct', 'input_1.go'));
const golden = fs.readFileSync(path.join(fixturePath, 'fillStruct', 'golden_1.go'), 'utf-8');
Expand Down

0 comments on commit c565b21

Please sign in to comment.