Re: [Freeipa-devel] [PATCH 0407] WIP: make-lint migration to config file and pylint plugin due pylint 1.5.2

2016-01-20 Thread Jan Cholasta

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

2016-01-19 Thread Christian Heimes
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

2016-01-19 Thread Martin Basti



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