Bug 171510 – check-webkit-style exits with an error while parsing Source/WebKit2/Scripts/webkit/ directory: KeyError: 'st'

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

When running check-webkit-style on Source/WebKit2, it exits with an error while parsing the Source/WebKit2/Scripts/webkit/ directory:

$ ./Tools/Scripts/check-webkit-style Source/WebKit2/Scripts/webkit
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessageReceiver-expected.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessageReceiver-expected.cpp:77:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessageReceiver-expected.cpp:96:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:25:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:45:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiver-expected.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiver-expected.cpp:77:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiver-expected.cpp:96:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiverSuperclass-expected.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:25:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:45:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessagesSuperclass-expected.h:25:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebKit2/Scripts/webkit/MessagesSuperclass-expected.h:33:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:160:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:213:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:302:  missing whitespace after ':'  [pep8/E231] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:315:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:318:  missing whitespace after ':'  [pep8/E231] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:346:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:414:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:465:  too many blank lines (2)  [pep8/E303] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:52:  trailing whitespace  [pep8/W291] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:246:  whitespace before ':'  [pep8/E203] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:311:  too many blank lines (3)  [pep8/E303] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:262:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:263:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:264:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/model.py:41:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/parser.py:124:  expected 2 blank lines, found 1  [pep8/E302] [5]
Traceback (most recent call last):
  File "./Tools/Scripts/check-webkit-style", line 48, in <module>
    sys.exit(CheckWebKitStyle().main())
  File "./Tools/Scripts/webkitpy/style/main.py", line 149, in main
    file_reader.process_paths(paths)
  File "./Tools/Scripts/webkitpy/style/filereader.py", line 134, in process_paths
    self._process_directory(directory=path)
  File "./Tools/Scripts/webkitpy/style/filereader.py", line 129, in _process_directory
    self.process_file(file_path)
  File "./Tools/Scripts/webkitpy/style/filereader.py", line 124, in process_file
    self._processor.process(lines, file_path, **kwargs)
  File "./Tools/Scripts/webkitpy/style/checker.py", line 896, in process
    checker.check(lines)
  File "./Tools/Scripts/webkitpy/style/checkers/python.py", line 43, in check
    self._check_pylint(lines)
  File "./Tools/Scripts/webkitpy/style/checkers/python.py", line 71, in _check_pylint
    output = pylinter.run(['-E', self._file_path])
  File "./Tools/Scripts/webkitpy/style/checkers/python.py", line 106, in run
    lint.Run(['--rcfile', self._pylintrc] + argv, reporter=ParseableTextReporter(output=output), exit=False)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 879, in __init__
    linter.check(args)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 502, in check
    self.check_astng_module(astng, walker, rawcheckers)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 574, in check_astng_module
    walker.walk(astng)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/utils.py", line 528, in walk
    self.walk(child)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/utils.py", line 525, in walk
    cb(astng)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/checkers/base.py", line 157, in visit_class
    self._check_redefinition('class', node)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/checkers/base.py", line 221, in _check_redefinition
    defined_self = node.parent.frame()[node.name]
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/logilab/astng/scoped_nodes.py", line 178, in __getitem__
    return self.locals[item][0]
KeyError: 'st'
$

[David Kilzer (:ddkilzer)]

Deleting parser.py "fixes" the error:

$ rm -f Source/WebKit2/Scripts/webkit/parser.py
$ ./Tools/Scripts/check-webkit-style Source/WebKit2/Scripts/webkit/
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessageReceiver-expected.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessageReceiver-expected.cpp:77:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessageReceiver-expected.cpp:96:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:25:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:45:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiver-expected.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiver-expected.cpp:77:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiver-expected.cpp:96:  _result is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessageReceiverSuperclass-expected.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:25:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:45:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/MessagesSuperclass-expected.h:25:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebKit2/Scripts/webkit/MessagesSuperclass-expected.h:33:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:160:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:213:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:302:  missing whitespace after ':'  [pep8/E231] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:315:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:318:  missing whitespace after ':'  [pep8/E231] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:346:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:414:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages.py:465:  too many blank lines (2)  [pep8/E303] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:52:  trailing whitespace  [pep8/W291] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:246:  whitespace before ':'  [pep8/E203] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:311:  too many blank lines (3)  [pep8/E303] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:262:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:263:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:264:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/model.py:41:  expected 2 blank lines, found 1  [pep8/E302] [5]
Total errors found: 28 in 17 files

However, running check-webkit-style on just parser.py works fine:

$ git checkout HEAD Source/WebKit2/Scripts/webkit/parser.py
$ ./Tools/Scripts/check-webkit-style Source/WebKit2/Scripts/webkit/parser.py
ERROR: Source/WebKit2/Scripts/webkit/parser.py:124:  expected 2 blank lines, found 1  [pep8/E302] [5]
Total errors found: 1 in 1 files

Okay, I've narrowed it down to two just files:

$ ./Tools/Scripts/check-webkit-style Source/WebKit2/Scripts/webkit/messages_unittest.py Source/WebKit2/Scripts/webkit/parser.py
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:52:  trailing whitespace  [pep8/W291] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:246:  whitespace before ':'  [pep8/E203] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:311:  too many blank lines (3)  [pep8/E303] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:262:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:263:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/messages_unittest.py:264:  [MessagesTest.setUp] Module 'parser' has no 'parse' member  [pylint/E1101] [5]
ERROR: Source/WebKit2/Scripts/webkit/parser.py:124:  expected 2 blank lines, found 1  [pep8/E302] [5]
Traceback (most recent call last):
  File "./Tools/Scripts/check-webkit-style", line 48, in <module>
    sys.exit(CheckWebKitStyle().main())
  File "./Tools/Scripts/webkitpy/style/main.py", line 149, in main
    file_reader.process_paths(paths)
  File "./Tools/Scripts/webkitpy/style/filereader.py", line 136, in process_paths
    self.process_file(path)
  File "./Tools/Scripts/webkitpy/style/filereader.py", line 124, in process_file
    self._processor.process(lines, file_path, **kwargs)
  File "./Tools/Scripts/webkitpy/style/checker.py", line 896, in process
    checker.check(lines)
  File "./Tools/Scripts/webkitpy/style/checkers/python.py", line 43, in check
    self._check_pylint(lines)
  File "./Tools/Scripts/webkitpy/style/checkers/python.py", line 71, in _check_pylint
    output = pylinter.run(['-E', self._file_path])
  File "./Tools/Scripts/webkitpy/style/checkers/python.py", line 106, in run
    lint.Run(['--rcfile', self._pylintrc] + argv, reporter=ParseableTextReporter(output=output), exit=False)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 879, in __init__
    linter.check(args)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 502, in check
    self.check_astng_module(astng, walker, rawcheckers)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 574, in check_astng_module
    walker.walk(astng)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/utils.py", line 528, in walk
    self.walk(child)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/utils.py", line 525, in walk
    cb(astng)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/checkers/base.py", line 157, in visit_class
    self._check_redefinition('class', node)
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/checkers/base.py", line 221, in _check_redefinition
    defined_self = node.parent.frame()[node.name]
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/logilab/astng/scoped_nodes.py", line 178, in __getitem__
    return self.locals[item][0]
KeyError: 'st'
$

[Daniel Bates]

For some reason the "import parser" line in Source/WebKit2/Scripts/webkit/messages_unittest.py is affecting check-webkit-style. It only seems to affect check-webkit-style when the file that has "import parser" (why?) is style checked before parser.py and when both files are in a subdirectory of the top-level WebKit checkout directory (why?) or outside the WebKit checkout directory (why?). Here is a simple way to reproduce this issue by running the following commands in a Terminal from inside the top-level WebKit checkout directory:

$ mkdir tmp
$ touch tmp/parser.py
$ echo "import parser" > A.py
$ Tools/Scripts/check-webkit-style tmp

[Daniel Bates]

(In reply to Daniel Bates from comment #2)
> [...]
> $ echo "import parser" > A.py

This should read:

$ echo "import parser" > tmp/A.py

[David Kilzer (:ddkilzer)]

As it turns out, fixing the warnings in messages_unittest.py (and parser.py) also fixes this error.

I think the source of the bug was a bad import statement:

-import parser
+from webkit import parser

The next step is to figure out how to make messages_unittest.py and messages.py both work with the same import statement (and still make check-webkit-style happy).

Note that running messages_unittest.py now fails as the expected output hasn't been updated with other changes (probably because messages_unittest.py isn't run regularly by anyone or any bot).

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #4)
> As it turns out, fixing the warnings in messages_unittest.py (and parser.py)
> also fixes this error.
> 
> I think the source of the bug was a bad import statement:
> 
> -import parser
> +from webkit import parser
> 
> The next step is to figure out how to make messages_unittest.py and
> messages.py both work with the same import statement (and still make
> check-webkit-style happy).
> 
> Note that running messages_unittest.py now fails as the expected output
> hasn't been updated with other changes (probably because
> messages_unittest.py isn't run regularly by anyone or any bot).

Dan basically covered this in the comments above.

[David Kilzer (:ddkilzer)]

(In reply to Daniel Bates from comment #3)
> (In reply to Daniel Bates from comment #2)
> > [...]
> > $ echo "import parser" > A.py
> 
> This should read:
> 
> $ echo "import parser" > tmp/A.py

Using Dan's test case, I can may pylint itself hit this error:

$ PYTHONPATH=`pwd`/Tools/Scripts/webkitpy/thirdparty/autoinstalled ./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/bin/pylint tmp/A.py tmp/parser.py 
No config file found, using default configuration
************* Module A
C:  1,0: Invalid name "A" (should match (([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$)
C:  1,0: Missing docstring
W:  1,0: Unused import parser
************* Module parser
I:  1,0: Unable to run raw checkers on built-in module parser
W:  1,0: Redefining built-in '__package__'
Traceback (most recent call last):
  File "./Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/bin/pylint", line 4, in <module>
    lint.Run(sys.argv[1:])
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 879, in __init__
    linter.check(args)
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 502, in check
    self.check_astng_module(astng, walker, rawcheckers)
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py", line 574, in check_astng_module
    walker.walk(astng)
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/utils.py", line 528, in walk
    self.walk(child)
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/utils.py", line 525, in walk
    cb(astng)
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/checkers/base.py", line 157, in visit_class
    self._check_redefinition('class', node)
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/checkers/base.py", line 221, in _check_redefinition
    defined_self = node.parent.frame()[node.name]
  File "/.../Tools/Scripts/webkitpy/thirdparty/autoinstalled/logilab/astng/scoped_nodes.py", line 178, in __getitem__
    return self.locals[item][0]
KeyError: 'st'

It would appear that pylint is getting confused by the built-in 'parser' module that comes with Python itself versus the local parser.py.

That's why changing this appears to fix the pylint issue (but may cause other issues at runtime depending on how the source is run):

-import parser
+from webkit import parser

Okay, I have a fix.

[David Kilzer (:ddkilzer)]

Created attachment 308894 [details]
Patch v1

[David Kilzer (:ddkilzer)]

Note that after this fix, you may run messages_unittests.py like this:

$ python ./Source/WebKit2/Scripts/webkit/messages_unittest.py

(The test fails because the expected files haven't been updated recently, but that's a separate issue.)

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #8)
> Note that after this fix, you may run messages_unittests.py like this:
> 
> $ python ./Source/WebKit2/Scripts/webkit/messages_unittest.py

Prior to this, you had to set PYTHONPATH yourself (which still works after this change, but the PYTHONPATH is now redundant):

$ PYTHONPATH=`pwd`/Source/WebKit2/Scripts python ./Source/WebKit2/Scripts/webkit/messages_unittest.py

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #8)
> Note that after this fix, you may run messages_unittests.py like this:
> 
> $ python ./Source/WebKit2/Scripts/webkit/messages_unittest.py
> 
> (The test fails because the expected files haven't been updated recently,
> but that's a separate issue.)

I filed:

Bug 171627: messages_unittest.py should support an [-u|--update] switch

[Daniel Bates]

Comment on attachment 308894 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=308894&action=review

> Source/WebKit2/ChangeLog:8
> +        The problem was that 'import parser' was abiguous since there is

abiguous => ambiguous

> Source/WebKit2/ChangeLog:9
> +        a built-in parser module that comes with python.  Changing

python => Python

> Source/WebKit2/ChangeLog:19
> +        As an added bonus, this patch also fixes all webkit-style issues
> +        in Source/WebKit2/Scripts/webkit/*.py files.

Yay!

[David Kilzer (:ddkilzer)]

Committed r216181: <http://trac.webkit.org/changeset/216181>