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

Make all names and fullnames into properties #7829

Merged
merged 3 commits into from
Nov 13, 2019
Merged

Make all names and fullnames into properties #7829

merged 3 commits into from
Nov 13, 2019

Conversation

msullivan
Copy link
Collaborator

SymbolNode and FuncBase had name and fullname as methods while everything else has them as attributes or properties. Turn them into properties.

Fixes #7640.

This was done with sed -i -e 's/\.name()/.name/g' -e 's/\.fullname()/.fullname/g' mypy/*.py mypy/*/*.py mypyc/*.py mypyc/*/*.py misc/proper_plugin.py test-data/unit/plugins/*.py.

This is an annoying plugin compatibility break unfortunately but the name/fullname situation is very annoying so it is probably worth it.

Plugins that want to work with old and new versions can do something like:

from typing import Union
from mypy.nodes import FuncBase, SymbolNode


def fullname(x: Union[FuncBase, SymbolNode]) -> str:
    fn = x.fullname
    if callable(fn):
        return fn()
    return fn


def name(x: Union[FuncBase, SymbolNode]) -> str:
    fn = x.name
    if callable(fn):
        return fn()
    return fn

I expect the odds that this will need to be refreshed before I merge it as approximately 100%.

@ilevkivskyi
Copy link
Member

SymbolNode and FuncBase had name and fullname as methods while everything else has them as attributes or properties.

This is not very fair comparison, as in my experience .fullname() appears in code three-four times more often than .fullname (this is also what grep says). I somehow thought the idea was to make the opposite (i.e. make them all methods). Of course properties are more natural and more usable, but this change will break literally every plugin I am aware of.

@msullivan
Copy link
Collaborator Author

I wasn't really trying to be fair, I guess :P. The main reason for doing it this way is that it is much nicer (and it seems pretty silly to turn simple attributes on ast nodes into methods).

Many plugins would be broken doing it the other way, too. I'd rather break all the plugins (instead of just most of them :P) and end up with code we'd rather maintain in the long run, I think.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many plugins would be broken doing it the other way, too. I'd rather break all the plugins (instead of just most of them :P) and end up with code we'd rather maintain in the long run, I think.

OK, I assume you are also volunteering to manage the community reaction to this change :-)

`sed -i -e 's/\.name()/.name/g' -e 's/\.fullname()/.fullname/g' mypy/*.py mypy/*/*.py mypyc/*.py mypyc/*/*.py misc/proper_plugin.py test-data/unit/plugins/*.py`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make all names and fullnames in mypy properties
2 participants