Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

slicker thinks a local variable is a reference to an import #2

Open
davidflanagan opened this issue Jan 4, 2018 · 3 comments
Open

Comments

@davidflanagan
Copy link

With webapp in this state: https://github.com/Khan/webapp/commit/aa39930c84ad17154b943b2c917105957248fb84

I ran this command:

$ slicker.py content_editing.handlers.TutorialNavData content_render.handlers

It copied this line into content_render/handlers.py:

from content_editing.api import articles

That line is unused and it seems like a bug that it got added.

@benjaminjkraft benjaminjkraft self-assigned this Jan 29, 2018
@benjaminjkraft
Copy link
Contributor

I'll try to look at this if I get some spare time this week, I'm pretty curious about it.

@benjaminjkraft
Copy link
Contributor

Ah, I see what's going on here.

The move-source file content_editing/handlers.py has the import from content_editing.api import articles, which is used, but additionally, the moved class TutorialNavData has a method with a local variable articles. Because we don't try very hard to understand what is a local variable, we think that this is a reference to the module. So we copy the import when we move the class.

I'll think about whether there's a good way to do this, short of handling local variables more carefully, which is a known TODO but quite hard.

@benjaminjkraft benjaminjkraft changed the title slicker brought in an unneeded import slicker thinks a local variable is a reference to an import Feb 2, 2018
@benjaminjkraft
Copy link
Contributor

Yeah, I don't see how to do this without tracking variables and scopes, which I've now opened as #19.

@benjaminjkraft benjaminjkraft removed their assignment Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants