zachallaun/mneme

Bug with nested templating strings

mindreframer opened this issue · 3 comments

mindreframer commented

Hey, it's me again! I seem to push Mneme rather hard. Another little bug.

test "bug" do
    res =
      {:ok,
       %{
         database: {:literal, "\"my_app_test\#{System.get_env(\"MIX_TEST_PARTITION\")}\""}
       }}

    assert res ==
             {:ok,
              %{database: {:literal, "\"my_app_test\#{System.get_env(\"MIX_TEST_PARTITION\")}\""}}}

    ### THIS fails
    auto_assert(res)
end

Message:

 ** (Mneme.InternalError) Mneme encountered an internal error. This is likely a bug in Mneme.

     Please consider reporting this error at https://github.com/zachallaun/mneme/issues. Thanks!

     ** (SyntaxError) nofile:18:67: unexpected token: "\" (column 67, code point U+005C)
         |
      18 |       {:ok, %{database: {:literal, "\"my_app_test#{System.get_env(\"MIX_TEST_PARTITION\")}\""}}} <-
         |                                                                   ^
         (sourceror 0.12.2) lib/sourceror.ex💯 Sourceror.parse_string!/1
         (rewrite 0.6.3) lib/rewrite/source.ex:649: Rewrite.Source.get_modules/1
         (rewrite 0.6.3) lib/rewrite/source.ex:338: Rewrite.Source.update_modules/2
         (rewrite 0.6.3) lib/rewrite/source.ex:319: Rewrite.Source.update/3
         (mneme 0.2.5) lib/mneme/patcher.ex:106: Mneme.Patcher.patch_assertion/3
         (mneme 0.2.5) lib/mneme/patcher.ex:44: Mneme.Patcher.patch!/4
         (mneme 0.2.5) lib/mneme/server.ex:149: Mneme.Server.do_patch_assertion/2
         (mneme 0.2.5) lib/mneme/server.ex:139: Mneme.Server.handle_continue/2
         (stdlib 4.2) gen_server.erl:1123: :gen_server.try_dispatch/4
         (stdlib 4.2) gen_server.erl:865: :gen_server.loop/7
         (stdlib 4.2) proc_lib.erl:240: :proc_lib.init_p_do_apply/3

Reproducible repo:

zachallaun commented

I sincerely appreciate these reports! Please keep them coming.

It's clear I need to re-evaluate how I'm handling characters that need to be escaped. The underlying issue is that the actual AST used to update the code needs to be different than the one that gets dynamically evaluated to actually run the assertion. For instance, to write a heredoc literal like this:

"""
foo
bar\
"""

You need an AST that looks like this:

{:__block__, [delimiter: ~S|"""|], ["foo\nbar\\\n"]}

But if you read in that heredoc, you get "foo\nbar", and if you eval that AST, you get "foo\nbar\\\n".

And it's like that for a lot of characters that need to be escaped. Unfortunately, I sort of found all these cases haphazardly and they're being handled in different places in the codebase. It's clear from all these bugs that I need to factor all of this escape handling into an independently testable module so that I can more systematically ensure they're being handled.

Anyways, I'll have a "bandaid" fix for this out soon and then will work on more comprehensively handling this.

Edit: the quick fix is probably going to still have a diff highlighting bug: #30. I'll address that a bit later on.

zachallaun commented

@mindreframer Went ahead and released v0.2.7 with the fix.

mindreframer commented

@zachallaun Awesome! I don't know, how you do this, but to say it again - this is crazy good bug resolution time. The highlighting bug does not bother me, I'm using mostly auto-accept during development.

Thanks!

  • ❤️1