Type tooltips in HTML output

[onnokort]

Hi,

this is likely related to #1123. This PR is meant as a basis for feedback and discussion and is not meant as ready to merge.

I was annoyed by always having to put in reveal_type(...) whenever I was confused what was going on with the typing system. This PR is a proposal to display the type for expressions in the HTML report, via a 'tooltip like' popup. This is how the HTML output looks like with this PR:

Questions I have:

• Is such a feature welcome?
• Is the general approach o.k.?
• should there be a command line flag to disable/enable HTML tooltips? (I guess so, if only because there is a performance impact: I measured 38s without tooltips and 68s with tooltips, generating a HTML report for a ~15k line project).

[gvanrossum]

This is a great idea! Reminds me of "icbd" by @kmod.

…

--Guido (mobile)

[kmod]

Here's a demo of what we had

I can't take credit for the visualization though.

[JukkaL]

@onnokort Can you rebase and fix merge conflicts, and look at the lint failures?

[onnokort]

@gvanrossum: Thanks a lot for the motivation!

@JukkaL : I fixed the lint errors and also added the HTML tooltips variant as another Reporter (usable with --xslt-html-tooltips-report).

I further reworked the code so that it is hopefully low overhead when the type tooltips functionality is not used.

I was looking into adding command line arguments just for a certain Reporter. However, I found no easy way to implement that, without doing a larger rework of the Reporter infrastructure. As I am not too versed in the deeper internal structures of mypy (and further development goals), I refrained from doing that so far.

I am not entirely sure that having another reporter for the HTML tooltips variant is the right way to go, but it works for me.

Somewhat OT: I used autopep8 to fix the linter failures; it appears however, that autopep8 standard settings would fix more than flake8 complains about and I didn't want to touch code needlessly. Is there a commit script/hook that has the right settings for mypy?

[gvanrossum]

Can you please undo all formatting-only changes that weren't triggered by a flake8 error? Our flake8 configuration is in setup.py. We don't use autopep8, we typically just rely on flake8 to tell us what it finds unacceptable.

[onnokort]

Fixed, commit only changes necessary lines now.

[gvanrossum]

I know I should really review your code, but so far I've just played with it a bit, and unfortunately I usually get crashes. E.g. here's the tail end of a typical crash I get when running it on mypy itself (always a good test case :-):

  File "/Users/guido/src/mypy/mypy/traverser.py", line 29, in visit_mypy_file
    d.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 812, in accept
    return visitor.visit_assignment_stmt(self)
  File "/Users/guido/src/mypy/mypy/stats.py", line 116, in visit_assignment_stmt
    super().visit_assignment_stmt(o)
  File "/Users/guido/src/mypy/mypy/traverser.py", line 70, in visit_assignment_stmt
    o.rvalue.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 1296, in accept
    return visitor.visit_index_expr(self)
  File "/Users/guido/src/mypy/mypy/stats.py", line 148, in visit_index_expr
    self.process_node(o)
  File "/Users/guido/src/mypy/mypy/stats.py", line 163, in process_node
    str(typ))
TypeError: __str__ returned non-string (type NoneType)

There's a simple fix for that (though I'm not sure if it's right):

diff --git a/mypy/types.py b/mypy/types.py
index 5df924f..942ecbe 100644
--- a/mypy/types.py
+++ b/mypy/types.py
@@ -1333,6 +1333,7 @@ class TypeStrVisitor(TypeVisitor[str]):
 
     def visit_instance(self, t: Instance) -> str:
         s = t.type.fullname() if t.type is not None else '<?>'
+        if s is None: s = '(NONE)'
         if t.erased:
             s += '*'
         if t.args != []:

With that in place, I get usable HTML, although I also get a bunch of errors of the form

ERROR: cycle in function expansion; skipping

which comes from mypy/stats.py:67, though I don't yet understand why.

I also have nits on the rendering for the tooltips -- the CSS is kind of wacky (why orange?) and I can't say I really like that you have to hover over exactly the first character of an expression. And it would be nicer if the tips were less than a full text line below the line to which they apply.

Here's a curious UI issue I hadn't thought of: when you have e.g. an expression x = y.foo().split() how would you figure out the intermediate type of y.foo()? The tooltip on y shows the type of y, and the tip on x shows the type of y.foo().split(), but there's nothing to show me what y.foo() is -- which might be relevant to figuring out why it can't be split.

Related, if there's an error on a line it would be nice to have that error available in the UI too.

I'm not quite sure how much of this would have to be addressed before it makes sense to merge this PR. It makes sense to merge first and plan to iterate rather than waiting for perfection, but at least it ought to not crash or produce odd errors, so we should probably handle those separately (in separate PRs would be best).

What I asked for first was done, but there are more questions.

[gvanrossum]

(FWIW I fixed the crash I reported in PR #2568.)

[gvanrossum]

@onnokort Hey Onno, are you still interested in pursuing this? I have more questions (see above -- you can disregard the crash since I've fixed it), although I am generally sympathetic to adding this without insisting on perfection (it's probably better to have it iterated on by interested users).

[onnokort]

Yes, however finding time to work on this again will take me some weeks at least.

[gvanrossum]

OK, that's fine. If we don't hear from you by mid February we'll ping you again.

[gvanrossum]

Mind if I just close this? I'm no fan of keeping lingering old PRs open that have open discussion and merge conflicts. This one is now nearly half a year old and repeated promises to continue to work on it have not born fruit. If you find the time to work on this again, just open a new PR, we'll look at it with fresh eyes!