wip: First try at linting imports

[kdmccormick]

This is my first pass at running import-linter on edx-platform.

My current attempt is here: https://github.com/edx/edx-platform/pull/27011

I added a bunch of provisional contracts. Some hold as-is, and others of require a small mountain of exceptions. See importlinter.cfg; I've annotated it with comments.

Notes

• My goal here was just to see if I could get the tool verifying contracts at all; I haven't yet dove into the ignore-lists that I built and searched for insights about LMS/Studio decoupling.

• I've found that import-linter, as a small but active open source project, leaves some things to be desired (namely, more flexible configuration and more helpful exception reporting). Overall, though, I'm really impressed with the tool and how it reports contract violations, and I'm pleasantly suprised at how quickly it is able to analyze contracts against edx-platform 🎉

• I needed to create empty __init__.py files in a few directories to make the linter happy. I bet that's a limitation that the import-linter maintainers would be open to relaxing, though.

• A big blocker in realizing the tool's full potential in edx-platform are the locally-installed packages in common/lib (as well as the locally-installed openedx/core/lib/xblock_builtin/xblock_discussion, but that's much smaller and less critical than, say, common/lib/xmodule). The contracts currently in importlinter.cfg essentially ignore all import chains going through those packages :/

• Lastly, I haven't checked whether this handles in-function imports (I predict it doesn't). Once pylint amnesty is going, though, it'll be easier to grep for wrong-import-position and check for all those cases. It looks like import-linter does check function-level imports, which is awesome!

Instructions & example output

Check out this branch, enter an lms-shell, and make lint-imports:

root@lms:/edx/app/edxapp/edx-platform# make lint-imports
pip install -r requirements/edx/importlinter.txt
# ...
# (requirements installation)
# ...
# ...
lint-imports --config importlinter.cfg
Could not find common.lib.xmodule.xmodule.course_module.get_available_providers when scanning cms.djangoapps.contentstore.rest_api.v1.views. This may be due to a missing __init__.py file in the parent package.
Could not find common.lib.xmodule.xmodule.course_module.get_available_providers when scanning cms.djangoapps.contentstore.rest_api.v1.serializers. This may be due to a missing __init__.py file in the parent package.
Could not find common.lib.xmodule.xmodule.capa_base.SHOWANSWER when scanning openedx.features.personalized_learner_schedules.show_answer.show_answer_field_override. This may be due to a missing __init__.py file in the parent package.
Could not find common.lib.xmodule.xmodule.capa_base.SHOWANSWER when scanning openedx.features.personalized_learner_schedules.show_answer.tests.test_show_answer_override. This may be due to a missing __init__.py file in the parent package.
=============
Import Linter
=============

---------
Contracts
---------

Analyzed 3657 files, 17163 dependencies.
----------------------------------------

LMS/Studio decoupling: lms may not *directly* import cms. KEPT
LMS/Studio decoupling: cms may not *directly* import lms, with exceptions. KEPT
LMS/Studio decoupling: lms may not *directly NOR indirectly* import cms, with exceptions. KEPT
LMS/Studio decoupling: cms may not *directly NOR indirectly* import lms, with exceptions. KEPT
Shared-code independence: common.djangoapps may not import cms. KEPT
Shared-code independence: openedx may not import cms, with exceptions. KEPT
Tooling isolation: Neither paver nor pavelib may be referenced outside of pavelib itself, with exceptions. KEPT
Tooling isolation: import_shims may not be referenced outside of import_shims itself. KEPT

Contracts: 8 kept, 0 broken.
root@lms:/edx/app/edxapp/edx-platform#

To see what a broken contract looks like, comment out an ignore_imports line, for example common.djangoapps.student.models -> lms.djangoapps.courseware.models within [importlinter:contract:cms_lms_forbidden_deep], and re-run.

pip install -r requirements/edx/importlinter.txt
# ...
# (requirements installation)
# ...
# ...
lint-imports --config importlinter.cfg
Could not find common.lib.xmodule.xmodule.course_module.get_available_providers when scanning cms.djangoapps.contentstore.rest_api.v1.views. This may be due to a missing __init__.py file in the parent package.
Could not find common.lib.xmodule.xmodule.course_module.get_available_providers when scanning cms.djangoapps.contentstore.rest_api.v1.serializers. This may be due to a missing __init__.py file in the parent package.
Could not find common.lib.xmodule.xmodule.capa_base.SHOWANSWER when scanning openedx.features.personalized_learner_schedules.show_answer.show_answer_field_override. This may be due to a missing __init__.py file in the parent package.
Could not find common.lib.xmodule.xmodule.capa_base.SHOWANSWER when scanning openedx.features.personalized_learner_schedules.show_answer.tests.test_show_answer_override. This may be due to a missing __init__.py file in the parent package.
=============
Import Linter
=============

---------
Contracts
---------

Analyzed 3657 files, 17163 dependencies.
----------------------------------------

LMS/Studio decoupling: lms may not *directly* import cms. KEPT
LMS/Studio decoupling: cms may not *directly* import lms, with exceptions. KEPT
LMS/Studio decoupling: lms may not *directly NOR indirectly* import cms, with exceptions. KEPT
LMS/Studio decoupling: cms may not *directly NOR indirectly* import lms, with exceptions. BROKEN
Shared-code independence: common.djangoapps may not import cms. KEPT
Shared-code independence: openedx may not import cms, with exceptions. KEPT
Tooling isolation: Neither paver nor pavelib may be referenced outside of pavelib itself, with exceptions. KEPT
Tooling isolation: import_shims may not be referenced outside of import_shims itself. KEPT

Contracts: 7 kept, 1 broken.


----------------
Broken contracts
----------------

LMS/Studio decoupling: cms may not *directly NOR indirectly* import lms, with exceptions.
-----------------------------------------------------------------------------------------

cms is not allowed to import lms:

-   cms.djangoapps.contentstore.management.commands.tests.test_sync_courses -> common.djangoapps.student.tests.factories (l.12)
    common.djangoapps.student.tests.factories -> common.djangoapps.student.models (l.17)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.tests.test_certificates -> common.djangoapps.student.models (l.22)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.utils -> common.djangoapps.student.models (l.27)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.tests.test_transcripts_utils -> common.djangoapps.student.tests.factories (l.21)
    common.djangoapps.student.tests.factories -> common.djangoapps.student.models (l.17)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.tests.test_libraries -> common.djangoapps.student.roles (l.21)
    common.djangoapps.student.roles -> common.djangoapps.student.models (l.188, l.16)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.checklists -> common.djangoapps.student.auth (l.7)
    common.djangoapps.student.auth -> common.djangoapps.student.roles (l.14)
    common.djangoapps.student.roles -> common.djangoapps.student.models (l.188, l.16)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.transcript_settings -> common.djangoapps.student.auth (l.26)
    common.djangoapps.student.auth -> common.djangoapps.student.roles (l.14)
    common.djangoapps.student.roles -> common.djangoapps.student.models (l.188, l.16)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.tests.test_users_default_role -> common.djangoapps.student.models (l.10)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.tests.test_organizations -> common.djangoapps.student.tests.factories (l.10)
    common.djangoapps.student.tests.factories -> common.djangoapps.student.models (l.17)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.tests.test_group_configurations -> openedx.features.content_type_gating.helpers (l.23)
    openedx.features.content_type_gating.helpers -> common.djangoapps.student.models (l.11)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.api.v1.serializers.course_runs -> common.djangoapps.student.models (l.18)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.error -> common.djangoapps.util.views (l.7)
    common.djangoapps.util.views -> common.djangoapps.student.roles (l.24)
    common.djangoapps.student.roles -> common.djangoapps.student.models (l.188, l.16)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

-   cms.djangoapps.contentstore.views.library -> common.djangoapps.student.roles (l.33)
    common.djangoapps.student.roles -> common.djangoapps.student.models (l.188, l.16)
    common.djangoapps.student.models -> lms.djangoapps.courseware.models (l.61)

# ...
# ... (many more lines)
# ...


make: *** [Makefile:140: lint-imports] Error 1
root@lms:/edx/app/edxapp/edx-platform# 

[kdmccormick]

we could instead put contracts in .importlinter and obviate the --config argument here, but I liked the GitIHub syntax highlighting that the .cfg file gives us.

[kdmccormick]

closed in favor of https://github.com/edx/edx-platform/pull/27011

[edx-status-bot]

Your PR has finished running tests. The following contexts failed:

• jenkins/python