feat: add more transforms - #3
Draft
AlphaKR93 wants to merge 15 commits into
Draft
Conversation
Adds every transform listed in transforms/README.md that wasn't yet built (unfold IIFE lambdas, dummy-assignment/docstring/type-statement/local-import removal, typing-class/generic/overload/decorator/typing_extensions handling, NamedTuple/TypedDict-to-constructor conversion, dynamic attribute access conversion, early-exit/inline/lambda conversions, dead-block collapsing, and extended constant folding), and ports the two unregistered legacy free-function transforms (exception-bracket removal, positional-arg conversion) to the SuiteTransformer architecture. Adds a shared qualified-name resolution helper and the terser_hints marker package for opt-out docstring preservation. Also fixes three pre-existing bugs that were latent until these new subtree-deleting/import-rewriting transforms started exercising them: an always-false hasattr(arg, "ref") typo in the scope resolver that mis-scoped parameter annotations, a mangler crash on namespaces orphaned by deleted subtrees, and CombineImports unconditionally rebuilding (and silently double-binding) single un-combinable import statements.
TransformCache.passes/.transforms were declared but never populated, so the "stop when nothing changed" break after each pass-loop was always comparing an empty dict - every multi-pass loop (single-file module transforms, project-wide transforms, and the mangle-sensitive tail pass) silently ran exactly once regardless of config.passes. Add transforms.apply_pass(), which applies one ordered stage of enabled transforms and records per-transform whether it changed the module (via an ast.dump diff) into cache.passes - the exact signal SuiteTransformer.__new__ was already built to consume for its skip-unchanged-upstream optimization. Wire it into the three loop sites in _minify.py, project.py, and terser.py. project.py additionally now gives each collected module its own TransformCache, since the shared one it reused before would have conflated unrelated modules' change state once the tracking actually started working. Also document the actual behavior of every implemented transform in transforms/README.md (previously just a plan; the constant-folding entry notes which sub-items - numeric literal reformatting, string unescaping, sys.version_info/platform folding - aren't implemented and why).
f"{x}" -> str(x) assumed x isn't already a str, with no way to verify
that statically - wrapping an already-str value just grows the source
instead of shrinking it. Remove the whole visit_JoinedStr path rather
than keep an optimization that can regress size.
visit_Module accessed node.bindings directly (not a real attribute - NodeRef metadata is only reachable via ref(node)) and compared binding.spec, a field Binding doesn't have. Both were always broken; this only surfaced now because remove_literal_statements defaults off and nobody had exercised it with the option enabled yet. Also: this transform is FLAGS=0 (runs before resolver.resolve()/bind()), so binding data wouldn't have existed yet even with the attribute access fixed - ref(node).bindings would just be empty. Replaced the check with a plain structural scan for a module-level `__doc__ = ...` assignment, which is all that's actually available at this stage and matches what the check was trying to detect.
HoistedBinding.rename() (mangler/_constants.py) builds three kinds of fresh nodes when hoisting a repeated literal to a local variable: a Load-context Name at each original occurrence (via replace()), a Store-context Name for the new assignment's target, and the Assign statement itself. None of them ever got a real NodeRef/binding: replace() attaches a NodeRef but never calls add_reference, so ref(node).binding raised AttributeError on the wrapper; the Assign and its target were never passed through NodeRef.new/add_child at all, so ref() on them raised AttributeError on the raw node itself (no wrapper attribute present). Both were always broken, but every existing transform happened to never call ref(...).binding on these particular nodes. RemoveDummyAssignments (added this session) is the first to unconditionally check the binding of both sides of every Assign, which is what surfaced the crash - reproduced via `terser <dir> --output ...` on a source with a literal repeated often enough to trigger hoisting. Also apply the same "namespace may be orphaned by a deleted subtree" defensive check already added to reserve_name() to its sibling is_available() in mangler/_locals.py - same crash class, just not yet hit by an actual repro.
…() call sites Third instance of the same class of bug: NameBinding.rename() (resolver/binding.py) inserts an alias assignment (`A = original_param_name`) at the top of a function body whenever a keyword-callable parameter gets shortened - the original name has to stay (call sites may use it as a keyword), so the short name is bound separately. The new Assign and its Name nodes were never passed through NodeRef.new, so ref() on them raised AttributeError on the raw node, exactly like the two mangler bugs fixed already this session. Reproduced via `terser <dir> --output ...` on a source with a method taking several positional-or-keyword parameters. Registered the new nodes the same way as the earlier _constants.py fix - the target gets no binding (it's a distinct identity from the parameter, so aliasing it to the same binding would make RemoveDummyAssignments mistake the alias for a no-op `x = x` and delete it, leaving the shortened name undefined everywhere it's used); the value does get one, since it's a genuine read of the parameter. Given this is now the third occurrence of "a mangler-synthesized node was never fully registered," also hardened every place that unconditionally calls ref(node).binding on an arbitrary Name it encounters during traversal (RemoveDummyAssignments, FoldConstants/ConvertDynamicAttributeAccess's _is_unshadowed_builtin, qualified_name) to treat an unregistered node as unresolvable instead of crashing - a general safety net for whichever mangler quirk turns up next, rather than chasing each one individually.
…' lines preprocess() scans source line-by-line at raw text level with no notion of string literals: any line whose stripped text starts with '#' is treated as a directive/comment line, and if it doesn't match an if/elif/else/endif directive, it's silently dropped from the output (never appended). This is fine for real comments, but a multi-line string whose content happens to contain lines starting with '#' - e.g. a string constant holding an example/generated Python code snippet, itself full of real comments - gets those lines deleted too, corrupting the string and frequently producing an "unterminated triple-quoted string literal" SyntaxError downstream (repro: beartype's _data/code/pep/datacodepep525.py, a ~250 line file with a large f-string constant full of comment-shaped lines; py_compile handles it fine, terser's preprocessor mangled it down to 90 lines). Added _multiline_string_body_lines(), which tokenizes the source once and returns every line number that falls inside a multi-line string/f-string body (excluding the opening line, which may have real code before the string starts). preprocess() now passes those lines through untouched regardless of what they look like, leaving the existing directive/comment handling for genuine code as-is.
AlphaKR93
changed the title
Resolves conflicts in pyproject.toml (wheel packages list), _minify.py (adopt dev's Task API, keep apply_pass helper), and project.py (adopt dev's task-graph/reporter rewrite while keeping per-module TransformCache instances so the SuiteTransformer skip-unchanged optimization doesn't leak state across independent module trees). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ko8o94qtQFeJputNeCbAN
Several per-module transforms (FLAGS <= REQUIRES_IMPORT_RESOLVE) run before project-wide linking, so they can't see how a name is used outside the module they're rewriting. Tighten them to stay correct in that blind spot: - convert_typing_constructors: don't drop a TypedDict class definition when it's exported (`__all__` or otherwise public) - it may only be referenced from other modules, which this pass can't observe. - remove_annotations: keep `Annotated[...]` annotations regardless of what they wrap, since the metadata (e.g. pydantic `Field(...)`) is often consumed at runtime and can't be reconstructed from a bare type or signature default. Plain string forward-ref annotations still get stripped unconditionally. - remove_dunder_all / remove_typing_classes: default to off. Both are only safe when no other module does `from x import *` or subclasses a stripped `Protocol`, which this pass has no way to check. - remove_all: keep the `__all__` assignment if anything else in the module still references the name (e.g. `__all__.append(...)`). - arg_rename_in_place: stop renaming `*args`/`**kwargs` parameter names in place - code may introspect them by name. - project.py: don't assert on a missing `--output` when writing a binary (FFI) file; skip it instead.
When a `from x import y` origin binding gets renamed, only the imported name was being synced (`alias_node.name`) - the local binding's own name (which may be preserved via `--preserve-globals`, e.g. `app:app`) was dropped whenever it didn't happen to match the origin's new name, since the old `asname == name` check compared against the already-updated `name`. A re-export like `app/__init__.py: from .main import app` would lose the `app` name entirely once `main.app` got mangled to something else, instead of becoming `from .main import <mangled> as app`.
Feature work:
- hint_modules config + terser/utils/hints.py: recognize terser_hints
decorators from additional user-configured modules, not just terser_hints
itself.
- terser_hints.constant / preserve_annotations markers, plus
ApplyConstantDecorator transform that un-sugars `@lambda _: _()` /
`@terser_hints.constant` into a plain call+rebind for later passes to
collapse.
- constant_folding: fold `if False and ...`-style BoolOp chains, bare
`TYPE_CHECKING` (not just `typing.TYPE_CHECKING`), and
`sys.version_info <op> (...)` comparisons against a new `target_version`
config option.
- remove_generics: also strip unused PEP 695 `class Foo[T]` type params,
guarded against exported classes and any use of the param name in the
class body.
- remove_dunder_all_modules: glob-pattern whitelist for removing `__all__`
per module, instead of the all-or-nothing `remove_dunder_all` switch.
- token_printer: prefer a raw string literal (`r"..."`) over `repr()`'s
escaped form when it's shorter and representable.
- __import__("mod") / __import__("mod").attr now resolve through a real
DynamicImportBinding (terser/_pipeline/resolver/dynamic_import.py),
not ad-hoc pattern matching - qualified_name, renaming, and typing-aware
transforms all see these the same as a real import. Designed to extend
to future forms (__lazy_import__, importlib.import_module) in one place.
Correctness fixes found along the way:
- CLI: TerserParsedArguments.from_argparse silently dropped most
TransformConfig fields (respect_all, remove_dunder_all, passes,
contracts, ...); type=bool made `--flag False` parse as True; a
Literal[...] field's own alias was passed as `type=`, breaking
`--optimize`. All fixed.
- Single-file `terser.minify()` defaulted `link_imports=False` and
`DummySpec.resolve()` unconditionally raised, so every qualified_name-
based check (typing decorator/class removal, TYPE_CHECKING folding, ...)
silently no-op'd outside project mode.
- mark_exports treated any plain import as part of a module's public
surface when there's no `__all__`; now only an explicit `import x as x`
re-export counts, matching ruff/pyflakes F401.
- resolve_all only recognized `__all__ = [...]`, not `__all__ = (...)`,
so tuple-style `__all__` (e.g. pydantic's) parsed as an empty export set.
- qualified_name used a binding's local (possibly aliased/mangled) name
as the remote symbol name; added ImportBinding.remote_name so
`from x import y as z` resolves as `x.y`, not `x.z`.
- Binding gained remove_reference(), used when a transform deletes the
node that made a reference (e.g. a stripped @typing.override) so stale
entries don't keep bindings looking used.
- apply_pass's per-transform skip check only looked at earlier-positioned
transforms in the current pass, missing changes a later-positioned
transform made *last* pass - added `cache.previous_passes` so the
round-robin dependency is checked in both directions.
- ConvertToLambda/ConvertToInline used `ref(parent).namespace` for a new
node's scope, which is one level too far up when `parent` is itself a
scope node - fixed to reuse `ref(node).namespace` from the node being
replaced. This was a real correctness bug (a mangler namespace-chain
walk could loop forever, or definition and use sites could get renamed
inconsistently, e.g. `def foo(): ...; return foo` -> `def foo(): ...;
return B` with `foo` never renamed at its own definition).
- ConvertToLambda/ConvertToInline also fully re-registered reused
subtrees via add_child, double-binding references (inflating a
builtin's apparent reference count) or, at FLAGS=0, prematurely
creating placeholder UnresolvedBindings that permanently squatted a
name. Both now do a lightweight reparent instead for reused nodes.
- resolver/binding.py: `Foo.__some_method`-style Python-mangled private
names in a class body are no longer blanket-disallowed from renaming
(Python's own compiler already makes them unreachable under their
literal spelling from outside the class).
- NameBinding.should_rename used `<=`, renaming on a byte-cost tie for no
actual gain; changed to `<` (strict improvement required).
- mangle_locals pre-reserves a global's own name to protect locals from
colliding with it; mangle_globals's later rename check for that same
binding then saw its own reservation as "taken by someone else" and
force-renamed it regardless of cost. NameAssigner.assign now clears a
binding's self-reservation before checking availability.
- ImportBinding.rename()'s `ast.arguments` case checked `node.vararg`
twice (copy-paste) instead of `node.kwarg` for `**kwargs`, so keyword-
varargs parameters were never actually renamed.
- constant_folding.visit_Name folded any qualified-name match regardless
of ast context, including a Store target (e.g. the `x` in
`x = __import__("typing").TYPE_CHECKING`) - restricted to Load.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Main purpose of this branch: implement every AST transform listed in
transforms/README.mdthat wasn't built yet (seea4531b8) - unfold IIFE lambdas, dummy-assignment/docstring/type-statement/local-import removal, typing-class/generic/overload/decorator/typing_extensions handling, NamedTuple/TypedDict-to-constructor conversion, dynamic attribute access conversion, early-exit/inline/lambda conversions, dead-block collapsing, extended constant folding, plus porting the two legacy free-function transforms (exception-bracket removal, positional-arg conversion) toSuiteTransformer.The remaining commits are fixes discovered while exercising those new transforms against real projects (see
~/test_project.zip/~/site-packages.zipin the linked session), grouped by theme:hasattr(arg, "ref")typo mis-scoping parameter annotations, a mangler crash on namespaces orphaned by deleted subtrees,CombineImportssilently double-binding single un-combinable imports,config.passesnot actually iterating to a fixed point, f-string folding inFoldConstantsproducing wrong output, a crash inRemoveLiteralStatements's__doc__guard, unregistered nodes from literal hoisting and keyword-param aliasing, and the preprocessor corrupting multi-line strings containing#lines.__init__.pynot written correctly in project-mode output.X is True/X is Falseconstant folding only sound when the compared value is provably abool- fixed to guard against non-bool truthy/falsy values (e.g.0 is False), andRemoveDebugnow keeps theelsebranch of a stripped__debug__block.ConvertTypingConstructorsno longer drops an exported (__all__/public) TypedDict class definition just because it's unreferenced within its own module.RemoveAnnotationsnow keeps anyAnnotated[...]annotation regardless of what it wraps (the metadata, e.g. pydanticField(...), is often consumed at runtime and can't be reconstructed from a bare type or signature default); plain string forward-ref annotations still get stripped unconditionally.remove_dunder_all,remove_typing_classes, andremove_type_statementsnow default to off - each is only safe when no other module doesfrom x import *, subclasses a strippedProtocol, or otherwise relies on the removed name, which a per-module pass can't check.RemoveAllkeeps the__all__assignment if anything else in the module still references the name (e.g.__all__.append(...)).arg_rename_in_placestops renaming*args/**kwargsin place - code may introspect them by name.project.pyno longer asserts on a missing--outputwhen writing a binary (FFI) file; it skips instead.Test plan
Annotated[...]-parametrized signatures survive minification unchanged in shape.from x import *to confirm__all__-dependent modules still resolve correctly withremove_dunder_allat its new default (off).