Add daemon command to get type of an expression

[ilevkivskyi]

Implementation is straightforward (I also tried to make it future-proof, so it is easy to add new inspections). Few things to point out:

• I added a more flexible traverser class, that supports shared visit logic and allows for O(log n) search by early return.
• I noticed a bunch of places where line/column/full name were not set. Did a quick pass and fixed as many as I can.
• I also implement basic support for expression attributes and go to definition (there are few tricky corner cases left, but they can be addressed later).

[ilevkivskyi]

OK, so the issues with mypy_primer seem to be that now a # type: ignore anywhere within a function silences all errors with that function as a context. It looks like previously it worked by accident because end_line on a function was not set properly. The logic in https://github.com/python/mypy/blob/master/mypy/errors.py#L457-L464 looks problematic.

Btw there may be the same issue with class definitions, as there was this line cdef.end_line = n.lineno (sic!).

Funny part is that such behaviour is technically correct. I can't find this in the docs, but we have tests checking that a type ignore that appears anywhere in the span of an expression silences all errors related to that expression. For example this works:

42 + ('no'
       +  # type: ignore
         'way')

because error context is the expression inside (...). In general, this kind of makes sense, as type ignores are more stable across Python/mypy versions this way. It however shouldn't probably apply to statements (of course we will need to take care of class/function decorators changing line numbers in Python 3.8).

[ilevkivskyi]

Also mypyc error on Python 3.6 looks really bizarre, I will try to figure it out later.

[JukkaL]

Also mypyc error on Python 3.6 looks really bizarre, I will try to figure it out later.

We'll be dropping Python 3.6 support soon (possibly today), so you may be able to just ignore it.

[ilevkivskyi]

OK, so now the mypy_primer errors are because some people do this:

    @property  # type: ignore[override]
    def host(self) -> str:  # type: ignore[override]
        ...

I think we should say there is an unused ignore, since either one of these would be enough to silence the error.

[JukkaL]

Requiring an exact match of the span here can make it hard to implement integrations. E.g., consider an editor extension that shows the type of an expression via a command in the right click menu. Now to get the correct span, the editor should probably use the Python parser, and maybe even the same version of the parser that is used by mypy, since I assume the spans could vary slightly between Python versions. So basically the editor would need to implement a similar visitor to find the span, which seems too complicated, especially if the editor is written in, say, Java or TypeScript.

Some ideas about how to deal with this below.

First, we could only take (line, column) as input, and return types for all the expressions that span this location. So if we have x = foo + bar and the user requests the type of the location in the middle of foo, we might return the type of foo and the type of foo + bar (with the corresponding spans). The caller can then decide what to do with the results: for example, they could show the innermost type only, or they could show all of the types (or the N innermost types), or they could ask the user to pick with expression they are interested in as a second step.

Second, we could always return the type for the innermost expression that includes the location (or intersects the span, if we take end line/column as well). This would means that to get the type of a binary operation, the location would have to be the between the operands (e.g. the operator). Here is an example (^ indicates the location requested by the caller):

foo + bar
 ^                return the type of "foo"
foo + bar
      ^           return the type of "bar"
foo + bar
    ^             return the type of "foo + bar"

[ilevkivskyi]

Hm, I think we can accept both forms, so that if user gives us full span (4 numbers), we return just the exact match (e.g. if user uses mouse to select a span). But if the user gives only 2 (i.e. line/column) we return the full stack starting from innermost (e.g. if user uses a right click).

I think we can also have flags to:

• Include span in response
• Include expression kind in response (maybe people want specifically CallExpr, not the argument etc)
• Always include only innermost expression (for 2 number requests)

[ilevkivskyi]

Another thing to consider to make this future proof is to move most of the logic to a separate module (e.g. inspections.py similar to suggestions.py) and change the API to be like dmypy inspect --type LOCATION, so we can also have dmypy inspect --attrs LOCATION (to suggest attributes after dot), dmypy --definition LOCATION, dmypy --args LOCATION (to suggest function argument names), etc.

[ilevkivskyi]

OK, all tests pass now, and mypy_primer changes are reasonable. As compared to previous versions of this PR:

• Refactored new daemon API to make it more future proof.
• Added support for two more inspections: attributes and definition (couple corner cases may be fixed separately).
• Added test support for inspection tests, and a bunch of inspection "unit" tests using it.
• Fixed few more missing line/column/full name as needed for tests.

This is now ready for review.

[ilevkivskyi]

It looks like @JukkaL is on vacation, so @jhance @hauntsaninja it would be great if one of you (or both) can take a look at this.

Btw while waiting for a review I was thinking again about some high-level design decisions here. While I still think it is probably a right thing to do, just wanted to mention this explicitly: the current default logic is optimized for speed, while it may give stale results (e.g if file was edited, but has full tree loaded in memory). My thinking here is that users typically want to get inspections really fast (like on mouse hover over an expression), so the editor integrations that would use this can call with default flag values, and only use --force-reload or a full dmypy recheck if they know some file(s) were edited.

[hauntsaninja]

Gave this a quick look over, everything looks reasonable to me. Tests look great.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/exceptions.py:208: error: Unused "type: ignore" comment

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/connection.py:174: error: Unused "type: ignore" comment

porcupine (https://github.com/Akuli/porcupine)
+ porcupine/plugins/pastebin.py:107: error: Unused "type: ignore" comment

core (https://github.com/home-assistant/core)
+ homeassistant/components/recorder/models.py:162: error: Unused "type: ignore" comment
+ homeassistant/components/recorder/models.py:174: error: Unused "type: ignore" comment
+ homeassistant/components/recorder/models.py:186: error: Unused "type: ignore" comment
+ homeassistant/components/recorder/models.py:201: error: Unused "type: ignore" comment

[ilevkivskyi]

@JukkaL I am sure you will have some comments on the docs :-), but to speed up things I will merge it now and you can make changes yourself.