Repository: python/mypy
Pull request: Support '# mypy: ' comments for inline configuration
Implements line configuration using '# mypy: ' comments, following the
blueprint I proposed in #2938.

It currently finds them just using a regex which means it is possible to pick
up a directive spuriously in a string literal or something but honestly I am
just not worried about that in practice.

Examples of what it looks like in the tests.
Still needs documentation.

Fixes #2938.

Thoughts?

~(This is stacked on #6838 so can't be merged yet)~

Problem statement:
ability to configure flags for a module from a comment within it
I would like to be able to use some syntax like

```
# MYPY: strict
```

to turn on all the useful stricter-checking flags, like

--disallow-subclassing-any
--warn-return-any
--disallow-untyped-calls
--strict-optional

Not totally sure what each one does.  Ideally we'd turn them all on globally, but ideally all our code would already be typed, and we don't live in that world.  I'd prefer this to our current mypy.ini for a few reasons:

- makes it really easy to turn on - no need to edit a separate file to add the flags
- the setting follows the file around during renames and moves
- it eliminates the need to know all the different flags

I suppose we'll still want to introduce new flags over time - to make stricter checking opt in so large codebases don't have to migrate all at once.  So either mypy standardizes ways to refer to a few flags (e.g. the first 3 could be called --strict-any? or --no-implicit-any like typescript?) or we could allow configuring these aliases in mypy.ini.  I think I'm a fan of the former - provides more standardization across codebases.

Hints from issue discussion:
I can see the value of this, in part because this was my original idea for per-file configuration options. The options that can be set within a file should probably be named identically to either the command-line options or the config file options to avoid redundancy. I'd also spell the comment like this:

```
# mypy: option1
# mypy: option2
```

We could also support multiple options per line (`# mypy: option1, option2`).

For example:

```
# mypy: strict, no-strict-optional  # like --strict --no-strict-optional
```

(There already is ` --strict` option that pretty does what was proposed. However, it's unclear if it should include things like `--strict-boolean` that may be a bit too opinionated.)

If we'd use the config file syntax, it would perhaps look like this (one option per line):

```
# mypy: strict=True
# mypy: strict_optional=False
```

I think that the options should be global to a file (i.e. no per-function options), and would be applied after config file options (but the config file would still be consulted as well).
OK we need to do this, I think, because our internal mypy.ini is thousands of lines long and I want to start doing things that would make it longer.

My proposal is to allow both multiple options on a line and multiple lines. I prefer command line style, since writing `=True` is ugly and I think hyphens look better.
For multiple options on a line, I propose `, ` (comma space) as the option separator, because just a space looks bad (without dashes in front of the arguments) and just a comma means we need some sort of escaping for things like always-true which takes a comma-delimited list.

So things would look like
```
# mypy: strict, no-strict-optional, always-true=WINDOWS,DEBUG
```
While I initially gave this a thumbs-up, on second thought I am not comfortable with the choice of using comma+space as separator. I also agree that a space is less than ideal. How about one of the following?

1. use `;` as separator
2. require quotes around values that need commas, e.g. `always-true="WINDOWS,DEBUG"`
3. switch the separator for `always-true` to something else, e.g. `/`

I slightly favor (2).
Yeah let's do option (2). Honestly I was mostly just trying to get out of writing the delimiter-quote-parsing code but (like every other part of this PR) it just wasn't actually that bad.
I like (2), since it seems to be the most general -- it allows both commas and semicolons as option values. I think that it also looks pretty nice in the common case. Maybe also allow escapes for double quotes and maybe things like newlines (just in case)?
My inclination is to not bother with more complicated escaping syntax until we want to add options that could take advantage of it, but it wouldn't be hard to support also.
The main potential use case for escaping that I can imagine is per-module flags to mypy plugins, which we don't support at the moment. So it seems fine to not support it for now. I couldn't even find an example where quoting the value would be useful, since none of the options where this is useful seem to be per-module options.
> none of the options where this is useful seem to be per-module options

Well, `always_true` and `always_false` would require quoting and are per-module. I could totally see putting one of those at the top of a module if there's a flag variable that's important in that module only. E.g. a module that has `debug = False` and later some hacky debug code under `if debug` -- you may not care about type-checking that.
(Oh wait, never mind. You were talking about escaping quotes inside quoted strings. Yeah, I suppose we don't need that.)

Commit message: Merge branch 'master' into per-module-flags-actually

Patch to review:
diff --git a/mypy/build.py b/mypy/build.py
index 8de60680f862..b54916f55e75 100644
--- a/mypy/build.py
+++ b/mypy/build.py
@@ -43,7 +43,7 @@
 from mypy.checker import TypeChecker
 from mypy.indirection import TypeIndirectionVisitor
 from mypy.errors import Errors, CompileError, report_internal_error
-from mypy.util import DecodeError, decode_python_encoding, is_sub_path
+from mypy.util import DecodeError, decode_python_encoding, is_sub_path, get_mypy_comments
 if MYPY:
     from mypy.report import Reports  # Avoid unconditional slow import
 from mypy import moduleinfo
@@ -61,6 +61,7 @@
 from mypy.metastore import MetadataStore, FilesystemMetadataStore, SqliteMetadataStore
 from mypy.typestate import TypeState, reset_global_state
 from mypy.renaming import VariableRenameVisitor
+from mypy.config_parser import parse_mypy_comments
 
 from mypy.mypyc_hacks import BuildManagerBase
 
@@ -1364,6 +1365,11 @@ def write_cache(id: str, path: str, tree: MypyFile,
 
     mtime = 0 if bazel else int(st.st_mtime)
     size = st.st_size
+    # Note that the options we store in the cache are the options as
+    # specified by the command line/config file and *don't* reflect
+    # updates made by inline config directives in the file. This is
+    # important, or otherwise the options would never match when
+    # verifying the cache.
     options = manager.options.clone_for_module(id)
     assert source_hash is not None
     meta = {'id': id,
@@ -1922,6 +1928,8 @@ def parse_file(self) -> None:
             else:
                 assert source is not None
                 self.source_hash = compute_hash(source)
+
+            self.parse_inline_configuration(source)
             self.tree = manager.parse_file(self.id, self.xpath, source,
                                            self.ignore_all or self.options.ignore_errors)
 
@@ -1937,6 +1945,16 @@ def parse_file(self) -> None:
 
         self.check_blockers()
 
+    def parse_inline_configuration(self, source: str) -> None:
+        # Check for inline mypy: options directive and parse them.
+        flags = get_mypy_comments(source)
+        if flags:
+            changes, config_errors = parse_mypy_comments(flags, self.options)
+            self.options = self.options.apply_changes(changes)
+            self.manager.errors.set_file(self.xpath, self.id)
+            for error in config_errors:
+                self.manager.errors.report(-1, 0, error)
+
     def semantic_analysis_pass1(self) -> None:
         """Perform pass 1 of semantic analysis, which happens immediately after parsing.
 
diff --git a/mypy/config_parser.py b/mypy/config_parser.py
index 5f0cefea691d..fb01eecf2e74 100644
--- a/mypy/config_parser.py
+++ b/mypy/config_parser.py
@@ -1,6 +1,7 @@
 import argparse
 import configparser
 import glob as fileglob
+from io import StringIO
 import os
 import re
 import sys
@@ -116,24 +117,22 @@ def parse_config_file(options: Options, filename: Optional[str],
             print("%s: No [mypy] section in config file" % file_read, file=stderr)
     else:
         section = parser['mypy']
-        prefix = '%s: [%s]' % (file_read, 'mypy')
-        updates, report_dirs = parse_section(prefix, options, section,
-                                             stdout, stderr)
+        prefix = '%s: [%s]: ' % (file_read, 'mypy')
+        updates, report_dirs = parse_section(prefix, options, section, stderr)
         for k, v in updates.items():
             setattr(options, k, v)
         options.report_dirs.update(report_dirs)
 
     for name, section in parser.items():
         if name.startswith('mypy-'):
-            prefix = '%s: [%s]' % (file_read, name)
-            updates, report_dirs = parse_section(prefix, options, section,
-                                                 stdout, stderr)
+            prefix = '%s: [%s]: ' % (file_read, name)
+            updates, report_dirs = parse_section(prefix, options, section, stderr)
             if report_dirs:
-                print("%s: Per-module sections should not specify reports (%s)" %
+                print("%sPer-module sections should not specify reports (%s)" %
                       (prefix, ', '.join(s + '_report' for s in sorted(report_dirs))),
                       file=stderr)
             if set(updates) - PER_MODULE_OPTIONS:
-                print("%s: Per-module sections should only specify per-module flags (%s)" %
+                print("%sPer-module sections should only specify per-module flags (%s)" %
                       (prefix, ', '.join(sorted(set(updates) - PER_MODULE_OPTIONS))),
                       file=stderr)
                 updates = {k: v for k, v in updates.items() if k in PER_MODULE_OPTIONS}
@@ -146,7 +145,7 @@ def parse_config_file(options: Options, filename: Optional[str],
 
                 if (any(c in glob for c in '?[]!') or
                         any('*' in x and x != '*' for x in glob.split('.'))):
-                    print("%s: Patterns must be fully-qualified module names, optionally "
+                    print("%sPatterns must be fully-qualified module names, optionally "
                           "with '*' in some components (e.g spam.*.eggs.*)"
                           % prefix,
                           file=stderr)
@@ -156,7 +155,6 @@ def parse_config_file(options: Options, filename: Optional[str],
 
 def parse_section(prefix: str, template: Options,
                   section: Mapping[str, str],
-                  stdout: TextIO = sys.stdout,
                   stderr: TextIO = sys.stderr
                   ) -> Tuple[Dict[str, object], Dict[str, str]]:
     """Parse one section of a config file.
@@ -176,17 +174,17 @@ def parse_section(prefix: str, template: Options,
                     if report_type in defaults.REPORTER_NAMES:
                         report_dirs[report_type] = section[key]
                     else:
-                        print("%s: Unrecognized report type: %s" % (prefix, key),
+                        print("%sUnrecognized report type: %s" % (prefix, key),
                               file=stderr)
                     continue
                 if key.startswith('x_'):
                     continue  # Don't complain about `x_blah` flags
                 elif key == 'strict':
-                    print("%s: Strict mode is not supported in configuration files: specify "
+                    print("%sStrict mode is not supported in configuration files: specify "
                           "individual flags instead (see 'mypy -h' for the list of flags enabled "
                           "in strict mode)" % prefix, file=stderr)
                 else:
-                    print("%s: Unrecognized option: %s = %s" % (prefix, key, section[key]),
+                    print("%sUnrecognized option: %s = %s" % (prefix, key, section[key]),
                           file=stderr)
                 continue
             ct = type(dv)
@@ -198,18 +196,18 @@ def parse_section(prefix: str, template: Options,
                 try:
                     v = ct(section.get(key))
                 except argparse.ArgumentTypeError as err:
-                    print("%s: %s: %s" % (prefix, key, err), file=stderr)
+                    print("%s%s: %s" % (prefix, key, err), file=stderr)
                     continue
             else:
-                print("%s: Don't know what type %s should have" % (prefix, key), file=stderr)
+                print("%sDon't know what type %s should have" % (prefix, key), file=stderr)
                 continue
         except ValueError as err:
-            print("%s: %s: %s" % (prefix, key, err), file=stderr)
+            print("%s%s: %s" % (prefix, key, err), file=stderr)
             continue
         if key == 'cache_dir':
             v = os.path.expanduser(v)
         if key == 'silent_imports':
-            print("%s: silent_imports has been replaced by "
+            print("%ssilent_imports has been replaced by "
                   "ignore_missing_imports=True; follow_imports=skip" % prefix, file=stderr)
             if v:
                 if 'ignore_missing_imports' not in results:
@@ -217,10 +215,79 @@ def parse_section(prefix: str, template: Options,
                 if 'follow_imports' not in results:
                     results['follow_imports'] = 'skip'
         if key == 'almost_silent':
-            print("%s: almost_silent has been replaced by "
+            print("%salmost_silent has been replaced by "
                   "follow_imports=error" % prefix, file=stderr)
             if v:
                 if 'follow_imports' not in results:
                     results['follow_imports'] = 'error'
         results[key] = v
     return results, report_dirs
+
+
+def split_directive(s: str) -> List[str]:
+    """Split s on commas, except during quoted sections"""
+    parts = []
+    cur = []  # type: List[str]
+    i = 0
+    while i < len(s):
+        if s[i] == ',':
+            parts.append(''.join(cur).strip())
+            cur = []
+        elif s[i] == '"':
+            i += 1
+            while i < len(s) and s[i] != '"':
+                cur.append(s[i])
+                i += 1
+        else:
+            cur.append(s[i])
+        i += 1
+    if cur:
+        parts.append(''.join(cur).strip())
+
+    return parts
+
+
+def mypy_comments_to_config_map(args: List[str], template: Options) -> Dict[str, str]:
+    """Rewrite the mypy comment syntax into ini file syntax"""
+    options = {}
+    for line in args:
+        for entry in split_directive(line):
+            if '=' not in entry:
+                name = entry
+                value = None
+            else:
+                name, value = entry.split('=', 1)
+
+            name = name.replace('-', '_')
+            if value is None:
+                if name.startswith('no_') and not hasattr(template, name):
+                    name = name[3:]
+                    value = 'False'
+                else:
+                    value = 'True'
+            options[name] = value
+
+    return options
+
+
+def parse_mypy_comments(
+        args: List[str], template: Options) -> Tuple[Dict[str, object], List[str]]:
+    """Parse a collection of inline mypy: configuration comments.
+
+    Returns a dictionary of options to be applied and a list of error messages
+    generated.
+    """
+
+    # In order to easily match the behavior for bools, we abuse configparser.
+    # Oddly, the only way to get the SectionProxy object with the getboolean
+    # method is to create a config parser.
+    parser = configparser.RawConfigParser()
+    parser['dummy'] = mypy_comments_to_config_map(args, template)
+
+    stderr = StringIO()
+    sections, reports = parse_section('', template, parser['dummy'], stderr=stderr)
+    errors = [x for x in stderr.getvalue().strip().split('\n') if x]
+    if reports:
+        errors.append("Reports not supported in inline configuration")
+
+    return sections, errors
diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py
index 6ee4ec312917..e7ee413707d2 100644
--- a/mypy/test/testcheck.py
+++ b/mypy/test/testcheck.py
@@ -83,6 +83,7 @@
     'check-redefine.test',
     'check-literal.test',
     'check-newsemanal.test',
+    'check-inline-config.test',
 ]
 
 # Tests that use Python 3.8-only AST features (like expression-scoped ignores):
diff --git a/mypy/util.py b/mypy/util.py
index a57499604b89..1e73c387db1d 100644
--- a/mypy/util.py
+++ b/mypy/util.py
@@ -18,6 +18,9 @@
 ENCODING_RE = \
     re.compile(br'([ \t\v]*#.*(\r\n?|\n))??[ \t\v]*#.*coding[:=][ \t]*([-\w.]+)')  # type: Final
 
+MYPY_RE = \
+    re.compile(r'^#.mypy: (.*)$', re.MULTILINE)  # type: Final
+
 default_python2_interpreter = \
     ['python2', 'python', '/usr/bin/python', 'C:\\Python27\\python.exe']  # type: Final
 
@@ -89,6 +92,10 @@ def decode_python_encoding(source: bytes, pyversion: Tuple[int, int]) -> str:
     return source_text
 
 
+def get_mypy_comments(source: str) -> List[str]:
+    return list(re.findall(MYPY_RE, source))
+
+
 _python2_interpreter = None  # type: Optional[str]
 
 
diff --git a/test-data/unit/check-inline-config.test b/test-data/unit/check-inline-config.test
new file mode 100644
index 000000000000..fd146319f836
--- /dev/null
+++ b/test-data/unit/check-inline-config.test
@@ -0,0 +1,108 @@
+-- Checks for 'mypy: option' directives inside files
+
+[case testInlineSimple1]
+# mypy: disallow-any-generics, no-warn-no-return
+
+from typing import List
+def foo() -> List:  # E: Missing type parameters for generic type
+    20
+
+[builtins fixtures/list.pyi]
+
+[case testInlineSimple2]
+# mypy: disallow-any-generics
+# mypy: no-warn-no-return
+
+from typing import List
+def foo() -> List:  # E: Missing type parameters for generic type
+    20
+
+[builtins fixtures/list.pyi]
+
+[case testInlineSimple3]
+# mypy: disallow-any-generics=true, warn-no-return=0
+
+from typing import List
+def foo() -> List:  # E: Missing type parameters for generic type
+    20
+
+[builtins fixtures/list.pyi]
+
+[case testInlineList]
+# mypy: disallow-any-generics,always-false="FOO,BAR"
+
+from typing import List
+
+def foo(FOO: bool, BAR: bool) -> List:  # E: Missing type parameters for generic type
+    if FOO or BAR:
+        1+'lol'
+    return []
+
+[builtins fixtures/list.pyi]
+
+[case testInlineIncremental1]
+import a
+[file a.py]
+# mypy: disallow-any-generics, no-warn-no-return
+
+from typing import List
+def foo() -> List:
+    20
+
+[file a.py.2]
+# mypy: no-warn-no-return
+
+from typing import List
+def foo() -> List:
+    20
+
+[file a.py.3]
+from typing import List
+def foo() -> List:
+    20
+[out]
+tmp/a.py:4: error: Missing type parameters for generic type
+[out2]
+[out3]
+tmp/a.py:2: error: Missing return statement
+
+[builtins fixtures/list.pyi]
+
+[case testInlineIncremental2]
+# flags2: --disallow-any-generics
+import a
+[file a.py]
+# mypy: no-warn-no-return
+
+from typing import List
+def foo() -> List:
+    20
+
+[file b.py.2]
+# no changes to a.py, but flag change should cause recheck
+
+[out]
+[out2]
+tmp/a.py:4: error: Missing type parameters for generic type
+
+[builtins fixtures/list.pyi]
+
+[case testInlineIncremental3]
+import a, b
+[file a.py]
+# mypy: no-warn-no-return
+
+def foo() -> int:
+    20
+
+[file b.py]
+[file b.py.2]
+# no changes to a.py and we want to make sure it isn't rechecked
+[out]
+[out2]
+[rechecked b]
+
+[case testInlineError1]
+# mypy: invalid-whatever
+[out]
+main: error: Unrecognized option: invalid_whatever = True
diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test
index ae233c5ba4cc..39a626592792 100644
--- a/test-data/unit/fine-grained.test
+++ b/test-data/unit/fine-grained.test
@@ -8846,6 +8846,42 @@ y = ''
 ==
 ==
 
+[case testInlineConfigFineGrained1]
+import a
+[file a.py]
+# mypy: no-warn-no-return
+
+from typing import List
+def foo() -> List:
+    20
+
+[file a.py.2]
+# mypy: disallow-any-generics, no-warn-no-return
+
+from typing import List
+def foo() -> List:
+    20
+
+[file a.py.3]
+# mypy: no-warn-no-return
+
+from typing import List
+def foo() -> List:
+    20
+
+[file a.py.4]
+from typing import List
+def foo() -> List:
+    20
+[out]
+==
+a.py:4: error: Missing type parameters for generic type
+==
+==
+a.py:2: error: Missing return statement
+
+[builtins fixtures/list.pyi]
+
 [case testWrongNumberOfArguments]
 
 [file a.py]


Code review comments:
diff_hunk: @@ -1937,6 +1945,16 @@ def parse_file(self) -> None:
 
         self.check_blockers()
 
+    def parse_inline_configuration(self, source: str) -> None:
+        # Check for inline mypy: options directive and parse them.
+        flags = get_mypy_comments(source)
+        if flags:
+            changes, config_errors = parse_mypy_comments(flags, self.options)
+            self.options = self.options.apply_changes(changes)
+            self.manager.errors.set_file(self.xpath, self.id)
+            for error in config_errors:
+                self.manager.errors.report(-1, 0, error)
path: mypy/build.py
text: @user1:
Too bad the line number isn't preserved. If someone accidentally has a `# mypy:` comment in their file, how would they investigate the error if they didn't know about such comments? The error message doesn't even mention `# mypy:` comments.

At the same time, I do understand this is complicated (esp. since the flag parsing is separated from the flag extraction, and shared with `mypy.ini` file parsing).

Perhaps you can parse each `# mypy:` comment separately, and associate the comments with line numbers?

@author:
Yeah we should do this.

diff_hunk: @@ -18,6 +18,9 @@
 ENCODING_RE = \
     re.compile(br'([ \t\v]*#.*(\r\n?|\n))??[ \t\v]*#.*coding[:=][ \t]*([-\w.]+)')  # type: Final
 
+MYPY_RE = \
path: mypy/util.py
text: @user1:
That's not a great name -- maybe `MYPY_COMMENT_RE`?