Skip to content

Refuse variable inlining across deferred annotation boundaries - #894

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

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

Conversation

@yangfan-yf-yf

@yangfan-yf-yf yangfan-yf-yf commented Oct 6, 2026 •

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

  • On the original baseline 2bd17a8 (tested on 2026-10-06), the 64-case regression file had 22 failures, 12 passes, and 30 skips on Python 3.12.3; on Python 3.14.7 it had 40 failures, 22 passes, and 2 skips.
  • Current head merges master 22f5e501. Python 3.12.3 full suite: 2,183 passed, 42 skipped, 5 xfailed. Python 3.14.7 new regressions plus the existing inline suite: 164 passed, 2 skipped. All applicable new cases pass.
  • All five configured pre-commit checks and the final diff check pass.

The regression programs run in subprocesses, check observable results, and verify that a refusal leaves project files unchanged. Eager controls and partial inlining with the original binding retained continue to execute successfully. The guard itself is unchanged from the original patch; the changelog includes the current master entries, and multiline test source samples use textwrap.dedent() under the latest contribution guideline.

@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 (22f5e50) to head (17c37cc).

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       26762    27016     +254     
==========================================
+ Hits        25516    25774     +258     
+ Misses       1246     1242       -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