Repository: python/mypy
Pull request: Add help for --shadow-file
Fixes #2119.  Partial fix for #3553.

Problem statement:
Feature request: checking files on stdin
Hi there,

I'd like to add support for mypy to Emacs' Flycheck plug-in.  We tend to prefer feeding checkers their input on stdin instead of writing temporary files; is this something that mypy could support too?

The changes should be minimal; probably just an extra flag, similar to `-c` for checking a string; maybe `--stdin`? Additionally, a `--stdin-display-name` flag like flake8 would allow us to tell `mypy` what the name of the original file was.

Thanks!

Hints from issue discussion:
I can prepare a PR, if there's consensus and it helps.

I'm curious how that would work -- mypy typically has to analyze more than
just one file, unless you're using it for a toy program (where it really
isn't that useful). I'm also curious how much of a slow-down you'd see if
you wrote the data to a temporary file first -- the I/O to do that would
seem minimal compared to the cost of starting mypy. Have you tried that
yet? And do you have evidence that it's too slow?

Hey Guido,

> I'm curious how that would work -- mypy typically has to analyze more than just one file, unless you're using it for a toy program (where it really isn't that useful).

It would work just like `-c`, which is already implemented :)

```
$ echo "x = 5; x = "a"; x = []" > a.py

$ mypy -c "import a; y = 1; y = 'b'"
<string>:1: note: In module imported here:
a.py:1: error: Name 'a' is not defined
a.py:1: error: Incompatible types in assignment (expression has type List[None], variable has type "int")
<string>:1: error: Incompatible types in assignment (expression has type "str", variable has type "int")
```

Flycheck would run mypy from the right directory, so `mypy` would be able to resolve imports.  Additionally, we'd pass `--stdin-display-name` with the file's name to `mypy`, so it really wouldn't be different from reading the actual file.

> I'm also curious how much of a slow-down you'd see if you wrote the data to a temporary file first -- the I/O to do that would seem minimal compared to the cost of starting mypy. Have you tried that yet? And do you have evidence that it's too slow?

There is no slowdown; it's mostly an issue of convenience.  Since `mypy` depends on the file's location, we need to write temporaries to the file's parent folder. Historically, this has caused plenty of issues, including rebuilds from badly configured build systems, leftover temporaries if Emacs doesn't exit cleanly, scripts watching the file system firing too often, and probably others that I don't remember.

I can experiment with a PR, if you want; maybe it'll be easier to decide whether this feature is worth it by looking at actual code?

Thanks for the prompt response!

If it's just the module search path, you can set MYPYPATH.

I'm pushing back because mypy already has too many flags, and making sure
they all work and interoperate correctly is a never-ending task.

We don't have a facility for setting this environment variables before calling checkers, unfortunately; and adding it to Flycheck sounds like a non-trivial task.

I understand your concern regarding complexity.  Would it make it better if the implementation of this feature piggy-backed on `-c`'s existing code path (then the feature would be implemented as "read stdin", then feed resulting string to `-c` handler)? Additionally, would it make it better if instead of a flag we just recognized `"-"` as a special file name?

Can't you write a wrapper shell script? I would much rather see you spend
your efforts on your side than on ours...

No, we don't usually distribute wrappers for individual checkers with Flycheck, unfortunately.  If you don't feel that this feature would be useful beyond Flycheck, then we can live with temporary files :)

On the other hand, this is a common feature for linters.  I think that at least Atom's linter by @steelbrain would benefit from it (see e.g. https://github.com/brigade/scss-lint/issues/578) and presumably SublimeLinter would benefit from it too.

Maybe you could explain how you envision a Flycheck user invoking mypy? (I'm an Emacs user but an incredibly unsophisticated one, and Flycheck is way too fancy for my tastes. :-)

Isn't usually your Python code in a file that you're editing anyway? (I know mine is.) I don't think sending mypy a fragment of code makes a lot of sense.

@gvanrossum Just a side note, that the contents are from the file but are different from the ones on disk (because the user has not saved yet). If your package does any relative resolutions for things like configs, you could require the stdin consumers to specify the path of the file as a param (like both jshint and scss-lint do).

Welcome to the wonderful world of editor integration. :-( I guess I'll have
to review another PR...

Commit message: Add help for --shadow-file

Patch to review:
diff --git a/mypy/main.py b/mypy/main.py
index 978db244b089..f0cb4246a694 100644
--- a/mypy/main.py
+++ b/mypy/main.py
@@ -321,13 +321,13 @@ def add_invertible_flag(flag: str,
         ", ".join(strict_flag_names))
     parser.add_argument('--strict', action='store_true', dest='special-opts:strict',
                         help=strict_help)
+    parser.add_argument('--shadow-file', nargs=2, metavar=('SOURCE_FILE', 'SHADOW_FILE'),
+                        dest='shadow_file',
+                        help='Typecheck SHADOW_FILE in place of SOURCE_FILE. '
+                        'Primarily intended for tooling. Allows tooling to make transformations '
+                        'to a file before type checking without having to change the file '
+                        'in-place.')
     # hidden options
-    # --shadow-file a.py tmp.py will typecheck tmp.py in place of a.py.
-    # Useful for tools to make transformations to a file to get more
-    # information from a mypy run without having to change the file in-place
-    # (e.g. by adding a call to reveal_type).
-    parser.add_argument('--shadow-file', metavar='PATH', nargs=2, dest='shadow_file',
-                        help=argparse.SUPPRESS)
     # --debug-cache will disable any cache-related compressions/optimizations,
     # which will make the cache writing process output pretty-printed JSON (which
     # is easier to debug).


Code review comments:
diff_hunk: @@ -321,13 +321,13 @@ def add_invertible_flag(flag: str,
         ", ".join(strict_flag_names))
     parser.add_argument('--strict', action='store_true', dest='special-opts:strict',
                         help=strict_help)
+    parser.add_argument('--shadow-file', nargs=2, metavar=('SOURCE_FILE', 'SHADOW_FILE'),
+                        dest='shadow_file',
+                        help='Typecheck SHADOW_FILE in place of SOURCE_FILE. '
+                        'Primarily intended for tooling. Allows tooling to make transformations '
+                        'to a file before type checking without having to change the file '
+                        'in-place.')
path: mypy/main.py
text: @user1:
I would prefer a shorter (one-line) help text here and the longer explanation in the docs.

@author:
Agreed -- that's definitely better.