Repository: facebookincubator/ptr

Issue:
Add pyre as a ptr run option
## How is `ptr` not doing what you expect?
I'd like to be able to add `pyre` to run as a ptr config option. Today I can not

## What is your suggestion to make this better?
Add a option to allow mypy or pyre to run based on configuration.

## Code/Bug example?
- Add config grammar / examples
- Add code to drive pyre
- Add unit tests

This needs a fix landed for `pyre` to allow the pyre wrapper to find pyre.bin for unactivated venvs: https://github.com/facebook/pyre-check/issues/160

Discussion:
cooperlees: This will fail until https://github.com/facebook/pyre-check/issues/160 is closed.

Patch:
diff --git a/.travis.yml b/.travis.yml
index aca08c6..069593e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -8,8 +8,6 @@ matrix:
     - python: 3.6
       env: PTR_INTEGRATION=1
     - python: 3.7
-    - python: 3.7
-      env: PYRE_CHECK=1
     - python: 3.7
       env: PTR_INTEGRATION=1
     - python: 3.8-dev
diff --git a/README.md b/README.md
index 76b8bfa..962e4ed 100644
--- a/README.md
+++ b/README.md
@@ -111,7 +111,7 @@ ptr_params = {
 
 This is per project in your repository and if exists is preferred over `setup.py`.
 
-Please refer to [`setup.cfg.sample`](http://github.com/facebookincubator/ptr/blob/master/ptrconfig.sample) for the options available + format.
+Please refer to [`setup.cfg.sample`](http://github.com/facebookincubator/ptr/blob/master/setup.cfg.sample) for the options available + format.
 
 ### mypy Specifics
 
@@ -122,19 +122,7 @@ To have `ptr` run mypy using you config:
 - OR add **[mypy]** section to your `setup.cfg`
 
 `mypy` Configuration Documentation can be found [here](https://mypy.readthedocs.io/en/stable/config_file.html)
-
-```
-[mypy]
-python_version = 3.5
-check_untyped_defs = True
-disallow_incomplete_defs = True
-disallow_untyped_decorators = True
-disallow_untyped_defs = True
-no_implicit_optional = True
-show_error_context = True
-warn_unused_ignores = True
-warn_return_any = True
-```
+- An example `setup.cfg` can be seen [here](http://github.com/facebookincubator/ptr/blob/master/mypy.ini).
 
 # Example Output 📝
 
@@ -150,6 +138,9 @@ Here is what you want to see in your CI logs!
 [2019-02-06 21:52:00,726] INFO: Running /Users/cooper/repos/ptr/ptr_tests.py tests via coverage (ptr.py:417)
 [2019-02-06 21:52:04,153] INFO: Analyzing coverage report for /Users/cooper/repos/ptr/setup.py (ptr.py:417)
 [2019-02-06 21:52:04,368] INFO: Running mypy for /Users/cooper/repos/ptr/setup.py (ptr.py:417)
+[2019-05-03 14:54:09,915] INFO: Running flake8 for /Users/cooper/repos/ptr/setup.py (ptr.py:417)
+[2019-05-03 14:54:10,422] INFO: Running pylint for /Users/cooper/repos/ptr/setup.py (ptr.py:417)
+[2019-05-03 14:54:14,020] INFO: Running pyre for /Users/cooper/repos/ptr/setup.py (ptr.py:417)
 [2019-02-06 21:52:07,733] INFO: /Users/cooper/repos/ptr/setup.py has passed all configured tests (ptr.py:509)
 -- Summary (total time 22s):
 
@@ -181,7 +172,7 @@ Here are some examples of runs failing. Any "step" can fail. All output is predo
 
 -- Failure Output --
 
-/Users/cooper/repos/ptr/setup.py:
+/Users/cooper/repos/ptr/setup.py (failed 'tests_run' step):
 ...F....................
 ======================================================================
 FAIL: test_config (__main__.TestPtr)
@@ -197,7 +188,7 @@ Ran 24 tests in 3.221s
 FAILED (failures=1)
 ```
 
-### Coverage Failure
+### coverage
 
 ```
 [2019-02-06 21:55:42,947] INFO: Starting ptr.py (ptr.py:782)
@@ -215,7 +206,7 @@ FAILED (failures=1)
 
 -- Failure Output --
 
-/Users/cooper/repos/ptr/setup.py:
+/Users/cooper/repos/ptr/setup.py (failed 'analyze_coverage' step):
 The following files did not meet coverage requirements:
   ptr.py: 84 < 99 - Missing: 146-147, 175, 209, 245, 269, 288-291, 334-336, 414-415, 425-446, 466, 497, 506, 541-543, 562, 611-614, 639-688
 ```
@@ -240,7 +231,7 @@ The following files did not meet coverage requirements:
 
 -- Failure Output --
 
-/Users/cooper/repos/ptr/setup.py:
+/Users/cooper/repos/ptr/setup.py (failed 'black_run' step):
 would reformat /Users/cooper/repos/ptr/ptr.py
 All done! 💥 💔 💥
 1 file would be reformatted, 4 files would be left unchanged.
@@ -265,13 +256,45 @@ All done! 💥 💔 💥
 
 -- Failure Output --
 
-/Users/cooper/repos/ptr/setup.py:
+/Users/cooper/repos/ptr/setup.py (failed 'mypy_run' step):
 /Users/cooper/repos/ptr/ptr.py: note: In function "_write_stats_file":
 /Users/cooper/repos/ptr/ptr.py:179: error: Argument 1 to "open" has incompatible type "Path"; expected "Union[str, bytes, int]"
 /Users/cooper/repos/ptr/ptr.py: note: In function "run_tests":
 /Users/cooper/repos/ptr/ptr.py:700: error: Argument 1 to "_write_stats_file" has incompatible type "str"; expected "Path"
 ```
 
+### pyre
+
+```
+cooper-mbp1:ptr cooper$ /tmp/tp/bin/ptr --venv /var/folders/tc/hbwxh76j1hn6gqjd2n2sjn4j9k1glp/T/ptr_venv_49117
+[2019-05-03 14:51:43,623] INFO: Starting /tmp/tp/bin/ptr (ptr.py:1023)
+[2019-05-03 14:51:43,657] INFO: Installing /Users/cooper/repos/ptr/setup.py + deps (ptr.py:565)
+[2019-05-03 14:51:44,840] INFO: Running ptr_tests tests via coverage (ptr.py:565)
+[2019-05-03 14:51:47,361] INFO: Analyzing coverage report for /Users/cooper/repos/ptr/setup.py (ptr.py:565)
+[2019-05-03 14:51:47,559] INFO: Running mypy for /Users/cooper/repos/ptr/setup.py (ptr.py:565)
+[2019-05-03 14:51:47,827] INFO: Running black for /Users/cooper/repos/ptr/setup.py (ptr.py:565)
+[2019-05-03 14:51:47,996] INFO: Running flake8 for /Users/cooper/repos/ptr/setup.py (ptr.py:565)
+[2019-05-03 14:51:48,566] INFO: Running pylint for /Users/cooper/repos/ptr/setup.py (ptr.py:565)
+[2019-05-03 14:51:52,301] INFO: Running pyre for /Users/cooper/repos/ptr/setup.py (ptr.py:565)
+[2019-05-03 14:51:54,983] INFO: /Users/cooper/repos/ptr/setup.py has passed all configured tests (ptr.py:668)
+-- Summary (total time 11s):
+
+✅ PASS: 1
+❌ FAIL: 0
+⌛️ TIMEOUT: 0
+💩 TOTAL: 1
+
+-- 1 / 1 (100%) `setup.py`'s have `ptr` tests running
+
+-- Failure Output --
+
+/Users/cooper/repos/ptr/setup.py (failed 'pyre_run' step):
+2019-05-03 14:54:14,173 INFO No binary specified, looking for `pyre.bin` in PATH
+2019-05-03 14:54:14,174 INFO Found: `/var/folders/tc/hbwxh76j1hn6gqjd2n2sjn4j9k1glp/T/ptr_venv_49117/bin/pyre.bin`
+... *(truncated)* ...
+ptr.py:602:25 Undefined name [18]: Global name `stdout` is not defined, or there is at least one control flow path that doesn't define `stdout`.
+```
+
 # FAQ ⁉️
 
 ### Q. How do I debug? I need output!
@@ -295,6 +318,11 @@ All done! 💥 💔 💥
   - [devpi](https://pypi.org/project/devpi/): Can be ran and used to *proxy* packages locally when pip goes out to grab your dependencies.
 - Please ensure you're using the `-k` or `--venv` option to no recreate a virtualenv each run when debugging your tests!
 
+### Q. Why is ptr not able to run `pyre` on Windows?
+
+- `pyre` (pyre-check on PyPI) does not ship a Windows wheel with the ocaml pyre.bin
+
+
 # Contact or join the ptr community 💬
 
 To chat in real time, hit us up on IRC. Otherwise, GitHub issues are always welcome!
diff --git a/ci.py b/ci.py
index f9f48c3..af375a9 100644
--- a/ci.py
+++ b/ci.py
@@ -75,13 +75,6 @@ def integration_test() -> int:
     return cp.returncode + check_ptr_stats_json(stats_file)
 
 
-def pyre_check() -> int:
-    print("Running `pyre` type checking", file=stderr)
-    pyre_cmd = ["pyre", "--source-directory", ".", "check"]
-    cp = run(pyre_cmd, check=True)
-    return cp.returncode
-
-
 def ci(show_env: bool = False) -> int:
     # Output exact python version
     cp = run(("python", "-V"), stdout=PIPE, universal_newlines=True)
@@ -92,9 +85,6 @@ def ci(show_env: bool = False) -> int:
         for key in sorted(environ.keys()):
             print("{}: {}".format(key, environ[key]), file=stderr)
 
-    if "PYRE_CHECK" in environ:
-        return pyre_check()
-
     # Azure sets CI_ENV=PTR_INTEGRATION
     # Travis sets PTR_INTEGRATION=1
     if "PTR_INTEGRATION" in environ or (
diff --git a/ptr.py b/ptr.py
index 7ced007..2460fb6 100755
--- a/ptr.py
+++ b/ptr.py
@@ -36,13 +36,18 @@ if sys.platform == "win32":
 
 
 def _config_default() -> ConfigParser:
+    if WINDOWS:
+        venv_pkgs = "black coverage flake8 mypy pip pylint setuptools"
+    else:
+        venv_pkgs = "black coverage flake8 mypy pip pylint pyre-check setuptools"
+
     LOG.info("Using default config settings")
     cp = ConfigParser()
     cp["ptr"] = {}
     cp["ptr"]["atonce"] = str(int((cpu_count() or 20) / 2))
     cp["ptr"]["exclude_patterns"] = "build* yocto"
     cp["ptr"]["pypi_url"] = "https://pypi.org/simple/"
-    cp["ptr"]["venv_pkgs"] = "black coverage flake8 mypy pip pylint setuptools"
+    cp["ptr"]["venv_pkgs"] = venv_pkgs
     return cp
 
 
@@ -87,6 +92,7 @@ class StepName(Enum):
     black_run = 5
     pylint_run = 6
     flake8_run = 7
+    pyre_run = 8
 
 
 coverage_line = namedtuple("coverage_line", ["stmts", "miss", "cover", "missing"])
@@ -280,6 +286,15 @@ def _generate_pylint_cmd(
     return (*cmds, *sorted(py_files))
 
 
+def _generate_pyre_cmd(
+    module_dir: Path, pyre_exe: Path, config: Dict
+) -> Tuple[str, ...]:
+    if not config.get("run_pyre", False) or WINDOWS:
+        return ()
+
+    return (str(pyre_exe), "--source-directory", str(module_dir), "check")
+
+
 def _get_test_modules(base_path: Path, stats: Dict[str, int]) -> Dict[Path, Dict]:
     get_tests_start_time = time()
     all_setup_pys = find_setup_pys(
@@ -468,6 +483,7 @@ async def _test_steps_runner(
     mypy_exe = venv_path / bin_dir / "mypy{}".format(exe)
     pip_exe = venv_path / bin_dir / "pip{}".format(exe)
     pylint_exe = venv_path / bin_dir / "pylint{}".format(exe)
+    pyre_exe = venv_path / bin_dir / "pyre{}".format(exe)
     config = tests_to_run[setup_py_path]
 
     steps = (
@@ -524,6 +540,13 @@ async def _test_steps_runner(
             "Running pylint for {}".format(setup_py_path),
             config["test_suite_timeout"],
         ),
+        step(
+            StepName.pyre_run,
+            bool("run_pyre" in config and config["run_pyre"] and not WINDOWS),
+            _generate_pyre_cmd(setup_py_path.parent, pyre_exe, config),
+            "Running pyre for {}".format(setup_py_path),
+            config["test_suite_timeout"],
+        ),
     )
 
     steps_ran = 0
diff --git a/ptrconfig.sample b/ptrconfig.sample
index 4fdfb86..d8201ab 100644
--- a/ptrconfig.sample
+++ b/ptrconfig.sample
@@ -12,4 +12,5 @@ venv_pkgs =
   mypy
   pip
   pylint
+  pyre-check
   setuptools
diff --git a/setup.cfg.sample b/setup.cfg.sample
index 56ff149..e637fdd 100644
--- a/setup.cfg.sample
+++ b/setup.cfg.sample
@@ -4,8 +4,9 @@ test_suite = ptr_tests
 test_suite_timeout = 120
 # `required_coverage_` will be stripped for use
 required_coverage_ptr.py = 84
-required_coverage_TOTAL = 90
+required_coverage_TOTAL = 89
 run_black = true
 run_mypy = true
 run_flake8 = true
 run_pylint = true
+run_pyre = true
diff --git a/setup.py b/setup.py
index 1144b1a..ec3e688 100644
--- a/setup.py
+++ b/setup.py
@@ -18,7 +18,7 @@ ptr_params = {
     "test_suite": "ptr_tests",
     "test_suite_timeout": 120,
     # Relative path from setup.py to module (e.g. ptr == ptr.py)
-    "required_coverage": {"ptr.py": 84, "TOTAL": 90},
+    "required_coverage": {"ptr.py": 84, "TOTAL": 89},
     # Run black or not
     "run_black": True,
     # Run mypy or not
@@ -27,6 +27,8 @@ ptr_params = {
     "run_flake8": True,
     # Run pylint or not
     "run_pylint": True,
+    # Run pyre or not
+    "run_pyre": True,
 }
 
 


Test patch:
diff --git a/ptr_tests.py b/ptr_tests.py
index 4e98353..884dfcd 100644
--- a/ptr_tests.py
+++ b/ptr_tests.py
@@ -156,7 +156,7 @@ class TestPtr(unittest.TestCase):
         self.assertEqual(
             sc["ptr"].get("pypi_url", ""), expected_pypi_url  # pyre-ignore
         )
-        self.assertEqual(len(sc["ptr"].get("venv_pkgs", "").split()), 7)  # pyre-ignore
+        self.assertEqual(len(sc["ptr"].get("venv_pkgs", "").split()), 8)  # pyre-ignore
 
     @patch("ptr._gen_check_output", async_none)  # noqa
     @patch("ptr._set_pip_mirror")  # noqa
@@ -260,6 +260,20 @@ class TestPtr(unittest.TestCase):
                 (str(pylint_exe), "--rcfile", str(cf), str(py1), str(py2)),
             )
 
+    def test_generate_pyre_cmd(self) -> None:
+        with TemporaryDirectory() as td:
+            td_path = Path(td)
+            pyre_exe = Path("pyre")
+
+            conf = {"run_pyre": True}
+            if ptr.WINDOWS:
+                self.assertEqual(ptr._generate_pyre_cmd(td_path, pyre_exe, conf), ())
+            else:
+                self.assertEqual(
+                    ptr._generate_pyre_cmd(td_path, pyre_exe, conf),
+                    (str(pyre_exe), "--source-directory", str(td_path), "check"),
+                )
+
     def test_get_site_packages_path_error(self) -> None:
         with TemporaryDirectory() as td:
             lib_path = Path(td) / "lib"
@@ -422,13 +436,20 @@ class TestPtr(unittest.TestCase):
                         True,
                     )
                 ),
-                (None, 6),
+                # Windows will not run pyre
+                (None, 6) if ptr.WINDOWS else (None, 7),
             )
 
-            # Run everything including black except on Windows + Python 3.7
-            expected = (
-                (None, 6) if ptr.WINDOWS and version_info >= (3, 7) else (None, 7)
-            )
+            if ptr.WINDOWS and version_info >= (3, 7):
+                # No pyre + black
+                expected = (None, 6)
+            elif ptr.WINDOWS:
+                # No pyre
+                expected = (None, 7)
+            else:
+                # Running all steps on everything else
+                expected = (None, 8)
+
             fake_tests_to_run = {fake_setup_py: ptr_tests_fixtures.EXPECTED_TEST_PARAMS}
             self.assertEqual(
                 self.loop.run_until_complete(
diff --git a/ptr_tests_fixtures.py b/ptr_tests_fixtures.py
index 793069a..884d438 100644
--- a/ptr_tests_fixtures.py
+++ b/ptr_tests_fixtures.py
@@ -27,11 +27,12 @@ EXPECTED_TEST_PARAMS = {
     "entry_point_module": "ptr",
     "test_suite": "ptr_tests",
     "test_suite_timeout": 120,
-    "required_coverage": {"ptr.py": 84, "TOTAL": 90},
+    "required_coverage": {"ptr.py": 84, "TOTAL": 89},
     "run_black": True,
     "run_mypy": True,
     "run_flake8": True,
     "run_pylint": True,
+    "run_pyre": True,
 }
 
 EXPECTED_COVERAGE_FAIL_RESULT = test_result(
@@ -192,11 +193,12 @@ entry_point_module = ptr
 test_suite = ptr_tests
 test_suite_timeout = 120
 required_coverage_ptr.py = 84
-required_coverage_TOTAL = 90
+required_coverage_TOTAL = 89
 run_black = true
 run_mypy = true
 run_flake8 = true
 run_pylint = true
+run_pyre = true
 """
 
 SAMPLE_SETUP_PY_PTR = {