From b9424d07e234eec09413659f237a861d34a05132 Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Wed, 18 Apr 2018 12:30:28 -0700 Subject: Run pylint on Python test code The test modules were not under pylint jurisdiction, and actual bugs have been found in tests that would have been prevented had we run static analysis on the test code as we do on the core modules. This is the first step to enable pylint on tests. Due to numerous warnings since the code is not ready and needs refactoring, a new `.pylintrc` specific to tests is added that suppresses a number of valid warnings. The goal is stepwise elimination of each class of warning while refactoring the code such that it will not emit any warnings in future commits, always keeping the sanity checks passing and keeping the disruption minimal. --- .pylintrc-tests | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 .pylintrc-tests (limited to '.pylintrc-tests') diff --git a/.pylintrc-tests b/.pylintrc-tests new file mode 100644 index 0000000000..e68fd47b08 --- /dev/null +++ b/.pylintrc-tests @@ -0,0 +1,120 @@ +[VARIABLES] + +# TODO(https://github.com/PyCQA/pylint/issues/1345): How does the inspection +# not include "unused_" and "ignored_" by default? +dummy-variables-rgx=^ignored_|^unused_ + +[DESIGN] + +# NOTE(nathaniel): Not particularly attached to this value; it just seems to +# be what works for us at the moment (excepting the dead-code-walking Beta +# API). +max-args=6 + +[MISCELLANEOUS] + +# NOTE(nathaniel): We are big fans of "TODO(): " and +# "NOTE(): ". We do not allow "TODO:", +# "TODO():", "FIXME:", or anything else. +notes=FIXME,XXX + +[MESSAGES CONTROL] + +disable= + # These suppressions are specific to tests: + # + # TODO(https://github.com/grpc/grpc/issues/261): investigate + # each of the following one by one and consider eliminating + # the suppression category. + # Eventually, the hope is to eliminate the .pylintrc-tests + # altogether and rely on .pylintrc for everything. + pointless-statement, + no-member, + no-self-use, + attribute-defined-outside-init, + unused-argument, + unused-variable, + unused-import, + redefined-builtin, + too-many-public-methods, + too-many-locals, + redefined-variable-type, + old-style-class, + redefined-outer-name, + bare-except, + broad-except, + ungrouped-imports, + too-many-branches, + too-many-arguments, + too-many-format-args, + too-many-return-statements, + too-many-statements, + undefined-variable, + function-redefined, + unnecessary-lambda, + wildcard-import, + line-too-long, + unreachable, + wrong-import-position, + wrong-import-order, + non-iterator-returned, + undefined-loop-variable, + raising-bad-type, + bad-continuation, + # -- END OF TEST-SPECIFIC SUPPRESSIONS -- + + + # TODO(https://github.com/PyCQA/pylint/issues/59#issuecomment-283774279): + # Enable cyclic-import after a 1.7-or-later pylint release that + # recognizes our disable=cyclic-import suppressions. + cyclic-import, + # TODO(https://github.com/grpc/grpc/issues/8622): Enable this after the + # Beta API is removed. + duplicate-code, + # TODO(https://github.com/grpc/grpc/issues/261): Doesn't seem to + # understand enum and concurrent.futures; look into this later with the + # latest pylint version. + import-error, + # TODO(https://github.com/grpc/grpc/issues/261): Enable this one. + # Should take a little configuration but not much. + invalid-name, + # TODO(https://github.com/grpc/grpc/issues/261): This doesn't seem to + # work for now? Try with a later pylint? + locally-disabled, + # NOTE(nathaniel): What even is this? *Enabling* an inspection results + # in a warning? How does that encourage more analysis and coverage? + locally-enabled, + # NOTE(nathaniel): We don't write doc strings for most private code + # elements. + missing-docstring, + # NOTE(nathaniel): In numeric comparisons it is better to have the + # lesser (or lesser-or-equal-to) quantity on the left when the + # expression is true than it is to worry about which is an identifier + # and which a literal value. + misplaced-comparison-constant, + # NOTE(nathaniel): Our completely abstract interface classes don't have + # constructors. + no-init, + # TODO(https://github.com/grpc/grpc/issues/261): Doesn't yet play + # nicely with some of our code being implemented in Cython. Maybe in a + # later version? + no-name-in-module, + # TODO(https://github.com/grpc/grpc/issues/261): Suppress these where + # the odd shape of the authentication portion of the API forces them on + # us and enable everywhere else. + protected-access, + # NOTE(nathaniel): Pylint and I will probably never agree on this. + too-few-public-methods, + # NOTE(nathaniel): Pylint and I wil probably never agree on this for + # private classes. For public classes maybe? + too-many-instance-attributes, + # NOTE(nathaniel): Some of our modules have a lot of lines... of + # specification and documentation. Maybe if this were + # lines-of-code-based we would use it. + too-many-lines, + # TODO(https://github.com/grpc/grpc/issues/261): Maybe we could have + # this one if we extracted just a few more helper functions... + too-many-nested-blocks, + # NOTE(nathaniel): I have disputed the premise of this inspection from + # the beginning and will continue to do so until it goes away for good. + useless-else-on-loop, -- cgit v1.2.3