Pull requests

#65 Declined
Repository
dirkbaechle dirkbaechle
Branch
default
Repository
scons scons
Branch
default

Getting the source compliant to pylint

Author
  1. dirkbaechle
Reviewers
Description

Hi,

I fiddled around with SCons and pylint a bit. With this proposed patch it is possible to run pylint on the whole current codebase. I'm not really after counting undocumented functions or unused arguments, but the further possibilities this would open:

  • automatic detection of cyclic imports (and boy do we have some!)
  • automatic creation of UML diagrams for classes via pyreverse (useful for design discussions and understanding/documenting the status quo)

I hope the rewrites of the Builder calls are okay like that, please give your comments everybody.

Best regards,

Dirk

  • Learn about pull requests

Comments (7)

    1. dirkbaechle author

      It's not that pylint actually wants the call notation, it just doesn't seem to like what we have now. ;) I'm not sure what causes pylint headaches with this, for some reason it doesn't see that the resulting builder is really callable.

      The proposed solution is what I could come up with, if you can think of a better, and nicer looking, approach let's hear about it.

  1. dirkbaechle author

    I use pylint 0.25.0 under Ubuntu 12.0.4.1 LTS. In the top-level folder of the repository I do a:

    export PYTHONPATH=/full_path_to_scons/src/engine

    first. Then, on calling

    pylint src/engine/SCons

    I get the following stacktrace:

    W:712,20:BuilderTestCase.test_single_source: Used builtin function 'map'
    C:713,12:BuilderTestCase.test_single_source: Invalid name "t" (should match [a-z_][a-z0-9_]{2,30}$)
    C:713,23:BuilderTestCase.test_single_source: More than one statement on a single line
    Traceback (most recent call last):
      File "/usr/bin/pylint", line 4, in <module>
        lint.Run(sys.argv[1:])
      File "/usr/lib/python2.7/dist-packages/pylint/lint.py", line 874, in __init__
        linter.check(args)
      File "/usr/lib/python2.7/dist-packages/pylint/lint.py", line 496, in check
        self.check_astng_module(astng, walker, rawcheckers)
      File "/usr/lib/python2.7/dist-packages/pylint/lint.py", line 568, in check_astng_module
        walker.walk(astng)
      File "/usr/lib/python2.7/dist-packages/pylint/utils.py", line 528, in walk
        self.walk(child)
      File "/usr/lib/python2.7/dist-packages/pylint/utils.py", line 528, in walk
        self.walk(child)
      File "/usr/lib/python2.7/dist-packages/pylint/utils.py", line 528, in walk
        self.walk(child)
      File "/usr/lib/python2.7/dist-packages/pylint/utils.py", line 528, in walk
        self.walk(child)
      File "/usr/lib/python2.7/dist-packages/pylint/utils.py", line 528, in walk
        self.walk(child)
      File "/usr/lib/python2.7/dist-packages/pylint/utils.py", line 525, in walk
        cb(astng)
      File "/usr/lib/python2.7/dist-packages/pylint/checkers/typecheck.py", line 248, in visit_callfunc
        called = safe_infer(node.func)
      File "/usr/lib/python2.7/dist-packages/pylint/checkers/utils.py", line 35, in safe_infer
        value = inferit.next()
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 302, in wrapped
        for res in _func(node, context, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 326, in wrapper
        for node in func(*args, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/inference.py", line 211, in infer_getattr
        for owner in self.expr.infer(context):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 302, in wrapped
        for res in _func(node, context, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 115, in _infer_stmts
        for infered in stmt.infer(context):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 302, in wrapped
        for res in _func(node, context, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/inference.py", line 351, in infer_ass
        stmts = list(self.assigned_stmts(context=context))
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 326, in wrapper
        for node in func(*args, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/protocols.py", line 192, in for_assigned_stmts
        for lst in self.iter.infer(context):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 302, in wrapped
        for res in _func(node, context, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 115, in _infer_stmts
        for infered in stmt.infer(context):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 302, in wrapped
        for res in _func(node, context, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 115, in _infer_stmts
        for infered in stmt.infer(context):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 302, in wrapped
        for res in _func(node, context, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 326, in wrapper
        for node in func(*args, **kwargs):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/inference.py", line 165, in infer_callfunc
        for infered in callee.infer_call_result(self, callcontext):
      File "/usr/lib/python2.7/dist-packages/logilab/astng/bases.py", line 201, in infer_call_result
        for res in node.infer_call_result(caller, context):
    TypeError: '_Yes' object is not iterable
    
  2. dirkbaechle author

    I think for now it's the best if someone simply rejects the patch (it's starting to bug me during development ;) ).

    I'll take the diff and put it in my scons_experimental fork, such that it doesn't get lost if we should ever come to using pylint.

    Thanks a lot in advance.

  3. Gary Oberbrunner

    OK, I'll reject the patch for now, on the grounds that it wouldn't make the SCons code itself any cleaner (the reverse in fact), even though it would make it pass pylint.