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

Abierto
  • ¿Algo no está bien?
#924 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 7 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
62/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python
Área
devtools

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.
Lenguaje dominante
Python
Estrellas
2.2k
Forks
197
Merge medio
2 d 13 h
PR fusionados (30 d)
5

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de python-rope/rope

Todos los issues de python-rope/rope

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.