Bug 171510

Summary: check-webkit-style exits with an error while parsing Source/WebKit2/Scripts/webkit/ directory: KeyError: 'st'
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, achristensen, aestes, andersca, dbates, dean_johnson, lforschler, sam, thorton, wenson_hsieh
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 171627    
Attachments:
Description Flags
Patch v1 dbates: review+, dbates: commit-queue-

Description David Kilzer (:ddkilzer) 2017-05-01 13:17:31 PDT
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'
$
Comment 1 David Kilzer (:ddkilzer) 2017-05-01 13:22:02 PDT
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'
$
Comment 2 Daniel Bates 2017-05-01 20:15:38 PDT
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
Comment 3 Daniel Bates 2017-05-01 20:21:18 PDT
(In reply to Daniel Bates from comment #2)
> [...]
> $ echo "import parser" > A.py

This should read:

$ echo "import parser" > tmp/A.py
Comment 4 David Kilzer (:ddkilzer) 2017-05-02 21:26:25 PDT
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).
Comment 5 David Kilzer (:ddkilzer) 2017-05-02 21:27:06 PDT
(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.
Comment 6 David Kilzer (:ddkilzer) 2017-05-03 04:30:00 PDT
(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.
Comment 7 David Kilzer (:ddkilzer) 2017-05-03 04:43:54 PDT
Created attachment 308894 [details]
Patch v1
Comment 8 David Kilzer (:ddkilzer) 2017-05-03 04:46:15 PDT
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.)
Comment 9 David Kilzer (:ddkilzer) 2017-05-03 04:50:22 PDT
(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
Comment 10 David Kilzer (:ddkilzer) 2017-05-03 15:38:06 PDT
(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
Comment 11 Daniel Bates 2017-05-03 21:50:47 PDT
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!
Comment 12 David Kilzer (:ddkilzer) 2017-05-04 05:30:30 PDT
Committed r216181: <http://trac.webkit.org/changeset/216181>