Extract method/variable and introduce parameter don't check whether the new name is already in use

Offen
#924 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 7 Tagen

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Anfängerfreundlichkeit
62/100
Issue-Typ
Bug
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
python
Bereich
devtools

Rechercherichtung

Start with the entry points in the reproduction: extract.ExtractMethod, extract.ExtractVariable, introduce_parameter.IntroduceParameter and MethodObject.get_changes. Use the name-clash checks that rename already performs (the #761 to #769 fixes) as the reference for the error to raise. Done means each of the three reproductions raises RefactoringError instead of producing changed or broken output, and the existing refactoring tests still pass.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

Extract method, extract variable and introduce parameter accept a new name that is already in use in the target scope. The result either doesn't run, or runs and silently changes behaviour. Rename already refuses several kinds of clashing names (#761 to #766, #768 and #769 were fixed that way, and #779 asks for the general check), so the other refactorings that introduce a name should do the same.

To Reproduce

import pathlib
import tempfile

from rope.base.project import Project
from rope.refactor import extract, introduce_parameter


def run(source, make):
    root = pathlib.Path(tempfile.mkdtemp())
    (root / "m.py").write_text(source)
    project = Project(str(root), ropefolder=None)
    changes = make(project, project.get_resource("m.py"), source)
    print(changes.changes[0].new_contents)


# 1. Extract method into the name of an existing function
run(
    "def helper():\n    return 42\n\ndef f(a):\n    return a + 1\n\nprint(helper())\n",
    lambda p, r, s: extract.ExtractMethod(
        p, r, s.index("a + 1"), s.index("a + 1") + 5
    ).get_changes("helper"),
)

# 2. Extract variable into the name of an existing parameter
run(
    "def f(a, b):\n    c = a * 2\n    return c + b\n",
    lambda p, r, s: extract.ExtractVariable(
        p, r, s.index("a * 2"), s.index("a * 2") + 5
    ).get_changes("b"),
)

# 3. Introduce parameter with the name of an existing parameter
run(
    "def f(a):\n    return a + LIMIT\n\nLIMIT = 3\n",
    lambda p, r, s: introduce_parameter.IntroduceParameter(
        p, r, s.index("LIMIT")
    ).get_changes("a"),
)

Output on master (46ebf37), Python 3.13:

# 1. The second definition replaces the first one; print(helper()) now raises TypeError
def helper():
    return 42

def f(a):
    return helper(a)

def helper(a):
    return a + 1

print(helper())

# 2. The parameter b is overwritten, so f(1, 10) returns 4 instead of 12
def f(a, b):
    b = a * 2
    c = b
    return c + b

# 3. SyntaxError: duplicate argument 'a'
def f(a, a=LIMIT):
    return a + a

LIMIT = 3

MethodObject.get_changes(classname) behaves the same way: with an existing class NewMethodObject, it adds a second class of that name.

Expected behaviour

Raise RefactoringError when the new name is already defined in the scope where the refactoring introduces it, or in a scope where the new definition would shadow it, as rename does for the cases fixed in #761 to #766, #768 and #769.

Editor information

  • Python 3.13
  • Rope master 46ebf37 (also 1.14.0)
  • Found while adding Rope refactorings to rope-language-server. A language server can't prompt for a name before running the refactoring, so it has to choose one, and it can't rely on Rope to reject a clash.
Vorherrschende Sprache
Python
Sterne
2.2k
Forks
197
Ø Merge
2 T. 13 Std.
Gemergte PRs (30 T.)
5

Entwicklungsumgebung

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus python-rope/rope

Alle Issues in python-rope/rope

Ähnliche Issues

Weitere Issues zu Python

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.