From cde2c3ca7af2bd31630b3059dabd822f1b58d47e Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Thu, 15 Dec 2016 10:43:51 -0600 Subject: [PATCH] [REF] duplicate-xml-record-id: Get xml section from manifest to skip different origin (#89) Fix OCA/pylint-odoo#83 --- .coveragerc | 1 + pylint_odoo/checkers/modules_odoo.py | 30 ++++++++++++------- pylint_odoo/misc.py | 3 +- pylint_odoo/test/main.py | 9 +++--- .../test_repo/broken_module/__openerp__.py | 6 +++- .../broken_module/demo/duplicated_id_demo.xml | 16 ++++++++++ 6 files changed, 48 insertions(+), 17 deletions(-) create mode 100644 pylint_odoo/test_repo/broken_module/demo/duplicated_id_demo.xml diff --git a/.coveragerc b/.coveragerc index 02e1caba..11233881 100644 --- a/.coveragerc +++ b/.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 diff --git a/pylint_odoo/checkers/modules_odoo.py b/pylint_odoo/checkers/modules_odoo.py index 962ee490..c834caac 100644 --- a/pylint_odoo/checkers/modules_odoo.py +++ b/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): diff --git a/pylint_odoo/misc.py b/pylint_odoo/misc.py index f8ce5593..611d4c06 100644 --- a/pylint_odoo/misc.py +++ b/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 diff --git a/pylint_odoo/test/main.py b/pylint_odoo/test/main.py index acfa9dfe..82e04428 100644 --- a/pylint_odoo/test/main.py +++ b/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) diff --git a/pylint_odoo/test_repo/broken_module/__openerp__.py b/pylint_odoo/test_repo/broken_module/__openerp__.py index f8807d64..f7049bda 100644 --- a/pylint_odoo/test_repo/broken_module/__openerp__.py +++ b/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', diff --git a/pylint_odoo/test_repo/broken_module/demo/duplicated_id_demo.xml b/pylint_odoo/test_repo/broken_module/demo/duplicated_id_demo.xml new file mode 100644 index 00000000..60d7487e --- /dev/null +++ b/pylint_odoo/test_repo/broken_module/demo/duplicated_id_demo.xml @@ -0,0 +1,16 @@ + + + + + + view.model.form + test.model + + + + + + + + +