Crash with --linecoverage-report on comment after decorator use

[gnprice]

We ran into a mypy crash in Zulip, which was fixed with the following patch:

-@register.filter(name='render_markdown_path', is_safe=True)
 # Much of the time, render_markdown_path is called with hashable
 # arguments, so this decorator is effective even though it only caches
 # the results when called if none of the arguments are unhashable.
 @ignore_unhashable_lru_cache(512)
+@register.filter(name='render_markdown_path', is_safe=True)
 def render_markdown_path(markdown_file_path, context=None):
     # type: (str, Optional[Dict[Any, Any]]) -> str
     """Given a path to a markdown file, return the rendered html.

The traceback (with long source paths abbreviated):

zerver/templatetags/app_filters.py: error: INTERNAL ERROR -- please report a bug at https://github.com/python/mypy/issues version: 0.560
Traceback (most recent call last):
  File "/srv/zulip-py3-venv/bin/mypy", line 11, in <module>
    sys.exit(console_entry())
  File ".../site-packages/mypy/__main__.py", line 7, in console_entry
    main(None)
  File ".../site-packages/mypy/main.py", line 66, in main
    res = type_check_only(sources, bin_dir, options)
  File ".../site-packages/mypy/main.py", line 119, in type_check_only
    options=options)
  File ".../site-packages/mypy/build.py", line 218, in build
    graph = dispatch(sources, manager)
  File ".../site-packages/mypy/build.py", line 1997, in dispatch
    process_graph(graph, manager)
  File ".../site-packages/mypy/build.py", line 2299, in process_graph
    process_stale_scc(graph, scc, manager)
  File ".../site-packages/mypy/build.py", line 2486, in process_stale_scc
    graph[id].finish_passes()
  File ".../site-packages/mypy/build.py", line 1912, in finish_passes
    manager.report_file(self.tree, self.type_map(), self.options)
  File "/usr/lib/python3.4/contextlib.py", line 77, in __exit__
    self.gen.throw(type, value, traceback)
  File ".../site-packages/mypy/build.py", line 1675, in wrap_context
    yield
  File ".../site-packages/mypy/build.py", line 1912, in finish_passes
    manager.report_file(self.tree, self.type_map(), self.options)
  File ".../site-packages/mypy/build.py", line 681, in report_file
    self.reports.file(file, type_map, options)
  File ".../site-packages/mypy/report.py", line 73, in file
    reporter.on_file(tree, type_map, options)
  File ".../site-packages/mypy/report.py", line 347, in on_file
    tree.accept(coverage_visitor)
  File ".../site-packages/mypy/nodes.py", line 228, in accept
    return visitor.visit_mypy_file(self)
  File ".../site-packages/mypy/traverser.py", line 30, in visit_mypy_file
    d.accept(self)
  File ".../site-packages/mypy/nodes.py", line 573, in accept
    return visitor.visit_decorator(self)
  File ".../site-packages/mypy/traverser.py", line 64, in visit_decorator
    o.func.accept(self)
  File ".../site-packages/mypy/nodes.py", line 497, in accept
    return visitor.visit_func_def(self)
  File ".../site-packages/mypy/report.py", line 317, in visit_func_def
    assert start_indent is not None and start_indent > old_indent
AssertionError:
zerver/templatetags/app_filters.py: : note: use --pdb to drop into pdb

So it looks like we got confused by the line after the decorator being neither a definition nor the next decorator, but instead a comment.

[gvanrossum]

Thanks for the report! Simpest repro is

@blah
# comment
def f(): pass

I can see a quick fix for the crash but I don't know if it's the right fix -- I haven't tried to understand the code around it yet.

[mattsb42-aws]

FWIW, I did some poking around this in one of our repos, and it looks like this issue was introduced in mypy 0.570

[msullivan]

@gvanrossum's test case crashes for me on mypy v0.560 too. What does your test case that doesn't crash on 0.560 look like, @mattsb42-aws?

[mattsb42-aws]

@msullivan That is a good question; I haven't isolated it to a single point yet. I'll do some digging and see if I can isolate one of the places where it is failing for us.

[mattsb42-aws]

Ok; I did a bit of poking and was able to isolate what is causing us to hit this.

This might be a specific bug when interacting with attrs, I'm not sure. I haven't been able to replicate the crash with any other decorator, so I'm not sure which piece exactly is triggering it, but the error seems to be coming from the same part of mypy as the error in the OP.

import attr


@attr.s
class Z(object):
    pass

error:

test.py: error: INTERNAL ERROR -- please report a bug at https://github.com/python/mypy/issues version: 0.570
Traceback (most recent call last):
  File "0570/bin/mypy", line 11, in <module>
    sys.exit(console_entry())
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/__main__.py", line 7, in console_entry
    main(None)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/main.py", line 80, in main
    type_check_only(sources, bin_dir, options, flush_errors)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/main.py", line 129, in type_check_only
    flush_errors=flush_errors)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 180, in build
    result = _build(sources, options, alt_lib_path, bin_dir, saved_cache, flush_errors)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 266, in _build
    graph = dispatch(sources, manager)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 2112, in dispatch
    process_graph(graph, manager)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 2413, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 2608, in process_stale_scc
    graph[id].finish_passes()
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 2005, in finish_passes
    manager.report_file(self.tree, self.type_map(), self.options)
  File "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/contextlib.py", line 99, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 1765, in wrap_context
    yield
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 2005, in finish_passes
    manager.report_file(self.tree, self.type_map(), self.options)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/build.py", line 734, in report_file
    self.reports.file(file, type_map, options)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/report.py", line 73, in file
    reporter.on_file(tree, type_map, options)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/report.py", line 347, in on_file
    tree.accept(coverage_visitor)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/nodes.py", line 237, in accept
    return visitor.visit_mypy_file(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/traverser.py", line 30, in visit_mypy_file
    d.accept(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/nodes.py", line 711, in accept
    return visitor.visit_class_def(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/traverser.py", line 61, in visit_class_def
    o.defs.accept(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/nodes.py", line 773, in accept
    return visitor.visit_block(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/traverser.py", line 34, in visit_block
    s.accept(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/nodes.py", line 508, in accept
    return visitor.visit_func_def(self)
  File "/Users/bullocm/tmp/mypy/0570/lib/python3.6/site-packages/mypy/report.py", line 317, in visit_func_def
    assert start_indent is not None and start_indent > old_indent
AssertionError: 
test.py: : note: use --pdb to drop into pdb

Arguments passed to mypy: --ignore-missing-imports --linecoverage-report build --show-traceback

Removing the line coverage flag bypasses the error.

[ilevkivskyi]

@euresti You might be interested in this, it may be related to attrs plugin (I mean the last proposed repro for the crash).

[euresti]

Based on the repro Guido posted I don't think it's the attrs plugin only. I wonder if it's caused by #4544

[euresti]

Ok never mind. I see a problem but I'm not sure what to do about it.

All the methods I add for attrs are given the same line number as the class they are created in.
i.e. func.line = self.info.line But this means that the line coverage report thinks there's a method that starts at that line. And it goes looking for it and doesn't find it and hits the assertion.

However to add insult to injury, when you add a decorator to a class, the TypeInfo.line seems to return the line number of the first decorator not the line number of the "class ...." which doesn't help or hurt here.

I also looked at NamedTuple to see why that wasn't hitting this problem and I found the reason why we're going into this code path to begin with. It's this at the bottom of add_method:

        # Add the created methods to the body so that they can get further semantic analysis.
        # e.g. Forward Reference Resolution.
        self.info.defn.defs.body.append(func)

So what do you suggest? I could add something to the FuncDef that says that this is a manufactured method or something like that. Or I could figure out some other way to do the further semantic analysis needed.

[msullivan]

@euresti That's a good question. Some way to distinguish generated methods might be the way to go.

In the mean time, I've put up a fix that makes coverage reports more resilient to dodgy line numbers.