Add -strict to exit on build file and directive errors

[dr-dime]

Gazelle tolerates common errors, which is fine for daily uses.
However, when being used for scripting, we will need a way to
tell if such errors have occurred. This CL adds -strict flag
to exit with a non-zero value after seeing those errors.

Updates #1029

[achew22]

Hey @dr-dime, thanks so much for your contribution! We just merged in a way to create test cases that cover things like this pretty simply. Could I have you add a test for this before we merge it?

The high level overview of how to create a test using this is to:

1. cp -r tests/diff_mode tests/strict_mode from the root of the repo
2. Manipulate the arguments.txt file so that it contains -strict
3. Put in a BUILD.in (note that it can't just be called BUILD because we want to be able to compare the before and after, which is called BUILD.out)
4. bazel test //tests:strict_mode and confirm that it passes. If it doesn't pass because of a diff in the stdout/stderr it will print out a command you can run to generate expected{ExitCode,Stdout,Stderr}.txt as well as any BUILD.out files that were created.

[dr-dime]

Will do.

[dr-dime]

@achew22 the error message gazelle generates includes the full path of the temp directory so it's very much impossible to match in the future. Now I have to fix generationtest first 🤣

gazelle: /private/var/folders/_9/15t9b0j51hz3fwb8v00mq44w0000gn/T/gazelle_test3489490209/fix_strict_mode/BUILD.bazel:3:13: syntax error near visibility
gazelle: Exit as strict mode is on

[dr-dime]

depends on #1231

[achew22]

As a temporary hack, we could tag this test specifically as not to be run in windows?

bazel-gazelle/.bazelci/presubmit.yml

Line 37 in 6adf619

- "-//internal:bazel_test"

[achew22]

What happens if there are multiple errors (maybe even across multiple files)? Do they accumulate and all get printed out? I presume this is mostly going to be used in CI pipelines, should we try to get all of the errors printed out before quitting?

[dr-dime]

It will exit at the first error. I give that a quick thought as well and decided not to do it.

The bzl parser bailed out at the first encountered error, so we can report at most one error per file. Also, the current interface/API doesn't return errors, so to collect all the errors, some serious refactor needs to take place first. See the discussion in #1029.

[achew22]

I guess the question is, would it be better to "accumulate" to stderr and have a single boolean tracking success and os.Exiting essentially in the main function?

[dr-dime]

It would be better to accumulate all errors, but there is no clean way to do so with the current API. I've tried this approach in master...dr-dime:cl/strict-mode-2, which doesn't LGTM as the error handling strategy is not clear at all.

I admit that those log.Fatals are not pretty, but it's following the comment on error handling in lang.Language interface. I can put some big TODOs next to them to make this more explicit as a refactor goal in the future.

[achew22]

Yeah, let's drop in some TODOs and hopefully address them down the road. It really would be better to accumulate some errors but it looks very painful

[dr-dime]

@achew22 Done. PTAL