diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 39794a818..b7a352e28 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -19,11 +19,16 @@ def CommonChecks(input_api, output_api, tests_to_black_list): r'^site-packages-py[0-9]\.[0-9][\/\\].+', r'^svn_bin[\/\\].+', r'^testing_support[\/\\]_rietveld[\/\\].+'] + disabled_warnings = [ + 'R0401', # Cyclic import + 'W0613', # Unused argument + ] results.extend(input_api.canned_checks.RunPylint( input_api, output_api, white_list=[r'.*\.py$'], - black_list=black_list)) + black_list=black_list, + disabled_warnings=disabled_warnings)) # TODO(maruel): Make sure at least one file is modified first. # TODO(maruel): If only tests are modified, only run them. diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index be84cd726..e7b503e0a 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -4,6 +4,9 @@ """Generic presubmit checks that can be reused by other presubmit checks.""" +import os as _os +_HERE = _os.path.dirname(_os.path.abspath(__file__)) + ### Description checks @@ -615,7 +618,8 @@ def _FetchAllFiles(input_api, white_list, black_list): return files -def RunPylint(input_api, output_api, white_list=None, black_list=None): +def RunPylint(input_api, output_api, white_list=None, black_list=None, + disabled_warnings=None): """Run pylint on python files. The default white_list enforces looking only a *.py files. @@ -632,6 +636,10 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None): if not input_api.AffectedSourceFiles(src_filter): return [] + extra_args = ['--rcfile=%s' % input_api.os_path.join(_HERE, 'pylintrc')] + if disabled_warnings: + extra_args.extend(['-d', ','.join(disabled_warnings)]) + # On certain pylint/python version combination, running pylint throws a lot of # warning messages. import warnings @@ -661,7 +669,7 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None): def run_lint(files): try: - lint.Run(files) + lint.Run(files + extra_args) assert False except SystemExit, e: # pylint has the bad habit of calling sys.exit(), trap it here. diff --git a/pylintrc b/pylintrc index 296bab032..3c5734b53 100644 --- a/pylintrc +++ b/pylintrc @@ -10,8 +10,8 @@ # Profiled execution. profile=no -# Add to the black list. It should be a base name, not a -# path. You may set this option multiple times. +# Add files or directories to the blacklist. They should be base names, not +# paths. ignore=CVS # Pickle collected data for later comparisons. @@ -31,16 +31,15 @@ load-plugins= # Disable the message, report, category or checker with the given id(s). You # can either give multiple identifier separated by comma (,) or put this option -# multiple time. +# multiple time (only on the command line, not in the configuration file where +# it should appear only once). +# CHANGED: # C0103: Invalid name "" # C0111: Missing docstring # C0302: Too many lines in module (N) # I0010: Unable to consider inline option '' # I0011: Locally disabling WNNNN # -# It's a problem but not something we can fix right now. -# R0401: Cyclic import -# # R0801: Similar lines in N files # R0901: Too many ancestors (8/7) # R0902: Too many instance attributes (N/7) @@ -60,10 +59,9 @@ load-plugins= # W0404: 41: Reimport 'XX' (imported line NN) # W0511: TODO # W0603: Using the global statement -# W0613: Unused argument '' # W0703: Catch "Exception" # W1201: Specify string format arguments as logging function parameters -disable=C0103,C0111,C0302,I0010,I0011,R0401,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0402,W0404,W0511,W0603,W0613,W0703,W1201 +disable=C0103,C0111,C0302,I0010,I0011,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0402,W0404,W0511,W0603,W0703,W1201 [REPORTS] @@ -81,39 +79,33 @@ include-ids=yes files-output=no # Tells whether to display a full report or only the messages -# CHANGE: No report. +# CHANGED: reports=no # Python expression which should return a note less than 10 (10 is the highest # note). You have access to the variables errors warning, statement which # respectively contain the number of errors / warnings messages and the total # number of statements analyzed. This is used by the global evaluation report -# (R0004). +# (RP0004). evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) # Add a comment according to your evaluation note. This is used by the global -# evaluation report (R0004). +# evaluation report (RP0004). comment=no -[MISCELLANEOUS] - -# List of note tags to take in consideration, separated by a comma. -notes=FIXME,XXX,TODO - - -[FORMAT] +[VARIABLES] -# Maximum number of characters on a single line. -max-line-length=80 +# Tells whether we should check for unused import in __init__ files. +init-import=no -# Maximum number of lines in a module -max-module-lines=1000 +# A regular expression matching the beginning of the name of dummy variables +# (i.e. not used). +dummy-variables-rgx=_|dummy -# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 -# tab). -# CHANGE: Use " " instead. -indent-string=' ' +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= [TYPECHECK] @@ -124,17 +116,50 @@ ignore-mixin-members=yes # List of classes names for which member attributes should not be checked # (useful for classes with attributes dynamically set). -ignored-classes=SQLObject +ignored-classes=SQLObject,twisted.internet.reactor,hashlib # When zope mode is activated, add a predefined set of Zope acquired attributes # to generated-members. zope=no # List of members which are set dynamically and missed by pylint inference -# system, and so shouldn't trigger E0201 when accessed. +# system, and so shouldn't trigger E0201 when accessed. Python regular +# expressions are accepted. generated-members=REQUEST,acl_users,aq_parent +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +notes=FIXME,XXX,TODO + + +[SIMILARITIES] + +# Minimum lines number of a similarity. +min-similarity-lines=4 + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + + +[FORMAT] + +# Maximum number of characters on a single line. +max-line-length=80 + +# Maximum number of lines in a module +max-module-lines=1000 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +# CHANGED: +indent-string=' ' + + [BASIC] # Required attributes for module, separated by a comma @@ -182,41 +207,6 @@ bad-names=foo,bar,baz,toto,tutu,tata no-docstring-rgx=__.*__ -[VARIABLES] - -# Tells whether we should check for unused import in __init__ files. -init-import=no - -# A regular expression matching names used for dummy variables (i.e. not used). -dummy-variables-rgx=_|dummy - -# List of additional names supposed to be defined in builtins. Remember that -# you should avoid to define new builtins when possible. -additional-builtins= - - -[SIMILARITIES] - -# Minimum lines number of a similarity. -min-similarity-lines=4 - -# Ignore comments when computing similarities. -ignore-comments=yes - -# Ignore docstrings when computing similarities. -ignore-docstrings=yes - - -[CLASSES] - -# List of interface methods to ignore, separated by a comma. This is used for -# instance to not check methods defines in Zope's Interface base class. -ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by - -# List of method names used to declare (i.e. assign) instance attributes. -defining-attr-methods=__init__,__new__,setUp - - [DESIGN] # Maximum number of arguments for function / method @@ -251,19 +241,39 @@ min-public-methods=2 max-public-methods=20 +[CLASSES] + +# List of interface methods to ignore, separated by a comma. This is used for +# instance to not check methods defines in Zope's Interface base class. +ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__,__new__,setUp + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + + [IMPORTS] # Deprecated modules which should not be used, separated by a comma deprecated-modules=regsub,string,TERMIOS,Bastion,rexec # Create a graph of every (i.e. internal and external) dependencies in the -# given file (report R0402 must not be disabled) +# given file (report RP0402 must not be disabled) import-graph= -# Create a graph of external dependencies in the given file (report R0402 must +# Create a graph of external dependencies in the given file (report RP0402 must # not be disabled) ext-import-graph= -# Create a graph of internal dependencies in the given file (report R0402 must +# Create a graph of internal dependencies in the given file (report RP0402 must # not be disabled) int-import-graph= + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 29f9a9310..75d140ada 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -13,6 +13,9 @@ import StringIO import sys import time +# TODO(maruel): Include inside depot_tools. +from pylint import lint + sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from testing_support.super_mox import mox, SuperMoxTestBase @@ -1449,6 +1452,7 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api = self.mox.CreateMock(presubmit.InputApi) input_api.cStringIO = presubmit.cStringIO input_api.json = presubmit.json + input_api.logging = logging input_api.os_listdir = self.mox.CreateMockAnything() input_api.os_walk = self.mox.CreateMockAnything() input_api.os_path = presubmit.os.path @@ -1462,6 +1466,7 @@ class CannedChecksUnittest(PresubmitTestsBase): def __str__(self): return 'foo' input_api.subprocess.CalledProcessError = fake_CalledProcessError + input_api.verbose = False input_api.change = change input_api.host_url = 'http://localhost' @@ -2130,6 +2135,20 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api, presubmit.OutputApi, ['test_module']) self.assertEquals(len(results), 0) + def testCannedRunPylint(self): + # lint.Run() always calls sys.exit()... + lint.Run = lambda x: sys.exit(0) + input_api = self.MockInputApi(None, True) + input_api.AffectedSourceFiles(mox.IgnoreArg()).AndReturn(True) + input_api.PresubmitLocalPath().AndReturn('/foo') + input_api.PresubmitLocalPath().AndReturn('/foo') + input_api.os_walk('/foo').AndReturn([('/foo', [], ['file1.py'])]) + self.mox.ReplayAll() + + results = presubmit_canned_checks.RunPylint( + input_api, presubmit.OutputApi) + self.assertEquals([], results) + def testCheckBuildbotPendingBuildsBad(self): input_api = self.MockInputApi(None, True) connection = self.mox.CreateMockAnything()