This change makes AddNamedImport more symmetrical to DeleteNamedImport
in behavior, signature, and documentation.
In Go, it is valid to import a package (identified by an import path)
with different names in a single .go file¹. For example, this is ok,
and the program will build as long as there are no unused imports²:
import (
"path"
. "path"
_ "path"
pathpkg "path"
)
The four imports (represented by ast.ImportSpec type) have the same
import path, but different names.
It's currently possible to use DeleteNamedImport to delete the exact
{name, import path} ast.ImportSpec from an AST.
Previously, it wasn't possible to use AddNamedImport to add an exact
{name, import path} ast.ImportSpec if the AST already has another
ast.ImportSpec with the same import path (but different name).
This change fixes that, making it possible to use AddNamedImport as
expected.
Rename the ipath parameter to path in AddNamedImport for consistency
with DeleteNamedImport. Also change the language used in its
documentation to be more precise and consistent with DeleteNamedImport.
Add test cases for this behavior. The DeleteNamedImport test cases
were passing before any changes to the astutil code.
Add test coverage for the return bool value of {Add,Delete}NamedImport.
Document the behavior of DeleteNamedImport when there are duplicate
matching imports.
Simplify string(buf.Bytes()) to buf.String() in test helper code.
¹ https://golang.org/ref/spec#Import_declarations
² https://play.golang.org/p/bi6L34rQbcDFixesgolang/go#28605Fixesgolang/go#16411
Change-Id: I70e887f5174764ab1cf32a761c8c734e216eea67
Reviewed-on: https://go-review.googlesource.com/c/147448
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This new package provides helper functions for traversal over the
syntax trees of a package, including node filtering by type, and
materialization of the traversal stack.
During construction, the inspector does a complete traversal and
builds a list of push/pop events and their node type. Subsequent
method calls that request a traversal scan this list, rather than walk
the AST, and perform type filtering using efficient bit sets.
Experiments suggest the inspector's traversals are about 2.5x faster
than ast.Inspect, but it may take around 5 traversals for this benefit
to amortize the inspector's construction cost.
This design is well-suited to the ongoing reworking of cmd/vet (see
docs.google.com/document/d/1-azPLXaLgTCKeKDNg0HVMq2ovMlD-e7n1ZHzZVzOlJk),
which historically made a single pass over the ASTs but is being
replaced by a design that requires a separate pass for each analysis.
Change-Id: I9a67aed6a3bf948076641d96447860d97ede67b4
Reviewed-on: https://go-review.googlesource.com/135655
Reviewed-by: Robert Griesemer <gri@golang.org>
This change implements an AST walker, Apply, with the capability
to perform various actions (including rewrites) during the AST
walk, in pre- and post-iteration order.
Ideally we would like to have this functionality in the std lib
(go/ast), but as this change contains significant new API, it
is better to first gain some experience with it in x/tools.
Credits: This is joint work of Josh Bleecher Snyder, Roger
Peppe, and myself (author of this CL). The code is based on
proposal golang/go#17108 which I had started, together with an
initial implementation. It was then reworked significantly by
Josh in golang.org/cl/55790, further refined by suggestions
from Roger (allocation reduction), and finally cleaned up a bit
by me (AST simplifications).
Fixesgolang/go#17108.
Change-Id: I56d56b7f2bc5be2acfeb927f76aea7f264bb7b94
Reviewed-on: https://go-review.googlesource.com/77811
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Before this change, astutil would only do a prefix match of a new import
with all the existing ones, to try to place it in the correct group. If
none was found, the new import would be placed at the beginning of the
first import group.
This works well for new std imports, but it doesn't work well for new
third-party packages that don't share any prefix with any of the
existing imports.
Example:
import (
"time"
"github.com/golang/snappy"
)
When adding "golang.org/x/sys/unix" with astutil.AddImport, the import
is inserted as follows:
import (
"golang.org/x/sys/unix"
"time"
"github.com/golang/snappy"
)
And goimports reorganizes the imports to separate std and third-party
packages:
import (
"time"
"golang.org/x/sys/unix"
"github.com/golang/snappy"
)
We usually don't want to introduce a new import group; in most cases,
the desired behavior is separating std from third-party packages.
With this CL, new imports that don't share prefix with any existing ones
will be placed with the first group of third-party imports, if any
exist. If no third-party import group exists, a new one will be added.
In the case of our example above, this will be the new outcome:
import (
"time"
"github.com/golang/snappy"
"golang.org/x/sys/unix"
)
Fixesgolang/go#19190.
Change-Id: Id4630015c029bd815234a6c8726cb97f4af16f1c
Reviewed-on: https://go-review.googlesource.com/37552
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
DeleteNamedImport assumes the import declaration is in the form of:
import (
"foo"
)
If an import path is deleted there might be a blank line-sized hole:
import (
)
It'll merge the black hole with the last line to change it to:
import (
)
However the import declaration might be in the following form as well:
import (
"foo")
Whic means after deleting the import path, it changes to:
import (
)
In this case it still tries to merge the line with a non existing line,
causing token.File.MergeLine to panic.
We fix the issue by checking that the import path line is not the last
line to avoid panicing.
Fixesgolang/go#20229
Change-Id: I37537a4eaa83d14db59a2926d7bb14c27167a2e4
Reviewed-on: https://go-review.googlesource.com/44372
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit 50193ec1c5.
Reason for revert: aliases will not be part of Go 1.8
Change-Id: Idb3b74243eeb8dbeb2a2f4bd69a248c1dafa5348
Reviewed-on: https://go-review.googlesource.com/32835
Reviewed-by: Robert Griesemer <gri@golang.org>
This CL only copies files and updates build tags.
Substantive changes will come in follow-ups.
This is a workaround for git's lack of rename/copy tracking.
Tested with go1.6, go1.7, and tip (go1.8).
Change-Id: Id88a05273fb963586b228d5e5dfacab32133a960
Reviewed-on: https://go-review.googlesource.com/32630
Reviewed-by: Robert Griesemer <gri@golang.org>
When adding imports to an existing import declaration, ensure that
the target declaration will be printed with parentheses. This allows
all of the imported packages to be printed, not just the first one.
Fixesgolang/go#17212
Change-Id: Ie5de5ec9bca6169650336ee2ea98334817e64e48
Reviewed-on: https://go-review.googlesource.com/29688
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If the parens of an ast.GenDecl are dropped, move the documentation
of the ImportSpec above the import statement, otherwise the the
code is invalid.
Fixesgolang/go#15432.
Change-Id: I715750b8f528380b96a6bc8b5f216043937976c2
Reviewed-on: https://go-review.googlesource.com/22415
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When merging import declarations into a single block, AddNamedImport
modifies the list of declarations in the provided file while
iterating over the list. Take care to adjust the index into the list
so as to not skip entries or fall off the end.
Fixesgolang/go#17213
Change-Id: I807246f762c965ea1fc51eb57759f6088336db86
Reviewed-on: https://go-review.googlesource.com/29681
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Only unary CallExprs with no ellipsis, f(x), are ambiguous.
Change-Id: If4f17445ab0725dee916992db133eac5536133a7
Reviewed-on: https://go-review.googlesource.com/24552
Reviewed-by: Robert Griesemer <gri@golang.org>
DeleteNamedImport deletes the import with a given
name and path from a parsed file, if present.
imports uses this function to delete named imports.
Fixesgolang/go#8149.
Change-Id: I84539d5627191c45f06db2d008507aee4d3b1eb2
Reviewed-on: https://go-review.googlesource.com/19951
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These nodes are common in incomplete programs.
Change-Id: Iff9750050c78762f0cb8bebc7739584c197d661e
Reviewed-on: https://go-review.googlesource.com/19509
Reviewed-by: Michael Matloob <matloob@golang.org>
Failing to set this when adding a named import
to an existing block at the 0th position
caused the Lparen position to be set to zero.
As a result, the specs were printed as if
they were a single spec, not a group.
This made it appear as if imports had
been swallowed.
See CL 8663 for more context
and the original bug report.
CL 2050 fixed most similar cases
but missed this one.
Change-Id: Ic578fbb8040fa3d3d41db5bde2b839e394801608
Reviewed-on: https://go-review.googlesource.com/10252
Reviewed-by: Robert Griesemer <gri@golang.org>
AddImport and AddNamedImport attempt to place new
imports in roughly the correct place--and thus the
correct group--by matching prefixes. Matching prefixes
byte-by-byte led to "regexp" being grouped with "rsc.io/p".
Instead, match prefixes by segments.
Fixesgolang/go#9961.
Change-Id: I52b7c58a9a2fbe85c2b5297e50c87d409364bda3
Reviewed-on: https://go-review.googlesource.com/8090
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Agniva De Sarker
Dmitri Shuralyov
Alan Donovan
LE Manh Cuong
Robert Griesemer
haya14busa
Fatih Arslan
Rhys Hiltner
David R. Jenni
Alan Donovan
Josh Bleecher Snyder
Peter Collingbourne