From 7f72dcc3c97fa7021472eb980ea57e00c2f90db4 Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Wed, 15 May 2019 16:29:18 -0700 Subject: [PATCH 1/3] Initial implementation of 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. Fixes #2938. Thoughts? --- mypy/build.py | 20 ++++- mypy/config_parser.py | 80 ++++++++++++++---- mypy/test/testcheck.py | 1 + mypy/util.py | 7 ++ test-data/unit/check-inline-config.test | 108 ++++++++++++++++++++++++ test-data/unit/fine-grained.test | 36 ++++++++ 6 files changed, 233 insertions(+), 19 deletions(-) create mode 100644 test-data/unit/check-inline-config.test 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..38899df53f0d 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,56 @@ 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 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 line.split(', '): + 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..c84b22f8ca6a --- /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 bda39b70d5d9..a16b65c40fda 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -8845,3 +8845,39 @@ y = '' [out] == == + +[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] From 096ec21e843378ced8ad722ab212f76c3956ff2e Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Wed, 15 May 2019 23:01:19 -0700 Subject: [PATCH 2/3] Implement quoting of comma separated values --- mypy/config_parser.py | 25 ++++++++++++++++++++++++- test-data/unit/check-inline-config.test | 2 +- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/mypy/config_parser.py b/mypy/config_parser.py index 38899df53f0d..fb01eecf2e74 100644 --- a/mypy/config_parser.py +++ b/mypy/config_parser.py @@ -224,11 +224,34 @@ def parse_section(prefix: str, template: Options, 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 line.split(', '): + for entry in split_directive(line): if '=' not in entry: name = entry value = None diff --git a/test-data/unit/check-inline-config.test b/test-data/unit/check-inline-config.test index c84b22f8ca6a..fd146319f836 100644 --- a/test-data/unit/check-inline-config.test +++ b/test-data/unit/check-inline-config.test @@ -29,7 +29,7 @@ def foo() -> List: # E: Missing type parameters for generic type [builtins fixtures/list.pyi] [case testInlineList] -# mypy: disallow-any-generics, always-false=FOO,BAR +# mypy: disallow-any-generics,always-false="FOO,BAR" from typing import List From 872b7468e5913e1b09c935008eb1f68bd6715577 Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Tue, 21 May 2019 11:55:25 -0700 Subject: [PATCH 3/3] Produce line numbers, address other feedback --- mypy/build.py | 6 +- mypy/config_parser.py | 84 +++++++++++++++---------- mypy/util.py | 17 +++-- test-data/unit/check-inline-config.test | 23 ++++++- 4 files changed, 88 insertions(+), 42 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index a330a41e2082..006126eeaa7b 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1981,14 +1981,14 @@ 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. + """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) + for lineno, error in config_errors: + self.manager.errors.report(lineno, 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 fb01eecf2e74..64b204fe90aa 100644 --- a/mypy/config_parser.py +++ b/mypy/config_parser.py @@ -224,10 +224,13 @@ def parse_section(prefix: str, template: Options, return results, report_dirs -def split_directive(s: str) -> List[str]: - """Split s on commas, except during quoted sections""" +def split_directive(s: str) -> Tuple[List[str], List[str]]: + """Split s on commas, except during quoted sections. + + Returns the parts and a list of error messages.""" parts = [] cur = [] # type: List[str] + errors = [] i = 0 while i < len(s): if s[i] == ',': @@ -238,56 +241,71 @@ def split_directive(s: str) -> List[str]: while i < len(s) and s[i] != '"': cur.append(s[i]) i += 1 + if i == len(s): + errors.append("Unterminated quote in configuration comment") + cur.clear() else: cur.append(s[i]) i += 1 if cur: parts.append(''.join(cur).strip()) - return parts + return parts, errors + +def mypy_comments_to_config_map(line: str, + template: Options) -> Tuple[Dict[str, str], List[str]]: + """Rewrite the mypy comment syntax into ini file syntax. -def mypy_comments_to_config_map(args: List[str], template: Options) -> Dict[str, str]: - """Rewrite the mypy comment syntax into ini file syntax""" + Returns + """ 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) + entries, errors = split_directive(line) + for entry in entries: + if '=' not in entry: + name = entry + value = None + else: + name, value = [x.strip() for x in 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 + 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 + return options, errors def parse_mypy_comments( - args: List[str], template: Options) -> Tuple[Dict[str, object], List[str]]: + args: List[Tuple[int, str]], + template: Options) -> Tuple[Dict[str, object], List[Tuple[int, 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") + errors = [] # type: List[Tuple[int, str]] + sections = {} + + for lineno, line in args: + # 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() + options, parse_errors = mypy_comments_to_config_map(line, template) + parser['dummy'] = options + errors.extend((lineno, x) for x in parse_errors) + + stderr = StringIO() + new_sections, reports = parse_section('', template, parser['dummy'], stderr=stderr) + errors.extend((lineno, x) for x in stderr.getvalue().strip().split('\n') if x) + if reports: + errors.append((lineno, "Reports not supported in inline configuration")) + sections.update(new_sections) return sections, errors diff --git a/mypy/util.py b/mypy/util.py index 1e73c387db1d..f819de84ce33 100644 --- a/mypy/util.py +++ b/mypy/util.py @@ -18,9 +18,6 @@ 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 @@ -92,8 +89,18 @@ 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)) +def get_mypy_comments(source: str) -> List[Tuple[int, str]]: + PREFIX = '# mypy: ' + # Don't bother splitting up the lines unless we know it is useful + if PREFIX not in source: + return [] + lines = source.split('\n') + results = [] + for i, line in enumerate(lines): + if line.startswith(PREFIX): + results.append((i + 1, line[len(PREFIX):])) + + return results _python2_interpreter = None # type: Optional[str] diff --git a/test-data/unit/check-inline-config.test b/test-data/unit/check-inline-config.test index fd146319f836..bdaa72f29ac1 100644 --- a/test-data/unit/check-inline-config.test +++ b/test-data/unit/check-inline-config.test @@ -28,6 +28,16 @@ def foo() -> List: # E: Missing type parameters for generic type [builtins fixtures/list.pyi] + +[case testInlineSimple4] +# 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" @@ -104,5 +114,16 @@ def foo() -> int: [case testInlineError1] # mypy: invalid-whatever +# mypy: no-warn-no-return; no-strict-optional +# mypy: always-true=FOO,BAR +# mypy: always-true="FOO,BAR +[out] +main:1: error: Unrecognized option: invalid_whatever = True +main:2: error: Unrecognized option: warn_no_return; no_strict_optional = False +main:3: error: Unrecognized option: bar = True +main:4: error: Unterminated quote in configuration comment + +[case testInlineError2] +# mypy: skip-file [out] -main: error: Unrecognized option: invalid_whatever = True +main:1: error: Unrecognized option: skip_file = True