Skip to content

Commit 4a8d4f2

Browse files
committed
Warn if non-protocol functions are defined in protocol modules
Previous approaches tried removing macros but they could always be added back. Therefore we now check for non-protocol functions in a before compile callback.
1 parent 6460c64 commit 4a8d4f2

File tree

5 files changed

+17
-20
lines changed

5 files changed

+17
-20
lines changed

lib/elixir/lib/list/chars.ex

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,6 @@ defprotocol List.Chars do
1919
"""
2020
@spec to_charlist(t) :: charlist
2121
def to_charlist(term)
22-
23-
@doc false
24-
@deprecated "Use List.Chars.to_charlist/1 instead"
25-
Kernel.def to_char_list(term) do
26-
__MODULE__.to_charlist(term)
27-
end
2822
end
2923

3024
defimpl List.Chars, for: Atom do

lib/elixir/lib/module/types.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ defmodule Module.Types do
8080
kind in [:def, :defmacro],
8181
reduce: {[], context()} do
8282
{types, context} ->
83-
# Optimized version of finder, since we already the definition
83+
# Optimized version of finder, since we already have the definition
8484
finder = fn _ ->
8585
default_domain(infer_mode(kind, infer_signatures?), def, fun_arity, impl)
8686
end

lib/elixir/lib/protocol.ex

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,8 +787,7 @@ defmodule Protocol do
787787
@before_compile Protocol
788788

789789
# We don't allow function definition inside protocols
790-
import Kernel,
791-
except: [def: 1, def: 2, defdelegate: 2, defguard: 1, defguardp: 1]
790+
import Kernel, except: [def: 1, def: 2]
792791

793792
# Import the new `def` that is used by protocols
794793
import Protocol, only: [def: 1]
@@ -851,6 +850,21 @@ defmodule Protocol do
851850
)
852851
end
853852

853+
extra =
854+
((Module.definitions_in(env.module, :def) ++ Module.definitions_in(env.module, :defmacro)) --
855+
functions) --
856+
[impl_for: 1, impl_for!: 1, __protocol__: 1, __deriving__: 2, __deriving__: 3]
857+
858+
# TODO: Make an error on Elixir v2.0
859+
if extra != [] do
860+
warn(
861+
"protocols can only define functions without implementation via def/1, found: " <>
862+
Enum.map_join(extra, ", ", fn {name, arity} -> "#{name}/#{arity}" end),
863+
env,
864+
nil
865+
)
866+
end
867+
854868
callback_metas = callback_metas(env.module, :callback)
855869
callbacks = :maps.keys(callback_metas)
856870

lib/elixir/test/elixir/fixtures/consolidation/sample.ex

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,4 @@ defprotocol Protocol.ConsolidationTest.Sample do
44
@deprecated "Reason"
55
@spec ok(t) :: boolean
66
def ok(term)
7-
8-
# Not a protocol function. While this is not "officially" supported,
9-
# it does happen in practice, so we need to make sure we preserve
10-
# its signature.
11-
Kernel.def(regular_fun(term), do: term + 1)
127
end

lib/elixir/test/elixir/protocol/consolidation_test.exs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,6 @@ defmodule Protocol.ConsolidationTest do
238238
assert %{{:ok, 1} => %{sig: {:strong, nil, clauses}}} = exports
239239
assert clauses == [{[none()], dynamic()}]
240240
end
241-
242-
test "handles regular function definitions" do
243-
exports = exports(sample_binary())
244-
245-
assert %{{:regular_fun, 1} => %{sig: :none}} = exports
246-
end
247241
end
248242

249243
test "consolidation errors on missing BEAM files" do

0 commit comments

Comments
 (0)