Skip to content

Commit

Permalink
Merge pull request #7 from CocoaPods/seg-parse-error-context
Browse files Browse the repository at this point in the history 
[Reader] Add context to parse errors
  • Loading branch information
@segiddins
segiddins authored Nov 2, 2016
2 parents 201a2c9 + 690486d commit 46d604e
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 21 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

##### Enhancements

* None.
* Add context to parse errors.
[Samuel Giddins](https://github.com/segiddins)
[#5](https://github.com/CocoaPods/Nanaimo/issues/5)

##### Bug Fixes

Expand Down
46 changes: 36 additions & 10 deletions lib/nanaimo/reader.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,32 @@ class ParseError < Error
# @return [String] The contents of the plist.
#
attr_accessor :plist_string

def to_s
"[!] #{super}#{context}"
end

def context(n = 2)
line_number, column = location
line_number -= 1
lines = plist_string.split(NEWLINE)

s = line_number.succ.to_s.size
indent = "#{' ' * s}# "
indicator = "#{line_number.succ}> "

m = ::String.new("\n")
m << "#{indent}-------------------------------------------\n"
m << lines[[line_number - n, 0].max...line_number].map do |l|
"#{indent}#{l}\n"
end.join
m << "#{indicator}#{lines[line_number]}\n"
m << ' ' * (column + s + 2) << "^\n"
m << Array(lines[line_number.succ..[lines.count.pred, line_number + n].min]).map do |l|
l.strip.empty? ? '' : "#{indent}#{l}\n"
end.join
m << "#{indent}-------------------------------------------\n"
end
end

# @param plist_contents [String]
Expand Down Expand Up @@ -73,7 +99,7 @@ def parse!
root_object = parse_object

eat_whitespace!
raise_parser_error ParseError, "unrecognized characters #{@scanner.rest.inspect} after parsing" unless @scanner.eos?
raise_parser_error ParseError, 'Found additional characters after parsing the root plist object' unless @scanner.eos?

Nanaimo::Plist.new(root_object, plist_format)
end
Expand All @@ -93,7 +119,7 @@ def read_string_encoding
def parse_object
_comment = skip_to_non_space_matching_annotations
start_pos = @scanner.pos
raise_parser_error ParseError, 'Unexpected eos while parsing' if @scanner.eos?
raise_parser_error ParseError, 'Unexpected end of string while parsing' if @scanner.eos?
if @scanner.skip(/\{/)
parse_dictionary
elsif @scanner.skip(/\(/)
Expand All @@ -112,15 +138,15 @@ def parse_object

def parse_string
eat_whitespace!
unless match = @scanner.scan(%r{[\w/.$-]+}o)
raise_parser_error ParseError, "not a valid string at index #{@scanner.pos} (char is #{current_character.inspect})"
unless match = @scanner.scan(%r{[\w/.$]+})
raise_parser_error ParseError, "Invalid character #{current_character.inspect} in unquoted string"
end
Nanaimo::String.new(match, nil)
end

def parse_quotedstring(quote)
unless string = @scanner.scan(/(?:([^#{quote}\\]|\\.)*)#{quote}/)
raise_parser_error ParseError, "unterminated quoted string started at #{@scanner.pos}, expected #{quote} but never found it"
raise_parser_error ParseError, "Unterminated quoted string, expected #{quote} but never found it"
end
string = Unicode.unquotify_string(string.chomp!(quote))
Nanaimo::QuotedString.new(string, nil)
Expand All @@ -137,7 +163,7 @@ def parse_array
eat_whitespace!
break if @scanner.skip(/\)/)
unless @scanner.skip(/,/)
raise_parser_error ParseError, "Array #{objects} missing ',' in between objects"
raise_parser_error ParseError, "Array missing ',' in between objects"
end
end

Expand All @@ -153,7 +179,7 @@ def parse_dictionary
key = parse_object
eat_whitespace!
unless @scanner.skip(/=/)
raise_parser_error ParseError, "Dictionary missing value after key #{key.inspect} at index #{@scanner.pos}, expected '=' and got #{current_character.inspect}"
raise_parser_error ParseError, "Dictionary missing value for key #{key.as_ruby.inspect}, expected '=' and found #{current_character.inspect}"
end

value = parse_object
Expand All @@ -162,7 +188,7 @@ def parse_dictionary
eat_whitespace!
break if @scanner.skip(/}/)
unless @scanner.skip(/;/)
raise_parser_error ParseError, "Dictionary (#{objects}) missing ';' after key-value pair (#{key} = #{value}) at index #{@scanner.pos} (got #{current_character})"
raise_parser_error ParseError, "Dictionary missing ';' after key-value pair for #{key.as_ruby.inspect}, found #{current_character.inspect}"
end
end

Expand All @@ -189,7 +215,7 @@ def current_character

def read_singleline_comment
unless comment = @scanner.scan_until(NEWLINE)
raise_parser_error ParseError, "failed to terminate single line comment #{@scanner.rest.inspect}"
raise_parser_error ParseError, 'Failed to terminate single line comment'
end
comment
end
Expand All @@ -208,7 +234,7 @@ def eat_whitespace!

def read_multiline_comment
unless annotation = @scanner.scan(%r{(?:.+?)(?=\*/)}m)
raise_parser_error ParseError, "#{@scanner.rest.inspect} failed to terminate multiline comment"
raise_parser_error ParseError, 'Failed to terminate multiline comment'
end
@scanner.skip(%r{\*/})

Expand Down
2 changes: 1 addition & 1 deletion lib/nanaimo/writer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def write_object(object)
when Data
write_data(object)
else
raise "Cannot write #{object} to an ascii plist"
raise "Cannot write #{object.inspect} to an ascii plist"
end
write_annotation(object) if pretty
output
Expand Down
214 changes: 205 additions & 9 deletions spec/nanaimo/reader_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,6 @@ module Nanaimo
expect(subject).to eq Nanaimo::Dictionary.new({ Nanaimo::String.new('key', '') => Nanaimo::String.new('$PROJECT_DIR/mogenerator/mogenerator', '') }, '')
end
end

describe 'that contain `-`' do
let(:unquoted_string) { 'foo/bar-baz' }

it 'are parsed correctly' do
expect(subject).to eq Nanaimo::Dictionary.new({ Nanaimo::String.new('key', '') => Nanaimo::String.new('foo/bar-baz', '') }, '')
end
end
end

describe 'quoted strings' do
Expand All @@ -147,9 +139,213 @@ module Nanaimo
let(:data) { '<12 3>' }

it 'raises an informative error' do
expect { subject }.to raise_error(Reader::ParseError, 'Data has an uneven number of hex digits')
expect { subject }.to raise_error(Reader::ParseError, <<-E)
[!] Data has an uneven number of hex digits
# -------------------------------------------
1> {key = <12 3>}
^
# -------------------------------------------
E
end
end
end

describe 'parse errors' do
shared_examples_for 'parse errors' do |name, plist, expected_error|
it "raises an informative error #{name}" do
if expected_error.is_a?(::String)
prefix = expected_error.scan(/^[ \t]*(?=\S)/).min
expected_error.gsub!(/^#{prefix}/, '')
end

expect { Reader.new(plist).parse! }
.to raise_error(Reader::ParseError) do |e|
if e.is_a?(Regexp)
expect(e.to_s).to match(expected_error)
else
expect(e.to_s).to eq(expected_error)
end
end
end
end

include_examples 'parse errors',
'with a dictionary without an `=`',
'{ a = ; }',
<<-E
[!] Invalid character ";" in unquoted string
# -------------------------------------------
1> { a = ; }
^
# -------------------------------------------
E

include_examples 'parse errors',
'with an unterminated array',
<<-PLIST,
(
a,
b,
(
c
)
PLIST
<<-E
[!] Array missing ',' in between objects
# -------------------------------------------
# c
# )
7>\s\s
^
# -------------------------------------------
E

include_examples 'parse errors',
'with an error in the middle of the plist',
<<-PLIST,
(
#{"a,\n" * 1000}
c,
d,
e,,,
f,
g,
#{"z,\n" * 250}
zz
)
PLIST
<<-E
[!] Invalid character "," in unquoted string
# -------------------------------------------
# c,
# d,
1005> e,,,
^
# f,
# g,
# -------------------------------------------
E

include_examples 'parse errors',
'with an array missing a comma between elements',
<<-PLIST,
(
a,
b,
(
c
d
)
)
PLIST
<<-E
[!] Array missing ',' in between objects
# -------------------------------------------
# (
# c
6> d
^
# )
# )
# -------------------------------------------
E

include_examples 'parse errors',
'with an unterminated dictionary',
<<-PLIST,
{
a = e;
b = j;
d = (
c
);
PLIST
<<-E
[!] Unexpected end of string while parsing
# -------------------------------------------
# c
# );
7>\s\s
^
# -------------------------------------------
E

include_examples 'parse errors',
'with an unterminated dictionary pair',
<<-PLIST,
{
a = e;
b = j;
d = (
c
)
PLIST
<<-E
[!] Dictionary missing ';' after key-value pair for "d", found ""
# -------------------------------------------
# c
# )
7>\s\s
^
# -------------------------------------------
E

include_examples 'parse errors',
'with a dictionary without an `=`',
'{a}',
<<-E
[!] Dictionary missing value for key "a", expected '=' and found "}"
# -------------------------------------------
1> {a}
^
# -------------------------------------------
E

include_examples 'parse errors',
'with a dictionary without a value',
'{ a = ; }',
<<-E
[!] Invalid character ";" in unquoted string
# -------------------------------------------
1> { a = ; }
^
# -------------------------------------------
E

include_examples 'parse errors',
'with an unterminated quoted string',
"{\na = 'bcd\n}",
<<-E
[!] Unterminated quoted string, expected ' but never found it
# -------------------------------------------
# {
2> a = 'bcd
^
# }
# -------------------------------------------
E

include_examples 'parse errors',
'with non-whitespace after the root object',
'abcd a',
<<-E
[!] Found additional characters after parsing the root plist object
# -------------------------------------------
1> abcd a
^
# -------------------------------------------
E

include_examples 'parse errors',
'when a data is unterminated',
'<1234',
<<-E
[!] Data missing closing '>'
# -------------------------------------------
1> <1234
^
# -------------------------------------------
E
end
end
end

0 comments on commit 46d604e

Please sign in to comment.