[Summer 2021] Mirror internal linter/formatter behaviour in opensource

[r0rshark]

At Facebook we use an internal formatter a way to replicate a similar behaviour in the Opensource repo is

1. Use µfmt instead of using black or µsort directly. This combines the import sorting and formatting into a single, atomic step, to minimize the chance of CI failures for inconsistent formatting between the two tools. $ ufmt <format|check|diff> <path>
2. Configure µsort to disable first party namespace detection. In your project's pyproject.toml. This matches the general behavior of the internal formatter , where anything is sorted with third-party packages:

   [tool.usort]
   first_party_detection = false

3. Make sure you pin dependencies in your requirements.txt to the same versions as the internal linter. We try to stay up to date, but may be delayed in upgrading.

   black version 21.4b2
   usort version 0.6.4
   libcst version 0.3.19

We can update the https://github.com/MLH-Fellowship/pyre-check/blob/master/.github/workflows/lint.yml lint action with this setup and test if it works :)

[abishekvashok]

Claiming this one as I feel it's an important issue.

[abishekvashok]

Hey @r0rshark I think we will need more configuration information here, as trying to run the latest ufmt on the project with the packages outlined above leads to a lot of files being detected (all .pyi files get flagged)

(I've tried using the given pyproject.toml setup and tweaking it a bit, black line-length for example, but couldn't get it working in sense that it's reporting many files need to be reformatted)

[abishekvashok]

Another interesting thing is that, running black and usort alone doesn't produce errors! It must be ufmt. But I checked the source code and all it does is to call usort and pipe the results to black (or the other way around).

[abishekvashok]

So I tried to hook a GitHub action: https://github.com/abishekvashok/pyre-check/runs/3047301951?check_suite_focus=true

But the diffs are kinda not what we want, its complaining that the ellipses shouldn't be inline.

eg)

-    def __init__(self, d1: int, d2: int) -> None: ...
+    def __init__(self, d1: int, d2: int) -> None:
+        ...

I tried excluding the pyi files but with no luck :/

[r0rshark]

@abishekvashok I am not very familiar with µfmt I found these guidelines internally. Did you find out why running black and usort/isort individually doesn't produce any issue but µfmt complains? This may help us defining a configuration to avoid this behaviour. Can you share the your tests and the output so I can ask someone internally about which is the best configuration to deal with this?

[abishekvashok]

@r0rshark this is the action I run: https://github.com/abishekvashok/pyre-check/runs/3047301951?check_suite_focus=true

The workflow file can be seen here: https://github.com/abishekvashok/pyre-check/actions/runs/1023071148/workflow

I ran black and usort individually and got the results that everything is fine. But for some reason when I run ufmt, I get diffs. I am supposing there should be some way to avoid checking .pyi files in ufmt. I tried to put it in the pyproject.toml file for black as:

[tool.black]
extend-exclude = '''
/.pyi$?
'''

And some other regex to filter out .pyi files but no hope so far :/

[abishekvashok]

Seems like ufmt calls the black functions and skips calling in the function that loads the toml file.

def ufmt_string(path: Path, content: str, config: Config) -> str:
    content = usort_string(content, config, path)
    mode = Mode()  # TODO

    try:
        content = format_file_contents(content, fast=False, mode=mode)
    except NothingChanged:
        pass

    return content

format_file_contents is the black function. Do we really want ufmt? I could either make a PR there, or code our own piece of python code which runs usort and black in the same "single atomic" way and obey configurations...

But idk, maybe there is some config that ufmt looks to, I couldn't find it in the source code tho :/

[r0rshark]

@abishekvashok found an example of Facebook opensource project using ufmt to format and flake8 to lint (https://github.com/pytorch/botorch/blob/master/.github/workflows/lint.yml) we may try do something similar. For the pyi exclusion have you tried using a syntax like:

[tool.black]
exclude = '''
/(
    \.pyi
  | stubs
)/
'''

I found this working in the facebookresearch fairscale project :) https://github.com/facebookresearch/fairscale/blob/master/pyproject.toml

[abishekvashok]

Hey @r0rshark thanks for looking up internally. Really appreciate it! https://github.com/facebookresearch/fairscale/blob/master/.circleci/config.yml , the fairscale project doesn't use ufmt but rather relies on running multiple jobs with black, flake, and isort. This is something we can pickup, running both these on two separate runners (containers) so that the output of one tool doesn't affect the other one. What do you feel?

The botch project on the other hand, uses ufmt but doesn't have any stubs, I tried with both the configurations, and variety of regexes but the same result (black and usort on their own runs fine, ufmt shows lots of formatting changes for .pyi files)

[r0rshark]

Let's use a similar approach to the one used in the fairscale project. I think it should work :)

[abishekvashok]

Got it!

[abishekvashok]

@r0rshark so I made a PR, guessing the only Internal tools are black and usort: https://github.com/facebook/pyre-check/pull/453

I tried flake8 as I saw .flake8 in the project root, however it says there are formatting issues: https://github.com/abishekvashok/pyre-check/actions/runs/1029878306

If you guys really use flake8 internally, or other tools, please let me know the exact version and we can include it as well :)

[abishekvashok]

Closed via https://github.com/facebook/pyre-check/commit/ed663bf8ba532215afd50f43c8c1bc120df0a7f5