Rewrite buildifier wrapper as bazel-bin/tools/lint/buildifier

[jwnimmer-tri]

Relates #6996.

Details:

• Add tools/lint:find_data and unit test.
• Add tools/lint:util and unit test.
• Rewrite buildifier.sh in python, now bazel-bin/tools/lint/buildifier:
  • Add explicit opt-in for --all.
  • Improve the command-line processing and help.
  • List out specific files that fail, instead of all.
• Port automated linting to use the new tool.
• Update docs.

---

This change is 

[soonho-tri]

+@soonho-tri

---

Review status: 0 of 13 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[soonho-tri]

(aside two minor suggestions)

---

Reviewed 14 of 14 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

drake/doc/bazel.rst, line 129 at r1 (raw file):

  cd /path/to/drake-distro
  bazel-bin/tools/lint/buildifier --all               # Reformat all Bazel files.

Do you think it's good to have the following line?

bazel build //tools/lint:buildifier

---

tools/lint/util.py, line 48 at r1 (raw file):

    for abs_dirpath, dirs, files in os.walk(workspace_root):
        assert abs_dirpath.startswith(workspace_root)
        rel_dirpath = abs_dirpath[len(workspace_root) + 1:]

BTW, how about the following (which might be more readable)?

rel_dirpath = os.path.relpath(abs_dirpath, workspace_root)

---

Comments from Reviewable

[jwnimmer-tri]

+@ggould-tri for platform review per rotation, please.

---

Review status: 12 of 13 files reviewed at latest revision, 2 unresolved discussions.

---

drake/doc/bazel.rst, line 129 at r1 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

Do you think it's good to have the following line?

bazel build //tools/lint:buildifier

Done, thanks for the reminder. I'd had this text in an original draft but lost it during rebasing.

---

tools/lint/util.py, line 48 at r1 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

BTW, how about the following (which might be more readable)?

rel_dirpath = os.path.relpath(abs_dirpath, workspace_root)

OK I hadn't of that, but I just tried it, and it leaves ./ at the start of all paths, which I would have to then strip off. I would still have to keep the assertion around, so I guess I think this parallel construction is clearest.

---

Comments from Reviewable

[jwnimmer-tri]

FYI /CC @SeanCurtis-TRI in case you want to jump in as a user representative. (I don't think CLion cares much about this one; in next PRs where I update cpplint and clang-format-includes similarly, I will be sure to tag you.)

[soonho-tri]

Review status: 12 of 13 files reviewed at latest revision, 1 unresolved discussion.

---

tools/lint/util.py, line 48 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

OK I hadn't of that, but I just tried it, and it leaves ./ at the start of all paths, which I would have to then strip off. I would still have to keep the assertion around, so I guess I think this parallel construction is clearest.

OK. I agree.

---

Comments from Reviewable

[soonho-tri]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[ggould-tri]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

tools/lint/BUILD.bazel, line 22 at r2 (raw file):

)

py_binary(

Is there a way to prevent this from being invoked via bazel run? I suspect that editing bazel files from within a bazel command is hazardous.

---

tools/lint/find_data.py, line 1 at r2 (raw file):

"""Helper for locating data=[] resources from a py_binary.

It is not clear from context that "data" and "py_binary" refer to the bazel concepts. Mention that this is is concerning itself with Bazel lint data.

---

Comments from Reviewable

[jwnimmer-tri]

Review status: 11 of 13 files reviewed at latest revision, 2 unresolved discussions.

---

tools/lint/BUILD.bazel, line 22 at r2 (raw file):

Previously, ggould-tri wrote…

Is there a way to prevent this from being invoked via bazel run? I suspect that editing bazel files from within a bazel command is hazardous.

Done.

In 'check' and 'diff' mode we should allow it, but in other modes now we fail.

---

tools/lint/find_data.py, line 1 at r2 (raw file):

Previously, ggould-tri wrote…

It is not clear from context that "data" and "py_binary" refer to the bazel concepts. Mention that this is is concerning itself with Bazel lint data.

Done.

---

Comments from Reviewable

[ggould-tri]

---

Reviewed 11 of 14 files at r1, 1 of 1 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[SeanCurtis-TRI]

FYI I'm assuming that CLion will be untouched by this.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from Reviewable

[jwnimmer-tri]

Yeah. And also I left a deprecation shim in place, so if you somehow see deprecation warnings, then we'll have to find what's going on.