Skip to content

RB: Safe-delete: remove a method, class, or variable only when nothing references it #433

Description

@ericwinger

Summary

Safe-delete is the one refactoring named in the "Done when" of #295 that has not been built. It
deletes a method, class, instance variable or class variable only after checking that nothing
still references it
— and when something does, it says so instead of leaving broken code behind.

Today deleting is unguarded: the Explorer's delete actions remove the thing and you find out
afterwards, from a doesNotUnderstand or a failed recompile, that something was still using it. Every
other refactoring in the family already asks its question up front and declines with a reason
(most recently the split-class and extract-superclass collision guards in #422), so delete is the
odd one out.

Acceptance criteria

  • Safe-delete is offered for a method, a class, an instance variable and a class
    variable
    , from the same places the ordinary delete lives today (Explorer rows and, where it
    applies, the editor's Refactor… menu).
  • Before deleting, the engine looks for references and declines with a reason naming what still
    uses the thing
    — senders for a method, references and subclasses for a class, accessing methods
    for a variable.
  • When references exist, the user can see them — the offending methods are listed, and clicking
    one opens it — rather than only being told the count.
  • When nothing references it, the deletion goes through the normal preview → apply flow: nothing is
    compiled or committed while building the change set, and the user commits explicitly.
  • After a successful delete, the Explorer selection lands somewhere sensible (the parent row), the
    same way the other refactorings reselect.
  • [GS SUnit] coverage on both 3.6.2 and 3.7.5, and apply-path tests per
    .claude/rules/refactoring-tests.md — assert the target is gone AND that everything it lived
    beside survived.

Implementation notes

Possible directions, nothing settled:

  • A new engine class (GsSafeDeleteRefactoring) on the shared infra, staging the existing
    #methodRemove change kind for a method and a class-definition edit for a variable. Class delete
    is the awkward one — there is no #classRemove change kind yet, and removing a class has to
    consider subclasses, references from other methods, and its class history.
  • The reference queries mostly exist: sendersOf / implementorsOf for methods,
    methodsAccessingClassVar:inHierarchyOf: and the instance-variable scans in
    GsRefactoringEnvironment for variables. A class-reference scan already exists in rename-class
    (it rewrites references), so the same scan can answer "who would break".
  • A variable is worth treating carefully: a method may mention the name without accessing that
    binding (comments, a same-named global, a shadowed name). The rename family already solved this by
    matching on association identity rather than name — safe-delete should reuse that, not re-derive it.
  • Worth deciding early whether "references exist" is always a hard decline, or whether the user may
    override with the references shown and a data-loss warning — the push-up/push-down overwrite rows
    are the precedent for an opt-in destructive path.

Part of #295.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requestideIDE component

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions