Update the current module when a new file is parsed

[chadrik]

FixAnnotateJson.current_module is currently a constant value set in __main__, but this causes bugs when pyannotate is run on a package directory, where the current module should be updated for each file that is parsed.

To fix this, I implemented set_filename, which is inherited from BaseFix, to update the current module each time the file changes.

To test, I removed the annotation from parse_type_comment and created a type_info.json file to provide the type data:

[
  {
    "func_name": "parse_type_comment", "line": 213, 
    "path": "/Users/chad/dev/pyannotate/pyannotate_tools/annotations/parse.py", 
    "samples": 0, 
    "signature": {
      "arg_types": ["str"], 
      "return_type": "Tuple[pyannotate_tools.annotations.parse:List[pyannotate_tools.annotations.parse.Argument], pyannotate_tools.annotations.parse.AbstractType]"
    }
  }
]

I saw two types of bugs with the current implementation.

1. imports statements were added that imported types from the current module:

pyannotate --uses-signature pyannotate_tools pyannotate_runtime
Refactored pyannotate_tools/annotations/parse.py
--- pyannotate_tools/annotations/parse.py	(original)
+++ pyannotate_tools/annotations/parse.py	(refactored)
@@ -10,6 +10,9 @@
 import sys
 
 from typing import Any, List, Mapping, Set, Tuple
+from pyannotate_tools.annotations.parse import AbstractType
+from pyannotate_tools.annotations.parse import Argument
+from pyannotate_tools.annotations.parse import List
 try:
     from typing import Text
 except ImportError:
@@ -211,6 +214,7 @@
 
 
 def parse_type_comment(comment):
+    # type: (str) -> Tuple[List[Argument], AbstractType]
     """Parse a type comment of form '(arg1, ..., argN) -> ret'."""
     return Parser(comment).parse()

2. If I passed two packages on the command line the second would be completely ignored.

pyannotate --uses-signature pyannotate_runtime pyannotate_tools
No files need to be modified.

[gvanrossum]

Excellent! Would it be possible to add a unit test?

[chadrik]

I looked into writing tests for this, but the test case base class from lib2to3 that’s in use here appears to be designed to test before-and-after code snippets. We would need something higher level that tested runs across multiple files. Having tests at that level would be great but it’s an order of magnitude more work, and I’m not sure I’ll have time for it. I’ll see if there is an existing test case from lib2to3 that we can use for this purpose, just to get a sense of the scope.

[chadrik]

Alternately I can write a super simple test function that demonstrates that calling set_filename on an instance of the fixer class updates the current module. Is that what you had in mind?

[gvanrossum]

Yes please, we can still write regular tests.

[chadrik]

Ok, added a test that confirms that our overridden set_filename is called. That should give us a reasonable assurance that the module will be set.

[gvanrossum]

Hm, it looks like I'm no longer on the team that can merge PRs on this repo. Hopefully @msullivan or @JukkaL can still do it?