{"action":"created","issue":{"url":"https://api.github.com/repos/pantsbuild/pants/issues/7023","repository_url":"https://api.github.com/repos/pantsbuild/pants","labels_url":"https://api.github.com/repos/pantsbuild/pants/issues/7023/labels{/name}","comments_url":"https://api.github.com/repos/pantsbuild/pants/issues/7023/comments","events_url":"https://api.github.com/repos/pantsbuild/pants/issues/7023/events","html_url":"https://github.com/pantsbuild/pants/pull/7023","id":395853489,"node_id":"MDExOlB1bGxSZXF1ZXN0MjQyMjA2NDg5","number":7023,"title":"use the asttokens 3rdparty lib to make @rule parsing errors very smooth","user":{"login":"cosmicexplorer","id":1305167,"node_id":"MDQ6VXNlcjEzMDUxNjc=","avatar_url":"https://avatars1.githubusercontent.com/u/1305167?v=4","gravatar_id":"","url":"https://api.github.com/users/cosmicexplorer","html_url":"https://github.com/cosmicexplorer","followers_url":"https://api.github.com/users/cosmicexplorer/followers","following_url":"https://api.github.com/users/cosmicexplorer/following{/other_user}","gists_url":"https://api.github.com/users/cosmicexplorer/gists{/gist_id}","starred_url":"https://api.github.com/users/cosmicexplorer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cosmicexplorer/subscriptions","organizations_url":"https://api.github.com/users/cosmicexplorer/orgs","repos_url":"https://api.github.com/users/cosmicexplorer/repos","events_url":"https://api.github.com/users/cosmicexplorer/events{/privacy}","received_events_url":"https://api.github.com/users/cosmicexplorer/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2019-01-04T09:02:20Z","updated_at":"2019-01-04T18:37:57Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/pantsbuild/pants/pulls/7023","html_url":"https://github.com/pantsbuild/pants/pull/7023","diff_url":"https://github.com/pantsbuild/pants/pull/7023.diff","patch_url":"https://github.com/pantsbuild/pants/pull/7023.patch"},"body":"### Problem\r\n\r\nAs @stuhood and @baroquebobcat [pointed](https://github.com/pantsbuild/pants/pull/7019#discussion_r245098273) [out](https://github.com/pantsbuild/pants/pull/7019#discussion_r245171933) in #7019, it seems a little silly to only be able to provide the location of the start of the offending `@rule` for `yield` statements which would lead to ambiguity (see #7019), instead of the actual `yield` statement to blame. In that PR, we punted and used `ast.dump()`, but in the meantime I was able to find a 3rdparty library ([`asttokens`](https://github.com/gristlabs/asttokens)) which performs the appropriate bookkeeping to get the correct source text as well as line and column offsets of individual nodes within an [`ast`](https://docs.python.org/2/library/ast.html) parse tree.\r\n\r\nIf you are interested in why this isn't trivial, I recommend trying an `ast.dump(node, include_attributes=True)` and trying to hook up the numbers yourself (and if it is obvious, let me know, but I don't think this particular 3rdparty lib is too difficult of a dependency to bring in).\r\n\r\n### Solution\r\n\r\n- Add `asttokens` 3rdparty requirement.\r\n- Use `asttokens.ASTTokens` and `asttokens.LineNumbers` to get the text and position of the offending `yield` statement in the original file.\r\n- Re-add indentation that we had to remove to parse the `@rule` body in the first place so that our error message is exactly what the source text is.\r\n\r\n### Result\r\n\r\nAn invalid rule like:\r\n\r\n```python\r\n@rule(A, [])\r\ndef f():\r\n  yield Get(SomeProduct, SomeInput, some_input)\r\n  yield A()\r\n```\r\n\r\nWill now raise an error like:\r\n\r\n```\r\nException message: In function f: A yield in an @rule without an assignment is equivalent to a return, and we\r\ncurrently require that it comes at the end of a series of statements.\r\nUse `_ = yield Get(...)` if you wish to yield control to the engine and discard the result.\r\n\r\nThe invalid statement was:\r\n/path/to/file/with/rule.py:45:2\r\n  yield Get(SomeProduct, SomeInput, some_input)\r\n\r\nThe rule defined by function `f` begins at:\r\n/path/to/file/with/rule.py:43:0\r\n@rule(A, [])\r\ndef f():\r\n```\r\n\r\nI can now jump to a parse error (an invalid `yield` statement, as above) and hit the return key in emacs's compilation-mode to jump again to the precise source text which caused the error. This makes the prospect of adding more `ast` magic in the future (or not, and continuing to use what was added in #7019) in a highly structured and specific way much more plausible and understandable to users not incredibly well-versed in the semantics of the v2 engine."},"comment":{"url":"https://api.github.com/repos/pantsbuild/pants/issues/comments/451529918","html_url":"https://github.com/pantsbuild/pants/pull/7023#issuecomment-451529918","issue_url":"https://api.github.com/repos/pantsbuild/pants/issues/7023","id":451529918,"node_id":"MDEyOklzc3VlQ29tbWVudDQ1MTUyOTkxOA==","user":{"login":"baroquebobcat","id":7378,"node_id":"MDQ6VXNlcjczNzg=","avatar_url":"https://avatars0.githubusercontent.com/u/7378?v=4","gravatar_id":"","url":"https://api.github.com/users/baroquebobcat","html_url":"https://github.com/baroquebobcat","followers_url":"https://api.github.com/users/baroquebobcat/followers","following_url":"https://api.github.com/users/baroquebobcat/following{/other_user}","gists_url":"https://api.github.com/users/baroquebobcat/gists{/gist_id}","starred_url":"https://api.github.com/users/baroquebobcat/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/baroquebobcat/subscriptions","organizations_url":"https://api.github.com/users/baroquebobcat/orgs","repos_url":"https://api.github.com/users/baroquebobcat/repos","events_url":"https://api.github.com/users/baroquebobcat/events{/privacy}","received_events_url":"https://api.github.com/users/baroquebobcat/received_events","type":"User","site_admin":false},"created_at":"2019-01-04T18:37:57Z","updated_at":"2019-01-04T18:37:57Z","author_association":"CONTRIBUTOR","body":"I think would be better if the top of the error message directly said what the issue was, with the explanation second. Maybe something like `Exception message: In function f: Yield in @rule without assignment not at end of a series of statements.`"}}
{"actor":{"display_login":"baroquebobcat"}}