nogo does not exclude external files with `--experimental_sibling_repository_layout`

[uhthomas]

What version of rules_go are you using?

v0.31.0

What version of gazelle are you using?

v0.25.0

What version of Bazel are you using?

5.1.1

Does this issue reproduce with the latest releases of all the above?

Yes.

What operating system and processor architecture are you using?

macOS aarch64

Any other potentially useful information about your toolchain?

No.

What did you do?

Given a project which uses nogo analysis and depends on external Go repositories.

What did you expect to see?

External repositories should be correctly excluded by nogo.

What did you see instead?

External go repositories are not excluded from nogo analysis when --experimental_sibling_repository_layout is enabled. This is a problem as it will be flipped in a future release. See https://github.com/bazelbuild/bazel/issues/12821.

[uhthomas]

A nice solution may be to use only_files instead where:

def nogo_config(name, config = {}, only_files = [], **kwargs):
    for v in only_files:
        config.setdefault(v, {}).update({
            "only_files": {
                "example.com/module": "first-party code",
            },
        })
    write_file(
        name = name,
        content = json.encode_indent(config),
        **kwargs
    )
    
def _write_file_impl(ctx):
    f = ctx.actions.declare_file(ctx.attr.out or ctx.attr.name)
    ctx.actions.write(f, ctx.attr.content)
    return [DefaultInfo(files = depset([f]))]

write_file = rule(
    implementation = _write_file_impl,
    attrs = {
        "out": attr.string(),
        "content": attr.string(mandatory = True),
    },
)

Though, this doesn't seem to work.

[tomqwpl]

I'm unclear what the expected behaviour is, but this issue looks relevant to what I'm seeing. I'm just starting out with bazel in any form, and am trying to get some kind of static code analysis going (we currently use golang-lint). So I'm trying to get nogo to run. However it seems to be analysing all third party dependencies, which obviously isn't going to work (I have no control over them). I can see that there is a way of using "only-files", but I apparently have to specify it independently for each analyser, and I've got no idea what the analysers it is running is (I'm just using the default nogo configuration). I appear to be using bazel 5.2. The referenced bazel ticket appears to still be open indicating that it hasn't been actioned yet, but what I see is consistent with it being the case in the version of bazel I've got.

I really hope they aren't going to change the behaviour and pollute my parent directory with loads of other directories. That sounds like a terrible idea.

[uhthomas]

@tomqwpl This isn't quite the same issue you're describing, though hopefully this will be helpful:

We generate our nogo config with starlark, which means we can much more easily exclude external repositories.

def _write_file_impl(ctx):
    f = ctx.actions.declare_file(ctx.attr.out or ctx.attr.name)
    ctx.actions.write(f, ctx.attr.content)
    return [DefaultInfo(files = depset([f]))]

write_file = rule(
    implementation = _write_file_impl,
    attrs = {
        "out": attr.string(),
        "content": attr.string(mandatory = True),
    },
)

def nogo_config(name, config = {}, exclude_files = [], **kwargs):
    for v in exclude_files:
        config.setdefault(v, {}).setdefault("exclude_files", {}).update({"external/": ""})
    write_file(
        name = name,
        content = json.encode_indent(config),
        **kwargs
    )

nogo_config(
    name = "nogo_config",
    config = {
        "nilness": {"exclude_files": {"cgo/": ""}},
        "unsafeptr": {"exclude_files": {"rules_go_work": "external sqlite3 fails vet, for some reason does not get a better workDir()"}},
    },
    exclude_files = [
        "unconvert",
        # ...
    ] + [
        # vet
        "asmdecl",
        "assign",
        "atomic",
        "bools",
        "buildtag",
        "cgocall",
        "composites",
        "copylocks",
        "httpresponse",
        "loopclosure",
        "lostcancel",
        "nilfunc",
        "nilness",
        "pkgfact",
        "printf",
        "shadow",
        "shift",
        "stdmethods",
        "stringintconv",
        "structtag",
        "tests",
        "unreachable",
        "unsafeptr",
        "unusedresult",
    ] + STATICCHECK_ANALYZERS,
    visibility = ["//visibility:public"],
)

---

It may be possible to use aspects or something to generate the list of external excludes automatically, but we haven't found the need yet.

The issue is that repositories aren't stored under external/ when using --experimental_sibling_repository_layout, and there is no obvious way to fix it. I suppose as I suggested earlier it may be possible to generate excludes for repository names with an aspect.

[tomqwpl]

@uhthomas All I can say is ughh! I'm coming from the perspective of having a makefile rule:

lint:
  bin/golangci-lint run

Given that I have no control over the code in external repositories, I wouldn't expect the static analyser to do anything other than analyse "my" code, on the basis that I can't do anything about anything that it might turn up in external repos anyway.

[uhthomas]

@tomqwpl That's a shame, I think nogo has a lot of potential. I do however understand the frustration, and you are not alone.

Google's gVisor wrote their "nogo" to address some of the limitations with the current tool, for instance.

https://github.com/google/gvisor/blob/c3a7b477f9f08ec04225fe561266a01646ceecc0/tools/nogo/README.md

I think that nogo analyses external repositories by design, though. A quote from @jayconrod:

Nogo definitely needs to be able to analyze packages in external repos though, and that should remain the default.

https://github.com/bazelbuild/rules_go/issues/2513#issuecomment-634058827