It seems that @mark_no_op was not correctly preserving type signatures
in pyre previous to this change. Preserving type signatures in the
decorator caused some pyre errors in tests where visitors are defined
that don't share the same arg names as their parent.
from circleci pyre tests:
> libcst/_typed_visitor_base.py:15:0 Invalid type [31]: Expression `Variable[F (bound to typing.Callable[[libcst._typed_visitor.CSTTypedBaseFunctions, Variable[libcst._typed_visitor_base.T]], None])]` is not a valid type. Type variables cannot contain other type variables in their constraints.
This is especially helpful for checking qualified names of nodes against one item
or a list of items that you wish to match against. I chose to create a new matcher
instead of widening the type of `MatchMetadata` to take in either a value or
a callable. I was originally going to do the former, but having a MatchIfTrue
and a MatchMetadataIfTrue felt more orthogonal and became easier to explain than
a single MatchIfTrue that could take two types of values.
Findall does what you expect given a matcher and a tree: It returns all nodes
that exist in that tree which match the given matcher. For convenience, findall
works with regular LibCST trees as well as MetadataWrappers. It is also provided
as a helper method on the matcher transforms.
This does a few things, since it was easier to combine than separate:
- Uses type aliases instead of fully-unrolled types, making it far easier for a human to read.
- Makes codegen approximately 3x faster, which has the side effect of halving our test run times.
- Consolidate the way we use type aliases by dropping the MetadataPredicate type for now, increasing consistency.
- Widen types for AllOf/OneOf matchers to allow for MatchIfTrue since this is supported under the hood.
This results in a generated matchers file that is 1/3 the size it was, more readable by a human, and most importantly, faster to codegen, parse, format and typecheck.
In certain cases (e.g. inside Instagram's lint framework) we know that
our tree originates from the parser, so we know that there shouldn't be
any duplicate nodes in our tree.
MetadataWrapper exists to copy the tree ensuring that there's no
duplicate nodes.
This diff provides an escape hatch on MetadataWrapper that allows us to
save a little time and avoid a copy when we know that it's safe to skip
the copy.
As part of this, I ran into some issues with `InitVar` and pyre, so I
removed `@dataclass` from the class. This means that this is techincally
a breaking change if someone depended on the MetadataWrapper being an
actual dataclass, but I think this is unlikely. I implemented `__repr__`
and added tests for hashing/equality behavior.
We don't need to worry about formatting changes since we already paid the cost
in b3253de9b8. So, now that there's a new 3.8-compatible
Black, lets use it!
