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