- Revision
- 134309
- Author
- [email protected]
- Date
- 2012-11-12 14:36:01 -0800 (Mon, 12 Nov 2012)
Log Message
webkitpy: integrate pylint into check-webkit-style, part I
https://bugs.webkit.org/show_bug.cgi?id=101285
Reviewed by Ojan Vafai.
This patch re-works lint-webkitpy so that the logic is pushed
into check-webkit-style (mostly); we don't yet control which
messages are displayed using the rules in webkitpy/style/checker.py
(we're still using the pylintrc to suppress messages instead),
but otherwise things work. For now we will only report pylint
"errors", not warnings.
* Scripts/lint-webkitpy:
* Scripts/webkitpy/style/checker.py:
* Scripts/webkitpy/style/checkers/python.py:
(PythonChecker):
(PythonChecker.check):
(PythonChecker._check_pep8):
(PythonChecker._check_pylint):
(Pylinter):
(Pylinter.__init__):
(Pylinter.run):
(_FilteredStringIO):
(_FilteredStringIO.__init__):
(_FilteredStringIO.write):
(_FilteredStringIO._filter):
* Scripts/webkitpy/style/checkers/python_unittest.py:
(PythonCheckerTest.test_check):
* Scripts/webkitpy/style/checkers/python_unittest_input.py:
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (134308 => 134309)
--- trunk/Tools/ChangeLog 2012-11-12 22:33:40 UTC (rev 134308)
+++ trunk/Tools/ChangeLog 2012-11-12 22:36:01 UTC (rev 134309)
@@ -1,5 +1,37 @@
2012-11-12 Dirk Pranke <[email protected]>
+ webkitpy: integrate pylint into check-webkit-style, part I
+ https://bugs.webkit.org/show_bug.cgi?id=101285
+
+ Reviewed by Ojan Vafai.
+
+ This patch re-works lint-webkitpy so that the logic is pushed
+ into check-webkit-style (mostly); we don't yet control which
+ messages are displayed using the rules in webkitpy/style/checker.py
+ (we're still using the pylintrc to suppress messages instead),
+ but otherwise things work. For now we will only report pylint
+ "errors", not warnings.
+
+ * Scripts/lint-webkitpy:
+ * Scripts/webkitpy/style/checker.py:
+ * Scripts/webkitpy/style/checkers/python.py:
+ (PythonChecker):
+ (PythonChecker.check):
+ (PythonChecker._check_pep8):
+ (PythonChecker._check_pylint):
+ (Pylinter):
+ (Pylinter.__init__):
+ (Pylinter.run):
+ (_FilteredStringIO):
+ (_FilteredStringIO.__init__):
+ (_FilteredStringIO.write):
+ (_FilteredStringIO._filter):
+ * Scripts/webkitpy/style/checkers/python_unittest.py:
+ (PythonCheckerTest.test_check):
+ * Scripts/webkitpy/style/checkers/python_unittest_input.py:
+
+2012-11-12 Dirk Pranke <[email protected]>
+
nrwt: remove a bunch of broken chromium-specific flags
https://bugs.webkit.org/show_bug.cgi?id=101979
Modified: trunk/Tools/Scripts/lint-webkitpy (134308 => 134309)
--- trunk/Tools/Scripts/lint-webkitpy 2012-11-12 22:33:40 UTC (rev 134308)
+++ trunk/Tools/Scripts/lint-webkitpy 2012-11-12 22:36:01 UTC (rev 134309)
@@ -27,14 +27,8 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-import os
import sys
-from webkitpy.thirdparty.autoinstalled.pylint import lint
+from webkitpy.style.checkers.python import Pylinter
-script_dir = os.path.abspath(os.path.dirname(__file__))
-if not script_dir in sys.path:
- sys.path.append(script_dir)
-
-pylintrc = os.path.join(script_dir, 'webkitpy', 'pylintrc')
-lint.Run(['--rcfile', pylintrc, '-f', 'parseable' ] + sys.argv[1:])
+sys.stdout.write(Pylinter().run(sys.argv[1:]).getvalue())
Modified: trunk/Tools/Scripts/webkitpy/style/checker.py (134308 => 134309)
--- trunk/Tools/Scripts/webkitpy/style/checker.py 2012-11-12 22:33:40 UTC (rev 134308)
+++ trunk/Tools/Scripts/webkitpy/style/checker.py 2012-11-12 22:36:01 UTC (rev 134309)
@@ -106,6 +106,9 @@
# with the 79 character limit, or some higher limit that is
# agreeable to the WebKit project.
'-pep8/E501',
+
+ # FIXME: Move the pylint rules from the pylintrc to here. This will
+ # also require us to re-work lint-webkitpy to produce the equivalent output.
]
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/python.py (134308 => 134309)
--- trunk/Tools/Scripts/webkitpy/style/checkers/python.py 2012-11-12 22:33:40 UTC (rev 134308)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/python.py 2012-11-12 22:36:01 UTC (rev 134309)
@@ -22,23 +22,32 @@
"""Supports checking WebKit style in Python files."""
+import os
+import re
+import sys
+from StringIO import StringIO
+
from webkitpy.thirdparty.autoinstalled import pep8
+from webkitpy.thirdparty.autoinstalled.pylint import lint
+from webkitpy.thirdparty.autoinstalled.pylint.reporters.text import ParseableTextReporter
class PythonChecker(object):
-
"""Processes text lines for checking style."""
-
def __init__(self, file_path, handle_style_error):
self._file_path = file_path
self._handle_style_error = handle_style_error
def check(self, lines):
+ self._check_pep8(lines)
+ self._check_pylint(lines)
+
+ def _check_pep8(self, lines):
# Initialize pep8.options, which is necessary for
# Checker.check_all() to execute.
pep8.process_options(arglist=[self._file_path])
- checker = pep8.Checker(self._file_path)
+ pep8_checker = pep8.Checker(self._file_path)
def _pep8_handle_error(line_number, offset, text, check):
# FIXME: Incorporate the character offset into the error output.
@@ -51,6 +60,69 @@
self._handle_style_error(line_number, category, 5, pep8_message)
- checker.report_error = _pep8_handle_error
+ pep8_checker.report_error = _pep8_handle_error
+ pep8_errors = pep8_checker.check_all()
- errors = checker.check_all()
+ def _check_pylint(self, lines):
+ pylinter = Pylinter()
+
+ # FIXME: for now, we only report pylint errors, but we should be catching and
+ # filtering warnings using the rules in style/checker.py instead.
+ output = pylinter.run(['-E', self._file_path])
+
+ lint_regex = re.compile('([^:]+):([^:]+): \[([^]]+)\] (.*)')
+ for error in output.getvalue().splitlines():
+ match_obj = lint_regex.match(error)
+ assert(match_obj)
+ line_number = int(match_obj.group(2))
+ category_and_method = match_obj.group(3).split(', ')
+ category = 'pylint/' + (category_and_method[0])
+ if len(category_and_method) > 1:
+ message = '[%s] %s' % (category_and_method[1], match_obj.group(4))
+ else:
+ message = match_obj.group(4)
+ self._handle_style_error(line_number, category, 5, message)
+
+
+class Pylinter(object):
+ # We filter out these messages because they are bugs in pylint that produce false positives.
+ # FIXME: Does it make sense to combine these rules with the rules in style/checker.py somehow?
+ FALSE_POSITIVES = [
+ # possibly http://www.logilab.org/ticket/98613 ?
+ "Instance of 'Popen' has no 'returncode' member",
+ "Instance of 'Popen' has no 'stdin' member",
+ "Instance of 'Popen' has no 'stdout' member",
+ "Instance of 'Popen' has no 'stderr' member",
+ "Instance of 'Popen' has no 'wait' member",
+ ]
+
+ def __init__(self):
+ self._script_dir = os.path.abspath(os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__)))))
+ self._pylintrc = os.path.join(self._script_dir, 'webkitpy', 'pylintrc')
+
+ def run(self, argv):
+ output = _FilteredStringIO(self.FALSE_POSITIVES)
+ lint.Run(['--rcfile', self._pylintrc] + argv, reporter=ParseableTextReporter(output=output), exit=False)
+ return output
+
+
+class _FilteredStringIO(StringIO):
+ def __init__(self, bad_messages):
+ StringIO.__init__(self)
+ self.dropped_last_msg = False
+ self.bad_messages = bad_messages
+
+ def write(self, msg=''):
+ if not self._filter(msg):
+ StringIO.write(self, msg)
+
+ def _filter(self, msg):
+ if any(bad_message in msg for bad_message in self.bad_messages):
+ self.dropped_last_msg = True
+ return True
+ if self.dropped_last_msg:
+ # We drop the newline after a dropped message as well.
+ self.dropped_last_msg = False
+ if msg == '\n':
+ return True
+ return False
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/python_unittest.py (134308 => 134309)
--- trunk/Tools/Scripts/webkitpy/style/checkers/python_unittest.py 2012-11-12 22:33:40 UTC (rev 134308)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/python_unittest.py 2012-11-12 22:36:01 UTC (rev 134309)
@@ -57,6 +57,7 @@
checker = PythonChecker(file_path, _mock_handle_style_error)
checker.check(lines=[])
- self.assertEquals(len(errors), 1)
- self.assertEquals(errors[0],
- (2, "pep8/W291", 5, "trailing whitespace"))
+ self.assertEquals(errors, [
+ (4, "pep8/W291", 5, "trailing whitespace"),
+ (4, "pylint/E0602", 5, "Undefined variable 'error'"),
+ ])
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/python_unittest_input.py (134308 => 134309)
--- trunk/Tools/Scripts/webkitpy/style/checkers/python_unittest_input.py 2012-11-12 22:33:40 UTC (rev 134308)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/python_unittest_input.py 2012-11-12 22:36:01 UTC (rev 134309)
@@ -1,2 +1,4 @@
-# This file is sample input for python_unittest.py and includes a single
-# error which is an extra space at the end of this line.
+# This file is sample input for python_unittest.py and includes two
+# problems, one that will generate a PEP-8 warning for trailing whitespace
+# and one that will generate a pylint error for an undefined variable.
+print error()
