proposal: x/tools/go/analysis/passes/shadow: option to ignore generated code

[ainar-g]

protoc-gen-go-grpc, in particular, generates code like:

func (c *myServiceClient) GetProfiles(ctx context.Context, in *ProfilesRequest, opts ...grpc.CallOption) (MyService_GetProfilesClient, error) {
	stream, err := c.cc.NewStream(ctx, &MyService_ServiceDesc.Streams[0], "/MyService/getProfiles", opts...)
	if err != nil {
		return nil, err
	}
	x := &myServiceGetProfilesClient{stream}
	if err := x.ClientStream.SendMsg(in); err != nil {
		return nil, err
	}
	if err := x.ClientStream.CloseSend(); err != nil {
		return nil, err
	}
	return x, nil
}

Another, more generic option would be providing a list of ignored files.

[timothy-king]

Another, more generic option would be providing a list of ignored files.

An already available option is post processing the json output to filter the list/regex of files to ignore.

But going more broadly, how do you want to be able to specify this option? My personal preference would be an annotation in the function or package, e.g. var _ = shadowannot.Ignore(myServiceClient.GetProfiles). Is this feasible for the generated code you are looking at? Do you have an alternative pathway for giving annotations/options in mind?

[ainar-g]

But going more broadly, how do you want to be able to specify this option?

In any way, really. The existing tools our projects use have a wild variety of ways to ignore things from the command line:

• gocyclo has --ignore=<regexp> to specify a regular expression for filenames.

• errcheck has three flags—--ignoregenerated, --ignorepkg=<pkg>, and --ignoretests—all of which are self-explanatory, but there is also a deprecated flag --ignore=<pkg:regexp> to once again ignore files based on a regular expression

• gosec has --exclude-dir=<dir> and --exclude-generate, which are again self-explanatory.

I personally think that gocyclo's approach is the most flexible, especially when talking about generated files, since they often have special filenames, like foo.pb.go.

On a more finegrained level, one could specify an identifier the same way go doc expects it. I.e. my.own/pkg/path.Identifier.method.

Is this feasible for the generated code you are looking at? Do you have an alternative pathway for giving annotations/options in mind?

I think there is a way to customize the gRPC code generator by either forking it or using some kind of a plugin, but that feels like on overkill to just add an annotation.

[timothy-king]

Ignoring based on a regexp at the file level makes sense to me. I think a combination of the package and the file's base name sounds fairly robust and helpful (*.pb.go likely covers a project well). I believe all of this information we need to make this string should be available in the analysis.Pass object. It should be possible to make this check fairly fast too. This could be done in shadow.Analyzer.Flags so no other analyzers would need to be impacted.

If someone deeply wanted fine grained filtering, they could use multiple files to contain the 'ignore shadow' functions.