From ee2bc8be3a1239911a5b56f69078ca50a1c3ed5a Mon Sep 17 00:00:00 2001 From: Peter Weinberger Date: Wed, 6 Apr 2022 10:43:05 -0400 Subject: [PATCH] go/ast/astutil: fix panic in DeleteNamedImport from line directive DeleteNamedImport used FileSet.Position() where it should have used FileSet.PositionFor(, false) to ignore line directives. There are likely many more of these panics lurking in x/tools. Change-Id: I11fc0ea5301fe873cb679a82e231b1720dbdaef6 Reviewed-on: https://go-review.googlesource.com/c/tools/+/398596 Run-TryBot: Peter Weinberger gopls-CI: kokoro Trust: Peter Weinberger Reviewed-by: Heschi Kreinick --- go/ast/astutil/imports.go | 4 ++-- go/ast/astutil/imports_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/go/ast/astutil/imports.go b/go/ast/astutil/imports.go index 2087ceec9cf..f7413ce19d3 100644 --- a/go/ast/astutil/imports.go +++ b/go/ast/astutil/imports.go @@ -270,8 +270,8 @@ func DeleteNamedImport(fset *token.FileSet, f *ast.File, name, path string) (del } if j > 0 { lastImpspec := gen.Specs[j-1].(*ast.ImportSpec) - lastLine := fset.Position(lastImpspec.Path.ValuePos).Line - line := fset.Position(impspec.Path.ValuePos).Line + lastLine := fset.PositionFor(lastImpspec.Path.ValuePos, false).Line + line := fset.PositionFor(impspec.Path.ValuePos, false).Line // We deleted an entry but now there may be // a blank line-sized hole where the import was. diff --git a/go/ast/astutil/imports_test.go b/go/ast/astutil/imports_test.go index 68f05ab6d92..2a383e467b7 100644 --- a/go/ast/astutil/imports_test.go +++ b/go/ast/astutil/imports_test.go @@ -1654,6 +1654,34 @@ import f "fmt" `, unchanged: true, }, + // this test panics without PositionFor in DeleteNamedImport + { + name: "import.44", + pkg: "foo.com/other/v3", + renamedPkg: "", + in: `package main +//line mah.go:600 + +import ( +"foo.com/a.thing" +"foo.com/surprise" +"foo.com/v1" +"foo.com/other/v2" +"foo.com/other/v3" +) +`, + out: `package main + +//line mah.go:600 + +import ( + "foo.com/a.thing" + "foo.com/other/v2" + "foo.com/surprise" + "foo.com/v1" +) +`, + }, } func TestDeleteImport(t *testing.T) {