Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Carry comments with the AST #68307

Closed
brettcannon opened this issue May 3, 2015 · 15 comments
Closed

Carry comments with the AST #68307

brettcannon opened this issue May 3, 2015 · 15 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@brettcannon
Copy link
Member

BPO 24119
Nosy @gvanrossum, @brettcannon, @rhettinger, @pitrou, @tiran, @markshannon, @serhiy-storchaka, @kernc, @mlouielu, @mbdevpl, @isidentical

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2015-05-03.15:21:10.716>
labels = ['type-feature', 'library']
title = 'Carry comments with the AST'
updated_at = <Date 2020-03-13.01:04:47.072>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2020-03-13.01:04:47.072>
actor = 'gvanrossum'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2015-05-03.15:21:10.716>
creator = 'brett.cannon'
dependencies = []
files = []
hgrepos = []
issue_num = 24119
keywords = []
message_count = 15.0
messages = ['242485', '242499', '242560', '242602', '242603', '289643', '289685', '292655', '292661', '292682', '292683', '333540', '333541', '334072', '364061']
nosy_count = 11.0
nosy_names = ['gvanrossum', 'brett.cannon', 'rhettinger', 'pitrou', 'christian.heimes', 'Mark.Shannon', 'serhiy.storchaka', 'kernc', 'louielu', 'mbdevpl', 'BTaskaya']
pr_nums = []
priority = 'low'
resolution = None
stage = 'test needed'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue24119'
versions = ['Python 3.5']

@brettcannon
Copy link
Member Author

One thing about https://www.python.org/dev/peps/pep-0484/ is that it makes comments potentially semantically meaningful. Unfortunately the AST doesn't carry comments with it in any way, making it difficult to build a tool to implement a linter for PEP-484 using purely the ast module. Even if comments were carried along side-band and could do correlation by line number would be useful in this scenario.

I thought an issue had previously existed for this topic but I could find it.

@brettcannon brettcannon added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels May 3, 2015
@markshannon
Copy link
Member

Comments don't belong on the AST. Where would you attach them?

The tokenizer module provides all information about comments. Tools can get the information quite easily if they need it.

@brettcannon
Copy link
Member Author

Normally I would agree comments don't belong there, but if we are going to start giving them semantic meaning then I don't think it's not so clear to me anymore.

As to where to attach, simple place is off of the Module node. Another is to have it be fundamental like lineno and only attach it when it is a line-trailing comment.

Yes, the tokenize module will give you the comments as well, but it is unfortunate you have to parse the code twice in order to get the comments and the AST.

@brettcannon
Copy link
Member Author

Another option is to provide a tool in 'tokenize' or 'ast' which will take the source and some comment regex and then attach the found comment metadata to the AST.

@pitrou
Copy link
Member

pitrou commented May 5, 2015

Or a separate AST node -> comment mapping.

@mbdevpl
Copy link
Mannequin

mbdevpl mannequin commented Mar 15, 2017

For some time now, there's an alternate ast implementation https://github.com/python/typed_ast that carries PEP-484 type comments with the AST as attributes of certain nodes.

Their approach is described here: https://github.com/python/typed_ast/blob/master/typed_ast/ast3.py#L5

If type comments become mainstream in Python, could this approach maybe be adopted as official Python AST at some point?

@brettcannon
Copy link
Member Author

The type annotation is already in the AST so there's nothing to carry over from typed_ast (we only care about the latest Python version while typed_ast tries to be version-agnostic).

@mlouielu
Copy link
Mannequin

mlouielu mannequin commented May 1, 2017

Brett, which implement method will you prefer?

If we want to carry comment at builtin_compile_impl, it will need to change the grammar since tokenize just drop the comment when dealing with source code.

But if just using regex, will it be more easy with just combine the exists tokenize and ast module?

@rhettinger
Copy link
Contributor

After PEP-526, the need for this proposal may have evaporated.

@brettcannon
Copy link
Member Author

There's potentially some usefulness from other tools, but Raymond is right that the main motivation is definitely gone long-term. Dropping this down to "low" priority simply because others have asked for this kind of support before.

@serhiy-storchaka
Copy link
Member

Where would comment be attached in the following case?

a = ('start'         # comment 1
     'continuation') # comment 2

@rhettinger
Copy link
Contributor

Since the AST carries the module/lineno attributes isn't there already a way trace back into the token stream to recover comments?

@rhettinger
Copy link
Contributor

Possible superceder: https://bugs.python.org/issue33337

@gvanrossum
Copy link
Member

See also bpo-35766.

@gvanrossum
Copy link
Member

I propose to close this issue, since (as of Python 3.8) we now have ast.parse(source, type_comments=True).

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@iritkatriel iritkatriel closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

7 participants