php icon indicating copy to clipboard operation
php copied to clipboard

Line number in AST Node?

Open otremblay opened this issue 9 years ago • 1 comments

Hi,

I'm using your tool to parse PHP code with the secret hope of building some kind of static analysis tool with it; I wondered if it would be feasible and/or desirable to track line numbers in Node structures? I don't mind lending a hand for that, but if it's definitely unwanted I'd rather not waste time :)

otremblay avatar Mar 12 '16 14:03 otremblay

Short answer: yes, I totally want to have line numbers available, and I'd love some help with that! That said, it's a feature I've worked toward a bit. I originally tried storing locations on the AST, but it didn't fit well. It sorely breaks down if you treat an AST as mutable, or if it's generated instead of parsed, both use cases I've had in mind.

Instead, I've put location information in the product of the lexer, basically a slice of token.Item. So far, it seems like a better location than the AST node. A list of token.Item can be an actual representation of the code we've read, instead of an abstract version of it because it's strictly ordered and we haven't removed things like whitespace. You can see a little bit of usage in how the parser handles errors.

The big open task is creating a good way to arbitrarily translate between AST nodes and a stream of tokens. I think a fairly naive solution would be to change Parser.Parse's signature to something like:

type NodeMap map[ast.Node]struct {
    Begin, End int
}

type ParseResult struct {
    File    *ast.File
    Tokens  []token.Item
    NodeMap NodeMap
}

func (p *Parser) Parse(filepath, input string) (ParseResult, error)

and to populate that information as the Parser does its work.

That said, as part of that, I think it'd be helpful (maybe necessary) to standardize on pointers for all AST node type usages, so map lookups go smoothly. Also, this mechanism still doesn't help you identify source locations in an AST that you've modified, but perhaps we could have a way to create a NodeMap from an *ast.File, or something like that.

It wouldn't be surprising if I'm missing something here :) so let me know what you think.

stephens2424 avatar Mar 13 '16 05:03 stephens2424