Remove false positive error of missing member in TextChoices tuples

[moritz89]

Why:

• The fields label and value are not marked as missing

This change addresses the need by:

• Adding a test to ensure that pylint does not mark these as errors

[coveralls]

Pull Request Test Coverage Report for Build 1354

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-86.9%) to 0.0%

---

Totals
Change from base Build 1349:	-86.9%
Covered Lines:	0
Relevant Lines:	0

---

💛 - Coveralls

[atodorov]

@moritz89 test looks good to me. Are you going to work on a fix next ?

[moritz89]

Hi, definitely. Could you give me a pointer to how this would be implemented or where I could gather examples of how this case can be handled?

[atodorov]

Hi, definitely. Could you give me a pointer to how this would be implemented or where I could gather examples of how this case can be handled?

See comments on the issue, they give enough pointers. The only other thing I can point out to is http://pylint.pycqa.org/en/latest/how_tos/transform_plugins.html

[moritz89]

As part of the transform, I have to call manager.register_transform(nodes.Call, inference_tip(<transform_fcn>), <predicate fcn>) in an add_transforms(manager) function. However, I'm having trouble with the predicate function as it never triggers for the code in the test. Methods I've used to verify this include:

file_names = set()

def transform_fcn(node):
    file_name = node.root().file
    if file_name not in file_names:
        file_names.update({file_name})
        import ipdb; ipdb.set_trace()
    else:
        return False

and

class_names = set()

def is_tuple_in_text_choices(node):
    if not (isinstance(node.parent, nodes.Assign)
            or isinstance(node.parent, nodes.FunctionDef)) :
        return False
    if not isinstance(node.parent.parent, nodes.ClassDef):
        return False
    if "TextChoices" in [i.name for i in node.parent.parent.bases]:
        import ipdb; ipdb.set_trace()

Would appreciate a pointer for where I'm going wrong. The full file I'm using to apply the transform:

# pylint_django/transforms/text_choices.py

from astroid import MANAGER, scoped_nodes, nodes, inference_tip

from pylint_django import utils

class_names = set()

def is_tuple_in_text_choices(node):
    file_name = node.root().file
    if file_name not in file_names:
        file_names.update({file_name})
        import ipdb; ipdb.set_trace()
    else:
        return False

    if not (isinstance(node.parent, nodes.Assign)
            or isinstance(node.parent, nodes.FunctionDef)) :
        return False
    if not isinstance(node.parent.parent, nodes.ClassDef):
        return False
    if "TextChoices" in [i.name for i in node.parent.parent.bases]:
        import ipdb; ipdb.set_trace()

    class_name = node.parent.parent.name
    if class_name not in class_names:
        class_names.update({class_name})
        # import ipdb; ipdb.set_trace()
    else:
        return False
    
    # if node.parent.parent.name == "SomeTextChoices":
    #     import ipdb; ipdb.set_trace()
    # else:
    #     return False

    # if node.parent.parent.parent.name in ["TextChoices", "SomeClass", "ChoicesMeta", "TextChoices"]:
    #     import ipdb; ipdb.set_trace()
    # else:
    #     return False

    # if node.root().file.endswith("enums.py"):
    #     import ipdb; ipdb.set_trace()
    # else:
    #     return False

    # try:
    #     if node.root().file.endswith("model_enum.py"):
    #         import ipdb; ipdb.set_trace()
    # except:
    #     import ipdb; ipdb.set_trace()
    # if (node.parent.parent.name != "LazyObject"
    #         and node.parent.parent.parent.name != "django.db.models.expressions"
    #         and node.parent.parent.parent.name != "django.db.models.fields"):
    #     import ipdb; ipdb.set_trace()

    if isinstance(node.func, nodes.Attribute):
        attr = node.func.attrname
    elif isinstance(node.func, nodes.Name):
        attr = node.func.name
    else:
        return False
    return True


def infer_key_classes(node, context=None):
    pass


def add_transforms(manager):
    manager.register_transform(nodes.Call, inference_tip(infer_key_classes),
                               is_tuple_in_text_choices)

[atodorov]

Would appreciate a pointer for where I'm going wrong. The full file I'm using to apply the transform:

Please submit what you think is your best implementation to this PR so I can debug without having to copy & paste.

One possible path is to try and suppress the no-member error message (depends on how exactly this pylint checker is written and where exactly this triggers for our examples). The path with ast transformations is probably more elegant although more rocky as well. In my experience they are not very well documented (or maybe just rarely used and not deeply understood, IDK). I need some quiet time to be able to dig into your implementation and figure out what's happening.

[moritz89]

Hi Alexander, I pushed the code with which I try to trigger the transform in the test class. The trouble I have is that during the parsing process I never seem to land in the test file itself

[atodorov]

Hi Alexander, I pushed the code with which I try to trigger the transform in the test class. The trouble I have is that during the parsing process I never seem to land in the test file itself

From what I can tell this is because you are triggerring the AST transform on a Call node, which the references to .value isn't. Hence nothing of what you wrote in the transform module makes any difference.

Something.other_thing.value is an Attribute node. The condition/pattern here is an Attribute (you may or may not want to limit attrname to value/label) whose .expr is another Attribute whose .expr is Name, which when .inferred() points to a ClassDef. Name.name & ClassDef.name have the same value. This is the model class definition. In this example SomeTextChoices.

Now SomeTextChoices inherits from models.TextChoices which itself inherits from enum.Choices which wraps up the built-in type enum.Enum.

In Django you have this signature class Choices(enum.Enum, metaclass=ChoicesMeta): and the ChoicesMeta reveals the cls.choices attribute. Then in __new__ we have cls.label = property(lambda self: cls._value2label_map_.get(self.value)).

This is how what you declare as a tuple gets a .label property. Not yet sure where .values comes from.

I'm not quite sure what the rest of your code does and why. You check for assign nodes, classdef, funcdef but I fail to see the idea behind. The code being not complete doesn't help either.

I did experiment with the following snippet:

def inherits_from_choices_class(node):
    return node_is_subclass(node,
        'django.db.models.Choices',
        'django.db.models.enums.Choices',
        '.Choices',
    )


def transform_choice_tuples(node):
    print("**** node=", node, node.locals)

    for local in node.locals:
        if local.startswith('__'):
            continue

        print("--- local=", local)

#    for member in node.member:
#        member.value = ""
#        member.label = ""
    return node


def add_transforms(manager):
    manager.register_transform(nodes.ClassDef, transform_choice_tuples,
                               inherits_from_choices_class)

It will trigger the transform function when the class in question inherits from Choices. Then you can loop around all members (e.g. all tuple attributes) and just set a value/label attributes on them and pylint will stop complaining.

I have some examples in this old talk - https://www.youtube.com/watch?v=3CkSKUNMLJc&list=PLFjlI7p-h1hxBP3cIjEqePSeoBDHud5Db&index=47&t=328s (after 40:00). IIRC I took them directly from pylint's own source code.

Hope this helps.