[pylint] E1101:Instance of 'ForeignKey' has no 'operator' member

[debuggerpk]

as instructed in #21 , creating a new issue.

if i call self.discount.name twice on a model, it raises a linting error. See screenshot. the error raised is

[pylint] E1101:Instance of 'ForeignKey' has no 'operator' member

WARNING: Please do not report issues about missing Django, see
README!

[atodorov]

Post the definition of the Discount model as plain text, not a screenshot.

[debuggerpk]

class Discount(TimeStamped):
  name = models.CharField(max_length=255)
  value = models.DecimalField(max_digits=8, decimal_places=2, blank=True, null=True)
  operator = models.CharField(max_length=255, blank=True, null=True)
  is_active = models.NullBooleanField()

  customer = models.ForeignKey('customers.Customer', on_delete=models.PROTECT, related_name='discounts')

[atodorov]

I tried with the following example (a bit different than yours)

from django.db import models


class Discount(models.Model):
    name = models.CharField(max_length=255)
    value = models.DecimalField(max_digits=8, decimal_places=2, blank=True, null=True)
    operator = models.CharField(max_length=255, blank=True, null=True)
    is_active = models.NullBooleanField()

class OutletDiscount(models.Model):
    discount = models.ForeignKey(Discount, on_delete=models.PROTECT)

    @property
    def name(self):
        return self.discount.name

    @property
    def operator(self):
        return self.discount.operator

    @property
    def value(self):
        return self.discount.value

I was not able to reproduce so closing.

Make sure you are using the latest version from PyPI and if you can still reproduce prepare a self contained example.

To do this place your code under the pylint_django/tests/input directory and execute scripts/test.sh. If you can manage to create a test file that reproduces this problem but does not complain about missing class definitions, etc then I can investigate more.

[debuggerpk]

@atodorov the problem appears when the FK field references a string not the actual class. In my example, the class is

class OutletDiscount(models.Model):
  outlet = models.ForeignKey(Outlet, on_delete=models.PROTECT, related_name='discounts')
  discount = models.ForeignKey('warehouse.Discount', on_delete=models.PROTECT)

  @property
  def name(self):
    return self.discount.name

  @property
  def operator(self):
    return self.discount.operator

  @property
  def value(self):
    return self.discount.value

notice the discount field on OutletDiscount, it class the FK from string.

[debuggerpk]

PS: i am sorry if it has been difficult for me to explain.

[dmytrokyrychuk]

I can confirm the issue reported by @ysfjwd.

@atodorov you were not able to reproduce because the referenced model was in scope. It appears that referenced models must be in astroid's cache for the string approach to work correctly, which seems to defeat the purpose of using strings completely.

[atodorov]

@KyrychukD thanks for the tip. So the FK model both needs to be referenced as string and needs to live in an external file I think. With that I was able to reproduce but I have no idea how to fix that yet.

[atodorov]

So the current implementation is based on #35 and #119 adds few tests. These are working well in the case where the referenced model is defined in the same module so I guess it is in the astroid cache.

The current problem appears to be that when the module defining the model is external it isn't loaded and not available to astroid so the original implementation rightly fails because it has no idea that there is a class named 'module.Model'.

One thing we can do is try to import the referenced module before performing the check. @carlio how do you feel about this ? I'll try a POC of this idea to see if it works.

[atodorov]

A few more thoughts on this:

1. There's the obvious problem of discovering the module before it is imported which depends on the exact filesystem layout and PYTHONPATH/sys.path modifications you are doing in your project which pylint doesn't know about.

2. assuming Python is able to find a module with the desired name I get a failure from Django:

E django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

because I'm trying to load one piece of the application without loading the entire app first.

I'm starting to gravitate towards wontfix b/c there are many edge cases where this will not work and we'll have to either special-case them one by one or explain to people why it's not working and why it's not going to be fixed. IMO I'd rather go with a "don't reference models as strings" known issue approach and forget about this!

[dmytrokyrychuk]

An important thing to note here is that models are usually referenced with strings in case there are circular dependencies between the modules they are defined in. So chances are that the referenced model's module will not be in cache. Is there a way in pylint to defer inspection somehow, e.g. until there's no more modules to load? Чт, 24 трав. 2018 20:53 користувач Alexander Todorov < [email protected]> пише:

…

A few more thoughts on this: 1. There's the obvious problem of discovering the module before it is imported which depends on the exact filesystem layout and PYTHONPATH/sys.path modifications you are doing in your project which pylint doesn't know about. 2. assuming Python is able to find a module with the desired name I get a failure from Django: E django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet. because I'm trying to load one piece of the application without loading the entire app first. I'm starting to gravitate towards *wontfix* b/c there are many edge cases where this will not work and we'll have to either special-case them one by one or explain to people why it's not working and why it's not going to be fixed. IMO I'd rather go with a "don't reference models as strings" known issue approach and forget about this! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#142 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAquOcrj9To_rXRMZ67fcK2tH9l7AvSgks5t1vN_gaJpZM4S3aCK> .

[atodorov]

Is there a way in pylint to defer inspection somehow, e.g. until there's no more modules to load?

I don't think so. What if I execute pylint path/to/some/models.py and the foreign key models are defined into another file? That other file will never be loaded if it isn't imported anywhere.

And as you say the reason for using string names is circular dependencies between modules which is a bad sign for the project. IMO the best course of action will be to clean up the offending application and have a simpler architecture of the underlying models so that there are no circular dependencies.

@carlio any thoughts on this issue ?

[carlio]

My thoughts on this remain that pylint (and therefore pylint-django) can only inspect code that exists and since there is no way to know what model strings are referring to without setting up and inspecting Django code, this seems like just a very tricky thing to do without overcomplicating this library.

I supposed I'd ask why pylint-django should be able to find the model if the models.py itself is not importing it but using a string to reference. Presumably there's some import problem preventing referencing the class directly, in which case pylint-django can't do a huge amount, or it's just done though convenience in which case that's really on the developer to fix.

Just my 2 cents :-)