From 6f2baffa8ac960f19cec99b87e9c5283b33e58e2 Mon Sep 17 00:00:00 2001 From: Nicholas Lundgaard Date: Mon, 3 May 2021 18:47:32 -0500 Subject: [PATCH] Fix issues with nested properties **Problem** I merged #18 a little too soon; It has 3 issues: 1. It doesn't handle comment lines between nested properties 2. It doesn't handle a blank (whitespace) after `=` in the nested property opener like the other forms of properties. 3. The test case was inserted in the wrong location in `eini_tests` module; it should have been in its own test function, and it should have had some error cases. **Solution** 1. Add support for comment lines in between nested properties. 2. Add support for blank after `=` in nested property opener. 3. Refactor and extend the tests for nested properties to cover more ground. **Known Issues** After I merged #18, I noticed this warning: ``` Compiled src/eini_lexer.xrl Parse action conflict scanning symbol blank in state 51: Reduce to properties_nested from property_nested (rule 28 at line 97) vs. shift to state 53, adding right sisters to property_nested. Conflict resolved in favor of shift. ``` It appears that the introduction of nesting has added some ambiguity to the grammar aroud the `blank`, and this change makes it a little worse: ``` Parse action conflict scanning symbol break in state 47: Reduce to single_value from blank (rule 44 at line 121) vs. shift to state 52, adding right sisters to blank. Conflict resolved in favor of shift. Parse action conflict scanning symbol blank in state 58: Reduce to properties_nested from property_nested (rule 30 at line 101) vs. shift to state 61, adding right sisters to property_nested. Conflict resolved in favor of shift. Parse action conflict scanning symbol comment in state 58: Reduce to properties_nested from property_nested (rule 30 at line 101) vs. shift to state 12, adding right sisters to property_nested. Conflict resolved in favor of shift. ``` Now there is some ambiguity around `comment` as well, and this appears to cause an issue parsing INI files with a comment line at the end of a block of nested properties when another (not-nested) key follows. I don't know if we should mark this as a known issue or not, but the trigger case is in a commented unit test in `eini_tests`. --- src/eini_parser.yrl | 20 ++++++-- test/eini_tests.erl | 112 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/src/eini_parser.yrl b/src/eini_parser.yrl index a3dbd37..4529d90 100644 --- a/src/eini_parser.yrl +++ b/src/eini_parser.yrl @@ -28,7 +28,8 @@ Nonterminals title property_with_skip_lines properties property_base property - properties_nested property_nested + properties_nested property_nested_open property_nested + indentation key_part values single_value skip_lines @@ -88,16 +89,23 @@ property_with_skip_lines -> property : '$1'. property_with_skip_lines -> property skip_lines : '$1'. property_base -> key_part '=' values break : - {list_to_atom(value_of('$1')), strip_values('$3')}. + {list_to_atom(value_of('$1')), strip_values('$3')}. property -> property_base : '$1'. -property -> key_part '=' break properties_nested : - {list_to_atom(value_of('$1')), '$4'}. +property -> property_nested_open properties_nested : + {list_to_atom(value_of('$1')), '$2'}. + +property_nested_open -> key_part '=' break : '$1'. +property_nested_open -> key_part '=' blank break : '$1'. properties_nested -> property_nested : ['$1']. properties_nested -> property_nested properties_nested : ['$1' | '$2']. -property_nested -> blank property_base : '$2'. +property_nested -> indentation property_base : '$2'. + +indentation -> blank : $1. +indentation -> blank skip_lines blank : $1. +indentation -> skip_lines blank : $2. key_part -> word : '$1'. key_part -> word blank : '$1'. @@ -116,6 +124,8 @@ single_value -> '[' : "[". single_value -> '=' : "=". single_value -> ']' : "]". +Expect 3. + Erlang code. -spec value_of({atom(), _Line, TokenChars::string()}) -> TokenChars::string(). diff --git a/test/eini_tests.erl b/test/eini_tests.erl index f7ce5dd..989820b 100644 --- a/test/eini_tests.erl +++ b/test/eini_tests.erl @@ -349,20 +349,6 @@ one_section_title_and_one_prop_test_() -> "[title] \n" "key1= value1 \n" )), - %% nested properties: https://docs.aws.amazon.com/credref/latest/refdocs/file-format.html - ?_assertEqual({ok, [ - {title, [{key1,[{key11,<<"value11">>},{key12,<<"value12">>},{key13,<<"value13">>},{key14,<<"value14">>}]},{key2,<<"value2">>}]} - ]}, - parse( - "[title]\n" - "key1 =\n" - " key11=value11\n" - " key12 =value12\n" - " key13= value13\n" - " key14 = value14\n" - "key2=value2\n" - )), - %% value has characters which can not used in titles or keys ?_assertEqual({ok, [ {title, [{key1, <<"value1$% '""#!+*=@/:+">>}]} @@ -461,6 +447,84 @@ binary_two_section_test_() -> )) ]}. +nested_properties_test_() -> + %% https://docs.aws.amazon.com/credref/latest/refdocs/file-format.html + {setup, + fun setup/0, + fun teardown/ 1, + [ + ?_assertEqual({ok, [ + {title, + [{key1, [{key11, <<"value11">>}, + {key12, <<"value12">>}]}]} + ]}, + parse( + "[title]\n" + "key1=\n" + " key11=value11\n" + " key12 =value12\n" + )), + ?_assertEqual({ok, [ + {title, + [{key1, [{key11, <<"value11">>}, + {key12, <<"value12">>}, + {key13, <<"value13">>}, + {key14, <<"value14">>}]}, + {key2, <<"value2">>}]} + ]}, + parse( + "[title]\n" + "key1 =\n" + " key11=value11\n" + " key12 =value12\n" + " key13= value13\n" + " key14 = value14\n" + "key2=value2\n" + )), + ?_assertEqual({ok, [ + {title, + [{key1, [{key11, <<"value11">>}]}, + {key2, <<"value2">>}]} + ]}, + parse( + "[title]\n" + "key1= \n" + " key11=value11\n" + "key2=value2\n" + )), + %% with comments in/between nested properties + ?_assertEqual({ok, [ + {title, + [{key1, [{key11, <<"value11">>}, + {key12, <<"value12">>}]}, + {key2, <<"value2">>}]} + ]}, + parse( + "[title]\n" + "key1= \n" + " ; comment in nested properties\n" + " key11 = value11\n" + " ; comment between nested properties\n" + " key12 = value12\n" + "key2=value2\n" + ))%, + %% with comments at end of nested properties (TODO Fix this case) + % ?_assertEqual({ok, [ + % {title, + % [{key1, [{key11, <<"value11">>}, + % {key12, <<"value12">>}]}, + % {key2, <<"value2">>}]} + % ]}, + % parse( + % "[title]\n" + % "key1= \n" + % " key11 = value11\n" + % " key12 = value12\n" + % "; comment after nested properties\n" + % "key2=value2\n" + % )) + ]}. + lex_error_title_test_() -> {setup, fun setup/0, @@ -534,6 +598,26 @@ syntax_error_property_test_() -> "key;comment=value\n")) ]}. +syntax_error_nested_property_test_() -> + {setup, + fun setup/0, + fun teardown/1, + [ + %% no nested properties after open + ?_assertMatch({error, {syntax_error, 2, ["syntax error before: ", _]}}, + parse( + "[title]\n" + "key1 =\n" + )), + %% no nested properties after open (with following key) + ?_assertMatch({error, {syntax_error, 3, ["syntax error before: ", _]}}, + parse( + "[title]\n" + "key1=\n" + "key2=value2\n" + )) + ]}. + dup_title_test_() -> {setup, fun setup/0,