buildtools icon indicating copy to clipboard operation
buildtools copied to clipboard

disable buildifier in buildozer

Open mobileink opened this issue 3 years ago • 3 comments

I don't want buildozer to reformat my code. If I want that I can run buildifier separately. Maybe -buildifier=disable?

Specific case: I have attribute lines like:

    opts = [
        "-w", "-32",  # Unused value declaration.
    ],

and that's the way I want them formatted. buildozer inserts a newline. but buildozer should not force me to adopt its idea of good formatting.

Even worse: I need to add pairs of options like the one above, in the correct order, but when I do buildifier sorts them.

An alternative to -buildifier=disable would be to provide a predefined identity formatter.

mobileink avatar Jan 06 '21 11:01 mobileink

There's no point in disabling buildifier there, buildozer will format the output files anyway, by design it's not even able to print files with custom formatting. Using a separate buildifier call is redundant in most of the cases, but sometimes can be useful, for example:

  1. If you have a presubmit check that makes sure that all your BUILD files are formatted, you can use a specific buildifier version to make sure it maches the version used by the presubmit check.
  2. In theory it's possible that buildozer may produce a not exactly correct AST which will be formatted incorrectly, for instance it adds an ident node containing something more complex than just an ident, e.g. f( x ). In this case the file should be re-parsed and re-formatted to make sure the function call above is parsed correctly as a function call and printed correctly, in this example without redundant spaces. But currently even if you don't provide an external buildifier binary, buildozer will try to that using its internal parser and formatter.

Custom formatting for chunks of a file is currently not supported, but if it sorts a list that shouldn't be sorted you can disable that with a special comment:

my_rule(
    ...
    # do not sort
    opts = [ ... ]
)

or

my_rule(
    ...
    opts = [
        # do not sort
        ...
    ],
)

vladmos avatar Jan 13 '21 13:01 vladmos

I use a no-op buildifier for large changes which just delete lines ----- snip ---- #!/bin/sh echo $* >/tmp/fooo for arg in $* ; do case $arg in */BUILD ) cat $arg ;;

  • ) ;; esac done cat ---- snip ----

aiuto avatar Jul 30 '21 01:07 aiuto

Based on recent experience with a very large change, I think I might be happy with an option to disable various buildifier rewrites on buildozers print.

  • nothing should be resorted by default
  • probably other things should be turned off.

The way to think about it is the Unix philosophy. One tool does one job well. buildozer should change the structure of the build graph, not the format of the file. If I want to also fully reformat the files, it is trivial to apply buildifier to my PR.

Related: https://github.com/bazelbuild/buildtools/issues/1004

aiuto avatar Sep 22 '21 17:09 aiuto