Hi all,
I'm sending a couple of patches that we wrote internally for our custom
extended pylint version. Some of the patches expose more configuration
options, other ones fix exotic crashes. I'll explain all of them separately.
The patches were written for 0.23, but I've tested all of them against 0.24,
and updated them so they apply cleanly.
* classmethod_first_arg_name
Exposes the valid name of the first argument to a classmethod as a
configuration option.
* import_assign_crash
Fixes an issue with conditionally assigning a nonexisting module to
intermediate modules in a dotted name.
This is only a cosmetic fix to avoid the crash, the actual fix probably
needs a lot of work on the inference module to support conditional
assignment and inspection of several places of assignment, rather than just
the latest one. Probably not worth it, either.
* infer__bases__
Fixes a bug with incorrectly inferring the base classes This led to a crash
in code like
class X(some bases...)
def __init__(self, ...):
for b in self.__class__.__bases__:
# do sth with b
* newstyle_type_fix
Fixes misleading wording in warnings E1001 and E1002.
* nocatch_exception_types
Exposes the list of exceptions that should not be caught as a configuration
option.
* no_main_import
Fixes a problem when a module imports __main__. While this is benign
normally (pylint just imports itself, and exposes its __main__ module to
astng, which is wrong, but not fatal), this led to a crash in our deployed,
standalone pylint version.
The fix is now that a completely empty module is returned, since neither the
linter nor the module in question can make any assumptions about __main__.
* no_special_attr_scop_lookup_crash
Fixes a crash that occurs when lookup up a special attribute in class scope
(i.e. during class construction). This is not a total fix, which would
introduce a mechanism similar to special attribute lookup in modules. Also,
in order to get correct inference in this case, the scope lookup rules would
need to be changed to return the initial assignment (which does not exist in
code per se) as well as any later modifications.
* str_getitem_infer_fix
This one is cute. It's a fix for a crash that occurs on code like
"any string literal"[0].upper()
due to the fact that the extracted string literal is not properly wrapped,
but returned as a naked string object.
The fixes for the crashes were all found while running gpylint over a
significant subset of the Python code at Google, the new configuration
options are needed to make gpylint adhere to our internal style guide, but
might be useful to others as well.
// Torsten
--
Site Reliability Engineer ∘ Google ✚ ∘ ⬕
--- a/pylint/checkers/classes.py
+++ b/pylint/checkers/classes.py
@@ -63,7 +63,7 @@ MSGS = {
'Used when a method has an attribute different the "self" as\
first argument. This is considered as an error since this is\
a so common convention that you shouldn\'t break it!'),
- 'C0202': ('Class method should have "cls" as first argument', # E0212
+ 'C0202': ('Class method should have %s as first argument', # E0212
'Used when a class method has an attribute different than "cls"\
as first argument, to easily differentiate them from regular \
instance methods.'),
@@ -156,6 +156,13 @@ in Zope\'s Interface base class.'}
'help' : 'List of method names used to declare (i.e. assign) \
instance attributes.'}
),
+ ('valid-classmethod-first-arg',
+ {'default' : ('cls',),
+ 'type' : 'csv',
+ 'metavar' : '<argument names>',
+ 'help' : 'List of valid names for the first argument in \
+a class method.'}
+ ),
)
@@ -385,8 +392,16 @@ instance attributes.'}
self.add_message('C0203', node=node)
# class method
elif node.type == 'classmethod':
- if first != 'cls':
- self.add_message('C0202', node=node)
+ if first not in self.config.valid_classmethod_first_arg:
+ if len(self.config.valid_classmethod_first_arg) == 1:
+ valid = repr(self.config.valid_classmethod_first_arg[0])
+ else:
+ valid = ', '.join(
+ repr(v)
+ for v in self.config.valid_classmethod_first_arg[:-1])
+ valid = '%s or %r' % (
+ valid, self.config.valid_classmethod_first_arg[-1])
+ self.add_message('C0202', args=valid, node=node)
# regular method without self as argument
elif first != 'self':
self.add_message('E0213', node=node)
--- a/pylint/test/messages/func_e0203.txt
+++ b/pylint/test/messages/func_e0203.txt
@@ -1,2 +1 @@
-C: 12:Abcd.abcd: Class method should have "cls" as first argument
-
+C: 12:Abcd.abcd: Class method should have 'cls' as first argument
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -483,6 +483,8 @@ builtins. Remember that you should avoid to define new builtins when possible.'
break
try:
module = module.getattr(name)[0].infer().next()
+ if module is astng.YES:
+ return None
except astng.NotFoundError:
self.add_message('E0611', args=(name, module.name), node=node)
return None
--- /dev/null
+++ b/pylint/test/regrtest_data/import_assign.py
@@ -0,0 +1,5 @@
+import shmixml.dom.minidom
+import xml.dom.minidom
+
+if 'dom' not in xml.__dict__:
+ xml.dom = shmixml.dom
--- a/pylint/test/test_regr.py
+++ b/pylint/test/test_regr.py
@@ -120,6 +120,9 @@ class NonRegrTC(TestCase):
got = linter.reporter.finalize().strip()
self.failUnlessEqual(got, '')
+ def test_import_assign_crash(self):
+ linter.check(join(REGR_DATA, 'import_assign.py'))
+
def test_module_global_crash(self):
linter.check(join(REGR_DATA, 'module_global.py'))
got = linter.reporter.finalize().strip()
--- a/logilab-astng/scoped_nodes.py
+++ b/logilab-astng/scoped_nodes.py
@@ -862,11 +862,15 @@ class Class(Statement, LocalsDictNodeNG, FilterStmtsMixin):
# you can just do [cf(tuple())] + values without breaking any test
# this is ticket http://www.logilab.org/ticket/52785
if name == '__bases__':
- return [cf(tuple(self.ancestors(recurs=False, context=context)))] + values
+ t = cf(tuple())
+ t.elts = tuple(self.ancestors(recurs=False, context=context))
+ return [t] + values
# XXX need proper meta class handling + MRO implementation
if name == '__mro__' and self.newstyle:
# XXX mro is read-only but that's not our job to detect that
- return [cf(tuple(self.ancestors(recurs=True, context=context)))] + values
+ t = cf(tuple())
+ t.elts = tuple(self.ancestors(recurs=True, context=context))
+ return [t] + values
return std_special_attributes(self, name)
# don't modify the list in self.locals!
values = list(values)
--- a/logilab-astng/test/unittest_inference.py
+++ b/logilab-astng/test/unittest_inference.py
@@ -1126,5 +1126,18 @@ n = NewTest()
infered = list(n.igetattr('arg'))
self.assertEqual(len(infered), 1, infered)
+ def test__bases__(self):
+ code = '''
+class SomeClass(object):
+ def __init__(self):
+ pass
+ '''
+ astng = builder.string_build(code, __name__, __file__)
+ inferred_class = astng['SomeClass'].igetattr('__class__').next()
+ inferred_bases = inferred_class.igetattr('__bases__').next()
+ self.assertEqual(1, len(inferred_bases.elts))
+ self.assertIsInstance(inferred_bases.elts[0], nodes.Class)
+ self.assertEqual('object', inferred_bases.elts[0].name)
+
if __name__ == '__main__':
unittest_main()
--- a/pylint/checkers/newstyle.py
+++ b/pylint/checkers/newstyle.py
@@ -23,10 +23,10 @@ from pylint.checkers import BaseChecker
from pylint.checkers.utils import check_messages
MSGS = {
- 'E1001': ('Use __slots__ on an old style class',
- 'Used when an old style class use the __slots__ attribute.'),
- 'E1002': ('Use super on an old style class',
- 'Used when an old style class use the super builtin.'),
+ 'E1001': ('Use of __slots__ on an old style class',
+ 'Used when an old style class uses the __slots__ attribute.'),
+ 'E1002': ('Use of super on an old style class',
+ 'Used when an old style class uses the super builtin.'),
'E1003': ('Bad first argument %r given to super class',
'Used when another argument than the current class is given as \
first argument of the super builtin.'),
--- a/pylint/test/messages/func_newstyle___slots__.txt
+++ b/pylint/test/messages/func_newstyle___slots__.txt
@@ -1 +1 @@
-E: 10:HaNonNonNon: Use __slots__ on an old style class
+E: 10:HaNonNonNon: Use of __slots__ on an old style class
--- a/pylint/test/messages/func_newstyle_super.txt
+++ b/pylint/test/messages/func_newstyle_super.txt
@@ -1,4 +1,4 @@
-E: 7:Aaaa.hop: Use super on an old style class
-E: 11:Aaaa.__init__: Use super on an old style class
+E: 7:Aaaa.hop: Use of super on an old style class
+E: 11:Aaaa.__init__: Use of super on an old style class
E: 20:NewAaaa.__init__: Bad first argument 'object' given to super class
--- a/pylint/checkers/exceptions.py
+++ b/pylint/checkers/exceptions.py
@@ -26,6 +26,8 @@ from pylint.checkers.utils import is_empty, is_raising
from pylint.interfaces import IASTNGChecker
+OVERGENERAL_EXCEPTIONS = ('Exception',)
+
MSGS = {
'E0701': (
'Bad except clauses order (%s)',
@@ -47,8 +49,9 @@ MSGS = {
'W0702': ('No exception type(s) specified',
'Used when an except clause doesn\'t specify exceptions type to \
catch.'),
- 'W0703': ('Catch "Exception"',
- 'Used when an except catches Exception instances.'),
+ 'W0703': ('Catching too general exception %s',
+ 'Used when an except catches a too general exception, \
+ possibly burying unrelated errors.'),
'W0704': ('Except doesn\'t do anything',
'Used when an except clause does nothing but "pass" and there is\
no "else" clause.'),
@@ -74,7 +77,14 @@ class ExceptionsChecker(BaseChecker):
name = 'exceptions'
msgs = MSGS
priority = -4
- options = ()
+ options = (('overgeneral-exceptions',
+ {'default' : OVERGENERAL_EXCEPTIONS,
+ 'type' :'csv', 'metavar' : '<comma-separated class names>',
+ 'help' : 'Exceptions that will emit a warning '
+ 'when being caught. Defaults to "%s"' % (
+ ', '.join(OVERGENERAL_EXCEPTIONS),)}
+ ),
+ )
def visit_raise(self, node):
"""visit raise possibly inferring value"""
@@ -163,10 +173,10 @@ class ExceptionsChecker(BaseChecker):
msg = '%s is an ancestor class of %s' % (
previous_exc.name, exc.name)
self.add_message('E0701', node=handler.type, args=msg)
- if (exc.name == 'Exception'
+ if (exc.name in self.config.overgeneral_exceptions
and exc.root().name == EXCEPTIONS_MODULE
and nb_handlers == 1 and not is_raising(handler.body)):
- self.add_message('W0703', node=handler.type)
+ self.add_message('W0703', args=exc.name, node=handler.type)
exceptions_classes += excs
@@ -185,4 +195,3 @@ def inherit_from_std_ex(node):
def register(linter):
"""required method to auto register this checker"""
linter.register_checker(ExceptionsChecker(linter))
-
--- a/pylint/test/messages/func_w0703.txt
+++ b/pylint/test/messages/func_w0703.txt
@@ -1 +1 @@
-W: 8: Catch "Exception"
+W: 8: Catching too general exception Exception
--- a/logilab-astng/test/unittest_manager.py
+++ b/logilab-astng/test/unittest_manager.py
@@ -89,6 +89,11 @@ class ASTNGManagerTC(TestCase):
'data.nonregr', 'data.notall']
self.assertListEqual(sorted(k for k in obj.keys()), expected)
+ def test_do_not_expose_main(self):
+ obj = self.manager.astng_from_module_name('__main__')
+ self.assertEqual(obj.name, '__main__')
+ self.assertEqual(obj.items(), [])
+
class BorgASTNGManagerTC(TestCase):
--- a/logilab-astng/manager.py
+++ b/logilab-astng/manager.py
@@ -127,6 +127,9 @@
"""given a module name, return the astng object"""
if modname in self.astng_cache:
return self.astng_cache[modname]
+ if modname == '__main__':
+ from logilab.astng.builder import ASTNGBuilder
+ return ASTNGBuilder(self).string_build('', modname)
old_cwd = os.getcwd()
if context_file:
os.chdir(dirname(context_file))
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -332,7 +332,8 @@ builtins. Remember that you should avoid to define new builtins when possible.'
astmts = [stmt for stmt in node.lookup(name)[1]
if hasattr(stmt, 'ass_type')]
# filter variables according their respective scope
- if not astmts or astmts[0].statement().parent_of(node):
+ if not astmts or (astmts[0].is_statement or astmts[0].parent) \
+ and astmts[0].statement().parent_of(node):
_astmts = []
else:
_astmts = astmts[:1]
--- /dev/null
+++ b/pylint/test/regrtest_data/special_attr_scope_lookup_crash.py
@@ -0,0 +1,3 @@
+class Klass(object):
+ """A"""
+ __doc__ += "B"
--- a/pylint/test/test_regr.py
+++ b/pylint/test/test_regr.py
@@ -120,6 +120,9 @@ class NonRegrTC(TestCase):
got = linter.reporter.finalize().strip()
self.failUnlessEqual(got, '')
+ def test_special_attr_scope_lookup_crash(self):
+ linter.check(join(REGR_DATA, 'special_attr_scope_lookup_crash.py'))
+
def test_module_global_crash(self):
linter.check(join(REGR_DATA, 'module_global.py'))
got = linter.reporter.finalize().strip()
--- a/logilab-astng/node_classes.py
+++ b/logilab-astng/node_classes.py
@@ -435,7 +435,7 @@ class Const(NodeNG, Instance):
def getitem(self, index, context=None):
if isinstance(self.value, basestring):
- return self.value[index]
+ return Const(self.value[index])
raise TypeError('%r (value=%s)' % (self, self.value))
def has_dynamic_getattr(self):
--- a/logilab-astng/test/unittest_inference.py
+++ b/logilab-astng/test/unittest_inference.py
@@ -397,6 +397,7 @@ l = [1]
t = (2,)
d = {}
s = ''
+s2 = '_'
'''
astng = builder.string_build(code, __name__, __file__)
n = astng['l']
@@ -427,6 +428,9 @@ s = ''
self.assertIsInstance(infered, Instance)
self.failUnlessEqual(infered.name, 'str')
self.failUnless('lower' in infered._proxied.locals)
+ n = astng['s2']
+ infered = n.infer().next()
+ self.failUnlessEqual(infered.getitem(0).value, '_')
def test_unicode_type(self):
if sys.version_info >= (3, 0):
_______________________________________________
Python-Projects mailing list
[email protected]
http://lists.logilab.org/mailman/listinfo/python-projects
