Produce reports containing file/line information of bad formatting

[ianfixes]

Problem

In some senses, black is a static analysis tool like a linter. I'd like to integrate black into other tooling at my organization that understands code errors at a file/line level, but there doesn't seem to be any way to do this.

Solution I'd Like

It would be awesome if in addition to the non-code-editing flags black --check and black --diff, there was one that just presented the results as a JSON report, something like black --report=json (or a simpler one as black --report=emacs). The JSON report might be made in a similar way to rubocop's:

{
  "metadata": {
    "rubocop_version": "0.55.0",
    "ruby_engine": "ruby",
    "ruby_version": "2.3.7",
    "ruby_patchlevel": "456",
    "ruby_platform": "universal.x86_64-darwin17"
  },
  "files": [
    {
      "path": "zapp/models/user_session.rb",
      "offenses": [
        {
          "severity": "warning",
          "message": "Lint/DuplicateMethods: Method `UserSession#geoip_data` is defined at both app/models/user_session.rb:40 and app/models/user_session.rb:125.",
          "cop_name": "Lint/DuplicateMethods",
          "corrected": false,
          "location": {
            "start_line": 125,
            "start_column": 3,
            "last_line": 125,
            "last_column": 5,
            "length": 3,
            "line": 125,
            "column": 3
          }
        }

I'm not very familiar with "emacs format", I've just seen some tools output a colon-separated line of filename:start_line:start_char:end_line:end_char (or some such) and seen it referred to as compatible with emacs in some way.

Alternatives

Create mini-report.sh

#!/bin/sh

# expects a python filename as input
# print out a mini report of black findings as file:line
# changes files but then (hopefully) un-does the changes

TMPFILE=.black.orig.txt

cp $1 $TMPFILE
black $1 2>/dev/null
diff --new-line-format= --unchanged-line-format= --old-line-format="$1:%dn%c'\12'" $TMPFILE $1
mv $TMPFILE $1

Then from the shell

$ black --check .
would reformat /path/to/mostly_good_formatting.py
Oh no! 💥 💔 💥
1 file would be reformatted, 36 files would be left unchanged.
$ black --check . 2>&1 | grep "would reformat" | cut -d" " -f 3- | xargs -I{} echo "sh mini-report.sh {}" | sh
/path/to/mostly_good_formatting.py:21
/path/to/mostly_good_formatting.py:55

Which works but is a fragile workaround.

[ErezAmihud]

As said in the following mypy discussion the golden standard for json output is the Static Analysis Results Interchange Format (SARIF) . Github has out of the box support for it, and it is verbose enough to convert to any other needed format (emacs, codeclimate, etc).

A problem that needs answering before implementing this is how do we determine on which line and location the reformatting is needed? As far as I can see black only report if it can change a whole file without givving specific lines where the problem exists.

[JelleZijlstra]

I'm not enthusiastic about adding a new and potentially complex output format to Black. The intended use case for Black is that you run it on a file and apply the format, without worrying about exactly how to apply the formatting.

I think what the OP requests could be implemented through a third-party tool that invokes black --diff and parses the result.

[ianfixes]

potentially complex output format

The file:line format -- e.g. /path/to/mostly_good_formatting.py:55 -- is perfectly sufficient here, and I wouldn't call it "complex". Not knowing your preference, I posted the JSON to illustrate the other extreme.

what the OP requests could be implemented through a third-party tool

Correct; the few lines of bash that I posted above demonstrate such an implementation. But using tempfiles and relying on certain features of diff to be available seems less portable than simply enabling black to report the file/line where it encounters non-compliant code.

At a bare minimum, would you find value in just bundling some version of my wrapper script(s) with the black installation? The end goal of this feature request is to enable richer CI/CD behavior, which can greatly reduce the friction between a maintainer and a junior external contributor.

[JelleZijlstra]

The file:line format -- e.g. /path/to/mostly_good_formatting.py:55 -- is perfectly sufficient here, and I wouldn't call it "complex". Not knowing your preference, I posted the JSON to illustrate the other extreme.

Black internally doesn't work that way at all, though. It simply modifies the syntax tree to look the way it wants it to look; it doesn't have a concept internally of specific issues that happen at particular places. So we'd either have to do diff parsing ourselves, or greatly change how Black works internally.

At a bare minimum, would you find value in just bundling some version of my wrapper script(s) with the black installation? The end goal of this feature request is to enable richer CI/CD behavior, which can greatly reduce the friction between a maintainer and a junior external contributor.

Personally I'd be hesitant because it would mean taking up maintenance load and I don't see a clear benefit over the other integrations we already recommend (e.g. GitHub Actions, editor integrations, pre-commit hooks). I would prefer for a tool like this to be maintained outside of the Black repo first; we could always fold it into Black itself later if it proves popular and stable. I am not speaking for the whole maintainer team, however, and others may have different opinions.

[ianfixes]

I'm hearing that this feature isn't related to any existing features of black, which is certainly unfortunate and I'll understand if this issue should be closed/wontfix as a result.

That said, I believe there is great value in being able to express a set of lint errors in file/line format because it makes it easier to integrate that functionality into IDEs and things like GitHub Annotations. Even if you don't support it in the executable itself, a more stable way to parse out the filenames containing violations would at least enable 3rd parties to (safely) add the diff functionality.

[ErezAmihud]

I'm hearing that this feature isn't related to any existing features of black, which is certainly unfortunate and I'll understand if this issue should be closed/wontfix as a result.

That said, I believe there is great value in being able to express a set of lint errors in file/line format because it makes it easier to integrate that functionality into IDEs and things like GitHub Annotations. Even if you don't support it in the executable itself, a more stable way to parse out the filenames containing violations would at least enable 3rd parties to (safely) add the diff functionality.

I would agree if black was a linter or an error finder, but it is a formatter, and I think there is not a lot of value in knowing where the formatting issues are when you can just run black <filename>, as most users would do. As JelleZijlstra said there are a lot of integrations that just run the formatting for you.

[reitzig]

Well, I for one use black --check as a linter in my CI pipeline. 💁

It would be nice to report errors/warnings to the CI system (similar to ruff, pytest, ...) but not essential; I view running it in the CI build as a safety net only. It's easy enough to run black locally to fix a rejected commit -- if you didn't integrate it into your IDE and didn't install the pre-commit hooks ... 🙄

[ianfixes]

The use case / user persona being overlooked here is that of a maintainer of a public GitHub project or InnerSource-style corporate project.

True, users can run this tool locally. That may even be a best practice. But that involves pushing work to a (new) user in 2 ways:

1. by expecting them to set up precisely the right development environment as a prerequisite to contributing
2. By not supporting a pipeline that can show them exactly where their problems are in code review -- it impedes their path to an acceptable PR

There is every reason not to add this feature and to call it out of scope for this project -- it's not a bug, and it's not up to me to say what this project's goal/vision is or should be.

However, you can make that choice without implying that the use case behind my request is invalid. I wouldn't have bothered to write this issue and submit a working implementation if it wasn't of great value to me and the developers I was supporting at the time I opened this

[steinuil]

I think what the OP requests could be implemented through a third-party tool that invokes black --diff and parses the result.

In case anybody finds this issue, I did just that and wrote https://github.com/steinuil/black-codeclimate a couple of months ago to use in pipelines at work. It uses unidiff to parse the black --diff output, and so far we haven't run into any issues.