From e06171e5bcf4bc1f1ef46832ad08212ff6c87136 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mois=C3=A9s=20L=C3=B3pez?= Date: Fri, 23 Sep 2016 22:50:55 +0000 Subject: [PATCH] [ADD] missing-import-error, missing-manifest-dependency --- pylint_odoo/checkers/modules_odoo.py | 101 ++++++++++++++++-- pylint_odoo/misc.py | 23 ++-- pylint_odoo/test/main.py | 2 + .../test_repo/no_odoo_module/myfile.py | 3 +- .../test_repo/test_module/__openerp__.py | 1 + .../test_repo/test_module/absolute_import.py | 5 + .../test_repo/test_module/osv_expression.py | 4 +- tox.ini | 1 + 8 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 pylint_odoo/test_repo/test_module/absolute_import.py diff --git a/pylint_odoo/checkers/modules_odoo.py b/pylint_odoo/checkers/modules_odoo.py index 37879556..3c1cb0b6 100644 --- a/pylint_odoo/checkers/modules_odoo.py +++ b/pylint_odoo/checkers/modules_odoo.py @@ -1,11 +1,11 @@ """Visit module to add odoo checks """ -import ast import os import re import astroid +import isort from pylint.checkers import utils from .. import misc, settings @@ -103,6 +103,20 @@ 'file-not-used', settings.DESC_DFLT ), + 'W%d35' % settings.BASE_OMODULE_ID: ( + 'External dependency "%s" without ImportError. More info: ' + 'https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md' + '#external-dependencies', + 'missing-import-error', + settings.DESC_DFLT + ), + 'W%d36' % settings.BASE_OMODULE_ID: ( + 'Missing external dependency "%s" from manifest. More info: ' + 'https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md' + '#external-dependencies', + 'missing-manifest-dependency', + settings.DESC_DFLT + ), } @@ -114,6 +128,15 @@ DFLT_EXTFILES_CONVERT = ['csv', 'sql', 'xml', 'yml'] DFLT_EXTFILES_TO_LINT = DFLT_EXTFILES_CONVERT + [ 'po', 'js', 'mako', 'rst', 'md', 'markdown'] +DFLT_IMPORT_NAME_WHITELIST = [ + # self-odoo + 'odoo', 'openerp', + # Known external packages of odoo + 'PIL', 'babel', 'dateutil', 'decorator', 'docutils', 'faces', + 'jinja2', 'ldap', 'lxml', 'mako', 'mock', 'odf', 'openid', 'passlib', + 'pkg_resources', 'psycopg2', 'pyPdf', 'pychart', 'pytz', 'reportlab', + 'requests', 'serial', 'simplejson', 'unittest2', 'usb', 'werkzeug', 'yaml', +] class ModuleChecker(misc.WrapperModuleChecker): @@ -145,6 +168,13 @@ class ModuleChecker(misc.WrapperModuleChecker): 'help': 'List of extension files supported to convert ' 'from manifest separated by a comma.' }), + ('import_name_whitelist', { + 'type': 'csv', + 'metavar': '', + 'default': DFLT_IMPORT_NAME_WHITELIST, + 'help': 'List of known import dependencies of odoo,' + ' separated by a comma.' + }), ) class_inherit_names = [] @@ -229,15 +259,74 @@ def check_odoo_relative_import(self, node): self.add_message('odoo-addons-relative-import', node=node, args=(self.odoo_module_name)) - @utils.check_messages('odoo-addons-relative-import') + @staticmethod + def _is_absolute_import(node, name): + modnode = node.root() + importedmodnode = ModuleChecker._get_imported_module(node, name) + if importedmodnode and importedmodnode.file and \ + modnode is not importedmodnode and \ + importedmodnode.name != name: + return True + return False + + @staticmethod + def _get_imported_module(importnode, modname): + try: + return importnode.do_import_module(modname) + except: + pass + + def _check_imported_packages(self, node, module_name): + """Check if the import node is a external dependency to validate it""" + if not module_name: + # skip local packages because is not a external dependency. + return + if not self.manifest_dict: + # skip if is not a module of odoo + return + if not isinstance(node.parent, astroid.Module): + # skip nested import sentences + return + if self._is_absolute_import(node, module_name): + # skip absolute imports + return + isort_obj = isort.SortImports( + file_contents='', + known_standard_library=self.config.import_name_whitelist, + ) + import_category = isort_obj.place_module(module_name) + if import_category not in ('FIRSTPARTY', 'THIRDPARTY'): + # skip if is not a external library or is a white list library + return + self.add_message('missing-import-error', node=node, + args=(module_name,)) + + ext_deps = self.manifest_dict.get('external_dependencies') or {} + py_ext_deps = ext_deps.get('python') or [] + if module_name not in py_ext_deps and \ + module_name.split('.')[0] not in py_ext_deps: + self.add_message('missing-manifest-dependency', node=node, + args=(module_name,)) + + @utils.check_messages('odoo-addons-relative-import', + 'missing-import-error', + 'missing-manifest-dependency') def visit_importfrom(self, node): self.check_odoo_relative_import(node) + if isinstance(node.scope(), astroid.Module): + package = node.modname + self._check_imported_packages(node, package) visit_from = visit_importfrom - @utils.check_messages('odoo-addons-relative-import') + @utils.check_messages('odoo-addons-relative-import', + 'missing-import-error', + 'missing-manifest-dependency') def visit_import(self, node): self.check_odoo_relative_import(node) + for name, _ in node.names: + if isinstance(node.scope(), astroid.Module): + self._check_imported_packages(node, name) def _check_rst_syntax_error(self): """Check if rst file there is syntax error @@ -578,10 +667,8 @@ def _get_manifest_referenced_files(self): referenced_files = [] data_keys = ['data', 'demo', 'demo_xml', 'init_xml', 'test', 'update_xml'] - with open(self.manifest_file) as f_manifest: - manifest_dict = ast.literal_eval(f_manifest.read()) - for key in data_keys: - referenced_files.extend(manifest_dict.get(key) or []) + for key in data_keys: + referenced_files.extend(self.manifest_dict.get(key) or []) return referenced_files def _get_module_files(self): diff --git a/pylint_odoo/misc.py b/pylint_odoo/misc.py index ab43d254..f8ce5593 100644 --- a/pylint_odoo/misc.py +++ b/pylint_odoo/misc.py @@ -1,4 +1,4 @@ - +import ast import csv import os import re @@ -61,6 +61,8 @@ class WrapperModuleChecker(BaseChecker): odoo_module_name = None manifest_file = None module = None + manifest_dict = None + is_main_odoo_module = None def get_manifest_file(self, node_file): """Get manifest file path @@ -110,16 +112,25 @@ def _check_{NAME_KEY}(self, module_path) :param node: A astroid.scoped_nodes.Module :return: None """ - self.manifest_file = self.get_manifest_file(node.file) - if self.manifest_file: + manifest_file = self.get_manifest_file(node.file) + if manifest_file: + self.manifest_file = manifest_file self.odoo_node = node self.odoo_module_name = os.path.basename( os.path.dirname(self.odoo_node.file)) - elif self.odoo_node and \ - not os.path.dirname(self.odoo_node.file) in \ + with open(self.manifest_file) as f_manifest: + self.manifest_dict = ast.literal_eval(f_manifest.read()) + elif self.odoo_node and not os.path.dirname(self.odoo_node.file) in \ os.path.dirname(node.file): + # It's not a sub-module python of a odoo module and + # it's not a odoo module self.odoo_node = None self.odoo_module_name = None + self.manifest_dict = None + self.manifest_file = None + self.is_main_odoo_module = False + if self.odoo_node and self.odoo_node.file == node.file: + self.is_main_odoo_module = True self.node = node self.module_path = os.path.dirname(node.file) self.module = os.path.basename(self.module_path) @@ -134,7 +145,7 @@ def _check_{NAME_KEY}(self, module_path) check_method = getattr( self, '_check_' + name_key.replace('-', '_'), None) - is_odoo_check = self.manifest_file and \ + is_odoo_check = self.is_main_odoo_module and \ msg_code[1:3] == str(settings.BASE_OMODULE_ID) is_py_check = msg_code[1:3] == str(settings.BASE_PYMODULE_ID) if callable(check_method) and (is_odoo_check or is_py_check): diff --git a/pylint_odoo/test/main.py b/pylint_odoo/test/main.py index d64d53f7..275fb4c2 100644 --- a/pylint_odoo/test/main.py +++ b/pylint_odoo/test/main.py @@ -37,6 +37,8 @@ 'method-inverse': 1, 'method-required-super': 8, 'method-search': 1, + 'missing-import-error': 3, + 'missing-manifest-dependency': 2, 'missing-newline-extrafiles': 3, 'missing-readme': 1, 'no-utf8-coding-comment': 3, diff --git a/pylint_odoo/test_repo/no_odoo_module/myfile.py b/pylint_odoo/test_repo/no_odoo_module/myfile.py index 96dd65c3..200f4426 100644 --- a/pylint_odoo/test_repo/no_odoo_module/myfile.py +++ b/pylint_odoo/test_repo/no_odoo_module/myfile.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import other_package if __name__ == '__main__': - var = True + var = other_package diff --git a/pylint_odoo/test_repo/test_module/__openerp__.py b/pylint_odoo/test_repo/test_module/__openerp__.py index eb548999..47c1e036 100644 --- a/pylint_odoo/test_repo/test_module/__openerp__.py +++ b/pylint_odoo/test_repo/test_module/__openerp__.py @@ -18,6 +18,7 @@ ], 'python': [ 'os', + 'manifest_lib', ], }, } diff --git a/pylint_odoo/test_repo/test_module/absolute_import.py b/pylint_odoo/test_repo/test_module/absolute_import.py new file mode 100644 index 00000000..cf45f2da --- /dev/null +++ b/pylint_odoo/test_repo/test_module/absolute_import.py @@ -0,0 +1,5 @@ +# coding: utf-8 +try: + import uninstalled_module +except ImportError: + uninstalled_module = None diff --git a/pylint_odoo/test_repo/test_module/osv_expression.py b/pylint_odoo/test_repo/test_module/osv_expression.py index e66e0ff9..bbb55e31 100644 --- a/pylint_odoo/test_repo/test_module/osv_expression.py +++ b/pylint_odoo/test_repo/test_module/osv_expression.py @@ -1,6 +1,8 @@ # coding: utf-8 import expression import expression as expr4 +import manifest_lib +import absolute_import import openerp.osv import openerp.osv.expression @@ -12,4 +14,4 @@ def dummy(): return (expression, osv, osv2, expr2, openerp.osv.expression, openerp.osv, - expr4, expr3) + expr4, expr3, absolute_import, manifest_lib) diff --git a/tox.ini b/tox.ini index e9c3625d..2d92fbbf 100644 --- a/tox.ini +++ b/tox.ini @@ -10,6 +10,7 @@ deps = Pygments==2.0.2 restructuredtext_lint==0.12.2 coveralls + isort [testenv] deps =