From 77ba38b4bdd6477caf934992dc47cc4ef1c0bb11 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Tue, 19 Mar 2024 14:24:33 +0100 Subject: [PATCH 1/8] Set up eqWAlizer --- rebar.config | 18 ++++++++++++++++-- src/exml.app.src | 3 ++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/rebar.config b/rebar.config index 7c017f5..9489a0b 100644 --- a/rebar.config +++ b/rebar.config @@ -1,4 +1,10 @@ -{deps, []}. +{deps, [ + {eqwalizer_support, + {git_subdir, + "https://github.com/whatsapp/eqwalizer.git", + {branch, "main"}, + "eqwalizer_support"}} +]}. {dialyzer, [{warnings, @@ -25,7 +31,15 @@ ]} ]}. -{project_plugins, [rebar3_ex_doc]}. +{project_plugins, + [ + rebar3_ex_doc, + {eqwalizer_rebar3, + {git_subdir, + "https://github.com/whatsapp/eqwalizer.git", + {branch, "main"}, + "eqwalizer_rebar3"}} + ]}. {plugins, [pc, rebar3_hex]}. % Interrupt compilation, if the artifact is not found diff --git a/src/exml.app.src b/src/exml.app.src index d3d734b..fbcafce 100644 --- a/src/exml.app.src +++ b/src/exml.app.src @@ -4,7 +4,8 @@ {registered, []}, {applications, [kernel, - stdlib + stdlib, + eqwalizer_support ]}, {env, []}, {modules, []}, From ca55de81cd90a0f1a9c107b305b56f78cc22fbc1 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Tue, 19 Mar 2024 16:36:17 +0100 Subject: [PATCH 2/8] Fix some elp type errors in src/exml.erl --- src/exml.erl | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/exml.erl b/src/exml.erl index 486ae47..b63f562 100644 --- a/src/exml.erl +++ b/src/exml.erl @@ -50,7 +50,8 @@ xml_size(#xmlstreamstart{ name = Name, attrs = Attrs }) -> byte_size(Name) + 2 + xml_size(Attrs); xml_size(#xmlstreamend{ name = Name }) -> byte_size(Name) + 3; -xml_size({Key, Value}) -> +xml_size({Key, Value}) when is_binary(Key) -> + % Attributes byte_size(Key) + 4 % ="" and whitespace before + byte_size(Value). @@ -66,7 +67,12 @@ xml_size({Key, Value}) -> %% @end %% The implementation of this function is a subtle modification of %% https://github.com/erszcz/rxml/commit/e8483408663f0bc2af7896e786c1cdea2e86e43d --spec xml_sort(item() | [item()]) -> item() | [item()]. +-spec xml_sort([item()]) -> [item()]; + (element()) -> element(); + (attr()) -> attr(); + (cdata()) -> cdata(); + (exml_stream:start()) -> exml_stream:start(); + (exml_stream:stop()) -> exml_stream:stop(). xml_sort(#xmlcdata{} = Cdata) -> Cdata; xml_sort(#xmlel{ attrs = Attrs, children = Children } = El) -> @@ -130,7 +136,10 @@ to_iolist([#xmlstreamstart{name = Name, attrs = Attrs} | Tail] = Elements, Prett case Last of #xmlstreamend{name = Name} -> %% Add extra nesting for streams so pretty-printing would be indented properly - Element = #xmlel{name = Name, attrs = Attrs, children = lists:reverse(RevChildren)}, + Children = lists:reverse(RevChildren), + %% The cast is safe, since we eliminated #xmlstreamstart{} and #xmlstreamend{}. + %% The rest are element()s, which are valid children. + Element = #xmlel{name = Name, attrs = Attrs, children = eqwalizer:dynamic_cast(Children)}, to_binary_nif(Element, Pretty); _ -> [to_iolist(El, Pretty) || El <- Elements] From bca39e9278adb62c7b707bd115467768eedfde9c Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Tue, 19 Mar 2024 16:41:01 +0100 Subject: [PATCH 3/8] Fix some elp type errors in src/exml_nif.erl --- src/exml_nif.erl | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/exml_nif.erl b/src/exml_nif.erl index 9509a4e..2c244b3 100644 --- a/src/exml_nif.erl +++ b/src/exml_nif.erl @@ -25,9 +25,15 @@ load() -> PrivDir = case code:priv_dir(?MODULE) of {error, _} -> - EbinDir = filename:dirname(code:which(?MODULE)), - AppPath = filename:dirname(EbinDir), - filename:join(AppPath, "priv"); + case code:which(?MODULE) of + Path when is_list(Path) -> + EbinDir = filename:dirname(Path), + AppPath = filename:dirname(EbinDir), + filename:join(AppPath, "priv"); + _ -> + %% cover_compiled | preloaded | non_existing + erlang:error({cannot_get_load_path, ?MODULE}) + end; Path -> Path end, From 07c4c1e481bc2444fbf13f910d9cc17733fce4ac Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Wed, 20 Mar 2024 10:23:11 +0100 Subject: [PATCH 4/8] Fix elp error: subelement/3 might return 'undefined' --- src/exml_query.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/exml_query.erl b/src/exml_query.erl index 2f7dadc..c17ee79 100644 --- a/src/exml_query.erl +++ b/src/exml_query.erl @@ -74,7 +74,7 @@ path(Element, Path) -> %% ''' %% will return `<<"Message from bob to alice">>' %% @end --spec path(exml:element(), path(), Default) -> exml:element() | binary() | Default. +-spec path(exml:element() | undefined, path(), Default) -> exml:element() | binary() | Default. path(#xmlel{} = Element, [], _) -> Element; path(#xmlel{} = Element, [{element, Name} | Rest], Default) -> From 48b9987c8383fc0c0e7e727355d3cea05862c68b Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Wed, 20 Mar 2024 10:23:33 +0100 Subject: [PATCH 5/8] Fix elp error: make sure subelement/3 does not return cdata This is tricky, but the result might actually be a cdata in the unlikely case the cdata content matches some element name, like this: elem Check it like this: > {ok, Elem} = exml:parse(<<"elem">>). ... > exml_query:subelement(Elem, <<"elem">>, default). #xmlcdata{content = <<"elem">>} This is because #xmlel.name is just an index and both #xmlel{} and #xmlcdata{} records do have a field with this index - it's just different fields. So the spec should properly specify this function can technically return a cdata... Or we can make sure it's a #xmlel{} by matching on Result. --- src/exml_query.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/exml_query.erl b/src/exml_query.erl index c17ee79..0b61478 100644 --- a/src/exml_query.erl +++ b/src/exml_query.erl @@ -129,10 +129,10 @@ subelement(Element, Name) -> -spec subelement(exml:element(), binary(), Default) -> exml:element() | Default. subelement(#xmlel{children = Children}, Name, Default) -> case lists:keyfind(Name, #xmlel.name, Children) of + #xmlel{} = Result -> + Result; false -> - Default; - Result -> - Result + Default end. %% @equiv path(Element, [{element_with_ns, NS}]) From caa867717566418562388b3eed33a4982049de47 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Wed, 20 Mar 2024 10:30:05 +0100 Subject: [PATCH 6/8] Add dynamic casts where we're certain of the returned types --- src/exml_query.erl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/exml_query.erl b/src/exml_query.erl index 0b61478..0e725e0 100644 --- a/src/exml_query.erl +++ b/src/exml_query.erl @@ -161,7 +161,9 @@ child_with_ns([_ | Rest], NS, Default) -> -spec subelement_with_attr(exml:element(), AttrName :: binary(), AttrValue :: binary()) -> exml:element() | undefined. subelement_with_attr(Element, AttrName, AttrValue) -> - subelement_with_attr(Element, AttrName, AttrValue, undefined). + %% eqwalizer can't tell that Default in subelement_with_attr/4 + %% in this case WILL be just 'undefined', which is a valid return type from this function. + eqwalizer:dynamic_cast(subelement_with_attr(Element, AttrName, AttrValue, undefined)). %% @equiv path(Element, [{element_with_attr, AttrName, AttrValue}], Default) -spec subelement_with_attr(Element, AttrName, AttrValue, Default) -> SubElement | Default when @@ -209,7 +211,7 @@ subelements(#xmlel{children = Children}, Name) -> true; (_) -> false - end, Children). + end, eqwalizer:dynamic_cast(Children)). %% @equiv paths(Element, [{element_with_ns, NS}]) -spec subelements_with_ns(exml:element(), binary()) -> [exml:element()]. @@ -218,7 +220,7 @@ subelements_with_ns(#xmlel{children = Children}, NS) -> NS =:= attr(Child, <<"xmlns">>); (_) -> false - end, Children). + end, eqwalizer:dynamic_cast(Children)). %% @equiv paths(Element, [{element_with_ns, Name, NS}]) -spec subelements_with_name_and_ns(exml:element(), binary(), binary()) -> [exml:element()]. @@ -228,7 +230,7 @@ subelements_with_name_and_ns(#xmlel{children = Children}, Name, NS) -> NS =:= attr(Child, <<"xmlns">>); (_) -> false - end, Children). + end, eqwalizer:dynamic_cast(Children)). %% @equiv paths(Element, [{element_with_attr, AttrName, AttrValue}]) -spec subelements_with_attr(exml:element(), binary(), binary()) -> [exml:element()]. @@ -237,7 +239,7 @@ subelements_with_attr(#xmlel{children = Children}, AttrName, Value) -> Value =:= attr(Child, AttrName); (_) -> false - end, Children). + end, eqwalizer:dynamic_cast(Children)). %% @equiv path(Element, [cdata]) -spec cdata(exml:element()) -> binary(). From f1147e8cecb8d5051037c677a2c350a1b0c917c1 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Wed, 20 Mar 2024 10:53:20 +0100 Subject: [PATCH 7/8] Fix invalid spec clause - dialyzer caught that, but eqwalizer didn't --- src/exml.erl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/exml.erl b/src/exml.erl index b63f562..02e1b8d 100644 --- a/src/exml.erl +++ b/src/exml.erl @@ -69,7 +69,6 @@ xml_size({Key, Value}) when is_binary(Key) -> %% https://github.com/erszcz/rxml/commit/e8483408663f0bc2af7896e786c1cdea2e86e43d -spec xml_sort([item()]) -> [item()]; (element()) -> element(); - (attr()) -> attr(); (cdata()) -> cdata(); (exml_stream:start()) -> exml_stream:start(); (exml_stream:stop()) -> exml_stream:stop(). From 4bbb77a53db820e7dc75beddc9bc4b607de5b6c1 Mon Sep 17 00:00:00 2001 From: Radek Szymczyszyn Date: Wed, 20 Mar 2024 12:11:05 +0100 Subject: [PATCH 8/8] Fix EUnit application_test --- test/exml_tests.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/exml_tests.erl b/test/exml_tests.erl index 9fccca1..01de778 100644 --- a/test/exml_tests.erl +++ b/test/exml_tests.erl @@ -16,7 +16,7 @@ -compile(export_all). application_test() -> - ?assertEqual(ok, application:start(exml)), + ?assertMatch({ok, _}, application:ensure_all_started(exml)), ?assertEqual(ok, application:stop(exml)). size_of_normal_xml_test() ->