Re: [Freeipa-devel] [PATCH 0407] WIP: make-lint migration to config file and pylint plugin due pylint 1.5.2
Hi, On 19.1.2016 13:43, Martin Basti wrote: New pylint version will broke our custom make-lint script again, attached patch migrates make-lint to: * config file * pylint plugin which are supported by pylint and should not have regular compatibility issues to test new approach run ./make-lint2 Advantages: * compatibility with pylint * works on both pylint-1.4.3-3.fc23.noarch and pylint-1.5.2-1.fc24.noarch * pylint plugin works in different way than the previous custom checker. Missing ("dynamic") attributes are added to abstract syntax tree instead of ignoring them and all their sub-members. This makes check better, pylint can detect more typos in tests configurations, api, env, etc.. Disadvantages: * any new attribute in api, test config, etc.. must be added to definition of missing members (pylint plugin) - this should not happen too often 1) Please "mv pylint_plugins/fix_ipa_members.py pylint_plugins.py" and "rm -rf pylint_plugins/", no need for this redundant directory structure. 2) Rename pylintrc to freeipa.pylintrc so you have to always specify it explicitly with --rcfile. 3) Use the load-plugins directive in freeipa.pylintrc to load the plugins rather than --load-plugins. 4) Instead of running pylint twice, run it only once with both normal and Python 3 checks enabled: [MESSAGE CONTROL] enable=all,python3 disable=...,no-absolute-import Q&TODO: * make-lint: should it be just bash script or rather python script? IMO neither, it should be a make target (make lint). * add dynamic detection of python files to be checked You can use "find . -type f -executable ! -path \*/.\* ! -name \*.py\* -exec grep -lsm1 '^#!.*\bpython' \{\} \;". * should I keep the current options from original make-lint? No, but allow pylint options to be overridable (make lint PYLINTFLAGS="--disable=python3") * several false positive errors I haven't been able to fix in plugin yet, in worst case they can be locally disabled: Disable them locally. Honza -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0407] WIP: make-lint migration to config file and pylint plugin due pylint 1.5.2
On 19.01.2016 16:34, Christian Heimes wrote: On 2016-01-19 13:43, Martin Basti wrote: + +def fake_class(name_or_class_obj, members=[]): Please use a non-mutable argument here. members=() will do the job just fine. Fixed. +if isinstance(name_or_class_obj, scoped_nodes.Class): +cl = name_or_class_obj +else: +cl = scoped_nodes.Class(name_or_class_obj, None) + +for m in members: +if isinstance(m, str): +if m in cl.locals: +_warning_already_exists(cl, m) +else: +cl.locals[m] = [scoped_nodes.Class(m, None)] +elif isinstance(m, dict): +for key, val in m.items(): +assert isinstance(key, str), "key must be string" +if key in cl.locals: +_warning_already_exists(cl, key) +fake_class(cl.locals[key], val) +else: +cl.locals[key] = [fake_class(key, val)] +else: +# here can be used any astroid type +if m.name in cl.locals: +_warning_already_exists(cl, m.name) +else: +cl.locals[m.name] = [copy.copy(m)] +return cl ... +ipa_class_members = { +# Python standard library & 3rd party classes +'socket._socketobject': ['sendall'], +# !!!'datetime.tzinfo': ['houroffset', 'minoffset', 'utcoffset', 'dst'], +# !!!'nss.nss.subject_public_key_info': ['public_key'], + +# IPA classes +'ipalib.base.NameSpace': [ +'add', +'mod', +'del', +'show', +'find' +], +'ipalib.cli.Collector': ['__options'], +'ipalib.config.Env': [ +{'__d': ['get']}, +{'__done': ['add']}, +'xmlrpc_uri', +'validate_api', +'startup_traceback', +'verbose' +] + LOGGING_ATTRS, The rules for __options, __d and __done may lead to false detection. Class member and attribute names with leading double underscore are mangled, so Collector.__options is turned into __Collector_options. self.__options works because it's also mangled. But from other classes, the attribute must be referred to as __Collector_options. Christian This doesn't work for pylint 'ipalib.config.Env': [ {'_Env__d': ['get']}, {'_Env__done': ['add']}, 'xmlrpc_uri', 'validate_api', 'startup_traceback', 'verbose' ] + LOGGING_ATTRS, * Module ipalib.config ipalib/config.py:240: [E1101(no-member), Env.__setitem__] Instance of 'Env' has no '__d' member) ipalib/config.py:242: [E1101(no-member), Env.__setitem__] Instance of 'Env' has no '__d' member) ipalib/config.py:263: [E1101(no-member), Env.__setitem__] Instance of 'Env' has no '__d' member) ipalib/config.py:269: [E1101(no-member), Env.__getitem__] Instance of 'Env' has no '__d' member) ipalib/config.py:292: [E1101(no-member), Env.__contains__] Instance of 'Env' has no '__d' member) ipalib/config.py:298: [E1101(no-member), Env.__len__] Instance of 'Env' has no '__d' member) ipalib/config.py:304: [E1101(no-member), Env.__iter__] Instance of 'Env' has no '__d' member) ipalib/config.py:408: [E1101(no-member), Env.__doing] Instance of 'Env' has no '__done' member) ipalib/config.py:412: [E1101(no-member), Env.__doing] Instance of 'Env' has no '__done' member) ipalib/config.py:415: [E1101(no-member), Env.__do_if_not_done] Instance of 'Env' has no '__done' member) ipalib/config.py:419: [E1101(no-member), Env._isdone] Instance of 'Env' has no '__done' member) ipalib/config.py:534: [E1101(no-member), Env._finalize_core] Instance of 'Env' has no '__d' member) -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0407] WIP: make-lint migration to config file and pylint plugin due pylint 1.5.2
On 2016-01-19 13:43, Martin Basti wrote: > + > +def fake_class(name_or_class_obj, members=[]): Please use a non-mutable argument here. members=() will do the job just fine. > +if isinstance(name_or_class_obj, scoped_nodes.Class): > +cl = name_or_class_obj > +else: > +cl = scoped_nodes.Class(name_or_class_obj, None) > + > +for m in members: > +if isinstance(m, str): > +if m in cl.locals: > +_warning_already_exists(cl, m) > +else: > +cl.locals[m] = [scoped_nodes.Class(m, None)] > +elif isinstance(m, dict): > +for key, val in m.items(): > +assert isinstance(key, str), "key must be string" > +if key in cl.locals: > +_warning_already_exists(cl, key) > +fake_class(cl.locals[key], val) > +else: > +cl.locals[key] = [fake_class(key, val)] > +else: > +# here can be used any astroid type > +if m.name in cl.locals: > +_warning_already_exists(cl, m.name) > +else: > +cl.locals[m.name] = [copy.copy(m)] > +return cl ... > +ipa_class_members = { > +# Python standard library & 3rd party classes > +'socket._socketobject': ['sendall'], > +# !!!'datetime.tzinfo': ['houroffset', 'minoffset', 'utcoffset', 'dst'], > +# !!!'nss.nss.subject_public_key_info': ['public_key'], > + > +# IPA classes > +'ipalib.base.NameSpace': [ > +'add', > +'mod', > +'del', > +'show', > +'find' > +], > +'ipalib.cli.Collector': ['__options'], > +'ipalib.config.Env': [ > +{'__d': ['get']}, > +{'__done': ['add']}, > +'xmlrpc_uri', > +'validate_api', > +'startup_traceback', > +'verbose' > +] + LOGGING_ATTRS, The rules for __options, __d and __done may lead to false detection. Class member and attribute names with leading double underscore are mangled, so Collector.__options is turned into __Collector_options. self.__options works because it's also mangled. But from other classes, the attribute must be referred to as __Collector_options. Christian signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH 0407] WIP: make-lint migration to config file and pylint plugin due pylint 1.5.2
New pylint version will broke our custom make-lint script again, attached patch migrates make-lint to: * config file * pylint plugin which are supported by pylint and should not have regular compatibility issues to test new approach run ./make-lint2 Advantages: * compatibility with pylint * works on both pylint-1.4.3-3.fc23.noarch and pylint-1.5.2-1.fc24.noarch * pylint plugin works in different way than the previous custom checker. Missing ("dynamic") attributes are added to abstract syntax tree instead of ignoring them and all their sub-members. This makes check better, pylint can detect more typos in tests configurations, api, env, etc.. Disadvantages: * any new attribute in api, test config, etc.. must be added to definition of missing members (pylint plugin) - this should not happen too often Q&TODO: * make-lint: should it be just bash script or rather python script? * add dynamic detection of python files to be checked * should I keep the current options from original make-lint? * several false positive errors I haven't been able to fix in plugin yet, in worst case they can be locally disabled: * Module ipalib.plugins.vault ipalib/plugins/vault.py:1654: [E1101(no-member), vault_archive.forward] Class 'subject_public_key_info' has no 'public_key' member) ipalib/plugins/vault.py:1858: [E1101(no-member), vault_retrieve.forward] Class 'subject_public_key_info' has no 'public_key' member) * Module ipatests.test_ipapython.test_ipautil ipatests/test_ipapython/test_ipautil.py:390: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'houroffset' member) ipatests/test_ipapython/test_ipautil.py:391: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'minoffset' member) ipatests/test_ipapython/test_ipautil.py:392: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'utcoffset' member) ipatests/test_ipapython/test_ipautil.py:392: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'dst' member) ipatests/test_ipapython/test_ipautil.py:398: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'houroffset' member) ipatests/test_ipapython/test_ipautil.py:399: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'minoffset' member) ipatests/test_ipapython/test_ipautil.py:400: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'utcoffset' member) ipatests/test_ipapython/test_ipautil.py:400: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'dst' member) ipatests/test_ipapython/test_ipautil.py:406: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'houroffset' member) ipatests/test_ipapython/test_ipautil.py:407: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'minoffset' member) ipatests/test_ipapython/test_ipautil.py:410: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'utcoffset' member) ipatests/test_ipapython/test_ipautil.py:410: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'dst' member) ipatests/test_ipapython/test_ipautil.py:416: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'houroffset' member) ipatests/test_ipapython/test_ipautil.py:417: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'minoffset' member) ipatests/test_ipapython/test_ipautil.py:418: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'utcoffset' member) ipatests/test_ipapython/test_ipautil.py:418: [E1101(no-member), TestTimeParser.test_time_zones] Class 'tzinfo' has no 'dst' member) From e322273a7e5dd6fa5d8f5fb799a09f0130aab0f2 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Fri, 15 Jan 2016 16:58:38 +0100 Subject: [PATCH] WIP: make-lint: use config file and plugin for pylint Our custom implementation of pylint checker is often broken by incompatible change on pylint side. Using supported solutions (config file, pylint plugins) should avoid this issue. The plugin adds missing (dynamic) member to classes in abstract syntax tree generated for pylint, instead of just ignoring missing members and all sub-members. This should improve pylint detection of typos and missing members in api. env and test config. https://fedorahosted.org/freeipa/ticket/5615 --- make-lint2| 70 +++ pylint_plugins/__init__.py| 0 pylint_plugins/fix_ipa_members.py | 211 pylintrc | 404 ++ 4 files changed, 685 insertions(+) create mode 100755 make-lint2 create mode 100644 pylint_plugins/__init__.py create mode 100644 pylint_plugins/fix_ipa_members.py create mode 100644 pylintrc diff --git a/make-lint2 b/make-lint2 new file mode 100755 index ..7c1c6453c189618fcf7e7e19ab91aebd580b4dbd --- /dev/null +++ b/make