t3rn0 / ast-comments

Extension to the built-in ast module. Finds comments in source code and adds them to the parsed tree.
MIT License
31 stars 10 forks source link

Add get_source_segment and support trailing comments in functions #19

Closed devdanzin closed 1 year ago

devdanzin commented 1 year ago

I've made some changes to ast-comments while trying to get a version of ast.get_source_segment that roundtrips comments to use in radon.

They allow getting trailing comments from within function bodies. Unfortunately, it'll also incorporate comments that appear right after a function's body. I'm not sure this is of interest to include in ast-comments, but wanted to keep a record of the exploration and allow assessing whether (an improved version of?) this would be desirable.

t3rn0 commented 1 year ago

Hi. Thanks for creating this PR. I understand that in some cases proposed behavior may be convenient. But I think the current one is ok. Consider following example:

source = """
a = 1; b = 2
"""
tree = ast.parse(source)
for node in tree.body:  # len(tree.body) == 2
    print(ast.get_source_segment(node))

## prints ##
a = 1
b = 2

There are two separate nodes and there are two separate source segments. Now, let's change the b-node to a comment:

source = """
a = 1  # some comment
"""
tree = ast_comments.parse(source)
for node in tree.body:  # len(tree.body) == 2
    print(ast.get_source_segment(node))

## prints ##
a = 1
# some comment

Again, we have two separate nodes (Expr and Comment) and two separate source segments. I don't see any difference between the first and second examples, except for the ast node type. If a Comment node is part of the body of some other node (function, forloop, etc. ), its source segment must also be part of the corresponding node's source segment:

source = """
def foo():  # some comment
    a = 1
"""
tree = ast_comments.parse(source)
for node in tree.body:  # len(tree.body) == 1 (!!!)
    print(ast.get_source_segment(node))

## prints ##
def foo():  # some comment
    a = 1

About PR: From "testing" perspective I find that ast_comments.get_source_segment doesn't actually do anything. If you remove it pytest results won't change.

I recommend creating this modification on your side (in radon).

It's really great you raised this issue. Review helped me to find one minor bug 🙂

devdanzin commented 1 year ago

Thank you for reviewing this! I agree with your analysis, glad to hear that it helped spot a minor bug :)