From 3dca72f9e1192e5088a452e207de23d3767d35f0 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 23 Apr 2017 22:18:34 +0200 Subject: [PATCH 1/3] Fix some --strict-optional errors: stubgen, stubgenc, checkstrformat --- mypy/checkstrformat.py | 37 +++++++++++++++++-------------------- mypy/parse.py | 2 +- mypy/stubgen.py | 9 +++++---- mypy/stubgenc.py | 9 ++++++--- mypy/stubutil.py | 3 ++- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/mypy/checkstrformat.py b/mypy/checkstrformat.py index bb3e02e99510..8d8d009140ff 100644 --- a/mypy/checkstrformat.py +++ b/mypy/checkstrformat.py @@ -2,7 +2,7 @@ import re -from typing import cast, List, Tuple, Dict, Callable, Union +from typing import cast, List, Tuple, Dict, Callable, Union, Optional from mypy.types import ( Type, AnyType, TupleType, Instance, UnionType @@ -18,7 +18,7 @@ from mypy.messages import MessageBuilder FormatStringExpr = Union[StrExpr, BytesExpr, UnicodeExpr] - +Checkers = Tuple[Callable[[Expression], None], Callable[[Type], None]] class ConversionSpecifier: def __init__(self, key: str, flags: str, width: str, precision: str, type: str) -> None: @@ -114,10 +114,10 @@ def analyze_conversion_specifiers(self, specifiers: List[ConversionSpecifier], if has_key and has_star: self.msg.string_interpolation_with_star_and_key(context) - return None + return False if has_key and not all_have_keys: self.msg.string_interpolation_mixing_key_and_non_keys(context) - return None + return False return has_key def check_simple_str_interpolation(self, specifiers: List[ConversionSpecifier], @@ -192,9 +192,8 @@ def check_mapping_str_interpolation(self, specifiers: List[ConversionSpecifier], def build_replacement_checkers(self, specifiers: List[ConversionSpecifier], context: Context, expr: FormatStringExpr - ) -> List[Tuple[Callable[[Expression], None], - Callable[[Type], None]]]: - checkers = [] # type: List[Tuple[Callable[[Expression], None], Callable[[Type], None]]] + ) -> Optional[List[Checkers]]: + checkers = [] # type: List[Checkers] for specifier in specifiers: checker = self.replacement_checkers(specifier, context, expr) if checker is None: @@ -203,13 +202,12 @@ def build_replacement_checkers(self, specifiers: List[ConversionSpecifier], return checkers def replacement_checkers(self, specifier: ConversionSpecifier, context: Context, - expr: FormatStringExpr) -> List[Tuple[Callable[[Expression], None], - Callable[[Type], None]]]: + expr: FormatStringExpr) -> Optional[List[Checkers]]: """Returns a list of tuples of two functions that check whether a replacement is of the right type for the specifier. The first functions take a node and checks its type in the right type context. The second function just checks a type. """ - checkers = [] # type: List[Tuple[Callable[[Expression], None], Callable[[Type], None]]] + checkers = [] # type: List[Checkers] if specifier.width == '*': checkers.append(self.checkers_for_star(context)) @@ -227,14 +225,13 @@ def replacement_checkers(self, specifier: ConversionSpecifier, context: Context, checkers.append(c) return checkers - def checkers_for_star(self, context: Context) -> Tuple[Callable[[Expression], None], - Callable[[Type], None]]: + def checkers_for_star(self, context: Context) -> Checkers: """Returns a tuple of check functions that check whether, respectively, a node or a type is compatible with a star in a conversion specifier """ expected = self.named_type('builtins.int') - def check_type(type: Type = None) -> None: + def check_type(type: Type) -> None: expected = self.named_type('builtins.int') self.chk.check_subtype(type, expected, context, '* wants int') @@ -246,8 +243,7 @@ def check_expr(expr: Expression) -> None: def checkers_for_regular_type(self, type: str, context: Context, - expr: FormatStringExpr) -> Tuple[Callable[[Expression], None], - Callable[[Type], None]]: + expr: FormatStringExpr) -> Optional[Checkers]: """Returns a tuple of check functions that check whether, respectively, a node or a type is compatible with 'type'. Return None in case of an """ @@ -255,7 +251,8 @@ def checkers_for_regular_type(self, type: str, if expected_type is None: return None - def check_type(type: Type = None) -> None: + def check_type(type: Type) -> None: + assert expected_type is not None self.chk.check_subtype(type, expected_type, context, messages.INCOMPATIBLE_TYPES_IN_STR_INTERPOLATION, 'expression has type', 'placeholder has type') @@ -268,8 +265,7 @@ def check_expr(expr: Expression) -> None: def checkers_for_c_type(self, type: str, context: Context, - expr: FormatStringExpr) -> Tuple[Callable[[Expression], None], - Callable[[Type], None]]: + expr: FormatStringExpr) -> Optional[Checkers]: """Returns a tuple of check functions that check whether, respectively, a node or a type is compatible with 'type' that is a character type """ @@ -277,7 +273,8 @@ def checkers_for_c_type(self, type: str, if expected_type is None: return None - def check_type(type: Type = None) -> None: + def check_type(type: Type) -> None: + assert expected_type is not None self.chk.check_subtype(type, expected_type, context, messages.INCOMPATIBLE_TYPES_IN_STR_INTERPOLATION, 'expression has type', 'placeholder has type') @@ -291,7 +288,7 @@ def check_expr(expr: Expression) -> None: return check_expr, check_type - def conversion_type(self, p: str, context: Context, expr: FormatStringExpr) -> Type: + def conversion_type(self, p: str, context: Context, expr: FormatStringExpr) -> Optional[Type]: """Return the type that is accepted for a string interpolation conversion specifier type. diff --git a/mypy/parse.py b/mypy/parse.py index ddcd226b9ff2..13fd58be3f60 100644 --- a/mypy/parse.py +++ b/mypy/parse.py @@ -7,7 +7,7 @@ def parse(source: Union[str, bytes], fnam: str, - errors: Errors, + errors: Optional[Errors], options: Options) -> MypyFile: """Parse a source file, without doing any semantic analysis. diff --git a/mypy/stubgen.py b/mypy/stubgen.py index 52646bda98ec..6e25ecdfd27c 100644 --- a/mypy/stubgen.py +++ b/mypy/stubgen.py @@ -311,11 +311,10 @@ def visit_decorator(self, o: Decorator) -> None: super().visit_decorator(o) def visit_class_def(self, o: ClassDef) -> None: + sep = None # type: Optional[int] if not self._indent and self._state != EMPTY: sep = len(self._output) self.add('\n') - else: - sep = None self.add('%sclass %s' % (self._indent, o.name)) self.record_name(o.name) base_types = self.get_base_types(o) @@ -465,7 +464,7 @@ def visit_import(self, o: Import) -> None: self.add_import_line('import %s as %s\n' % (id, target_name)) self.record_name(target_name) - def get_init(self, lvalue: str, rvalue: Expression) -> str: + def get_init(self, lvalue: str, rvalue: Expression) -> Optional[str]: """Return initializer for a variable. Return None if we've generated one already or if the variable is internal. @@ -511,7 +510,9 @@ def output(self) -> str: def is_not_in_all(self, name: str) -> bool: if self.is_private_name(name): return False - return self.is_top_level() and bool(self._all_) and name not in self._all_ + if self._all_: + return self.is_top_level() and name not in self._all_ + return False def is_private_name(self, name: str) -> bool: return name.startswith('_') and (not name.endswith('__') diff --git a/mypy/stubgenc.py b/mypy/stubgenc.py index ade21afe2b3e..92b78fa115de 100644 --- a/mypy/stubgenc.py +++ b/mypy/stubgenc.py @@ -111,12 +111,15 @@ def generate_c_function_stub(module: ModuleType, self_arg = '%s, ' % self_var else: self_arg = '' - if name in ('__new__', '__init__') and name not in sigs and class_name in class_sigs: + if (name in ('__new__', '__init__') and name not in sigs and class_name and + class_name in class_sigs): sig = class_sigs[class_name] else: docstr = getattr(obj, '__doc__', None) - sig = infer_sig_from_docstring(docstr, name) - if not sig: + inferred = infer_sig_from_docstring(docstr, name) + if inferred: + sig = inferred + else: if class_name and name not in sigs: sig = infer_method_sig(name) else: diff --git a/mypy/stubutil.py b/mypy/stubutil.py index f0d8a174be4f..5c7807e92b52 100644 --- a/mypy/stubutil.py +++ b/mypy/stubutil.py @@ -90,7 +90,8 @@ def is_c_module(module: ModuleType) -> bool: return '__file__' not in module.__dict__ or module.__dict__['__file__'].endswith('.so') -def write_header(file: IO[str], module_name: str, pyversion: Tuple[int, int] = (3, 5)) -> None: +def write_header(file: IO[str], module_name: str = None, + pyversion: Tuple[int, int] = (3, 5)) -> None: if module_name: if pyversion[0] >= 3: version = '%d.%d' % (sys.version_info.major, From 0a414c10590e697e8cc3dcdf29b3966c0ae9790e Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 23 Apr 2017 22:49:26 +0200 Subject: [PATCH 2/3] More fixes: report, strconv --- mypy/checkstrformat.py | 7 ++++--- mypy/report.py | 23 +++++++++++++---------- mypy/strconv.py | 5 ++--- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/mypy/checkstrformat.py b/mypy/checkstrformat.py index 8d8d009140ff..3dde6d7ac0af 100644 --- a/mypy/checkstrformat.py +++ b/mypy/checkstrformat.py @@ -20,6 +20,7 @@ FormatStringExpr = Union[StrExpr, BytesExpr, UnicodeExpr] Checkers = Tuple[Callable[[Expression], None], Callable[[Type], None]] + class ConversionSpecifier: def __init__(self, key: str, flags: str, width: str, precision: str, type: str) -> None: self.key = key @@ -105,7 +106,7 @@ def parse_conversion_specifiers(self, format: str) -> List[ConversionSpecifier]: return specifiers def analyze_conversion_specifiers(self, specifiers: List[ConversionSpecifier], - context: Context) -> bool: + context: Context) -> Optional[bool]: has_star = any(specifier.has_star() for specifier in specifiers) has_key = any(specifier.has_key() for specifier in specifiers) all_have_keys = all( @@ -114,10 +115,10 @@ def analyze_conversion_specifiers(self, specifiers: List[ConversionSpecifier], if has_key and has_star: self.msg.string_interpolation_with_star_and_key(context) - return False + return None if has_key and not all_have_keys: self.msg.string_interpolation_mixing_key_and_non_keys(context) - return False + return None return has_key def check_simple_str_interpolation(self, specifiers: List[ConversionSpecifier], diff --git a/mypy/report.py b/mypy/report.py index e30f1eef7af1..51eff26db881 100644 --- a/mypy/report.py +++ b/mypy/report.py @@ -200,7 +200,7 @@ def visit_func_def(self, defn: FuncDef) -> None: if cur_indent is None: # Consume the line, but don't mark it as belonging to the function yet. cur_line += 1 - elif cur_indent > start_indent: + elif start_indent is not None and cur_indent > start_indent: # A non-blank line that belongs to the function. cur_line += 1 end_line = cur_line @@ -211,7 +211,7 @@ def visit_func_def(self, defn: FuncDef) -> None: is_typed = defn.type is not None for line in range(start_line, end_line): old_indent, _ = self.lines_covered[line] - assert start_indent > old_indent + assert start_indent is not None and start_indent > old_indent self.lines_covered[line] = (start_indent, is_typed) # Visit the body, in case there are nested functions @@ -304,7 +304,7 @@ def __init__(self, reports: Reports, output_dir: str) -> None: self.css_html_path = os.path.join(reports.data_dir, 'xml', 'mypy-html.css') xsd_path = os.path.join(reports.data_dir, 'xml', 'mypy.xsd') self.schema = etree.XMLSchema(etree.parse(xsd_path)) - self.last_xml = None # type: etree._ElementTree + self.last_xml = None # type: Optional[etree._ElementTree] self.files = [] # type: List[FileInfo] def on_file(self, @@ -535,7 +535,8 @@ def on_finish(self) -> None: out_path = os.path.join(self.output_dir, 'index.xml') out_xslt = os.path.join(self.output_dir, 'mypy-html.xslt') out_css = os.path.join(self.output_dir, 'mypy-html.css') - last_xml.write(out_path, encoding='utf-8') + if last_xml is not None: + last_xml.write(out_path, encoding='utf-8') shutil.copyfile(self.memory_xml.xslt_html_path, out_xslt) shutil.copyfile(self.memory_xml.css_html_path, out_css) print('Generated XML report:', os.path.abspath(out_path)) @@ -577,9 +578,10 @@ def on_finish(self) -> None: last_xml = self.memory_xml.last_xml out_path = os.path.join(self.output_dir, 'index.html') out_css = os.path.join(self.output_dir, 'mypy-html.css') - transformed_html = bytes(self.xslt_html(last_xml, ext=self.param_html)) - with open(out_path, 'wb') as out_file: - out_file.write(transformed_html) + if last_xml is not None: + transformed_html = bytes(self.xslt_html(last_xml, ext=self.param_html)) + with open(out_path, 'wb') as out_file: + out_file.write(transformed_html) shutil.copyfile(self.memory_xml.css_html_path, out_css) print('Generated HTML report (via XSLT):', os.path.abspath(out_path)) @@ -608,9 +610,10 @@ def on_finish(self) -> None: last_xml = self.memory_xml.last_xml out_path = os.path.join(self.output_dir, 'index.txt') stats.ensure_dir_exists(os.path.dirname(out_path)) - transformed_txt = bytes(self.xslt_txt(last_xml)) - with open(out_path, 'wb') as out_file: - out_file.write(transformed_txt) + if last_xml is not None: + transformed_txt = bytes(self.xslt_txt(last_xml)) + with open(out_path, 'wb') as out_file: + out_file.write(transformed_txt) print('Generated TXT report (via XSLT):', os.path.abspath(out_path)) diff --git a/mypy/strconv.py b/mypy/strconv.py index 169d44bdf9aa..4714e8d41c88 100644 --- a/mypy/strconv.py +++ b/mypy/strconv.py @@ -24,10 +24,9 @@ class StrConv(NodeVisitor[str]): def __init__(self, show_ids: bool = False) -> None: self.show_ids = show_ids + self.id_mapper = None # type: IdMapper if show_ids: self.id_mapper = IdMapper() - else: - self.id_mapper = None def get_id(self, o: object) -> int: return self.id_mapper.id(o) @@ -504,7 +503,7 @@ def visit_backquote_expr(self, o: 'mypy.nodes.BackquoteExpr') -> str: return self.dump([o.expr], o) -def dump_tagged(nodes: Sequence[object], tag: str, str_conv: 'StrConv') -> str: +def dump_tagged(nodes: Sequence[object], tag: Optional[str], str_conv: 'StrConv') -> str: """Convert an array into a pretty-printed multiline string representation. The format is From 53759faeac28180f6bc11abff60bc3341e893951 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Wed, 26 Apr 2017 01:55:06 +0200 Subject: [PATCH 3/3] Address review comments --- mypy/report.py | 20 ++++++++++---------- mypy/strconv.py | 9 ++++++--- mypy/stubutil.py | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/mypy/report.py b/mypy/report.py index 51eff26db881..74b44ac1f995 100644 --- a/mypy/report.py +++ b/mypy/report.py @@ -532,11 +532,11 @@ def on_file(self, def on_finish(self) -> None: last_xml = self.memory_xml.last_xml + assert last_xml is not None out_path = os.path.join(self.output_dir, 'index.xml') out_xslt = os.path.join(self.output_dir, 'mypy-html.xslt') out_css = os.path.join(self.output_dir, 'mypy-html.css') - if last_xml is not None: - last_xml.write(out_path, encoding='utf-8') + last_xml.write(out_path, encoding='utf-8') shutil.copyfile(self.memory_xml.xslt_html_path, out_xslt) shutil.copyfile(self.memory_xml.css_html_path, out_css) print('Generated XML report:', os.path.abspath(out_path)) @@ -576,12 +576,12 @@ def on_file(self, def on_finish(self) -> None: last_xml = self.memory_xml.last_xml + assert last_xml is not None out_path = os.path.join(self.output_dir, 'index.html') out_css = os.path.join(self.output_dir, 'mypy-html.css') - if last_xml is not None: - transformed_html = bytes(self.xslt_html(last_xml, ext=self.param_html)) - with open(out_path, 'wb') as out_file: - out_file.write(transformed_html) + transformed_html = bytes(self.xslt_html(last_xml, ext=self.param_html)) + with open(out_path, 'wb') as out_file: + out_file.write(transformed_html) shutil.copyfile(self.memory_xml.css_html_path, out_css) print('Generated HTML report (via XSLT):', os.path.abspath(out_path)) @@ -608,12 +608,12 @@ def on_file(self, def on_finish(self) -> None: last_xml = self.memory_xml.last_xml + assert last_xml is not None out_path = os.path.join(self.output_dir, 'index.txt') stats.ensure_dir_exists(os.path.dirname(out_path)) - if last_xml is not None: - transformed_txt = bytes(self.xslt_txt(last_xml)) - with open(out_path, 'wb') as out_file: - out_file.write(transformed_txt) + transformed_txt = bytes(self.xslt_txt(last_xml)) + with open(out_path, 'wb') as out_file: + out_file.write(transformed_txt) print('Generated TXT report (via XSLT):', os.path.abspath(out_path)) diff --git a/mypy/strconv.py b/mypy/strconv.py index 4714e8d41c88..b6295e6e0064 100644 --- a/mypy/strconv.py +++ b/mypy/strconv.py @@ -24,12 +24,14 @@ class StrConv(NodeVisitor[str]): def __init__(self, show_ids: bool = False) -> None: self.show_ids = show_ids - self.id_mapper = None # type: IdMapper + self.id_mapper = None # type: Optional[IdMapper] if show_ids: self.id_mapper = IdMapper() - def get_id(self, o: object) -> int: - return self.id_mapper.id(o) + def get_id(self, o: object) -> Optional[int]: + if self.id_mapper: + return self.id_mapper.id(o) + return None def format_id(self, o: object) -> str: if self.id_mapper: @@ -46,6 +48,7 @@ def dump(self, nodes: Sequence[object], obj: 'mypy.nodes.Context') -> str: """ tag = short_type(obj) + ':' + str(obj.get_line()) if self.show_ids: + assert self.id_mapper is not None tag += '<{}>'.format(self.get_id(obj)) return dump_tagged(nodes, tag, self) diff --git a/mypy/stubutil.py b/mypy/stubutil.py index 5c7807e92b52..93153f2464d4 100644 --- a/mypy/stubutil.py +++ b/mypy/stubutil.py @@ -90,7 +90,7 @@ def is_c_module(module: ModuleType) -> bool: return '__file__' not in module.__dict__ or module.__dict__['__file__'].endswith('.so') -def write_header(file: IO[str], module_name: str = None, +def write_header(file: IO[str], module_name: Optional[str] = None, pyversion: Tuple[int, int] = (3, 5)) -> None: if module_name: if pyversion[0] >= 3: