Notes on go/packages

[Dominik Honnef]

I'd like to document my notes on go/packages. These are either

things that I need, or things that I have noticed.

How are we supposed to use go/packages if we also want to support Go 1.10?

go/packages depends on behavior in 'go list' that was added in Go

1.11, and has no graceful fallback. This means we cannot use

go/packages as long as Go 1.10 is still supported, which for most

people is until Go 1.12 is released. At the same time, support

for Go modules only exists in go/packages, and I'd really want to

support them once Go 1.11 is released.

Are there any plans for a fallback to go/build in go/packages? Or

are we expected to use both go/loader and go/packages and to

layer our own abstraction on top?

Mapping of generated files to source files

In the context of 'go build', I need to know which Go files are the

result of cgo preprocessing. This is because of an interesting UI need

in linters; quoting my own code comment:

> // The //line compiler directive can be used to change the file

> // name and line numbers associated with code. This can, for

> // example, be used by code generation tools. The most prominent

> // example is 'go tool cgo', which uses //line directives to refer

> // back to the original source code.

> //

> // In the context of our linters, we need to treat these

> // directives differently depending on context. For cgo files, we

> // want to honour the directives, so that line numbers are

> // adjusted correctly. For all other files, we want to ignore the

> // directives, so that problems are reported at their actual

> // position and not, for example, a yacc grammar file. This also

> // affects the ignore mechanism, since it operates on the position

> // information stored within problems. With this implementation, a

> // user will ignore foo.go, not foo.y

In other words, it matters whether a file was generated during the

build process, or if it was generated offline (e.g. with yacc)

I'm not familiar with Bazel or Blaze, but I'll pessimistically assume

that it has flexible code generation mechanisms, and that having the

equivalent to build.Package.CgoFiles isn't sufficient.

The first version of the CL introducing go/packages actually had a

GeneratedBy map[string]string field. Why was it removed?

The way flags and the environment are passed to go list, Bazel etc

It's been previously stated that there are no current plans for an

abstraction of tags, GOOS, GOARCH etc across the different build

systems. Instead, flags and the environment will be passed through.

This focusses heavily on the interactive use case, but ignores

programmatic approaches. For example, I'd want to check different

GOOS/GOARCH/tag combinations for a specified package to intersect the

reported problems.

Even without a provided abstraction, I could synthesize the right

flags and environment myself, but that requires the option to

explicitly pass these in, versus being infered from os.Args and os.Env.

Additionally, relying on flags and the environment makes it necessary

that tool writers are aware of these flags, as not to accidentally

reuse them for something else. I reckon that the majority of us tool

writers are not familiar with all build systems, especially not Blaze,

so I'd like the specific list of flags, and environment variables, of

each build system to be documented as part of go/packages's

documentation.

x/tools APIs that currently assume go/loader

There are some helper libraries, such as ssautil, that depend on

go/loader. What are the plans for making these work with go/packages?

Timeframe, backwards compatibility, etc.

Build tags

What's the story with build tags? Do Bazel and Blaze have this

concept? Is something similar to build.Package.AllTags viable with

go/packages, or does this concept not span build systems?

I'll follow up with more emails if I discover additional issues while

porting staticcheck to go/packages.

[Russ Cox]

Hi Dominik,

Thanks for this great feedback. We definitely want to work with you and other tool authors to make sure that go/packages supports what you need to do.

On Thu, Jul 12, 2018 at 11:44 PM, Dominik Honnef <dominik...@gmail.com> wrote:

I'd like to document my notes on go/packages. These are either

things that I need, or things that I have noticed.

How are we supposed to use go/packages if we also want to support Go 1.10?

go/packages depends on behavior in 'go list' that was added in Go

1.11, and has no graceful fallback. This means we cannot use

go/packages as long as Go 1.10 is still supported, which for most

people is until Go 1.12 is released. At the same time, support

for Go modules only exists in go/packages, and I'd really want to

support them once Go 1.11 is released.

Are there any plans for a fallback to go/build in go/packages? Or

are we expected to use both go/loader and go/packages and to

layer our own abstraction on top?

Even go/build doesn't really work with Go 1.10 due to build caching. We should have had go/packages out for the Go 1.10 release, and we dropped the ball on that. My apologies. To correct the problem you identified, I think once we are sure that we understand what go/packages needs from cmd/go, we can backport those additional list flags to a Go 1.10 point release and then go/packages will work with it. Probably we won't reach back to Go 1.9 though.

In the interim, I assume you've got something in golint already (probably a use of go/build) that kind of works, and I would say to keep that code around until go/packages handles all the Go versions you want to support. I know this is not even close to ideal.

Mapping of generated files to source files

In the context of 'go build', I need to know which Go files are the

result of cgo preprocessing. This is because of an interesting UI need

in linters; quoting my own code comment:

> // The //line compiler directive can be used to change the file

> // name and line numbers associated with code. This can, for

> // example, be used by code generation tools. The most prominent

> // example is 'go tool cgo', which uses //line directives to refer

> // back to the original source code.

> //

> // In the context of our linters, we need to treat these

> // directives differently depending on context. For cgo files, we

> // want to honour the directives, so that line numbers are

> // adjusted correctly. For all other files, we want to ignore the

> // directives, so that problems are reported at their actual

> // position and not, for example, a yacc grammar file. This also

> // affects the ignore mechanism, since it operates on the position

> // information stored within problems. With this implementation, a

> // user will ignore foo.go, not foo.y

In other words, it matters whether a file was generated during the

build process, or if it was generated offline (e.g. with yacc)

I am not sure I agree with this premise. If I make a Go mistake in a yacc rule, I want golint to report the source line I can edit - the one in the yacc file - not some line in generated code that I have to manually back out to the corresponding line in an editable file. If the yacc //line annotations are somehow wrong, we should fix them. Some tools choose to show both, like

    x.y:123[y.tab.go:2345]: problem here

I believe we dropped GeneratedBy precisely because the //line information should be used instead.

The way flags and the environment are passed to go list, Bazel etc

It's been previously stated that there are no current plans for an

abstraction of tags, GOOS, GOARCH etc across the different build

systems. Instead, flags and the environment will be passed through.

This focusses heavily on the interactive use case, but ignores

programmatic approaches. For example, I'd want to check different

GOOS/GOARCH/tag combinations for a specified package to intersect the

reported problems.

Even without a provided abstraction, I could synthesize the right

flags and environment myself, but that requires the option to

explicitly pass these in, versus being infered from os.Args and os.Env.

Additionally, relying on flags and the environment makes it necessary

that tool writers are aware of these flags, as not to accidentally

reuse them for something else. I reckon that the majority of us tool

writers are not familiar with all build systems, especially not Blaze,

so I'd like the specific list of flags, and environment variables, of

each build system to be documented as part of go/packages's

documentation.

The idea here was that tools shouldn't in general be trying to imagine all possible worlds. In general there are going to be environment variables and command-line flags that affect the way the underlying build system interprets source code, so there should be a way to pass through arbitrary additional environment variables and flags. I think it would suffice for tools using go/packages to have flags like -env X=Y and -buildflags '-whatever' and just pass those through to the go/packages. Note that this matters not just for GOOS/GOARCH/build tags but also for plain configuration about where to find the code or how to build it (necessary to get export data), like CC, CC_FOR_linux_amd64, CGO_CFLAGS, GOCACHE, GOPATH, -compiler, -ldflags, -getmode, -pkgdir (hopefully not anymore), -toolexec, and so on. Trying to abstract away GOOS/GOARCH/build tags does not do anything for all the other important adjustments users might need to make. That's why we said let's just have a simple pass-through. Users running golint on code in build system X are already familiar with the environment variables and command-line flags necessary for build system X. The tools just need to be able to let the users pass them through.

I do sympathize with the fact that go/packages is not helping to answer the question "what are all the possible build configurations for this code?" but honestly the underlying build system may not even know. Maybe in a future version we could add something to suggest guesses about interesting alternate configurations to try (different GOOS, GOARCH etc). But right now I'd just like to get back to being able to load packages at all (see note above about Go 1.10 breaking everything).

x/tools APIs that currently assume go/loader

There are some helper libraries, such as ssautil, that depend on

go/loader. What are the plans for making these work with go/packages?

Timeframe, backwards compatibility, etc.

Build tags

What's the story with build tags? Do Bazel and Blaze have this

concept? Is something similar to build.Package.AllTags viable with

go/packages, or does this concept not span build systems?

Bazel/Blaze don't have a concept of build tags in general - they are unique to Go. But the Go support for these systems does apply build tags to winnow the file list before invoking the compiler, same as the go command.

I think it would be reasonable to put AllTags into the go list output and expose it in go/packages. Want to file an issue for that?

Russ

[Alan Donovan]

On 12 July 2018 at 23:44, Dominik Honnef <dominik...@gmail.com> wrote:

I'd like to document my notes on go/packages. These are either

things that I need, or things that I have noticed.

Hi Dominik, thanks for these good questions, and sorry for not having done a better job of communicating our plans (or more accurately, still-evolving thoughts) on them.

How are we supposed to use go/packages if we also want to support Go 1.10?

go/packages depends on behavior in 'go list' that was added in Go
1.11, and has no graceful fallback. This means we cannot use
go/packages as long as Go 1.10 is still supported, which for most
people is until Go 1.12 is released. At the same time, support
for Go modules only exists in go/packages, and I'd really want to
support them once Go 1.11 is released.
Are there any plans for a fallback to go/build in go/packages? Or
are we expected to use both go/loader and go/packages and to
layer our own abstraction on top?

The first drafts of go/packages were based on the go1.10 go list command, which lacks many necessary features (-export, -cgo, -deps, ImportMap) and thus had to be invoked three times (once to expand the command line arguments, once to compute transitive dependencies, and once to obtain complete information for each package), with more logic to convert the results into the appropriate form. We will need to revive this approach to handle older versions of go list. Even then, handling generated test main packages and cgo will require unappealing workarounds: either scraping the output of 'go build -x -work', or duplicating the necessary logic.

In the meantime an immediate stopgap measure would be for go/packages to return "ErrTooOld" and for an application to fall back to go/loader, with inevitable minor differences in semantics.

The first version of the CL introducing go/packages actually had a

GeneratedBy map[string]string field. Why was it removed?

Two reasons: first, we hadn't yet written the logic to populate it and didn't want to expose it until it actually worked. Second, we wanted to first exhaust the possibility that the necessary information can be extracted from the files themselves, either //line directives or uniform "Generated" / "DO NOT EDIT" comments. It's possible that some code generators don't add such comments, but if possible we should fix them.

The way flags and the environment are passed to go list, Bazel etc

At many points in the design, configuration problems can be solved by a specific high-level API (e.g. GOARCH=x) or a generic low-level pass-through (Environ, Flags). The low-level API may be more flexibIe but it is less portable and harder to use and does not abstract the underlying build system. We've deferred implementing the specific APIs on the basis that the pass-throughs, which we know we will need, are sufficient for now, but that does move the burden to the client, which seems like the wrong choice for such everyday parameters as OS and ARCH.

x/tools APIs that currently assume go/loader

There are some helper libraries, such as ssautil, that depend on

go/loader. What are the plans for making these work with go/packages?

Timeframe, backwards compatibility, etc.

I have some small pending CLs to make it easy to load SSA from a packages.TypeCheck call, but porting existing tools to the go/packages depends on us first addressing the migration problem.

What's the story with build tags? Do Bazel and Blaze have this

concept? Is something similar to build.Package.AllTags viable with

go/packages, or does this concept not span build systems?

Again, this is a case where we know we need something and we haven't yet implemented it.

go/packages certainly needs a way to allow applications to pass tags through to the query tool. Although Bazel and Blaze both have mechanisms to support arbitrary build tags, they are essentially never used in practice, and they provide no way to ascertain which tags were relevant to file selection. The choice here is between adding a go-only Tags []string field to Options, or a universal Flags []string pass-through, which would provide an escape hatch for this and other configuration factors although in low-level, non-portable way. I'm inclined to do the latter because I'm sure we will need it eventually, and we can always add a dedicated Options.Tags field later if it is important.

 

I'll follow up with more emails if I discover additional issues while

porting staticcheck to go/packages.

Please do. 

[Dominik Honnef]

Hi,

thanks for working with me on this!

On Friday, July 13, 2018 at 3:52:11 PM UTC+2, rsc wrote:

Hi Dominik,

Thanks for this great feedback. We definitely want to work with you and other tool authors to make sure that go/packages supports what you need to do.

On Thu, Jul 12, 2018 at 11:44 PM, Dominik Honnef <dominik...@gmail.com> wrote:

I'd like to document my notes on go/packages. These are either

things that I need, or things that I have noticed.

How are we supposed to use go/packages if we also want to support Go 1.10?

go/packages depends on behavior in 'go list' that was added in Go

1.11, and has no graceful fallback. This means we cannot use

go/packages as long as Go 1.10 is still supported, which for most

people is until Go 1.12 is released. At the same time, support

for Go modules only exists in go/packages, and I'd really want to

support them once Go 1.11 is released.

Are there any plans for a fallback to go/build in go/packages? Or

are we expected to use both go/loader and go/packages and to

layer our own abstraction on top?

Even go/build doesn't really work with Go 1.10 due to build caching. We should have had go/packages out for the Go 1.10 release, and we dropped the ball on that. My apologies. To correct the problem you identified, I think once we are sure that we understand what go/packages needs from cmd/go, we can backport those additional list flags to a Go 1.10 point release and then go/packages will work with it. Probably we won't reach back to Go 1.9 though.

In the interim, I assume you've got something in golint already (probably a use of go/build) that kind of works, and I would say to keep that code around until go/packages handles all the Go versions you want to support. I know this is not even close to ideal.

Go 1.10 broke relatively few things, actually. Most third party tools use go/loader, loading code entirely from source. They aren't affected by changes to GOPATH/pkg or caching. Go 1.11, however, will have a lot more people trying out Go modules, and I'd rather not see the issues of the vendor experiment repeat themself (people using vendor, tools not understanding vendor.)

Backporting the cmd/go changes would be great. Supporting Go 1.10 and Go 1.11 ought to be enough.

 

Mapping of generated files to source files

In the context of 'go build', I need to know which Go files are the

result of cgo preprocessing. This is because of an interesting UI need

in linters; quoting my own code comment:

> // The //line compiler directive can be used to change the file

> // name and line numbers associated with code. This can, for

> // example, be used by code generation tools. The most prominent

> // example is 'go tool cgo', which uses //line directives to refer

> // back to the original source code.

> //

> // In the context of our linters, we need to treat these

> // directives differently depending on context. For cgo files, we

> // want to honour the directives, so that line numbers are

> // adjusted correctly. For all other files, we want to ignore the

> // directives, so that problems are reported at their actual

> // position and not, for example, a yacc grammar file. This also

> // affects the ignore mechanism, since it operates on the position

> // information stored within problems. With this implementation, a

> // user will ignore foo.go, not foo.y

In other words, it matters whether a file was generated during the

build process, or if it was generated offline (e.g. with yacc)

I am not sure I agree with this premise. If I make a Go mistake in a yacc rule, I want golint to report the source line I can edit - the one in the yacc file - not some line in generated code that I have to manually back out to the corresponding line in an editable file. If the yacc //line annotations are somehow wrong, we should fix them. Some tools choose to show both, like

    x.y:123[y.tab.go:2345]: problem here

I believe we dropped GeneratedBy precisely because the //line information should be used instead.

Generated code and linting is a nuanced problem. Yes, if x.y includes inlined Go code, it'd be better to use the positions in x.y. However, code generation generates a lot of supporting code that isn't controlled by the user, which may, however, still contain issues. In the case of staticcheck, these would likely be bugs in the code generator itself. My users have found it easier to understand when these errors were reported in the .go file. The input to a code generator may not even include any Go code at all.

Even if I were to show both positions, as in x.y:123[y.tab.go:2345], I'd still like to discern transparent code generation (cgo) from other generated files. In the case of cgo, it wouldn't be useful to show positions in the generated code; there may not even be useful file names.

The way flags and the environment are passed to go list, Bazel etc

It's been previously stated that there are no current plans for an

abstraction of tags, GOOS, GOARCH etc across the different build

systems. Instead, flags and the environment will be passed through.

This focusses heavily on the interactive use case, but ignores

programmatic approaches. For example, I'd want to check different

GOOS/GOARCH/tag combinations for a specified package to intersect the

reported problems.

Even without a provided abstraction, I could synthesize the right

flags and environment myself, but that requires the option to

explicitly pass these in, versus being infered from os.Args and os.Env.

Additionally, relying on flags and the environment makes it necessary

that tool writers are aware of these flags, as not to accidentally

reuse them for something else. I reckon that the majority of us tool

writers are not familiar with all build systems, especially not Blaze,

so I'd like the specific list of flags, and environment variables, of

each build system to be documented as part of go/packages's

documentation.

The idea here was that tools shouldn't in general be trying to imagine all possible worlds. In general there are going to be environment variables and command-line flags that affect the way the underlying build system interprets source code, so there should be a way to pass through arbitrary additional environment variables and flags. I think it would suffice for tools using go/packages to have flags like -env X=Y and -buildflags '-whatever' and just pass those through to the go/packages. Note that this matters not just for GOOS/GOARCH/build tags but also for plain configuration about where to find the code or how to build it (necessary to get export data), like CC, CC_FOR_linux_amd64, CGO_CFLAGS, GOCACHE, GOPATH, -compiler, -ldflags, -getmode, -pkgdir (hopefully not anymore), -toolexec, and so on. Trying to abstract away GOOS/GOARCH/build tags does not do anything for all the other important adjustments users might need to make. That's why we said let's just have a simple pass-through. Users running golint on code in build system X are already familiar with the environment variables and command-line flags necessary for build system X. The tools just need to be able to let the users pass them through.

Using -buildflags does sidestep this issue, you're right. I was under the impression that the plan was for tools to offer the same flags as the build system, as first class citizens, as in 'golint -tags foo -pkgdir bar'. Looking at the sheer number of flags, however, this would get very unwieldy.

An interesting observation, however, is that most tools already have a -tags flag.

 

I do sympathize with the fact that go/packages is not helping to answer the question "what are all the possible build configurations for this code?" but honestly the underlying build system may not even know. Maybe in a future version we could add something to suggest guesses about interesting alternate configurations to try (different GOOS, GOARCH etc). But right now I'd just like to get back to being able to load packages at all (see note above about Go 1.10 breaking everything).

The build system may not know, but the user does. A user could say "please check this project for the following configurations", specified as a list of GOOS, GOARCH, tags, and possibly more. This may be done in the form of command line flags, or project configuration files. Some code analysis is simply not possible when looking at a single build configuration in isolation. Identifiers unused in one configuration may be used in another configuration. Unnecessary conversions (c.f. https://github.com/mdempsky/unconvert) may not be unnecessary under all architectures.

Of course, considering -buildflags, this may not actually need any form of abstraction. The user could just specify a list of values for -buildflags, and tools could be oblivious to what the different values mean.

As an aside, "what are all the possible build configurations for this code" isn't actually a useful question to ask. Once we consider transitive dependencies, we pull in the runtime, and the number of build configurations explodes.

I should be clear and say that I don't expect go/packages to solve all my issues in Go 1.11. I'd be happy to see these solved in 1.12 or later. Some of these things, however, could be complicated by unfortunate API choices now, especially if go/packages makes its way into the standard library, subject to backwards compatibility rules. At the same time, not all of my expectations make sense, or can be solved differently, so I appreciate your input.

 

x/tools APIs that currently assume go/loader

There are some helper libraries, such as ssautil, that depend on

go/loader. What are the plans for making these work with go/packages?

Timeframe, backwards compatibility, etc.

Build tags

What's the story with build tags? Do Bazel and Blaze have this

concept? Is something similar to build.Package.AllTags viable with

go/packages, or does this concept not span build systems?

Bazel/Blaze don't have a concept of build tags in general - they are unique to Go. But the Go support for these systems does apply build tags to winnow the file list before invoking the compiler, same as the go command.

I think it would be reasonable to put AllTags into the go list output and expose it in go/packages. Want to file an issue for that?

I'm not sure anymore that we need it.

[Russ Cox]

On Fri, Jul 13, 2018 at 3:26 PM, Dominik Honnef <dominik...@gmail.com> wrote:

Go 1.10 broke relatively few things, actually. Most third party tools use go/loader, loading code entirely from source. They aren't affected by changes to GOPATH/pkg or caching. Go 1.11, however, will have a lot more people trying out Go modules, and I'd rather not see the issues of the vendor experiment repeat themself (people using vendor, tools not understanding vendor.)

Yes, we definitely want go/packages ready for Go 1.11.

Our internal testing of Go 1.11 is not where it usually is (as evidenced by not even talking about Go1.11rc1 yet), so the release is more likely the end of August than the beginning of August. We have a little bit of time to get go/packages hammered out.

 Backporting the cmd/go changes would be great. Supporting Go 1.10 and Go 1.11 ought to be enough.

Great.

Generated code and linting is a nuanced problem. Yes, if x.y includes inlined Go code, it'd be better to use the positions in x.y. However, code generation generates a lot of supporting code that isn't controlled by the user, which may, however, still contain issues. In the case of staticcheck, these would likely be bugs in the code generator itself. My users have found it easier to understand when these errors were reported in the .go file. The input to a code generator may not even include any Go code at all.

In general code generators should be printing something like //line "file i'm writing":123 when they flip back into code they are generating from scratch, to avoid attributing that code to random unrelated lines in the source file. In some tools I've taken care of this by printing all the "not derived from the actual inputs" code at the top of the file, before the first //line comment. But I appreciate that not all tools are doing that right.

Maybe a heuristic would be that if the //line refers back to a .go file, then use that Go file, otherwise report the generated Go file? Then at least you're always pointing into a Go file, hopefully the right one.

Of course, considering -buildflags, this may not actually need any form of abstraction. The user could just specify a list of values for -buildflags, and tools could be oblivious to what the different values mean.

That's what I hope. I do think it's reasonable for tools to say "well -tags is very common so I'll provide -tags=foo as meaning -buildflags=-tags=foo."

As an aside, "what are all the possible build configurations for this code" isn't actually a useful question to ask. Once we consider transitive dependencies, we pull in the runtime, and the number of build configurations explodes.

I should be clear and say that I don't expect go/packages to solve all my issues in Go 1.11. I'd be happy to see these solved in 1.12 or later. Some of these things, however, could be complicated by unfortunate API choices now, especially if go/packages makes its way into the standard library, subject to backwards compatibility rules. At the same time, not all of my expectations make sense, or can be solved differently, so I appreciate your input.

FWIW go/packages will not go into the standard library until Go 1.12 at the earliest, more likely Go 1.13. It will be x/tools/go/packages until we are sure it's right.

Thanks again.

Russ

[Jan Mercl]

On Fri, Jul 13, 2018 at 3:52 PM Russ Cox <r...@golang.org> wrote:

> I am not sure I agree with this premise. If I make a Go mistake in a yacc rule, I want golint to report the source line

> I can edit - the one in the yacc file - not some line in generated code that I have to manually back out to the corresponding line in an editable file.

FTR: This never worked and without changing some things in the Go compiler implementation, it never will.

jnml@4670:~/src/tmp> cat -n foo.y

     1 %{

     2 package main

     3

     4 import (

     5 "log"

     6 )

     7 %}

     8

     9 %union {

    10 i int

    11 }

    12

    13 %%

    14

    15 start: 'X'

    16 {

    17 log.Println("I'm foo.y:17")

    18 goto ret0

    19 }

    20

    21 %%

    22

    23 type lexer struct {}

    24

    25 func (lexer) Lex(*yySymType) int { return 'X' }

    26 func (lexer) Error(string) {}

    27

    28 func main() {

    29 log.SetFlags(log.Lshortfile)

    30 log.Println("I'm foo.y:30")

    31 yyParse(lexer{})

    32 }

jnml@4670:~/src/tmp> goyacc foo.y && go run y.go

foo.y:29: I'm foo.y:30

yaccpar:335: I'm foo.y:17

jnml@4670:~/src/tmp> 

The first problem, line 29 vs 30, is just an off by one error. AFAIK it was never reported, probably because the third yacc %%-separated section is rarely used. The really interesting line info is that of foo.y:17 for exactly the reasons you stated. The interesting part of the generated code is

switch yynt {

case 1:

yyDollar = yyS[yypt-1 : yypt+1]

//line foo.y:16

{

log.Println("I'm foo.y:17")

goto ret0

}

}

goto yystack /* stack new state and value */

This time there's no off by one error, but properly gofmt'ed code puts the //line comment aligned to the block it belongs to. So it does not start at column 1, so it's ignored by the logic in the Go compiler that handles //line comments.

I wonder if the rule should be changed such that the //line comment must be the first non-blank item in a line. That would make the thing you and others want: we will be pointed to foo.y:17, not to yaccpar:355, which isn't actually a file at all, probably just some leftover from the C version of Plan 9 yacc.

--

-j

[Dominik Honnef]

I have some more questions and feedback.

My first question is regarding setting the TypeChecker.Sizes field
when using the WholeProgram loader. How is one expected to set this,
in a build-system-agnostic way, without understanding the flags passed
to the build system? For 'go build', we would use something like
types.SizesFor(ctx.Compiler, ctx.GOARCH), where ctx is a
go/build.Context, usually build.Default, which gets GOARCH from the
runtime or the environment. But we can't do that if we want to support
multiple build systems.

My next question is regarding test packages. These include a copy of
the code of the package under test In turn, if we lint both a package
and its tests we'll report errors twice. It is of course trivial to
deduplicate errors, but I was wondering if there was a way that would
avoid the cost of actually linting code twice. This probably brings us
back to the ForTest field, which would let us lint only the test
package.

Finally, I was wondering if you would consider adding a field WasStale
to the Package struct, which would get set to true if the underlying
build system had to regenerate export data (for the Types and Syntax
modes). This would allow tool writers to store additional information
about packages on disk (e.g. function summaries, such as the "function
is a printf wrapper" flag) and to refresh them when a package changed.

Cheers

[Ian Cottrell]

Hi,

The TypeChecker.Sizes question is going to have to wait for Alan, who is currently away, I don't have enough context to be sure of any answers I gave on that one.

Test packages are an interesting issue, for golint I have just deduplicated the reported Problems because it was the easiest way for now.

I am not sure ForTest is the right solution, because it still does not really let you behave correctly anyway. 

It would let you know you were processing a package with test files, but that is not enough information to know which files in that package are only there in that test, and it also does not cope with cases where you have multiple tests for the same package (something that is possible in other build systems).

Because the package API can give you back fully type checked packages, you should not be worrying about the cost of re-parsing or type checking anyway, if its possible to avoid it the API should be doing it for you, so it's only the analysis phase you have to worry about, and the cost of the linters was so small compared to the rest that I was okay with it running multiple times for some files.

We do have plans for a higher level of API that will manage that side of things for you anyway, so you just add checkers and it will invoke them with the right information, rather than asking everyone to do that complicated work, or complicating the packages API to make it easier. We think a framework like that could replace the core of golint, govet and a few other things too.

the x/tools/go/packages API is really intended for direct use by a command line tool, not repeated use by a long running tool or underlying use for a library. As such it does things like pass flags directly through to the build system assuming they come from the user, and uses build system specific patterns for package selection, and has no agnostic way of controlling things like the build tags.

We have plans for another level of API for people writing tool libraries that allows you to operate in a build system independent way, that supports repeated queries and caching of results, for writing higher level libraries on top of. Anything you would need WasStale for probably falls into that category. If you would like to discuss what kind of features that API should have I would love to hear your thoughts. For now we have just been focusing on the packages API and using it to fix all the tools it is appropriate for, seeing how far we can stretch it without making it overly complicated.

[Dominik Honnef]

> We have plans for another level of API for people writing tool libraries that allows you to operate in a build system independent way, that supports repeated queries and caching of results, for writing higher level libraries on top of.

I'll make sure to give my input when time comes, and I'll try to keep my go/packages feedback to things that I need to port my existing tools at this time. Things like WasStale are for future plans of mine, so if there's going to be another place for that, that is fine by me.

My only question regarding this other API is how it will interact with go/packages. For example, will it attempt to reuse go/packages's Package structure? go/packages's doc.go reads like it is supposed to be _the_ way of getting package metadata from build systems, so the mention of another API surprises me. It would seem to me that any other API would have to sit on top of go/packages, but it sounds like go/packages would actually be a client of this other, more powerful API.

> We do have plans for a higher level of API that will manage that side of things for you anyway, so you just add checkers and it will invoke them with the right information, rather than asking everyone to do that complicated work, or complicating the packages API to make it easier. We think a framework like that could replace the core of golint, govet and a few other things too.

I reckon that this API will work fine for the majority of simpler tools but that staticcheck will keep sticking to its own framework (for reasons we can get into when you actually release that API)

> Because the package API can give you back fully type checked packages, you should not be worrying about the cost of re-parsing or type checking anyway, if its possible to avoid it the API should be doing it for you, so it's only the analysis phase you have to worry about, and the cost of the linters was so small compared to the rest that I was okay with it running multiple times for some files.

It occured to me that there are two cases: 1) the linter is simple enough that it is cheap enough to make simple deduplication viable 2) the linter applies such advanced analyses that the difference between test and non-test code is actually relevant and may produce different results. So yes, in either case, this doesn't seem to be a problem.

[Dominik Honnef]

While I'm asking questions:

For Go 1.11 'go list' and cgo, is Package.GoFiles supposed to contain the unprocessed files or the processed ones? Currently, it contains the unprocessed files, which means that it doesn't match the actual inputs to the parser and type checker.

[Paul Jolly]

Perhaps you're after https://go-review.googlesource.com/c/go/+/126695?

> --
> You received this message because you are subscribed to the Google Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.