buildifier(feature): .buildifier.json config file

[pcj]

• Add new package github.com/bazelbuild/buildtools/buildifier/config
• Refactor flags from buildifier.go into the Config struct.
• Refactor buildifier.go to use a type buildifier struct initialized with a Config.
• Move github.com/bazelbuild/buildtools/buildifier/edit.isFile to github.com/bazelbuild/buildtools/buildifier/wspace.IsRegularFile
• Move github.com/bazelbuild/buildtools/buildifier/utils.ValidateInputType to github.com/bazelbuild/buildtools/buildifier/config.validateInputType (and similar functions)
• Add new convention of {WORKSPACE_DIR}/.buildifier.json where the config file can optionally exist.
• Add new environment variable BUILDIFIER_CONFIG that can override location of config file.
• Add new flag -config that can specify location of config file.
• Add new behavior -config=example that will print a sample JSON config initialized with warn.AllWarnings to stdout.
• Add new validation tests of the flag combinations.
• Add integration tests

Fixes #479

[pcj]

Can be tested with the bazel-stack-vscode extension by putting the following in your settings.json (workspace or user):

{
    "bsv.buildifier.githubOwner": "pcj",
    "bsv.buildifier.githubRelease": "5.1.1-rc1"
}

This will download artifacts from test release https://github.com/pcj/buildtools/releases/tag/5.1.1-rc1.

[JayBazuzi]

Looking forward to this new feature!

When the config file is malformed I get a message like:

buildifier: invalid character ']' looking for beginning of value

This message needs context, e.g. "while reading config file {PATH}".

[JayBazuzi]

JSON is great as a data interchange format, but has some annoyances for human use:

I wish for comments:

{
    "warningsList": [
        // TODO: Fix this
        "-unused-variable",

        // This is annoying and encourages 0-value comments that just take up space
        "-function-docstring-args"
    ]
}

And trailing commas:

{
    "warningsList": [
        "-name-conventions",
        "-print",
        "-unnamed-macro",
        "-load",
        "-unused-variable",
    ],
}

[JayBazuzi]

I suggest taking this PR as-is, and and consider the above feedback for future improvements. @pcj how does that sound to you?

[pcj]

@JayBazuzi that sounds fine to me. Regarding the JSON comments, I considered using a YAML config file but there is no yaml library in the stdlib and didn't want to take on additional dependencies. Looks like it needs to be rebased given the age of the PR.

@vladmos or @fmeum can you review?

[pcj]

Updated and squashed to https://github.com/bazelbuild/buildtools/pull/1080/commits/ee3f889a752c9bd6e3c2e136baa4788ff2e51eee

[fmeum]

@pcj Sorry, I'm not officially affiliated with this project, I just happened to contribute to it frequently in the last couple of weeks. I can just say that I really like what this PR is doing.

[JayBazuzi]

Apologies for the spam, but could one of @vladmos @meisterT @pmbethe09 review this change?

[vladmos]

Thanks for the PR, I'll just need to make sure it doesn't break any existing internal usages and will merge it.

[meisterT]

For the JSON vs yaml discussion: how heavy is the dep for yaml?

Another format that we could consider is textproto.

[vladmos]

For JSON vs yaml: I think we can merge it as is now and add a support for .buildifier.yaml later if needed.