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

Ensure AST nodes have correct locations #13453

Open
straight-shoota opened this issue May 10, 2023 · 3 comments
Open

Ensure AST nodes have correct locations #13453

straight-shoota opened this issue May 10, 2023 · 3 comments

Comments

@straight-shoota
Copy link
Member

Originally posted by @oprypin in #13452 (comment)

Do you think it's possible to make a generated test suite that asserts that all nodes have correct locations?

@straight-shoota
Copy link
Member Author

I think it shouldn't be difficult to build a visitor that traverses the entire tree and checks all nodes' locations.

That's at least for validating presence. Correctness is a different story. But I suppose when there's a location, it should be reasonably correct. Some sanity check could also cover that child nodes are enclosed between location and end location.

Should be a fun excercise for anyone who would like to get acquainted with the compiler's visitor pattern 😏

@FnControlOption
Copy link
Contributor

😳 I actually have a branch that checks node locations for each test string in parser_spec.cr that doesn't raise a syntax error. Basically, I just call ASTNode#accept at the end of assert_end_location, which, in turn, is called at the end of it_parses. Here's what my visitor looks like:

class LocationVisitor < Crystal::Visitor
  def initialize(*, @file : String, @line : Int32)
  end

  def visit(node)
    return false unless check_node?(node)
    node.location.should_not be_nil, file: @file, line: @line, failure_message: "Unexpected nil location of #{node.class_desc} #{node.to_s.inspect}"
    node.end_location.should_not be_nil, file: @file, line: @line, failure_message: "Unexpected nil end_location of #{node.class_desc} #{node.to_s.inspect}"
    return false unless check_children?(node)
    true
  end

  private def check_node?(node)
    # ...
  end

  private def check_children?(node)
    # ...
  end
end

assert_end_location already checks if the root node's location and end location matches up with the source string, so what's missing is checking if each child node matches with its corresponding substring.

@Blacksmoke16
Copy link
Member

I noticed that named tuple keys are lacking a location, meaning it's not currently possible (as far as I can tell) to have a compile time error in a macro pointing at a specific key. Probably because the keys are read as strings without any location info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants