[REF] duplicate-xml-record-id: Get xml section from manifest to skip …

…different origin (#89)

Fix #83

.coveragerc

@@ -11,6 +11,7 @@ omit =
 
 # Regexes for lines to exclude from consideration
 exclude_lines =
+    pragma: no cover
     # Don't complain about self-execute
     if __name__ == '__main__':
     # Don't complain about uninstalled packages

pylint_odoo/checkers/modules_odoo.py

@@ -394,8 +394,11 @@ def _get_duplicate_xml_record_id(self, records):
         """
         all_records = {}
         for record in records:
-            record_id = record.attrib.get('id', '') + '.' + "noupdate" + \
-                record.getparent().attrib.get('noupdate', '0')
+            record_id = "%s/%s_noupdate_%s" % (
+                record.attrib.get('section', ''),
+                record.attrib.get('id', ''),
+                record.getparent().attrib.get('noupdate', '0'),
+            )
             all_records.setdefault(record_id, []).append(record)
         # Remove all keys which not duplicated
         for key, items in all_records.items():
@@ -404,16 +407,22 @@ def _get_duplicate_xml_record_id(self, records):
         return all_records
 
     def _check_duplicate_xml_record_id(self):
-        """Check duplicate xml record id all xml files of a odoo module.
+        """Check duplicated XML-IDs inside of the files of
+        each manifest-section treated them separately
         :return: False if exists errors and
                  add list of errors in self.msg_args
         """
         self.msg_args = []
-        all_xml_ids = []
-        for xml_file in self.filter_files_ext('xml', relpath=False):
-            all_xml_ids.extend(self.get_xml_records(xml_file))
+        xml_records = []
+        for fname, section in self._get_manifest_referenced_files().items():
+            if os.path.splitext(fname)[1].lower() != '.xml':
+                continue
+            fname = os.path.join(self.module_path, fname)
+            for xml_record in self.get_xml_records(fname):
+                xml_record.attrib['section'] = section
+                xml_records.append(xml_record)
         for name, fobjs in \
-                self._get_duplicate_xml_record_id(all_xml_ids).items():
+                self._get_duplicate_xml_record_id(xml_records).items():
             self.msg_args.append((
                 "%s:%d" % (os.path.relpath(fobjs[0].base, self.module_path),
                            fobjs[0].sourceline),
@@ -670,11 +679,12 @@ def _check_missing_newline_extrafiles(self):
         return True
 
     def _get_manifest_referenced_files(self):
-        referenced_files = []
+        referenced_files = {}
         data_keys = ['data', 'demo', 'demo_xml', 'init_xml', 'test',
                      'update_xml']
-        for key in data_keys:
-            referenced_files.extend(self.manifest_dict.get(key) or [])
+        for data_type in data_keys:
+            for fname in self.manifest_dict.get(data_type) or []:
+                referenced_files[fname] = data_type
         return referenced_files
 
     def _get_module_files(self):

pylint_odoo/misc.py

@@ -207,7 +207,8 @@ def filter_files_ext(self, fext, relpath=True, skip_examples=True):
                     fnames.remove(fname)
                     break
         if not relpath:
-            fnames = [
+            # Unused section is not delete it for compatibility
+            fnames = [  # pragma: no cover
                 os.path.join(self.module_path, fname)
                 for fname in fnames]
         return fnames

pylint_odoo/test/main.py

@@ -23,7 +23,7 @@
     'duplicate-id-csv': 2,
     'duplicate-xml-fields': 6,
     'duplicate-xml-record-id': 2,
-    'file-not-used': 8,
+    'file-not-used': 6,
     'incoherent-interpreter-exec-perm': 3,
     'invalid-commit': 4,
     'javascript-lint': 2,
@@ -84,6 +84,7 @@ def setUp(self):
         ]
         self.profile = Profile()
         self.sys_path_origin = list(sys.path)
+        self.maxDiff = None
 
     def tearDown(self):
         sys.path = list(self.sys_path_origin)
@@ -118,8 +119,7 @@ def test_20_expected_errors(self):
         pylint_res = self.run_pylint(self.paths_modules)
         # Expected vs found errors
         real_errors = pylint_res.linter.stats['by_msg']
-        self.assertEqual(sorted(real_errors.items()),
-                         sorted(EXPECTED_ERRORS.items()))
+        self.assertEqual(EXPECTED_ERRORS, real_errors)
         # All odoolint name errors vs found
         msgs_found = pylint_res.linter.stats['by_msg'].keys()
         plugin_msgs = misc.get_plugin_msgs(pylint_res)
@@ -139,8 +139,7 @@ def test_30_disabling_errors(self):
         real_errors = pylint_res.linter.stats['by_msg']
         global EXPECTED_ERRORS
         EXPECTED_ERRORS.pop('dangerous-filter-wo-user')
-        self.assertEqual(sorted(real_errors.items()),
-                         sorted(EXPECTED_ERRORS.items()))
+        self.assertEqual(EXPECTED_ERRORS, real_errors)
         sum_fails_found = misc.get_sum_fails(pylint_res.linter.stats)
         sum_fails_expected = sum(EXPECTED_ERRORS.values())
         self.assertEqual(sum_fails_found, sum_fails_expected)

pylint_odoo/test_repo/broken_module/__openerp__.py

@@ -6,7 +6,11 @@
     'description': 'Should be a README.rst file',
     'version': '1.0',
     'depends': ['base'],
-    'data': ['model_view.xml', 'model_view2.xml'],
+    'data': [
+        'model_view.xml', 'model_view2.xml', 'model_view_odoo.xml',
+        'model_view_odoo2.xml',
+    ],
+    'demo': ['demo/duplicated_id_demo.xml'],
     'test': ['test.yml'],
     'installable': True,
     'name': 'Duplicated value',

pylint_odoo/test_repo/broken_module/demo/duplicated_id_demo.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8"?>
+<odoo>
+    <data>
+        <!-- duplicate record id but from demo -->
+        <record id="view_model_form10" model="ir.ui.view">
+            <field name="name">view.model.form</field>
+            <field name="model">test.model</field>
+            <field name="arch" type="xml">
+                <tree string="Test model">
+                    <field name="name"></field>
+                </tree>
+            </field>
+        </record>
+
+    </data>
+</odoo>