-
Notifications
You must be signed in to change notification settings - Fork 7
Fix parsing of quoted keys #41
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
Conversation
We mustn't unconditionally call unescape_str. It might be a raw string (single quotes). Rather than open-coding this, call tokenize_string which already knows all the details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM.
subtest 'no dequoting of raw string keys' => sub { | ||
my $funky_string = "a \\\\ \\x42 b \\\""; | ||
diag 'RAW FUNKY STRING'; | ||
diag $funky_string; | ||
diag ''; | ||
my $toml = <<END; | ||
key.'$funky_string' = 'a' | ||
itable = { '$funky_string' = 'b' } | ||
[table.'$funky_string'] | ||
x = 'c' | ||
[[array.'$funky_string']] | ||
y = 'd' | ||
END | ||
diag 'INPUT TOML'; | ||
diag $toml; | ||
diag ''; | ||
my $data = eval { from_toml($toml) }; | ||
my $error = $@; | ||
ok !$error, "parsed ok" or diag $error; | ||
diag 'PARSED AS'; | ||
diag Dumper($data); | ||
diag ''; | ||
ok $data->{key}{$funky_string} eq 'a', "key.funky"; | ||
ok $data->{itable}{$funky_string} eq 'b', "not in inline table"; | ||
ok $data->{table}{$funky_string}{x} eq 'c', "not in table heading"; | ||
ok $data->{array}{$funky_string}[0]{y} eq 'd', "not in array item heading"; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the extra tests. Could we make the diagnostic output optional? That would keep the default test output more succinct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I'm not sure I've done it in the best way, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird. This actually fails for me:
perl t/parser.t
# Seeded srand with seed '20250324' from local date.
Use of uninitialized value in string eq at t/parser.t line 68.
not ok 1 - oddballs and regressions {
ok 1 - strings that look like numbers {
ok 1 - strings do not inflate as integers
ok 2 - integers do inflate with inflate_integer
ok 3 - strings do not inflate as floats
ok 4 - floats do inflate with inflate_float
1..4
}
not ok 2 - no dequoting of raw string keys {
ok 1 - parsed ok
not ok 2 - key.funky
# Failed test 'key.funky'
# at t/parser.t line 68.
#
# RAW FUNKY STRING
# a \\ \x42 b \"
#
# INPUT TOML
# key.'a \\ \x42 b \"' = 'a'
# itable = { 'a \\ \x42 b \"' = 'b' }
# [table.'a \\ \x42 b \"']
# x = 'c'
# [[array.'a \\ \x42 b \"']]
# y = 'd'
#
# PARSED AS
# $VAR1 = {
# "array" => {
# "a \\ \\x42 b \"" => [
# {
# "y" => "d"
# }
# ]
# },
# "itable" => {
# "a \\ \\x42 b \"" => "b"
# },
# "key" => {
# "a \\ \\x42 b \"" => "a"
# },
# "table" => {
# "a \\ \\x42 b \"" => {
# "x" => "c"
# }
# }
# };
1..2
}
# Failed test 'no dequoting of raw string keys'
# at t/parser.t line 74.
1..2
}
# Failed test 'oddballs and regressions'
# at t/parser.t line 75.
1..1
Summary of my perl5 (revision 5 version 40 subversion 0) configuration:
Platform:
osname=darwin
osvers=23.6.0
archname=darwin-2level
uname='darwin olaf-h4tjn0jbq6x9 23.6.0 darwin kernel version 23.6.0: mon jul 29 21:14:21 pdt 2024; root:xnu-10063.141.2~1release_arm64_t8103 arm64 '
config_args='-Dprefix=/Users/olaf/.plenv/versions/5.40.0 -de -Dversiononly -A'eval:scriptdir=/Users/olaf/.plenv/versions/5.40.0/bin''
hint=recommended
useposix=true
d_sigaction=define
useithreads=undef
usemultiplicity=undef
use64bitint=define
use64bitall=define
uselongdouble=undef
usemymalloc=n
default_inc_excludes_dot=define
Compiler:
cc='cc'
ccflags ='-fno-common -DPERL_DARWIN -mmacosx-version-min=14.6 -DNO_THREAD_SAFE_QUERYLOCALE -DNO_POSIX_2008_LOCALE -fno-strict-aliasing -pipe -fstack-protector-strong'
optimize='-O3'
cppflags='-fno-common -DPERL_DARWIN -mmacosx-version-min=14.6 -DNO_THREAD_SAFE_QUERYLOCALE -DNO_POSIX_2008_LOCALE -fno-strict-aliasing -pipe -fstack-protector-strong'
ccversion=''
gccversion='Apple LLVM 15.0.0 (clang-1500.3.9.4)'
gccosandvers=''
intsize=4
longsize=8
ptrsize=8
doublesize=8
byteorder=12345678
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=8
longdblkind=0
ivtype='long'
ivsize=8
nvtype='double'
nvsize=8
Off_t='off_t'
lseeksize=8
alignbytes=8
prototype=define
Linker and Libraries:
ld='cc'
ldflags =' -mmacosx-version-min=14.6 -fstack-protector-strong -L/usr/local/lib'
libpth=/Library/Developer/CommandLineTools/usr/lib/clang/15.0.0/lib /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/lib /Library/Developer/CommandLineTools/usr/lib /usr/local/lib /usr/lib
libs=
perllibs=
libc=
so=dylib
useshrplib=false
libperl=libperl.a
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_dlopen.xs
dlext=bundle
d_dlsymun=undef
ccdlflags=' '
cccdlflags=' '
lddlflags=' -mmacosx-version-min=14.6 -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector-strong'
Characteristics of this binary (from libperl):
Compile-time options:
HAS_LONG_DOUBLE
HAS_STRTOLD
HAS_TIMES
PERLIO_LAYERS
PERL_COPY_ON_WRITE
PERL_DONT_CREATE_GVSV
PERL_HASH_FUNC_SIPHASH13
PERL_HASH_USE_SBOX32
PERL_MALLOC_WRAP
PERL_OP_PARENT
PERL_PRESERVE_IVUV
PERL_USE_SAFE_PUTENV
USE_64_BIT_ALL
USE_64_BIT_INT
USE_LARGE_FILES
USE_LOCALE
USE_LOCALE_COLLATE
USE_LOCALE_CTYPE
USE_LOCALE_NUMERIC
USE_LOCALE_TIME
USE_PERLIO
USE_PERL_ATOF
Built under darwin
Compiled at Aug 13 2024 12:30:06
@inc:
/Users/olaf/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0/darwin-2level
/Users/olaf/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0
/Users/olaf/.plenv/versions/5.40.0/lib/perl5/5.40.0/darwin-2level
/Users/olaf/.plenv/versions/5.40.0/lib/perl5/5.40.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fixed now. 👍🏻
Given the changes to structure here, I have checked that deliberately introducing discrepancies in the test case still causes test failures. It would be nice if there were some automatic way of including common debug output to be included iff any item in test case fails, but I'm not aware of one.
See also #40. It turned out that this didn't overlap in tests or in the code.