Skip to content

fix(source-edit): place MoveSymbol imports at syntax boundaries - #4531

Merged
ScriptedAlchemy merged 4 commits into
masterfrom
fix/move-import-syntax-boundaries
Oct 28, 2026
Merged

ScriptedAlchemy merged 4 commits into
masterfrom
fix/move-import-syntax-boundaries

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Merge #4478 first, followed by #4526, #4510, #4524, #4489, #4485, #4527, #4528, #4530, #4531, #4529, #4534, and #4541.

This PR is number 10 of 13, based on #4530. Merge #4529 next.

Use merge commits. After merging a parent PR, change its child's base to master before merging the child. Auto-merge is disabled.

Fixes #4420. Stack parent: #4530. Merge after the existing ordered stack using a merge commit.

Problem

insert_imports advanced one physical line per use and treated inner and outer attributes alike. A destination with a multi-line use std::{ ... }; got the new import spliced inside the group, and a destination starting with #![allow(dead_code)] got the import ahead of the inner attribute. Both produce invalid Rust.

Root cause

Insertion position came from a per-line prefix classifier (classify_leading_line), not from the Rust parse the dependency analyzer already uses.

Fix

  • The insertion byte boundary uses the existing canonical Rust parser: after module inner attributes, inner docs, leading comments and existing use/extern crate declarations, never inside one. Outer docs and attributes stay attached to the item they annotate.
  • A destination that has syntax errors (or no grammar) refuses the move with a typed error instead of guessing. Empty and missing destinations behave as before.
  • The line classifier and LeadingKind are deleted; the only remaining caller (inner-doc skip) uses is_inner_doc_line.

Verification

Red first (commit 1): move_symbol_inserts_imports_after_a_multiline_use_group, ..._after_module_inner_attributes and move_symbol_refuses_a_destination_that_does_not_parse failed (the first two with rustc errors on the generated destination). With the fix, bazel test //crates/tracedecay-source-edit:unit_test passes (all tests) and bazel build --config=clippy //crates/tracedecay-source-edit/... is clean.

🤖 Generated with Claude Code

Independent boundary regression and correction

An existing import and an outer attribute may share a physical line. Rounding the import's syntax end to the next line detached #[cfg(any())] from the following item and applied it to the newly inserted import. The real MoveSymbol fixture then failed Rust compilation because the moved function's Widget import was disabled.

The insertion uses the parsed node's byte boundary, preserves the destination text around it, and inserts a newline where necessary. The new regression executes the existing preview/publication journey and compiles the resulting fixture.

  • BEFORE: 0 passed / 1 failed; invocation 6fe80e3b-a163-429d-b651-e981eddd13c0, missing Widget after the move.
  • AFTER exact: 1 passed / 114 filtered; invocation eca65863-6a1e-4bff-9e79-7cb0d30b880d; affected formatting passed.
  • Full source-edit unit suite: 115 passed / 0 failed; invocation 7e338640-af8b-4361-926b-50d59a944904.

All runs used canonical Bazel targets with fresh test results. No timeout, budget, or host setting changed.

Scoped Clippy and final formatting passed in 8420704d-e70a-481c-b928-2d1a34aac5ef. The first Clippy run caught the now-unused splice_lines import; removing that import was the only change after the 115-test pass.

Stack integration

The parent stack already contains canonical byte-based destination rendering, typed source-edit-destination-syntax-unsupported refusal, validation for dependency-free Rust moves, and newline preservation. Integration retains that implementation rather than duplicating the parser. This layer keeps the real execution regressions and removes the obsolete line classifier in favor of the remaining inner-doc predicate. No destination validation or existing regression is dropped.

Integrated parent validation: 121 source-edit unit tests passed, with formatting, in c759f31b-2115-407e-aa85-1ffe9eb18855. This includes the parent destination tests and every new execution regression.

Integrated scoped Clippy passed in 2e8d67bd-73a9-4027-a613-d1ccadf75a10. Parent integration uses a merge commit and preserves the canonical renderer byte-for-byte.

Import binding and destination-scope follow-up

Physical-line matching treated import text in comments and nested scopes as a module binding, while the text alias splitter missed valid multiline/commented as clauses. Import discovery and destination matching now use the maintained Rust grammar's path and alias nodes. Equivalent grouped or multiline imports are reused, and aliases keep their original meaning and bytes. Existing public exports and relative-source import safeguards remain intact.

Automatic insertion now returns the existing typed refusal before publication when it would overlap a conditional or unknown attributed import, a different known binding or item name, or an unresolved wildcard binding. Exact unconditional matches and legitimate self-import suppression remain supported; anonymous as _ imports remain distinct from wildcards. These checks use syntax-visible declarations and do not perform macro expansion or full namespace resolution.

The existing preview/apply fixture was extended for commented/nested imports, alias trivia, equivalent grouped paths, alias self-imports, manual dependency hints, and conservative refusal with unchanged files and no edit journal. A focused native check used the exact committed import-binding helper and pinned Rust grammar: the original helper failed the multiline-alias case (0 passed / 1 failed), while the repaired helper passed both alias and grouped/unnamed-binding checks (2 passed / 0 failed / 0 ignored). This helper check does not execute the full source-edit application journeys. Pinned formatting, diff checks, and normal hooks passed for both commits; the complete native suite remains a hosted CI requirement.

Unconditional lint-attribute follow-up

The destination availability check now recognizes direct, unqualified built-in lint attributes (allow, expect, warn, deny, and forbid) with parsed parenthesized arguments. These attributes do not disable the existing import, so a matching binding is reused with its original bytes and lint policy intact. Every attached attribute must qualify. Conditional, qualified, procedural, unknown, and documentation attributes retain the conservative refusal; source relocation still treats all attached attributes as owned syntax.

Actual preview/apply fixtures now cover a matching allow(unused_imports) import and a grouped public alias under multiple lint attributes. Additional refusal cases combine lint attributes with cfg_attr, a qualified attribute, or a procedural attribute. Existing assertions, compiler warning expectations, and mutation boundaries are preserved.

Pinned formatting, whitespace checks, normal pre-commit/commit-msg hooks, and two independent source reviews passed. A focused native check compiled the exact committed attribute helpers with the pinned Rust grammar and nightly compiler. The original rule reproduced the allow(unused_imports) failure (0 passed / 1 failed); the repaired helpers passed both checks (2 passed / 0 failed / 0 ignored), including all five lint names, trivia, strict source attachment ownership, and refusal of active, unknown, documentation, or malformed attribute forms. Full preview/apply application journeys remain a hosted CI requirement.

@changeset-bot

changeset-bot Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0b6e8b5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from d5b13e2 to 46532d2 Compare October 25, 2026 10:35
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from 05f3027 to 770114e Compare October 25, 2026 10:35
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from 46532d2 to deb6c24 Compare October 25, 2026 11:27
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from 770114e to 017328f Compare October 25, 2026 11:27
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from deb6c24 to e6d05d4 Compare October 25, 2026 19:42
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from 8b07ca6 to eea3ca0 Compare October 25, 2026 19:42
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from e6d05d4 to e5d734b Compare October 25, 2026 20:06
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from eea3ca0 to 443b1f9 Compare October 25, 2026 20:06
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from 379a571 to e852784 Compare October 26, 2026 03:42
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from e0f3699 to 5e25f96 Compare October 26, 2026 03:42
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from e852784 to 3e31ec1 Compare October 26, 2026 03:51
ScriptedAlchemy added a commit that referenced this pull request Oct 26, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from 87ffb73 to a6ba355 Compare October 26, 2026 19:14
ScriptedAlchemy added a commit that referenced this pull request Oct 26, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from c3ed21b to 96e4c2d Compare October 26, 2026 19:26
ScriptedAlchemy added a commit that referenced this pull request Oct 26, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from a6ba355 to 72bfe1e Compare October 26, 2026 19:26
ScriptedAlchemy added a commit that referenced this pull request Oct 26, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/search-eval-canonical-corpus branch from 96e4c2d to f419918 Compare October 26, 2026 19:59
ScriptedAlchemy added a commit that referenced this pull request Oct 26, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from 72bfe1e to 92a2c31 Compare October 26, 2026 19:59
ScriptedAlchemy added a commit that referenced this pull request Oct 26, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@ScriptedAlchemy
ScriptedAlchemy changed the base branch from fix/search-eval-canonical-corpus to release-please--branches--master--components--tracedecay October 27, 2026 06:45
@ScriptedAlchemy
ScriptedAlchemy force-pushed the release-please--branches--master--components--tracedecay branch from 1141108 to ffc8eba Compare October 27, 2026 06:55
@ScriptedAlchemy
ScriptedAlchemy changed the base branch from release-please--branches--master--components--tracedecay to fix/search-eval-canonical-corpus October 27, 2026 07:05
ScriptedAlchemy added a commit that referenced this pull request Oct 27, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from dd25e3d to 5acd312 Compare October 27, 2026 07:05
ScriptedAlchemy and others added 4 commits October 27, 2026 14:09
Restack unique #4531 onto the rewritten #4530 tip.

(cherry picked from commit f2cf33e)
Recognize existing destination imports from complete top-level Rust
declarations instead of matching physical lines in comments or nested
scopes. Preserve existing declaration visibility and attachments.

Refuse automatic insertion when only an attributed matching import is
available: its configuration may disable the binding or make an
unconditional duplicate invalid.

Exercise comment and nested-scope imports through real preview,
publication, and fixture compilation. Assert typed refusal and unchanged
files for cfg-disabled and cfg-enabled matching imports.

Pinned formatting and diff checks pass. Native execution remains pending
hosted CI because this workspace has exhausted its build disk budget.

(cherry picked from commit 7a9446c)
Preserve aliases across import trivia and grouping, and reuse exact
destination bindings without inserting duplicate names. Refuse attributed
or conflicting names and unresolved wildcard shadowing before planning.

Exercise the existing move preview/apply journeys for equivalent imports,
alias preservation, manual dependency hints, and unchanged refusal paths.

(cherry picked from commit 5a7f8e7)
Recognize direct built-in lint attributes when reusing existing destination
imports. Keep conditional, unknown, qualified and procedural attributes
on the conservative refusal path, and preserve source attribute ownership.

Extend the real move preview/apply fixtures for lint-attributed equivalent
imports and mixed conditional or unknown attributes without changing
existing assertions, warnings, or mutation boundaries.

(cherry picked from commit 5acd312)
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/move-import-syntax-boundaries branch from 5acd312 to 0b6e8b5 Compare October 27, 2026 21:13
@ScriptedAlchemy
ScriptedAlchemy changed the base branch from fix/search-eval-canonical-corpus to master October 27, 2026 21:13
@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) October 27, 2026 21:13
ScriptedAlchemy added a commit that referenced this pull request Oct 27, 2026
Restack unique #4529 onto the rewritten #4531 tip.

(cherry picked from commit 1e4aa82)
@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Included in the integration PR #4589, which runs CI once for all open PRs. This PR will be marked merged automatically when #4589 lands.

@ScriptedAlchemy
ScriptedAlchemy merged commit 833e697 into master Oct 28, 2026
9 of 15 checks passed
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.

fix(source-edit): insert MoveSymbol imports at syntax boundaries, not inside grouped imports or before inner attributes

1 participant