review comments

elixir-ecto · Jan 9, 2025 · 6be76a6 · 6be76a6
1 parent c161ae3
commit 6be76a6
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 53 deletions.
diff --git a/lib/ecto/query/api.ex b/lib/ecto/query/api.ex
@@ -479,19 +479,6 @@ defmodule Ecto.Query.API do
   """
   def fragment(fragments), do: doc!([fragments])
 
-  @doc """
-  Allows a literal identifier to be injected into a fragment:
-
-      collation = "es_ES"
-      fragment("? COLLATE ?", ^name, literal(^collation))
-
-  The example above will inject `collation` into the query as
-  a literal identifier instead of a query parameter. Note that
-  each different value of `collation` will emit a different query,
-  which will be independently prepared and cached.
-  """
-  def literal(binary), do: doc!([binary])
-
   @doc """
   Allows a dynamic identifier to be injected into a fragment:
 

diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex
@@ -751,18 +751,15 @@ defmodule Ecto.Query.Builder do
     )
   end
 
-  defp escape_fragment({:literal, _meta, [expr]}, params_acc, _vars, _env) do
-    case expr do
-      {:^, _, [expr]} ->
-        checked = quote do: Ecto.Query.Builder.literal!(unquote(expr))
-        escaped = {:{}, [], [:literal, [], [checked]]}
-        {escaped, params_acc}
+  defp escape_fragment({:literal, meta, [expr]}, params_acc, vars, env) do
+    env = if {env, _fun} = env, do: env, else: env
 
-      _ ->
-        error!(
-          "literal/1 in fragment expects an interpolated value, such as literal(^value), got `#{Macro.to_string(expr)}`"
-        )
-    end
+    IO.warn(
+      "`literal/1` is deprecated. Please use `identifier/1` instead.",
+      Macro.Env.stacktrace(env)
+    )
+
+    escape_fragment({:identifier, meta, [expr]}, params_acc, vars, env)
   end
 
   defp escape_fragment({:identifier, _meta, [expr]}, params_acc, _vars, _env) do
@@ -1281,18 +1278,6 @@ defmodule Ecto.Query.Builder do
     end
   end
 
-  @doc """
-  Called by escaper at runtime to verify literal in fragments.
-  """
-  def literal!(literal) do
-    if is_binary(literal) do
-      literal
-    else
-      raise ArgumentError,
-            "literal(^value) expects `value` to be a string, got `#{inspect(literal)}`"
-    end
-  end
-
   @doc """
   Called by escaper at runtime to verify identifier in fragments.
   """

diff --git a/test/ecto/query_test.exs b/test/ecto/query_test.exs
@@ -976,23 +976,6 @@ defmodule Ecto.QueryTest do
       end
     end
 
-    test "supports literals" do
-      query = from p in "posts", select: fragment("? COLLATE ?", p.name, literal(^"es_ES"))
-      assert {:fragment, _, parts} = query.select.expr
-
-      assert [
-               raw: "",
-               expr: {{:., _, [{:&, _, [0]}, :name]}, _, _},
-               raw: " COLLATE ",
-               expr: {:literal, _, ["es_ES"]},
-               raw: ""
-             ] = parts
-
-      assert_raise ArgumentError, "literal(^value) expects `value` to be a string, got `123`", fn ->
-        from p in "posts", select: fragment("? COLLATE ?", p.name, literal(^123))
-      end
-    end
-
     test "supports identifiers" do
       query = from p in "posts", select: fragment("? COLLATE ?", p.name, identifier(^"es_ES"))
       assert {:fragment, _, parts} = query.select.expr