Buildifier formats inconsistently and contrary to PEP-8

[mwoehlke-kitware]

Buildifier apparently wants spaces around named parameters, which is contrary to PEP-8. However, it doesn't enforce them consistently, making it near impossible to maintain code style consistency. It is also lacking vs. PEP-8 in several other ways.

For example:

A = [3 ]
B = [ 42]

def bar(x = None):
    if x > 0:
      bar(x = x - 1)
    print(x)

def foo(
  x=None
        ):
  bar(x=0)

How many style problems/inconsistencies can you spot in the above? There are several, but buildifier sees none.

(If buildifier followed PEP-8, this wouldn't be a problem; pycodestyle could be used to fill in the gaps that buildifier leaves.)

[pmbethe09]

1) skylark != python and the choice of spaces around named-parameters was intentional to differentiate. 2) buildifier does not currently format full skylark that could occur in .bzl files, only the BUILD file subset which we find in {BUILD, BUILD.bazel, BUCK} and that does not include any 'def's.

Work is being done on #2 to create a 'skylarkifier' which will likely be part of buildifier and create a consistent skylark format. This will not be quite the same as py-format for a few reasons including spaces around named-parameters.

[mwoehlke-kitware]

skylark != python and the choice of spaces around named-parameters was intentional to differentiate.

That's fine, if buildifier can format everything that pycodestyle can check. Right now, it's leaving our code inconsistently formatted... and I can't use pycodestyle, which would catch the inconsistency, because pycodestyle's enforced style is contrary to buildifier. So, please, fix the holes.

p.s. Please, for the love of readability, support this:

#------------------------------------------------------------------------------
def foo():
    pass

#------------------------------------------------------------------------------
def bar():
    pass

...rather than the asinine, less-readable format that pycodestyle wants. (Particularly, don't reject "rule" (as in rule(3)) comments the way pycodestyle does.)

Also, not in my original example, but buildifier is also missing maximum line length (which can occur in BUILD).

[laurentlb]

Buildifier can currently format only BUILD files. If you pass a .bzl file, it will skip the def blocks.

As in Go, there is no strict maximum line length. In particular, it's possible to have long labels in BUILD files (e.g. when you have a deep directory structure) and you shouldn't split them.

Shall we close this issue, or is there an action item?

[mwoehlke-kitware]

The lack of a functioning style tool for .bzl is the action item. (Maybe that is duplicated elsewhere?)

[swolchok]

In this vein, it would be nice if buildifier could format and buildozer could see rule/macro invocations that were gated by a top-level if statement or contained in a top-level for loop. (If I had to pick one, I personally care more about if SOME_FLAG: totally hiding gated rules from buildozer.) BUCK files are arbitrary Python, and while I'm very sympathetic to Bazel's approach of heavily restricting the contents of its input files, Buck users will probably be saddled withifs and fors for a long time and handling them at top-level doesn't seem that onerous.