Skip to content

Skip analyzers that don't emit facts when ignoring diagnostics#4402

Merged
fmeum merged 1 commit intomasterfrom
faster-nogo-1
Jul 29, 2025
Merged

Skip analyzers that don't emit facts when ignoring diagnostics#4402
fmeum merged 1 commit intomasterfrom
faster-nogo-1

Conversation

@fmeum
Copy link
Copy Markdown
Member

@fmeum fmeum commented Jul 29, 2025

What type of PR is this?

Performance improvement

What does this PR do? Why is it needed?

Avoid unnecessary work by skipping analyzers that don't emit facts when reported diagnostics are known to be ignored.

Which issues(s) does this PR fix?

Work towards #4327

Other notes for review

@fmeum fmeum requested review from jayconrod and linzhp July 29, 2025 16:25
@fmeum fmeum marked this pull request as ready for review July 29, 2025 16:25
@fmeum fmeum enabled auto-merge (squash) July 29, 2025 19:48
@fmeum fmeum merged commit 9badb60 into master Jul 29, 2025
4 checks passed
@fmeum fmeum deleted the faster-nogo-1 branch July 29, 2025 20:17
@lbcjbb
Copy link
Copy Markdown
Contributor

lbcjbb commented Aug 11, 2025

After upgrading rules_go from v0.55.1 to v0.56.1, I have errors from nogo when I execute gazelle, or building any Go library with external Go dependencies :

ERROR: .../external/gazelle++go_deps+org_golang_x_sync/errgroup/BUILD.bazel:3:11: output 'external/gazelle++go_deps+org_golang_x_sync/errgroup/errgroup.facts' was not created
ERROR: .../external/gazelle++go_deps+org_golang_x_sync/errgroup/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+org_golang_x_sync//errgroup:errgroup failed: not all outputs were created or valid
ERROR: .../external/gazelle++go_deps+com_github_bazelbuild_buildtools/labels/BUILD.bazel:3:11: output 'external/gazelle++go_deps+com_github_bazelbuild_buildtools/labels/labels.facts' was not created
ERROR: .../external/gazelle++go_deps+com_github_bazelbuild_buildtools/labels/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+com_github_bazelbuild_buildtools//labels:labels failed: not all outputs were created or valid
ERROR: .../external/gazelle++go_deps+in_gopkg_yaml_v3/BUILD.bazel:5:11: output 'external/gazelle++go_deps+in_gopkg_yaml_v3/yaml_v3.facts' was not created
ERROR: .../external/gazelle++go_deps+in_gopkg_yaml_v3/BUILD.bazel:5:11: Running nogo on @@gazelle++go_deps+in_gopkg_yaml_v3//:yaml_v3 failed: not all outputs were created or valid
ERROR: .../external/gazelle++go_deps+org_golang_x_sys/execabs/BUILD.bazel:3:11: output 'external/gazelle++go_deps+org_golang_x_sys/execabs/execabs.facts' was not created
ERROR: .../external/gazelle++go_deps+org_golang_x_sys/execabs/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+org_golang_x_sys//execabs:execabs failed: not all outputs were created or valid
...

I used nogo with several custom analysis tools without FactTypes, but I no longer have any errors if I configure these analysis tools with a fake Fact.

@fmeum
Copy link
Copy Markdown
Member Author

fmeum commented Aug 11, 2025

Could you share one of these analyzers? Is it possible that they are failing in a way that doesn't cause nogo itself to fail (we had issues with panics in the past, but those have been fixed). Do you have analyzers that don't emit facts but depend on analyzers that do?

@lbcjbb
Copy link
Copy Markdown
Contributor

lbcjbb commented Aug 11, 2025

Tested with this dummy analyzer:

package dummy

import (
	"golang.org/x/tools/go/analysis"
)

var Analyzer = &analysis.Analyzer{
	Name: "dummy",
	Doc:  "Dummy analyzer",
	Run: func(pass *analysis.Pass) (any, error) {
		return nil, nil
	},
}
nogo(
    name = "nogo",
    config = "//:.nogoconfig.json",
    visibility = ["//visibility:public"],
    deps = [
        "<my analyzer>",
    ],
)
go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")
go_sdk.nogo(
    nogo = "<my nogo target>",
)

.nogoconfig.json:

{
  "dummy": {}
}
  • With rules_go v0.56.1:

$ bazel run //:gazelle
ERROR: .../external/gazelle++go_deps+org_golang_x_sync/errgroup/BUILD.bazel:3:11: output 'external/gazelle++go_deps+org_golang_x_sync/errgroup/errgroup.facts' was not created
ERROR: .../external/gazelle++go_deps+org_golang_x_sync/errgroup/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+org_golang_x_sync//errgroup:errgroup failed: not all outputs were created or valid
...

✔️

fmeum pushed a commit that referenced this pull request Aug 11, 2025
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

After #4402, it is no longer possible to use a custom analyzer without
having to declare a new `Fact` type. The build of a Go library will fail
if the library contains any external dependencies.

That's the case for executing gazelle by example (if we don't use a
precompiled binary):

```
$ bazel run //:gazelle
ERROR: .../external/gazelle++go_deps+org_golang_x_sync/errgroup/BUILD.bazel:3:11: output 'external/gazelle++go_deps+org_golang_x_sync/errgroup/errgroup.facts' was not created
ERROR: .../external/gazelle++go_deps+org_golang_x_sync/errgroup/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+org_golang_x_sync//errgroup:errgroup failed: not all outputs were created or valid
ERROR: .../external/gazelle++go_deps+com_github_bazelbuild_buildtools/labels/BUILD.bazel:3:11: output 'external/gazelle++go_deps+com_github_bazelbuild_buildtools/labels/labels.facts' was not created
ERROR: .../external/gazelle++go_deps+com_github_bazelbuild_buildtools/labels/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+com_github_bazelbuild_buildtools//labels:labels failed: not all outputs were created or valid
ERROR: .../external/gazelle++go_deps+in_gopkg_yaml_v3/BUILD.bazel:5:11: output 'external/gazelle++go_deps+in_gopkg_yaml_v3/yaml_v3.facts' was not created
ERROR: .../external/gazelle++go_deps+in_gopkg_yaml_v3/BUILD.bazel:5:11: Running nogo on @@gazelle++go_deps+in_gopkg_yaml_v3//:yaml_v3 failed: not all outputs were created or valid
ERROR: .../external/gazelle++go_deps+org_golang_x_sys/execabs/BUILD.bazel:3:11: output 'external/gazelle++go_deps+org_golang_x_sys/execabs/execabs.facts' was not created
ERROR: .../external/gazelle++go_deps+org_golang_x_sys/execabs/BUILD.bazel:3:11: Running nogo on @@gazelle++go_deps+org_golang_x_sys//execabs:execabs failed: not all outputs were created or valid
...
```

**Other notes for review**

This review continue to skip analysers when ignoring diagnostics, but
return a valid `pkg` to create the missing `.facts` file required by the
validation action, and incidentally, fix a potential crash at line
https://github.com/bazel-contrib/rules_go/blob/1f993522f463c142c5516f94852c3d6597f717c6/go/tools/builders/nogo_main.go#L98.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants