Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

Improve 'from import' detection in BadModuleAttributeUseLinter #1

Closed
mschwager opened this issue May 20, 2019 · 0 comments
Closed

Improve 'from import' detection in BadModuleAttributeUseLinter #1

mschwager opened this issue May 20, 2019 · 0 comments

Comments

@mschwager
Copy link
Collaborator

There is currently room for false positives and false negatives in BadModuleAttributeUseLinter. Consider the following:

from foo import bar

bar.baz("example")
import foo

foo.bar.baz("example")

Currently, to catch both of these use cases we have to use two illegal module attributes:

@property
def illegal_module_attributes(self):
    return {
        'foo.bar': [
            'baz',
        ],
        'bar': [
            'baz',
        ],
    }

This is bad for two reasons:

  1. Developers implementing new BadModuleAttributeUseLinter classes may not realize they have to include both (or more), which opens the door for false negatives. I.e. illegal attributes are being used, but not detected.
  2. False positives can arise in the bar.baz case. E.g.
from foo import bar  # bad
from qux import bar  # okay

bar.baz("example")

We should improve detection in BadModuleAttributeUseLinter to account for this. We should also make it work for arbitrary depth imports, e.g. one illegal module attribute definition should catch all of these:

import really
from really import long
from really.long import deep
from really.long.deep import import_statement

really.long.deep.import_statement("example")
@property
def illegal_module_attributes(self):
    return {
        'really.long.deep': [
            'import_statement',
        ],
    }

Instead of requiring multiple:

@property
def illegal_module_attributes(self):
    return {
        'really.long.deep': [
            'import_statement',
        ],
        'long.deep': [
            'import_statement',
        ],
        'deep': [
            'import_statement',
        ],
    }
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

1 participant