Update rules to match bazelbuild/buildtools

[erikkerber]

Largely matches the current implementation of bazelbuild/buildtools. I don't necessarily love the new semantics, but this does allow seamless transition between the two.

• Adds buildifier_test
• Removes 'mode' from tests
• Adds srcs to buildifier.tests

40

[erikkerber]

Integration tests failed for me due to buildifier warning on:

buildifier: warning categories with modifiers ("+" or "-") can't be mixed with raw warning categories

Seems the lint_warnings are invalid cc: @cgrindel

[keith]

isn't this failure a usage issue? https://github.com/bazelbuild/buildtools/blob/7186f635531bf1c395f246cc2687aecb56ff0572/buildifier/utils/flags.go#L127

[erikkerber]

It is. Well, sort of. I assumed that failure had just always been there since the existing usage appears to violate that rule.

There's a slightly different --warnings format passed before and after this PR, and I found a bug in the rule impl I copied over. Let me figure out which one was the actual problem.

[erikkerber]

Alright, good to go. The lint_warnings format does allow that "mixing" works with the semantics in this repo:

    if len(ctx.attr.lint_warnings) > 0 and not ctx.attr.lint_mode:
        fail("Cannot pass 'lint_warnings' without a 'lint_mode'")
    for warning in ctx.attr.lint_warnings:
        args.append("--warnings={}".format(warning))

But doesn't work with what is used upstream:

    if ctx.attr.lint_warnings:
        if not ctx.attr.lint_mode:
            fail("Cannot pass 'lint_warnings' without a 'lint_mode'")
        args.append("--warnings={}".format(",".join(ctx.attr.lint_warnings)))

I kept the arg style used in this repo, which does work the integration suite.