Skip to content

Refuse variable inlining across deferred annotation boundaries - #894

Open
yangfan-yf-yf wants to merge 1 commit into
python-rope:masterfrom
yangfan-yf-yf:fix/inline-deferred-annotations
Open

yangfan-yf-yf wants to merge 1 commit into
python-rope:masterfrom
yangfan-yf-yf:fix/inline-deferred-annotations

Conversation

@yangfan-yf-yf

Copy link
Copy Markdown
Contributor

Description

Inlining target = original into def func(x: target) can change typing.get_type_hints(func)["x"] from int to str when original is rebound after the function definition. The same substitution can defer side effects until annotations are accessed. This occurs with from __future__ import annotations and with default deferred annotations on Python 3.14+.

I added a conservative preflight check that refuses variable inlining across these deferred annotation boundaries. It retains the original binding and source rather than moving an initializer into a different evaluation context. The check handles parameter and return annotations and simple module/class variable annotations; local variable annotations, non-simple annotated assignments, function defaults, and eager annotation controls remain available for ordinary inlining. Keeping the binding with only_current=True, remove=False also permits an eager reference to be inlined.

When a non-local binding would be removed, the check also scans Python files outside the requested write scope. For stringified annotations, it checks name and attribute expressions against their module namespace, including imported aliases, because default get_type_hints may use module globals even when static name lookup finds a class member or parameter.

Explicit string-literal forward references such as x: "target" remain outside this change. This does not replace the separate type-alias handling in #890 or promise preservation for arbitrary caller-provided namespaces.

Checklist

  • I have added tests that prove my fix is effective
  • I have updated CHANGELOG.md

Validation

  • The 64-case regression file on unmodified master: Python 3.12.3 had 22 failures, 12 passes, and 30 skips; Python 3.14.7 had 40 failures, 22 passes, and 2 skips.
  • With the fix: all applicable new cases pass. Python 3.12.3 full suite: 2,182 passed, 42 skipped, 5 xfailed. Python 3.14.7 new regressions plus the existing inline suite: 164 passed, 2 skipped.
  • All five pre-commit checks and the staged diff check pass.

The tests execute the original and retained/transformed programs in subprocesses, compare their observable results, and verify that a refusal leaves the project files unchanged. They cover rebinding, side-effect timing, parameter/return/module/class annotations, qualified and aliased imports, default get_type_hints namespace behavior, partial refactoring, and scope/eager controls. After the full-suite run, the resources positive control was narrowed to a tuple supported by the existing API; the affected tests passed on both versions with no further product changes.

@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.60630% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.40%. Comparing base (2bd17a8) to head (65c1340).

Files with missing lines Patch % Lines
rope/refactor/inline.py 97.72% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #894      +/-   ##
==========================================
+ Coverage   95.34%   95.40%   +0.05%     
==========================================
  Files         134      135       +1     
  Lines       26819    27073     +254     
==========================================
+ Hits        25570    25828     +258     
+ Misses       1249     1245       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant