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: * 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 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
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