facebookincubator / Bowler

Safe code refactoring for modern Python.
https://pybowler.io/
MIT License
1.53k stars 89 forks source link

Better introductory documentation #62

Open afoglia opened 5 years ago

afoglia commented 5 years ago

This looks like a great library, but the documentation is not very helpful in teaching it.

I think I need to do a "from bowler import *" to get the examples to work as scripts from the command line, but I don't see that in any docs.

I don't see how to run a query on an module. "bowler run" says it takes a path to a python script, or a module name, but if I do "bowler run my_query.py path.to.module" it still only checks stuff directly under my current directory.

I guess I'm expected to run it by walking the entire tree and doing "bowler run" repeatedly?

thatch commented 5 years ago

You can do bowler run my_query.py /path/to/tree and it will walk it (assuming your source files end with .py). I've talked with @jreese about improving the docs, and would love some help on the outline you'd like it to cover.

afoglia commented 5 years ago

Here's my code mod file (with the actual names changed):

(
    Query()
    .select_function("my_function")
    .rename("FOUND!")
    .diff()
)

$ bowler --version bowler 0.6.0 $ bowler run my_codemod.py common Traceback (most recent call last): File "/usr/local/google/home/afoglia/.local/bin/bowler", line 11, in sys.exit(main()) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 764, in call return self.main(args, kwargs) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 717, in main rv = self.invoke(ctx) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 1137, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 956, in invoke return ctx.invoke(self.callback, ctx.params) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 555, in invoke return callback(args, **kwargs) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/bowler/main.py", line 111, in run spec.loader.exec_module(module) # type: ignore File "", line 678, in exec_module File "", line 219, in _call_with_frames_removed File "drop_repetitive_testitem_args.py", line 13, in Query() NameError: name 'Query' is not defined

Okay, so I add the import at the top, and I get a bunch of "failed to parse" messages

$ bowler run my_codemod.py common ERROR:bowler.tool:Skipping ./setup_script.py: failed to parse [ and so on for many, many files until...] Traceback (most recent call last): File "/usr/local/google/home/afoglia/.local/bin/bowler", line 11, in sys.exit(main()) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 764, in call return self.main(args, kwargs) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 717, in main rv = self.invoke(ctx) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 1137, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 956, in invoke return ctx.invoke(self.callback, ctx.params) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 555, in invoke return callback(args, **kwargs) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/bowler/main.py", line 118, in run main() TypeError: 'module' object is not callable

Ugh, okay, so run is probably not what I want. Let's try "do".

Not much better.

$ bowler do my_codemod.py common/ Traceback (most recent call last): File "/usr/local/google/home/afoglia/.local/bin/bowler", line 11, in sys.exit(main()) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 764, in call return self.main(args, kwargs) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 717, in main rv = self.invoke(ctx) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 1137, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 956, in invoke return ctx.invoke(self.callback, ctx.params) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/click/core.py", line 555, in invoke return callback(*args, **kwargs) File "/usr/local/google/home/afoglia/.local/lib/python3.6/site-packages/bowler/main.py", line 73, in do result = eval(code) # noqa eval() - developer tool, hopefully they're not dumb File "", line 1, in NameError: name 'my_codemod' is not defined

If I try building the same query from bowler do -i, I can get it to trigger on the files one level down at least, but not multiple levels.

I can't find my session from when I did it last week. Somehow, I was able to get it to offer changes to one of the files in my directory, but only if it was the top-level file. It failed to find the same function in files two or more levels down. (It might have only been in bowler do -i.)

I suspect part of it is that we do some meta-path tricks to rewrite imports and perhaps that messes up the parsing, but this function is imported through modules that don't trigger that.

As for the documentation, there is no simple end-to-end example. A simple "hello world" like example that I user can follow and type along and do the same things and do a simple refactoring on files on disk. Like I said, as far as I can tell, nowhere in the documentation is the "from bowler import *" mentioned. It's entirely possible to believe it's unnecessary--bowler could be smart enough to do the import automatically, and inject its classes when importing the codemod scripts. Most developers who are going to use bowler will catch the error, but it would be better to guide them around the error in the first place.

afoglia commented 5 years ago

Also, when I run "bowler --debug" I see it looking for those deeper files, but claiming there's no matches, when a simple ag search shows there are.

thatch commented 5 years ago

No, bowler run is what you want. Typically you'd wrap your Query in a function called main but from the error you might have an import called main, and maybe have the query executing at import-time too? Here's what a typical Bowler script looks like:

import sys
from bowler import Query

def main():
  (
    Query(sys.argv[1:]).select(...).modify(...).diff()
  )

If you're saying it's not matching the function, is it perhaps a method? That uses select_method instead of select_function....

Any chance you could try to make a minimal repro to share the code?

afoglia commented 5 years ago

I didn't realize I needed to import main, and instantiate Query with sys.argv[1:]. Again, not in the docs.

It's actually a class. I believe I saw in the examples to use select_function for refactoring class constructors.

If I change it to query.select_class("MyClass"), it doesn't seem to make much difference. (The only difference: it does catch the class declaration, as I would expect.)

If I change it to query.select_class("my_module.MyClass") which is how it's called the 99% of the time I want to catch it, that gets me no results.

I can try to make a minimal repro, but I don't have much time, and although this is for code cleanup at work, I feel that is not rewarded by my employer, and I'm trying to cut back on doing it, if only for my own sanity.