Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can I write a plugin to change imports? #10

Closed
pylint-bot opened this issue Jan 13, 2014 · 6 comments
Closed

How can I write a plugin to change imports? #10

pylint-bot opened this issue Jan 13, 2014 · 6 comments
Labels

Comments

@pylint-bot
Copy link

Originally reported by: Ethan Glasser-Camp (BitBucket: glasserc, GitHub: @glasserc?)


This is the same as pylint issue 95 (https://bitbucket.org/logilab/pylint/issue/95/how-can-i-write-a-plugin-to-change-imports). sthenault asked me to create this new issue specific to astroid and indicate that it is a follow-up.

Hi, I use flask. Flask defines a module called flask.ext. Importing flask.ext.foo is automagically mapped to importing flask_foo. I wrote a plugin on an old version of pylint to handle this: http://engineering.pave.com/post/51820567994/welcome-linting-flask-ext

I'm trying now to upgrade to pylint 1.0.0 and I see that the API has changed. I'm trying to use the new API but having trouble. I can define a transform function, as follows:

MANAGER.register_transform(node_classes.From, replace_from_flask_ext,
                           is_from_flask_ext)

With is_from_flask_ext and replace_from_flask_ext defined as:

def copy_node_info(src, dest):
    """Copy information from src to dest

    Every node in the AST has to have line number information.  Get
    the information from the old stmt."""
    for attr in ['lineno', 'fromlineno', 'tolineno',
                 'col_offset', 'parent']:
        if hasattr(src, attr):
            setattr(dest, attr, getattr(src, attr))


def splice(stmt, new_stmt):
    """Replace stmt with new_stmt in the AST

    Also, copy useful information from stmt to new_stmt.

    This assumes that stmt and new_stmt are of the same type and
    define the same names.
    """
    copy_node_info(stmt, new_stmt)

    # Replace stmt with new_stmt in the sequence of statements that
    # included stmt.
    # body = stmt.parent.body
    # i = body.index(stmt)
    # stmt.parent.body[i] = new_stmt

    # The names defined by an import statement are kept in stmt.names
    # as a pair of (exported_name, as_name). For example, "import foo,
    # bar as baz" corresponds to an import statement with
    # names=[("foo", None), ("bar", "baz")].
    #
    # All names that stmt defined should now be defined by new_stmt.
    for (name, as_name) in stmt.names:
        stmt.parent.set_local(as_name or name, new_stmt)

    return new_stmt

def is_from_flask_ext(node):
    """Is this a 'from flask.ext import wtf' statement?"""
    return node.modname == 'flask.ext'

def replace_from_flask_ext(node):
    """Replace 'from flask.ext import wtf' with 'import flask_wtf as wtf'."""
    new_stmt = node_classes.Import()
    new_stmt.names = []
    for pair in node.names:
        (name, as_name) = pair
        new_stmt.names.append(('flask_'+name, as_name or name))

    return splice(node, new_stmt)

I'm running it on code like (from the test):

from flask.ext import wtf

print wtf.Flboltolo
class MyForm(wtf.Form):
    """Test form"""
    field = StringField()

Unfortunately, this doesn't infer anything about the "wtf" variable. (In particular I don't see the "E: 15, 6: Module 'flask_wtf' has no 'Flboltolo' member" error.) I think this is due to the fact that the TreeRebuilder stores all From nodes in a member variable called _from_nodes, even when I replace them with other nodes. Commenting out self._from_nodes.append(newnode) on line 490 of rebuilder.py gives me the expected result. There appears to be no other way for me to achieve this effect, because the TreeRebuilder isn't passed to the transform functions and it cannot be accessed in any other way.


@pylint-bot
Copy link
Author

Original comment by Ethan Glasser-Camp (BitBucket: glasserc, GitHub: @glasserc?):


Relatedly, I have another plugin that tries to add instance variables to classes if they're inherited from certain classes. Right now it's impossible to trace their inheritance chains from a register_transform function if those classes are imported using "from module import Class".

@pylint-bot
Copy link
Author

Original comment by Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):


This used to work but doesn't anymore since you updated your code to the new api?

@pylint-bot
Copy link
Author

Original comment by Ethan Glasser-Camp (BitBucket: glasserc, GitHub: @glasserc?):


As far as I know, although I haven't tried it since January.

@pylint-bot
Copy link
Author

Original comment by BitBucket: mdbostwick:


Did anything happen with this? I am using pylint 1.3.1 and astroid 1.2.1, common 0.63.0, that I am getting false positives on flask.ext. and would very much like to resolve those.

@pylint-bot
Copy link
Author

Original comment by Joe Schafer (BitBucket: jschaf, GitHub: @jschaf?):


I created a pylint-flask plugin that solves this issue in a different way. My code is at https://github.com/jschaf/pylint-flask

Instead of reusing the From node, I just made new ones.

@pylint-bot
Copy link
Author

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):


I'm really sorry for the delay, having bugs lurking around for years is not acceptable.

The original problem, the fact that inference wasn't really possible in transforms was fixed in the commit e22bcea38c2cfe9de535819cca3047ce5ad06426, which added a separate step for transforms. The problem with the transforms was that they were running at the same time the AST tree was built, which meant that whenever a transform was doing inference, it was doing inference on a partially constructed tree, with results similar as the one experienced in this issue. Since now the transforms are separated and called after the AST is built, inference is fully possible inside transforms, which means that transforms as the one provided by Ethan are working properly now. The fix will be part of astroid 1.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant