Rework refactoring interface

[jorgenschaefer]

The refactoring interface needs to be redone more or less completely, and made less Rope-centric.

Two things I'd really like to include are isort and autopep8.

[jorgenschaefer]

This won't make it into 1.5 due to time constraints. My local development notes, which could do with some clean-up:

- elpy-refactor (C-c C-r)
  - runs a save-some-buffers (and a syntax check)
  - Runs elpy-refactor-start-hook
  - Pops up a window with refactoring options and reads two keys to
    select one.
  - This window shows all options, but can show some as disabled

- Refactoring options:
  - No restriction:
    - froms_to_imports: Needs offset
    - organize_imports
    - module_to_package
    - rename_current_module: new_name
    - move_module: dest_package
  - Needs Symbol:
    - rename_symbol_at_point: offset, new_name
    - inline_call: offset, only_this(y/n)
      - Make two calls, inline_this_call and inline_all_calls
    - use_function: offset
  - Needs region:
    - region_to_method: region, method_name, make_global(y/n)
      - Remove make_global argument, make this two calls,
        region_to_method, region_to_function

- Patch window
  - We don't need this. Simply open files and modify them with a
    single undo history item, but don't save. If more than one file is
    affected, show a list of affected files. Or maybe do indeed show
    diffs.
  - This window should be in diff mode... diff-mode can also apply
    hunks and stuff, make sure that works
    - Read-only
    - Change some key bindings:
      - RET should apply current hunk and delete it from buffer
      - d should delete the current hunk without applying it (query)
      - C-c C-c should do that with all hunks
      - Intro lines should give some hints on how to use it
  - Probably only if it affects multiple files?

- RPC flow:
  - get_refactoring_options()
  - refactor(<name>, <file>, <position>, [args...])
  - Returns a list of change descriptions as before

Notes:
- Keep the Rope stuff for the most part (#243)
- Run elpy-check before popping up the Rope window
- Pop up menu with refactoring options.
- Always show all of them, but show them as disabled if they are not
  applicable to the environment, or if the respective command /
  library is not available
- Support new:
  - "Attributes from arguments": Add self.OPTION = OPTION for all
    arguments of current method
  - "Sort Imports": isort: https://github.com/timothycrosley/isort
  - "Format Region", "Format File":
    - autopep8: https://github.com/hhatto/autopep8
    - Has a nice .fix_code() method for regions
  - "Format Strings": https://github.com/myint/unify
  - "Format Docstrings": https://github.com/myint/docformatter
  - "Add Imports": https://github.com/alecthomas/importmagic

[whirm]

Please, keep the patch window or at least make it optional, I like to see what's going on when I refactor stuff.

The rest looks amazing, I would kill to have import magic integrated. Does it work fine with non-bundled packages? (IE twisted) If not, maybe an "add new import" tool would be nice. It's annoying to keep having to jump to the top of the file to add imports and then jump back again all the time.

Thanks!

[jorgenschaefer]

I have not done extensive tests with Import Magic, but from my current understanding, it builds a full list of available identifiers from sys.path, so if Twisted is installed in the current virtualenv, it should be supported.

To do this, import magic needs to build this index at some point. It supports writing it to a file ala tags, but I don't like such things. My current plan would be to do the symbol collection in a separate thread within elpy, and re-run the collection regularly, but I have not tested this at all so that might not work as I think it should.

And yes, the jumping back and forth to add imports is extremely annoying to me, too. I tried to add something like import magic myself before, but that was just extremely crude and didn't work at all :-D

[whirm]

before knowing about import magic I thought about having a function that would save a bookmark, jump to the imports area and back or clone/narrow the buffer temporariiy. But not having to even write the imports is definitely better :)

[jorgenschaefer]

importmagic idea:

<jlf> is there something like elpy-doc that knows about things that
    haven't yet been imported?  e.g. if i hadn't known that the sleep
    function resided in the time module, is there some sort of
    elpy-apropos-importable-functions thing that would have found it
    for me?

[jorgenschaefer]

Some more ideas here: https://github.com/chrisbarrett/emacs-refactor

[humitos]

Today I found docformatter (I didn't know about it) and I wrote a plugin (99.9% stolen from autopep8) and I configured it as a hook on save.

• https://github.com/humitos/py-docformatter.el

Maybe you find it useful / easy to include it in elpy.

I really appreciate the work you do, I use elpy all days in my work and I'm really happy :) Thanks,

[jorgenschaefer]

Nice tool, thank you!

[vindarel]

@whirm Hi, about easily or magically adding imports I'd like to point to two projects:

• https://github.com/Wilfred/pyimport: can remove unused imports (thanks to pyflakes) and can import the module at point, but: it will search in another python buffer an example on how to import it. In melpa.
• https://gitlab.com/emacs-stuff/python-import-add: quickly add one or many imports statements and optionally sort them (with pip's isort and py-isort). It's not magic, it asks for a list of comma-separated list of modules to import, for example "os, sys, from foo import bar". It suggests the symbol at point. But: the comma isn't a good choice and it's not on melpa. However it's my project and I can do it.

Hope it helps !

[ghost]

An interesting tool: https://redbaron.readthedocs.io/en/latest/tuto.html

A proof-of-concept Emacs interface https://github.com/vindarel/redbaron4emacs

[vindarel]

A few notes about this (very clever :D ) redbaron for emacs:

• it uses baron, which doesn't target python 3. I believe it's mypy's parser the most complete now.
• the approach of firing a python process at each operation is too slow, even for a small edit.
• redbaron4emacs only does local edits, it doesn't do cross-files refactorings.

Still, I use it daily for small and quick edits.

traad (and emacs-traad) fixes those. It uses a client-server approach. (didn't use it very much myself).

[ghost]

FWIW, from the redbaron Github:

Disclaimer: RedBaron (and baron) is working with python3 but it is NOT fully parsing it yet.

[TauPan]

Have you seen Bowler https://pybowler.io/ ?

I have to admit I didn't take a closer look, just skimmed the page and it looks very intriguing.

[azzamsa]

‘elpy-refactor’ is an obsolete command (as of elpy 1.5.0); Refactoring has been unstable and flakey, support will be dropped in the future.

Is this issue still relevant if refactoring support will be dropped ?

[galaunay]

The idea was to drop rope refactoring and find a better alternative.

For now, candidates are:

• pybowler and redbaron, that seems promising, but only provide low-level refactoring (to my knowledge), which means a lot of work would have to be done on Elpy's side to provide useful features.
• Jedi apparently wants to provide refactoring, which would be nice, but they need some workforce for that:

  Jedi’s parser would support refactoring, but there’s no API to use it right now. If you’re interested in helping out here, let me know. With the latest parser changes, it should be very easy to actually make it work.

[azzamsa]

The idea was to drop rope refactoring and find a better alternative.

oh great.

I think we have to do some research on what refactoring tool other text editor are using.

[sten0]

Continuing from https://github.com/jorgenschaefer/elpy/issues/1449#issuecomment-534637362, so these issues are linked (so many overlapping issues! 'hope this helps disambiguate them). This one appears to be the authoritative top-level issue on the topic--sorry I didn't find it sooner.

Here's something I wrote on the Spyder issue:

Bowler (uses fissix, a backported lib2to3 that is intended to enable Bowler to always support the latest Python grammar)

• Pro: Allegedly much easier to work with than Rope.
• Pro: Living upstream.
• Fissix is bleeding edge. That's either a pro or a con, depending on one's perspective. It's also being used to provide addition QA for Python upstream lib2to3.

RedBaron (Supports up to Python 3.7 grammar)

• From the upstream description "For now, RedBaron can be considered in alpha, the core is quite stable but it is not battle tested yet and is still a bit rough"

@galaunay

I barely use refactoring in Elpy, so I don't know how bad it is. But I made some quick tests just now, and it seems to do a decent job (for simple stuff at least). We also didn't had any reported issues regarding rope in ages (which may or may not be an indicator of rope working well...).

I just didn't wanted to remove rope without something to replace it.

That's completely reasonable, and kind to users :-)

So we could go for bowler, but it require a decent amount of work, and may not happen soon... Redbaron may be an alternative, with the advantage of already having an emacs package: redbaron4emacs ?

One reason I would never use redbaron4emacs is that it is requires Helm (for its imenu interface), and I really don't enjoy Helm. Also, that would mean Elpy from MELPA installs both Ivy (for find-file-in-project) and Helm, which seems rather heavy.

Parso is allegedly the fastest parser, which makes me wonder if Jedi will be the fastest work with after this issue is solved https://github.com/davidhalter/jedi/issues/667

Finally there's EMR aka emacs-refactor which could be extended to support Python. @galaunay if you have 10min, I'd appreciate if you could take a look at its interfaces to see if they'd be a good fit for Elpy and NACK this option early if they're categorically not.

[galaunay]

If I understand properly, EMR is only providing refactoring for a few langages and relying on other refactoring libraries for the rest. Somebody mentioned using rope to provide python support in EMR here, so we are kind of back to square one.

Jedi would be nice and easy to implement, but functionalities seem to be quite limited at the moment (see refactoring.py).

So maybe Bowler ?

[sten0]

@galaunay, sorry for the delay, replying to this issue fell through the cracks.

I agree that Jedi isn't ready yet, which is really a shame. (P.S. Jedi/Parso is a friendly upstream!), so I've begun work packaging Fissix and Bowler for Debian.

Another alternative that I just learned about today is Prospector.

RE: EMR, as I see it the theoretical advantage is that EMR could support all of Rope, Bowler, and Propector, thus providing an abstracted interface to all three, and this would reduce the maintenance burden of supporting all three in Elpy by shifting it to EMR. Of course that possibility is blocked until EMR supports two or more Python backends. If EMR's abstracted interface is too generic to provide a good Elpy user experience, or to make adequate use of the tools' power, then it's also a non-starter...which drops us back to #1449 which will be a lot of work.

For the sake of completeness there's also Language Server Protocol...

[galaunay]

Jedi 0.17 now includes some refactoring tools (rename, inline, extract variable and extract function).

It is not as exhaustive as what rope does, but should be pretty straightforward to add to Elpy.