|
| 1 | +defmodule Castle.NoMixTasksTest do |
| 2 | + # **Castle ships no Mix tasks.** Every build-time task lives in Forecastle, |
| 3 | + # whatever it is called. |
| 4 | + # |
| 5 | + # The two halves divide one job between them, and since forecastle#24 the |
| 6 | + # build-time tasks are named `castle.*` while still being compiled into |
| 7 | + # Forecastle: the namespace follows the vocabulary a developer thinks in |
| 8 | + # rather than the package that implements it. That leaves `Mix.Tasks.Castle.*` |
| 9 | + # a namespace both projects could write into, and Mix resolves a task by |
| 10 | + # module name alone. If both ever defined the same module, whichever `ebin` |
| 11 | + # came first on the code path would win - and nothing would say which, not the |
| 12 | + # compiler, not Mix, not the release. |
| 13 | + # |
| 14 | + # A convention nobody enforces is a convention that lapses, so it is asserted |
| 15 | + # here rather than remembered. That is the same move the project already makes |
| 16 | + # for the coverage floor, for `verify_relup!/2` and for the |
| 17 | + # `restart_new_emulator` refusal. |
| 18 | + use ExUnit.Case, async: true |
| 19 | + |
| 20 | + # Mix's own rule, and deliberately the same one. `Mix.Task.load_all/0` walks |
| 21 | + # `:code.get_path()` and matches each directory entry against |
| 22 | + # `Elixir.Mix.Tasks.<name>.beam` - a filename, in an `ebin`, with no reference |
| 23 | + # to application metadata anywhere in it. |
| 24 | + @task_beam ~r/^Elixir\.Mix\.Tasks\..+\.beam$/ |
| 25 | + |
| 26 | + # A sample for the heredoc case below. `\"\"\"` is an escaped delimiter rather |
| 27 | + # than a nested heredoc, so `mix format` leaves it alone. |
| 28 | + @heredoc_sample """ |
| 29 | + defmodule Castle.Docs do |
| 30 | + @moduledoc \"\"\" |
| 31 | + Do not do this: |
| 32 | +
|
| 33 | + defmodule Mix.Tasks.Castle.Nope do |
| 34 | + end |
| 35 | + \"\"\" |
| 36 | + end |
| 37 | + """ |
| 38 | + |
| 39 | + # `Application.spec(:castle, :modules)` was the obvious source here and is the |
| 40 | + # wrong one. `Mix.Tasks.Compile.App` fills `:modules` in with |
| 41 | + # `Keyword.put_new_lazy/3`, so a project that supplies its own list in |
| 42 | + # `application/0` *keeps* it - the metadata then says whatever that list says, |
| 43 | + # while Mix goes on finding every task beam in the directory regardless. The |
| 44 | + # metadata is a description of the artefact that something else is allowed to |
| 45 | + # write; the artefact is what Mix reads. |
| 46 | + # |
| 47 | + # `Mix.Project.compile_path/0` is Castle's *own* `ebin`, which is the point: |
| 48 | + # Castle takes Forecastle as a build-time dependency, so Forecastle's `ebin` is |
| 49 | + # on the code path during this very test and `Mix.Tasks.Castle.Relup` is in it. |
| 50 | + # That is Forecastle's and is supposed to be there. A check over the code path |
| 51 | + # would fail on it; this one is scoped to the only side of the collision this |
| 52 | + # project controls. |
| 53 | + test "no Mix task beam is compiled into Castle's ebin" do |
| 54 | + entries = File.ls!(Mix.Project.compile_path()) |
| 55 | + |
| 56 | + # Without this the check could pass by looking at the wrong directory, or at |
| 57 | + # one a failed build left empty: no entries filters to no tasks, which is |
| 58 | + # indistinguishable from a clean result. |
| 59 | + assert "Elixir.Castle.beam" in entries, |
| 60 | + "#{Mix.Project.compile_path()} does not contain Elixir.Castle.beam, so this " <> |
| 61 | + "test was not looking at Castle's compiled output" |
| 62 | + |
| 63 | + tasks = Enum.filter(entries, &Regex.match?(@task_beam, &1)) |
| 64 | + |
| 65 | + assert tasks == [], """ |
| 66 | + Castle must ship no Mix tasks, and these beams are in its ebin: |
| 67 | +
|
| 68 | + #{Enum.map_join(tasks, "\n", &" #{&1}")} |
| 69 | +
|
| 70 | + Mix finds a task by looking for exactly this filename on the code path, so a |
| 71 | + task beam here shares a namespace with the build-time tasks Forecastle |
| 72 | + compiles into its own ebin - which are called `castle.*` precisely because |
| 73 | + the name follows the user's vocabulary and not the package implementing it. |
| 74 | + If both projects ever define the same module, whichever ebin comes first |
| 75 | + wins, silently. |
| 76 | +
|
| 77 | + Every build-time task lives in Forecastle, whatever it is called. Move this |
| 78 | + one there - it can keep the `castle.*` name it has. |
| 79 | + """ |
| 80 | + end |
| 81 | + |
| 82 | + # The beam check can only see what this environment compiled, and `mix test` |
| 83 | + # compiles one. A module defined behind a `Mix.env()` condition would be |
| 84 | + # absent from that build and present in another, so the source is checked too: |
| 85 | + # it is the one form of the answer that does not depend on which environment |
| 86 | + # asked the question. |
| 87 | + test "no Mix task is defined in Castle's lib" do |
| 88 | + lib = Mix.Project.project_file() |> Path.dirname() |> Path.join("lib") |
| 89 | + sources = Path.wildcard(Path.join(lib, "**/*.ex")) |
| 90 | + |
| 91 | + assert Path.join(lib, "castle.ex") in sources, |
| 92 | + "#{lib} does not contain castle.ex, so this test was not looking at Castle's source" |
| 93 | + |
| 94 | + defined = |
| 95 | + Enum.flat_map(sources, fn path -> |
| 96 | + path |> File.read!() |> task_definitions(Path.relative_to(path, lib)) |
| 97 | + end) |
| 98 | + |
| 99 | + assert defined == [], """ |
| 100 | + Castle must ship no Mix tasks, and these are defined in its lib: |
| 101 | +
|
| 102 | + #{Enum.map_join(defined, "\n", &" #{&1}")} |
| 103 | +
|
| 104 | + Every build-time task lives in Forecastle, whatever it is called. Move this |
| 105 | + one there - it can keep the `castle.*` name it has. |
| 106 | + """ |
| 107 | + end |
| 108 | + |
| 109 | + # Both of these are about the detector rather than about Castle, and they are |
| 110 | + # here because the first version of it was a regex over lines and was wrong in |
| 111 | + # both directions. |
| 112 | + # |
| 113 | + # `defmodule(Mix.Tasks.Castle.Hidden)` is ordinary Elixir that `mix format` |
| 114 | + # preserves, and a regex anchored on whitespace after `defmodule` never saw |
| 115 | + # it. Wrapped in an environment branch it is invisible to the beam check too, |
| 116 | + # so between them the two checks reported clean on a tree that ships a task - |
| 117 | + # exactly the hazard this file exists to prevent. |
| 118 | + test "the source check sees a parenthesised definition inside an environment branch" do |
| 119 | + source = """ |
| 120 | + defmodule Castle.Conditional do |
| 121 | + if Mix.env() == :prod do |
| 122 | + defmodule(Mix.Tasks.Castle.Hidden) do |
| 123 | + use Mix.Task |
| 124 | + end |
| 125 | + end |
| 126 | + end |
| 127 | + """ |
| 128 | + |
| 129 | + assert task_definitions(source, "conditional.ex") == [ |
| 130 | + "conditional.ex:3: Mix.Tasks.Castle.Hidden" |
| 131 | + ] |
| 132 | + end |
| 133 | + |
| 134 | + # And the other direction: relaxing that regex would have started matching |
| 135 | + # module-looking prose. Documentation showing what not to do is a string |
| 136 | + # literal rather than a definition, and the parser is what knows the |
| 137 | + # difference. |
| 138 | + test "the source check ignores module-looking text in a heredoc" do |
| 139 | + assert task_definitions(@heredoc_sample, "docs.ex") == [] |
| 140 | + end |
| 141 | + |
| 142 | + # `Elixir.Mix.Tasks.X` names the same module as `Mix.Tasks.X` and parses with |
| 143 | + # an extra leading segment, which the first version of the matcher did not |
| 144 | + # expect. Guarded by an environment branch it was invisible to the beam check |
| 145 | + # too, so it escaped the pair the same way the parenthesised form did. |
| 146 | + test "the source check sees a fully qualified alias" do |
| 147 | + source = ~S""" |
| 148 | + if Mix.env() == :prod do |
| 149 | + defmodule Elixir.Mix.Tasks.Castle.Qualified do |
| 150 | + use Mix.Task |
| 151 | + end |
| 152 | + end |
| 153 | + """ |
| 154 | + |
| 155 | + assert task_definitions(source, "qualified.ex") == [ |
| 156 | + "qualified.ex:2: Mix.Tasks.Castle.Qualified" |
| 157 | + ] |
| 158 | + end |
| 159 | + |
| 160 | + # The other written form of a module name. |
| 161 | + test "the source check sees a literal atom module name" do |
| 162 | + source = ~S""" |
| 163 | + defmodule :"Elixir.Mix.Tasks.Castle.Atom" do |
| 164 | + use Mix.Task |
| 165 | + end |
| 166 | + """ |
| 167 | + |
| 168 | + assert task_definitions(source, "atom.ex") == ["atom.ex:1: Mix.Tasks.Castle.Atom"] |
| 169 | + end |
| 170 | + |
| 171 | + # A limit, pinned so that it is a decision on the record rather than a gap |
| 172 | + # someone rediscovers. `alias Mix.Tasks, as: N` and then `defmodule N.Castle.X` |
| 173 | + # defines `Mix.Tasks.Castle.X`, and nothing short of alias resolution sees it. |
| 174 | + # The beam check does, for the environment it compiled - see the note on |
| 175 | + # `task_definitions/2` for why that is where this stops. |
| 176 | + test "the source check does not resolve aliases, and says so" do |
| 177 | + source = ~S""" |
| 178 | + alias Mix.Tasks, as: N |
| 179 | +
|
| 180 | + defmodule N.Castle.Aliased do |
| 181 | + use Mix.Task |
| 182 | + end |
| 183 | + """ |
| 184 | + |
| 185 | + assert task_definitions(source, "aliased.ex") == [] |
| 186 | + end |
| 187 | + |
| 188 | + # Elixir's own parser rather than a pattern over the text. A `defmodule` is an |
| 189 | + # AST node whatever the spacing, parenthesisation or line breaks around it |
| 190 | + # are, and text inside a string literal is a binary in that AST rather than a |
| 191 | + # node - so both of the cases above fall out of asking the parser instead of |
| 192 | + # the characters. It matches the forms a module name is *written* in: an alias |
| 193 | + # (`Mix.Tasks.X`), the same alias fully qualified (`Elixir.Mix.Tasks.X`), and a |
| 194 | + # literal atom (`:"Elixir.Mix.Tasks.X"`). |
| 195 | + # |
| 196 | + # **What it does not do is resolve names, and that is a deliberate limit |
| 197 | + # rather than an oversight.** `alias Mix.Tasks, as: N` followed by |
| 198 | + # `defmodule N.Castle.X` names the same module and is invisible here, as are a |
| 199 | + # name built by `Module.concat/1`, one produced by a macro, and |
| 200 | + # `Module.create/3`. Resolving those means implementing alias scoping and |
| 201 | + # constant folding - a compiler, in a test - and the honest stopping point is |
| 202 | + # to say so instead. |
| 203 | + # |
| 204 | + # It costs less than it looks. The beam check is exact and cannot be fooled by |
| 205 | + # *any* of them, because every one still writes |
| 206 | + # `Elixir.Mix.Tasks.<name>.beam` into `ebin`, which is the file Mix reads. The |
| 207 | + # only gap the pair leaves is a module named indirectly *and* compiled only in |
| 208 | + # an environment `mix test` does not build, and reaching that state is not a |
| 209 | + # mistake anyone makes by accident - it is circumvention of an invariant this |
| 210 | + # file, `AGENTS.md` and `design/upgrade-tooling.md` all state in words. This |
| 211 | + # guards against the slip, which is someone adding `lib/mix/tasks/foo.ex` |
| 212 | + # because it seemed like the natural home for it. |
| 213 | + defp task_definitions(source, label) do |
| 214 | + ast = |
| 215 | + case Code.string_to_quoted(source) do |
| 216 | + {:ok, ast} -> ast |
| 217 | + {:error, reason} -> flunk("#{label} does not parse: #{inspect(reason)}") |
| 218 | + end |
| 219 | + |
| 220 | + {_ast, found} = |
| 221 | + Macro.prewalk(ast, [], fn |
| 222 | + {:defmodule, meta, [{:__aliases__, _, segments} | _]} = node, acc -> |
| 223 | + if task_alias?(segments), |
| 224 | + do: {node, [{line(meta), Module.concat(segments)} | acc]}, |
| 225 | + else: {node, acc} |
| 226 | + |
| 227 | + {:defmodule, meta, [module | _]} = node, acc when is_atom(module) -> |
| 228 | + if match?("Elixir.Mix.Tasks." <> _, Atom.to_string(module)), |
| 229 | + do: {node, [{line(meta), module} | acc]}, |
| 230 | + else: {node, acc} |
| 231 | + |
| 232 | + node, acc -> |
| 233 | + {node, acc} |
| 234 | + end) |
| 235 | + |
| 236 | + found |
| 237 | + |> Enum.reverse() |
| 238 | + |> Enum.map(fn {line, module} -> "#{label}:#{line}: #{inspect(module)}" end) |
| 239 | + end |
| 240 | + |
| 241 | + # `Mix.Tasks.X` parses to `[:Mix, :Tasks, :X]`, and the fully qualified |
| 242 | + # `Elixir.Mix.Tasks.X` to `[Elixir, :Mix, :Tasks, :X]` - the same module, |
| 243 | + # written twice. `Module.concat/1` already renders both as `Mix.Tasks.X`, so |
| 244 | + # only the recognition needs to know about the prefix. |
| 245 | + defp task_alias?([Elixir | rest]), do: task_alias?(rest) |
| 246 | + defp task_alias?([:Mix, :Tasks | _]), do: true |
| 247 | + defp task_alias?(_segments), do: false |
| 248 | + |
| 249 | + defp line(meta), do: Keyword.get(meta, :line, 0) |
| 250 | +end |
0 commit comments