* Add ImportAssignment class and record it from Scope
* Add overrides for LocalScope and ClassScope
* Clean scope_provider code and use ImportAssignment class in `unusued_imports` codemod
* Add missing types
* Fix fixit errors
* Fix ScopeProvider when string type annotation is unparsable
* Handle nested function calls w/in type declarations
* Edit stack in place
* Add unparsed test to test_cast
* Swallow parsing errors in string annotations.
This is the same behavior as cPython.
I've also rewritten the test that was relying on this exception to check where type parsing was happening
* Fix pyre error
Based on diff review of https://github.com/Instagram/LibCST/pull/536,
I investigated relatvie import handling and realized that with minor
changes we can now handle them correctly.
Relative imports aren't likely in code coming from an automated
tool, but they could happen in hand-written stubs if anyone tries
to use this codemod tool to merge stubs with code.
Added a new test:
```
> python -m unittest libcst.codemod.visitors.tests.test_apply_type_annotations
.............................................
----------------------------------------------------------------------
Ran 45 tests in 2.195s
OK
```
Make sure that the MetadataWrapper to resolves the requested providers vs the existing metadata results and prevent a single provider from being invoked multiple times.
Note: I'm pushing this because it works, but I actually want to
add annotation counting first and then modify the code so that we
only add the import if an annotation was actually included.
In ApplyTypeAnnotationsVisitor, there are edge cases where we
might have changed the module imports even though we never wound
up applying any type annotations.
This will become even more common if we support adding
`from __future__ import annotations`, which I would like to do
soon.
To handle this, we can simply return the original tree from
`transform_module_impl` (discarding any changes from either
`self` or `AddImportsVisitor`) whenever there are no changes
in `self.annotation_counts`.
I updated the no-annotations-changed test to reflect this:
```
> python -m unittest libcst.codemod.visitors.tests.test_apply_type_annotations.TestApplyAnnotationsVisitor
...............................................
----------------------------------------------------------------------
Ran 47 tests in 2.312s
OK
```
The existing TypeCollector visitor logic attempted to
fold actual imports from stubs together with the module
we were annotating, and separately do nice things with the
names of types so that we could parse stubs written either
with various sorts of proper imports *or* stubs written
using bare fully-qualified type names (which isn't
actually legal python, but is easy to produce from automated
tools like `pyre infer`).
In this commit I simplify things in principle - meaning the
data flow is simpler, although the code is still similarly
complex - by using `QualifiedNameProvider` plus a fallback
to `get_full_name_for_node` to handle all cases via
fully-qualified names, so that the way a stub chooses to
lay out its imports is no longer relevant to how we will
understand it.
As a result, we can scrap a whole test suite where we
were understanding edge cases in the import handling, and
moreover one of the weird unsupported edge cases is now
well supported.
The tests got simpler because some edge cases no longer
matter (the whole imports test is no longer relevant),
and a couple of weird edge cases were fixed.
I ran tests with
```
python -m unittest libcst.codemod.visitors.tests.test_apply_type_annotations.TestApplyAnnotationsVisitor
```
I tried to make this change minimal in that I preserve the
existing data flow, so that it's easy to review. But it's worth
considering whether to follow up with a diff where we change
the TypeAnnotationCollector into a *transform* rather than a
*visitor*, because that would allow us to scrap quite a bit
of logic - all we would need to know is a couple of bits
of context from higher up in the tree and we could process
Names and Attributes without needing all this recursion.
Refactor ApplyTypeAnnotationsVisitor so that all annotation
information is added via a smart constructor method starting
with `_apply_annotation_to`. This makes it much easier to
skim the code and understand where annotations are actually
added with a simple forward search.
Then, add an AnnotationCounts dataclass and count up all the
annotations we add inside the transform. This should be helpful
for a few reasons:
- First, it just makes counting the annotations easier. Prior
to this change, we would have to run some separate command
to count annotations before and after a codemod, which is
not as convenient as doing it directly, and would also fail
to account for cases where we changed an annotation.
- Second, I want to be able to avoid altering the import
statements in cases where we never actually made any changes.
Having annotation counts will help us do this - we can just
return the original tree (without import changes) in that
situtation.
```
> python -m unittest libcst.codemod.visitors.tests.test_apply_type_annotations.TestApplyAnnotationsVisitor
................................................
----------------------------------------------------------------------
Ran 48 tests in 1.773s
OK
```
(
I'm not really sure how the method got there, but it was calling
itself recursively... fortunately, it was also overwritten by
an identically named method so it was actually impossible to access.
The existing two tests didn't make it clear what exactly we wanted
to verify, which is two things:
- that we can successfully annotate async functions with decorators
- that it doesn't matter whether or not the async and decorator
information is part of the stubs - we need it to be permissible
because a "real" stubs file would have this, but stubs generated
by tools like pyre infer shouldn't need to care, they only
really need to care about types
All of our tests follow one of two patterns: either populate
a context and transform using the default behavior, or test
when setting flags in either the context population and transform
steps (and verify that the behavior is the same in both cases).
So, extract these two patterns into helper functions. This improves
readability of the existing code a bit, and will be even more helpful
if we split apart the monster test `test_annotate_functions` (which
I would like to do soon - the list of test cases is so big that it's
hard to jump to the relevant section when trying to verify behaviors).
which ensures we won't have inconsistent black-vs-isort errors
going forward. We can always format by running `ufmt format .`
at the root, and check with `ufmt check .` in our CI actions.
* Use setuptools-scm to derive the current version from git metadata
* Add Github Action equivalent to the current circleci tasks
* Run pyre integration test in GH action / tox
* Add tests to verify that LibCST handles string annotations.
This is an important property for certain use cases, so it
makes sense to verify it in tests so that we can safely
depend on it.
At present, the reason we want to be able to rely on this is:
- at the moment, imports added by infer can make pysa traces
hard to understand, because the line numbers are off
- if we add the ability to use fully-qualified string annotations
for the stubs from infer, then we can do so without adding
any import lines and pyre will understand the types.
* ApplyTypeAnnotations: add unit test of how import statments are merged
Add a unit test illustrating how the codemod handles various cases
of import statments in the stub file.
Explicitly call out each of the unsupported patterns:
- bare imports (we probably should support this)
- relative imports (we probably should support this)
star imports (we probably don't want to support this)
* Add .python-version to .gitignore
This will be helpful for anyone using pyenv (I accidentally committed
my python version file in a draft branch).
**What?**
I added tests of how TypeAnnotationsVisitor reacts to
- stubs that ignore decorators and the async keyword
- stubs that lack parameter and/or return annotations where
a callable has them.
I discovered one case where TypeAnnotationsVisitor didn't do what
I want it to do: if we're missing a return annotation in the stubs
but have one in our function and `overwrite_existing_annotations` is
set, we'll actually strip out the existing annotation. So I tweaked
the logic to only use the annotation from the stub if it is nonmissing.
**Why?**
I'm working on a project (pyre infer) that depends on
TypeAnnotationsVisitor, and a problem we've run into is that we don't
yet have perfect logic for resolving imports in a way that's
sufficiently sensitive to local context to be good for codemods - for
example, we're not yet sure how to reliably diff against the existing
imports for aliased imports (when bar imports foo.Foo and then baz
includes `from bar import Foo`) and relative imports.
Because we try to give full type annotations on parameters, even where
there is a preexisting annotation, this can lead to a lot of noise
where diffs fail because of imports changing due to the stub even
though there was no actual change to the annotations.
In the short term, the easiest solution is to simply omit given
annotations from the stubs we generate. But that means we have to
be sure TypeAnnotations
**Test Plan**
```
> python -m unittest libcst.codemod.visitors.tests.test_apply_type_annotations
...........................................
----------------------------------------------------------------------
Ran 43 tests in 1.548s
OK
```
When `scope.get_qualified_names_for()` is called with a node that's an `Assignment`, return the qualified name for that node instead of all the assignments for the same name.
