gazelle should exit with non-zero value for syntax errors in BUILD files

[ashi009]

What version of gazelle are you using?

v0.22.2

What version of rules_go are you using?

v0.24.7

What version of Bazel are you using?

4.0

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

Yes

What operating system and processor architecture are you using?

Mac Intel

What did you do?

$ cat BUILD.bazel
objc_library(
   	...
    pch = "label"
    visibility = ["//visibility:public"],
	...
)
...
$ bazel run //:gazelle
$ echo $?

What did you expect to see?

1

What did you see instead?

gazelle: <redacted>/BUILD.bazel:7:15: syntax error near visibility
0

[jayconrod]

This seems pretty reasonable.

Gazelle doesn't have a really well-defined policy on error handling or exit codes. Being unable to parse a build file seems like it should be a hard error though.

[aslonnie]

gazelle also does not emit non-zero exit value on other type of errors, like when having errors on running go get:

$ gazelle update
gazelle: finding module path for import <redacted>: go get: module <redacted>: git ls-remote -q origin in <redacted>: exit status 128:
	fatal: could not read Username for 'https://github.com': terminal prompts disabled
Confirm the import path was entered correctly.
If this is a private repository, see https://golang.org/doc/faq#git_https for additional information.
$ echo $?
0

I am not sure if there is a reliable way, say in a CI step, to tell that if gazelle runs successfully in these cases.

[aslonnie]

Just read some gazelle code; it is actually non-trivial to fix.

The interfaces gazelle using generally does not return an error. For example:

bazel-gazelle/resolve/index.go

Line 61 in 41b542f

Resolve(c *config.Config, ix *RuleIndex, rc *repo.RemoteCache, r *rule.Rule, imports interface{}, from label.Label)

which where the error is being dropped for go language:

bazel-gazelle/language/go/resolve.go

Line 96 in 41b542f

log.Print(err)

And changing the interface would be quite disruptive.

One idea to mitigate this can be adding maybe a ReportError function pointer into *config.Config, and hoping that important steps that need to report failing errors will have access to this config. The implementation of that function pointer can save the reported errors in a slice that can be picked up later by the upper caller (e.g. in runFixUpdate()). Not sure if that sounds like an acceptable design to the maintainers here.

[dr-dime]

I've created #1214 as a step to make gazelle more vigilant to errors.

[dr-dime]

Also found #499 for a similar request.