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

Parser: fix wrong/missing locations of various AST nodes #11798

Merged
merged 55 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
9682141
Parser: fix end location of call name
FnControlOption Oct 23, 2021
92b3b20
Parser: fix location of implicit tuple literal of multi-return
FnControlOption Nov 6, 2021
41d371d
Parser: fix `else_location` of if statement without `else`
FnControlOption Feb 3, 2022
0662d52
Check the text between an AST node's start and end locations
FnControlOption Jan 24, 2022
4a1d081
Parser: fix location of multi-assign if expression
FnControlOption Jan 24, 2022
1fe58da
Parser: fix end location of def with parenthesized parameters
FnControlOption Jan 24, 2022
4e15582
Parser: fix location of global call
FnControlOption Jan 24, 2022
0eadd4a
Parser: fix end location of `#[]=` call
FnControlOption Jan 24, 2022
a6b063b
Parser: fix end location of var with type declaration
FnControlOption Jan 24, 2022
0f56113
Parser: fix end location of `#[]?` call
FnControlOption Jan 24, 2022
930be1c
Parser: fix end location of array literal
FnControlOption Feb 3, 2022
8e54f89
Parser: fix end location of multi-line call with block argument
FnControlOption Feb 3, 2022
c7b1e58
Parser: fix end location of call with receiver and block
FnControlOption Feb 3, 2022
94f5252
Parser: add end location of array literal
FnControlOption Feb 4, 2022
bc085a9
Parser: fix end location of multi-line macro if
FnControlOption Feb 4, 2022
a3dd73a
Parser: add end location of `out` parameter
FnControlOption Feb 4, 2022
84ff78d
Parser: add end location of nilable generic type
FnControlOption Feb 4, 2022
c5b8f8c
Parser: add end location of type with suffix
FnControlOption Feb 4, 2022
b6b1496
Parser: add end location of proc pointer
FnControlOption Feb 4, 2022
cd32b38
Parser: add end location of splat argument
FnControlOption Feb 4, 2022
79b62f8
Parser: add end location of custom array literal
FnControlOption Feb 4, 2022
a154a90
Parser: add end location of negation suffix
FnControlOption Jan 24, 2022
4792a84
Parser: add end location of attribute assignment
FnControlOption Jan 24, 2022
2e94f4f
Parser: add end location of proc type
FnControlOption Jan 24, 2022
7ee0dad
Comment out line to try and fix compiler error
FnControlOption Feb 4, 2022
8863963
Revert "Comment out line to try and fix compiler error"
FnControlOption Feb 4, 2022
9677086
Revert "Parser: add end location of array literal"
FnControlOption Feb 4, 2022
68aaff6
Run `crystal tool format`
FnControlOption Feb 4, 2022
0787432
Re-add end location of array literal
FnControlOption Feb 4, 2022
cab803f
Merge branch 'master' into ast
FnControlOption Nov 18, 2022
4e30c38
Fix error
FnControlOption Nov 18, 2022
db5697e
Apply suggestions from code review
FnControlOption Nov 21, 2022
502d63a
Merge branch 'master' into ast
FnControlOption Nov 21, 2022
5ec08de
Fix off-by-one error
FnControlOption Nov 21, 2022
aed506a
Refactor consume_def_equals_sign_skip_space
FnControlOption Nov 21, 2022
a5ceac7
Merge branch 'master' into ast2
FnControlOption Nov 22, 2022
19cf9b7
Add end location of `require`
FnControlOption Nov 22, 2022
536df1f
Add location of `begin ... end`
FnControlOption Nov 22, 2022
377aa74
Add end location of endless range literal
FnControlOption Nov 22, 2022
a75e424
Add end location of `.responds_to?`
FnControlOption Nov 22, 2022
839c307
Add end location of `.nil?`
FnControlOption Nov 22, 2022
8f76dd4
Add end location of uninitialized var
FnControlOption Nov 22, 2022
981215f
Add end location of trailing `rescue`/`ensure`
FnControlOption Nov 22, 2022
d1407ac
Add end location of property assignment with splat
FnControlOption Nov 22, 2022
89c1958
Don't set end_location in parse_atomic
FnControlOption Nov 22, 2022
0df6a12
Don't change location of exception handler body
FnControlOption Nov 22, 2022
a958e3c
Add end location of local var
FnControlOption Nov 22, 2022
a92f6d5
Merge branch 'master' into ast
FnControlOption Nov 24, 2022
af8d754
Delete `focus: true`
FnControlOption Nov 24, 2022
e3e29e0
Add location of array literal element
FnControlOption Nov 24, 2022
0032caf
Add location of trailing ensure
FnControlOption Nov 24, 2022
b511000
Add location of trailing rescue
FnControlOption Nov 24, 2022
0da7e2e
Delete pseudo `Var#end_location`
FnControlOption Nov 24, 2022
ccb84e3
Add back TypeDeclaration location
FnControlOption Nov 24, 2022
c52c105
Merge branch 'master' into ast
asterite Dec 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 77 additions & 1 deletion spec/compiler/parser/parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,32 @@ private def it_parses(string, expected_node, file = __FILE__, line = __LINE__)
end
end

private def location_to_index(string, location)
index = 0
line = 1
while line != location.line_number
index = string.index('\n', index).not_nil! + 1
line += 1
end
FnControlOption marked this conversation as resolved.
Show resolved Hide resolved
index + location.column_number - 1
end

private def source_between(string, loc, end_loc)
beginning = location_to_index(string, loc.not_nil!)
ending = location_to_index(string, end_loc.not_nil!)
string[beginning..ending]
end

private def assert_end_location(source, line_number = 1, column_number = source.size, file = __FILE__, line = __LINE__)
it "gets corrects end location for #{source.inspect}", file, line do
parser = Parser.new("#{source}; 1")
string = "#{source}; 1"
parser = Parser.new(string)
node = parser.parse.as(Expressions).expressions[0]
loc = node.location.not_nil!
end_loc = node.end_location.not_nil!
end_loc.line_number.should eq(line_number)
end_loc.column_number.should eq(column_number)
source_between(string, loc, end_loc).should eq(source)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this expectation? I don't see how the source string comparison adds any relevant information to just the end location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s useful for checking if the start location is set correctly (so maybe I should move this to a separate test). However, I think the string comparison is easier to understand at a glance than line/column numbers (when the test fails), so perhaps we can delete the number comparisons altogether?

end
end

Expand Down Expand Up @@ -2023,6 +2042,45 @@ module Crystal
assert_end_location "extend Foo"
assert_end_location "1.as(Int32)"
assert_end_location "puts obj.foo"
assert_end_location "a, b = 1, 2 if 3"
assert_end_location "abstract def foo(x)"
assert_end_location "::foo"
assert_end_location "foo.[0] = 1"
assert_end_location "x : Foo(A, *B, C)"
assert_end_location "Int[8]?"
assert_end_location "[1, 2,]"
assert_end_location "foo(\n &.block\n)", line_number: 3, column_number: 1
assert_end_location "foo.bar(x) do; end"
assert_end_location "%w(one two)"
assert_end_location "{%\nif foo\n bar\n end\n%}", line_number: 5, column_number: 2
assert_end_location "foo bar, out baz"
assert_end_location "Foo?"
assert_end_location "foo : Foo.class"
assert_end_location "foo : Foo?"
assert_end_location "foo : Foo*"
assert_end_location "foo : Foo**"
assert_end_location "foo : Foo[42]"
assert_end_location "foo ->bar"
assert_end_location "foo ->bar="
assert_end_location "foo ->self.bar"
assert_end_location "foo ->self.bar="
assert_end_location "foo ->Bar.baz"
assert_end_location "foo ->Bar.baz="
assert_end_location "foo ->@bar.baz"
assert_end_location "foo ->@bar.baz="
assert_end_location "foo ->@@bar.baz"
assert_end_location "foo ->@@bar.baz="
assert_end_location "foo ->bar(Baz)"
assert_end_location "foo *bar"
assert_end_location "foo **bar"
assert_end_location "Foo { 1 }"
assert_end_location "foo.!"
assert_end_location "foo.!()"
assert_end_location "f.x = foo"
assert_end_location "f.x=(*foo)"
assert_end_location "f.x=(foo).bar"
assert_end_location "x : Foo ->"
assert_end_location "x : Foo -> Bar"

assert_syntax_error %({"a" : 1}), "space not allowed between named argument name and ':'"
assert_syntax_error %({"a": 1, "b" : 2}), "space not allowed between named argument name and ':'"
Expand Down Expand Up @@ -2220,6 +2278,12 @@ module Crystal
node.location.not_nil!.line_number.should eq(1)
node.else_location.not_nil!.line_number.should eq(2)
node.end_location.not_nil!.line_number.should eq(3)

parser = Parser.new("if foo\nend")
node = parser.parse.as(If)
node.location.not_nil!.line_number.should eq(1)
node.else_location.should be_nil
node.end_location.not_nil!.line_number.should eq(2)
end

it "sets correct location of `elsif` of if statement" do
Expand Down Expand Up @@ -2269,6 +2333,18 @@ module Crystal
node.end_location.not_nil!.line_number.should eq(5)
end

it "sets correct location of call name" do
source = "foo(bar)"
node = Parser.new(source).parse.as(Call)
source_between(source, node.name_location, node.name_end_location).should eq ("foo")
end

it "sets correct location of implicit tuple literal of multi-return" do
source = "def foo; return 1, 2; end"
node = Parser.new(source).parse.as(Def).body.as(Return).exp.not_nil!
source_between(source, node.location, node.end_location).should eq ("1, 2")
end

it "doesn't override yield with macro yield" do
parser = Parser.new("def foo; yield 1; {% begin %} yield 1 {% end %}; end")
a_def = parser.parse.as(Def)
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/syntax/ast.cr
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ module Crystal
loc = @name_location
return unless loc

Location.new(loc.filename, loc.line_number, loc.column_number + name_size)
Location.new(loc.filename, loc.line_number, loc.column_number + name_size - 1)
end

def_equals_and_hash obj, name, args, block, block_arg, named_args, global?
Expand Down
Loading