Skip to content

Commit

Permalink
Fix ASTNode#to_s to use escape sequence correctly (crystal-lang#5842)
Browse files Browse the repository at this point in the history
* Fix ASTNode#to_s to use escape sequence correctly

ASTNode#to_s against StringInterpolation, RegexLiteral, and Call of
backtick operator is buggy, it is missing to use escape sequence.

For example `pp "#{1}\0"` cannot be compiled by current compiler.

* Regex: make append_source as class method for RegexLiteral#to_s

* Use Regex.append_source directly

* Remove escape_regex and use Regex.append_source directly for all

crystal-lang#5842 (comment)

It is super tricky way.
But @straight-shoota said, so it is right way also.
  • Loading branch information
makenowjust authored and chris-huxtable committed Jun 6, 2018
1 parent b457815 commit f722696
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
6 changes: 6 additions & 0 deletions spec/compiler/parser/to_s_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,10 @@ describe "ASTNode#to_s" do
expect_to_s %(if (1 + 2\n3)\n 4\nend)
expect_to_s "%x(whoami)", "`whoami`"
expect_to_s %(begin\n ()\nend)
expect_to_s %q("\e\0\""), %q("\e\u0000\"")
expect_to_s %q("#{1}\0"), %q("#{1}\u0000")
expect_to_s %q(%r{\/\0}), %q(/\/\0/)
expect_to_s %q(%r{#{1}\/\0}), %q(/#{1}\/\0/)
expect_to_s %q(`\n\0`), %q(`\n\u0000`)
expect_to_s %q(`#{1}\n\0`), %q(`#{1}\n\u0000`)
end
10 changes: 5 additions & 5 deletions src/compiler/crystal/syntax/to_s.cr
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ module Crystal

def visit(node : StringInterpolation)
@str << '"'
visit_interpolation node, &.gsub('"', "\\\"")
visit_interpolation node, &.inspect_unquoted
@str << '"'
false
end
Expand Down Expand Up @@ -499,9 +499,9 @@ module Crystal
@str << '`'
case exp
when StringLiteral
@str << exp.value.inspect[1..-2]
@str << exp.value.inspect_unquoted.gsub('`', "\\`")
when StringInterpolation
visit_interpolation exp, &.gsub('`', "\\`")
visit_interpolation exp, &.inspect_unquoted.gsub('`', "\\`")
end
@str << '`'
false
Expand Down Expand Up @@ -968,9 +968,9 @@ module Crystal
@str << '/'
case exp = node.value
when StringLiteral
@str << exp.value.gsub('/', "\\/")
Regex.append_source exp.value, @str
when StringInterpolation
visit_interpolation exp, &.gsub('/', "\\/")
visit_interpolation(exp) { |s| Regex.append_source s, @str }
end
@str << '/'
@str << 'i' if node.options.includes? Regex::Options::IGNORE_CASE
Expand Down
7 changes: 4 additions & 3 deletions src/regex.cr
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ class Regex
# ```
def inspect(io : IO)
io << '/'
append_source(io)
Regex.append_source(source, io)
io << '/'
io << 'i' if options.ignore_case?
io << 'm' if options.multiline?
Expand Down Expand Up @@ -514,11 +514,12 @@ class Regex
io << 'x' unless options.extended?

io << ':'
append_source(io)
Regex.append_source(source, io)
io << ')'
end

private def append_source(io)
# :nodoc:
def self.append_source(source, io) : Nil
reader = Char::Reader.new(source)
while reader.has_next?
case char = reader.current_char
Expand Down

0 comments on commit f722696

Please sign in to comment.