Instagram / LibCST

A concrete syntax tree parser and serializer library for Python that preserves many aspects of Python's abstract syntax tree
https://libcst.readthedocs.io/
Other
1.55k stars 191 forks source link

`CodeRange` provided by `PositionProvider` doesn't include decorators #972

Open notEvil opened 1 year ago

notEvil commented 1 year ago

Hi,

since decorators are sub nodes of FunctionDef and ClassDef I expected the code range to cover them. Is this intended?

import libcst
import libcst.metadata as l_metadata

metadata_wrapper = libcst.MetadataWrapper(libcst.parse_module("""
@decorator
def function():
    pass

@decorator
class Class:
    pass
"""))

positions = metadata_wrapper.resolve(l_metadata.PositionProvider)

function_node, class_node = metadata_wrapper.module.body
print("function :", positions[function_node])
print("decorator:", positions[function_node.decorators[0]])
print()
print("class    :", positions[class_node])
print("decorator:", positions[class_node.decorators[0]])

# function : CodeRange(start=CodePosition(line=3, column=0), end=CodePosition(line=4, column=8))
# decorator: CodeRange(start=CodePosition(line=2, column=0), end=CodePosition(line=2, column=10))
#
# class    : CodeRange(start=CodePosition(line=7, column=0), end=CodePosition(line=8, column=8))
# decorator: CodeRange(start=CodePosition(line=6, column=0), end=CodePosition(line=6, column=10))
zsol commented 1 year ago

Dunno if this is intentional or not, but I think it would make sense to include the decorators in the span. Have to move the context manager here to the right place: https://github.com/Instagram/LibCST/blob/a3f5bf97d631e79c3395a249e15645cfbc225a4d/libcst/_nodes/statement.py#L1800

On Wed, 12 Jul 2023, 10:31 notEvil, @.***> wrote:

Hi,

since decorators are sub nodes of FunctionDef and ClassDef I expected the code range to cover them. Is this intended?

import libcstimport libcst.metadata as l_metadata

metadata_wrapper = @. function(): @. Class: pass""")) positions = metadata_wrapper.resolve(l_metadata.PositionProvider) function_node, class_node = metadata_wrapper.module.bodyprint("function :", positions[function_node])print("decorator:", positions[function_node.decorators[0]])print()print("class :", positions[class_node])print("decorator:", positions[class_node.decorators[0]])

function : CodeRange(start=CodePosition(line=3, column=0), end=CodePosition(line=4, column=8))# decorator: CodeRange(start=CodePosition(line=2, column=0), end=CodePosition(line=2, column=10))## class : CodeRange(start=CodePosition(line=7, column=0), end=CodePosition(line=8, column=8))# decorator: CodeRange(start=CodePosition(line=6, column=0), end=CodePosition(line=6, column=10))

— Reply to this email directly, view it on GitHub https://github.com/Instagram/LibCST/issues/972, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQJNBQX5VJZJNS4F5TAKTXPZVFZANCNFSM6AAAAAA2HGPC7E . You are receiving this because you are subscribed to this thread.Message ID: @.***>

notEvil commented 1 year ago

I found another inconsistency related to code ranges: Tuple doesn't include parens

import libcst
import libcst.metadata as l_metadata

metadata_wrapper = libcst.MetadataWrapper(libcst.parse_module("""
(None,)
[None]
{None}
{None: None}
"""))

code_ranges = metadata_wrapper.resolve(l_metadata.PositionProvider)

tuple_node, list_node, set_node, dict_node = (
    cst_node.body[0].value for cst_node in metadata_wrapper.module.body
)  # SimpleStatementLine, Expr
print("tuple:", code_ranges[tuple_node])
print("list :", code_ranges[list_node])
print("set  :", code_ranges[set_node])
print("dict :", code_ranges[dict_node])

# tuple: CodeRange(start=CodePosition(line=2, column=1), end=CodePosition(line=2, column=6))
# list : CodeRange(start=CodePosition(line=3, column=0), end=CodePosition(line=3, column=6))
# set  : CodeRange(start=CodePosition(line=4, column=0), end=CodePosition(line=4, column=6))
# dict : CodeRange(start=CodePosition(line=5, column=0), end=CodePosition(line=5, column=12))

Should I open a separate issue for this?

edit: It seems no node includes parens, since they are usually not significant. And I remember https://github.com/Instagram/LibCST/pull/458, so you can mark this as off-topic