Merge pull request #81 from vauxoo-dev/master-vauxoo-whlst-pack-moy3

* Merge pull request OCA#68 from vauxoo-dev/master-oca-whlst-pack-moy3

* [REF] import-error: Increase test expected

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
+    ),
 }
 
 
@@ -129,6 +143,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',
+]
 
 
 DFTL_JSLINTRC = os.path.join(
@@ -200,6 +223,13 @@ class ModuleChecker(misc.WrapperModuleChecker):
             'default': DFLT_PO_LINT_DISABLE,
             'help': 'List of disabled po-lint checks separated by a comma.'
         }),
+        ('import_name_whitelist', {
+            'type': 'csv',
+            'metavar': '<comma separated values>',
+            'default': DFLT_IMPORT_NAME_WHITELIST,
+            'help': 'List of known import dependencies of odoo,'
+            ' separated by a comma.'
+        }),
     )
 
     class_inherit_names = []
@@ -280,13 +310,72 @@ 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)
 
-    @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
@@ -664,10 +753,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):

pylint_odoo/misc.py

@@ -1,11 +1,12 @@
 
+import ast
+import csv
 import os
 import re
 import sys
 import subprocess
 from contextlib import contextmanager
 
-import csv
 import polib
 from lxml import etree
 from pylint.checkers import BaseChecker
@@ -106,6 +107,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
@@ -155,16 +158,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)
@@ -179,7 +191,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):

pylint_odoo/test/main.py

@@ -26,7 +26,7 @@
     'duplicate-xml-fields': 6,
     'duplicate-xml-record-id': 2,
     'file-not-used': 8,
-    'import-error': 4,
+    'import-error': 6,
     'incoherent-interpreter-exec-perm': 3,
     'invalid-commit': 4,
     'javascript-lint': 8,
@@ -40,6 +40,8 @@
     'method-inverse': 1,
     'method-required-super': 8,
     'method-search': 1,
+    'missing-import-error': 5,
+    'missing-manifest-dependency': 4,
     'missing-newline-extrafiles': 4,
     'missing-readme': 1,
     'no-utf8-coding-comment': 3,

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

pylint_odoo/test_repo/test_module/__openerp__.py

@@ -18,6 +18,7 @@
         ],
         'python': [
             'os',
+            'manifest_lib',
         ],
     },
 }

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

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)

tox.ini

@@ -17,6 +17,7 @@ deps =
    polib==1.0.7
    coveralls
    whichcraft
+   isort
    singledispatch==3.4
 
 [testenv]