Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support a project/repo-wide config file #1249

Closed
gnprice opened this issue Feb 27, 2016 · 10 comments
Closed

Support a project/repo-wide config file #1249

gnprice opened this issue Feb 27, 2016 · 10 comments
Assignees

Comments

@gnprice
Copy link
Collaborator

gnprice commented Feb 27, 2016

We have a couple of different ad-hoc scripts for invoking mypy developing in different repositories where people are using it, which generally

  • pass flags like --py2 and --silent-imports, and
  • work out through various logic what files mypy should process.

Having bits of shell script grow is never fun, and the logic to find what files to run on isn't totally trivial -- it ends up something like "files with *.py names that match # type: with grep." Even as I write that it's actually more subtle as # type: is both slightly too strict (#type:, no space, is a perfectly good start to a type comment) and too loose (string literals.)

Moreover, when there's a complicated script and the arguments to the underlying mypy become hard to identify, then when people are debugging an issue and need to vary the mypy invocation at the command line they can end up reconstructing versions that miss some of the details, which leads to a super frustrating debugging session -- @gvanrossum saw this happen yesterday.

So I think it'd be really helpful to get to a point where a user can sit anywhere inside the source tree of a codebase that uses Mypy, and type just the command

mypy

to have the typechecker run in the normal way over that codebase. Plus arguments as desired to modify what it does, but starting from that normal configuration.

The basic tactic that a lot of development tools take to make that kind of interface possible (including Hack, git, arc, and many others) is to have

  • a file (or subdirectory) sitting at the root directory of the project, which
  • doubles as a configuration file (or directory with room for config files.)

So e.g. a file .mypy, and when the mypy command is run, it looks up in the filesystem to find such a file. If there, then (a) the default operation is on all annotated files in the tree (by some specified definition), and (b) it's read as a configparser config file, which could be empty or could specify the equivalent of --py2, --silent-imports, etc.

Possible variations include

  • different filenames; or perhaps it's a directory, which could double as the home for cache files
  • maybe instead of mypy as the command to get this behavior, we start giving ourselves room for subcommands:
    • e.g., mypy check means "run the typechecker on this project, requiring a .mypy file"
    • then mypy ls could mean "list what files you would typecheck", which will be very helpful in some debugging situations especially in the presence of --silent-imports
    • the clash with e.g. mypy check.py is awkward, but we could say something like "if it ends in .py or / it's a file, otherwise it can only be a subcommand" -- just existing invocations like mypy myscript would break
    • alternatively we could avoid that clash by using some command name other than mypy

/cc @ddfisher who was also discussing this today.

@gnprice gnprice modified the milestone: 0.3.2 Mar 1, 2016
@gvanrossum
Copy link
Member

One potential feature that this doesn't seem to support is automatically finding files to check by grepping for e.g. # type:. (We do this at Dropbox in one project and it works reasonably well.)

@gvanrossum gvanrossum modified the milestones: 0.3.3, 0.3.2 Mar 17, 2016
@wittekm
Copy link
Contributor

wittekm commented May 26, 2016

A potential use case would be to enforce --disallow-untyped-defs on a per-file (or perhaps per-module) basis. A co-worker says they'd gotten a file fully typed and didn't want any regressions on that in the future.

@JukkaL
Copy link
Collaborator

JukkaL commented May 26, 2016

There is another issue about per-file build flags: #1235

@gvanrossum
Copy link
Member

IIRC @ddfisher is working on this -- David, how's this going?

@messense
Copy link

+1 for this feature.

@gvanrossum
Copy link
Member

I'm looking into this. As an intermediate step I propose to support a configuration file, default mypy.ini, that contains options corresponding to the options found in mypy/options.py. The options should be in a section labeled [mypy]. For example:

[mypy]
silent_imports = True
python_version = 2.7

I also propose to add fromfile_prefix_chars=@ to the ArgumentParser() constructor call, so that arbitrary flags can be read from a file (one per line). For example

mypy @flagsfile

will read arguments (flags and filenames to type-check) from file flagsfile. This can be combined with regular flags, and you can even specify multiple files this way -- argparse just consumes them all.

Why two different formats? The @flagsfile format is convenient if you you want to type-check a large list of filenames that you've already written to a file, one filename per line. But it's pretty unconventional to use this format for specifying flags (e.g. --silent-imports or --python-version); all other tools I've looked at (e.g. pytest and flake8) support a more conventional ".ini" configuration file. The @flagsfile format also doesn't support reading flags from a file implicitly -- you must pass an argument starting with @. All in all I recommend using mypy.ini for flags and @flagsfile for the list of files to type-check.

This proposal is an intermediate step -- it doesn't support per-file flags yet. For that I propose to use dynamic sections with additional flags, where the section name is something like [mypy-GLOB] and GLOB is a glob pattern that selects a set of files. But that's a next step.

gvanrossum pushed a commit that referenced this issue Sep 16, 2016
Also support reading command line flags using `mypy @flagsfile`.

Addresses #1249 (but does not completely fix it).

The mypy.ini file has the format:
```
[mypy]
silent_imports = True
python_version = 2.7
```
Errors in this config file are non-fatal.  Comments and blank lines
are supported.

The `@flagsfile` option reads additional argparse-style flags,
including filenames, from `flagsfile`, one per line.  This is
typically used for passing in a list of files, but it can also be used
for passing flags:
```
--silent-imports
--py2
mypy
```
This format does not allow comments or blank lines.  Each option must
appear on a line by itself.  Errors are fatal.

The mypy.ini file serves as a set of defaults that can be overridden
(or in some cases extended) by command line flags.  An alternate
config file may be specified using a command line flag:
`--config-file anywhere.ini`.  (There's a trick involved in making
this work, read the source. :-)
@gvanrossum
Copy link
Member

More thoughts on per-file options

There's a litmus test for whether an option could potentially vary per file: is it (or should it be!) included in OPTIONS_AFFECTING_CACHE? Note that this is currently missing strict-optional[-whitelist]; see #2126.

There's a fairly easy choke point for per-file options; or at least, two choke points that should hopefully handle everything. These are the visit_file() methods in TypeChecker and SemanticAnalyzer. Here's a possible design:

  • Create a way to specify per-file options on the command line and/or in the mypy.ini file (see Support reading options from a config file, default mypy.ini. #2148). E.g. --disallow-untyped-defs GLOB would mean to disallow untyped defs for files matching GLOB (a pattern).
  • This creates an additional data structure in the Options object that can be queried to determine whether a particular option should be on or off for a given filename. E.g. --disallow-untyped-defs GLOB would set options.disallow_untyped_defs = False but set options.per_file_globs['disallow_untyped_defs'] = GLOB.
  • Add a clone_for_file() method to the Options class that takes a filename and returns a new Options object whose values are the same as those of the cloned Options object, except that for those options that can vary per file, the value is set to True if the filename matches the GLOB for that option. (This requires that all per-file options are simple Booleans defaulting to False.)
  • In both visit_file() methods, self.options is replaced by self.options.clone_for_file(self.path). (We may have to restore it when leaving.)

That's it! The semantic analysis and type checking phases should now support per-file options.

@gvanrossum
Copy link
Member

I've got all that implemented in @2148 too.

Now I'm concerned about how to shoe-horn in the one flag that already has per-file behavior, --strict-optional-whitelist. The problem with that one is that it's really two flags: --strict-optional turns on the alternate, strict None-checking behavior globally; and the pattern (or patterns) select whether certain errors are suppressed. Only the error suppression can be selected per file. The strict None-checking behavior must be global, in part due to the way it's implemented (using checks for extensions.STRICT_OPTIONAL in functions that don't have easy access to an Options object), in part because its effects are necessarily cross-file, e.g. when one file calls a function defined in another.

Maybe it can be done as follows:

  • --strict-optional remains a global flag that cannot be set per file
  • We kill (or at least deprecate) --strict-optional-whitelist GLOB
  • We add a new per-file Boolean flag --suppress-strict-errors

So then here's how you would specify the effect of --strict-optional-whitelist foo/* in the mypy.ini file:

[mypy]
strict_optional = True
suppress_strict_errors = True
[mypy-foo/*]
suppress_strict_errors = False

This will enable strict None checking globally, and suppress the errors in all files except those matching the glob pattern foo/* (i.e., only show the errors in foo/*).

Alternatively, if you wanted to hide the errors in foo/* only, use:

[mypy]
strict_optional = True
[mypy-foo/*]
suppress_strict_errors = True

Or maybe we should reverse the sense of the per-file flag and name it show_strict_errors? It should then default to True. Note that in either case, strict_optional must be turned on separately for the show/hide flags to have an effect. It would then be possible to have the show/hide configuration dormant in the mypy.ini file, and trigger the actual strict None-checking behavior via the command line.

gvanrossum added a commit that referenced this issue Sep 20, 2016
Also support reading command line flags using `mypy @flagsfile`.

Addresses #1249 (but does not completely fix it).

The mypy.ini file has the format:
```
[mypy]
silent_imports = True
python_version = 2.7
```
Errors in this config file are non-fatal.  Comments and blank lines
are supported.

There are also sections with glob patterns for per-file options, e.g.
`[mypy-dir1/*,dir2/*]` (I'll document those later).

The `@flagsfile` option reads additional argparse-style flags,
including filenames, from `flagsfile`, one per line.  This is
typically used for passing in a list of files, but it can also be used
for passing flags:
```
--silent-imports
--py2
mypy
```
This format does not allow comments or blank lines.  Each option must
appear on a line by itself.  Errors are fatal.

The mypy.ini file serves as a set of defaults that can be overridden
(or in some cases extended) by command line flags.  An alternate
config file may be specified using a command line flag:
`--config-file anywhere.ini`.  (There's a trick involved in making
this work, read the source. :-)
@JukkaL
Copy link
Collaborator

JukkaL commented Dec 2, 2016

Can we close this now and create new issues for any remaining work?

@gvanrossum
Copy link
Member

Sure. What remaining work/ideas are there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants