Mailglass.Credo.NoRawAppEnvRestore (Mailglass v2.2.2)

Copy Markdown View Source

Basics

This check is disabled by default.

Learn how to enable it via .credo.exs.

This check has a base priority of high and works with any version of Elixir.

Explanation

Test code restores Application env through Mailglass.TestSupport.SandboxOwnership.with_app_env!/2, never through Application.put_all_env/1.

Application.put_all_env/1 MERGES. It writes every key in the list it is given and touches nothing else, so it is structurally incapable of removing a key the test ADDED — a key absent from the captured snapshot is absent from the write set, and the test's value survives into every later module in the run. The idiom reads like a restore and is one only for keys that already existed.

That is not a hypothetical. Seven test/ modules used it as their restore, and all seven set config :mailglass, :compliance, which is in no config/*.exs — so all seven leaked it on every run. Two also install a @behaviour Mailglass.Tenancy resolver whose scope/2 applies as: :scoped, and Mailglass.Operator.SupportSummary's private orphan_backlog_summary/2 (not backticked: ExDoc auto-links a Mod.fun/arity reference and --warnings-as-errors then fails on the private target) builds a query already aliased as: :orphan, so a leaked resolver turns every later caller into ** (Ecto.Query.CompileError) can't apply alias :scoped, binding in from is already aliased to :orphan. Observed in CI run 30571989203 on a DOCS-ONLY commit, and green two commits later with lib/ byte-identical.

with_app_env!/2 re-puts every captured key AND deletes every key that appeared since the capture, then verifies the result — the step put_all_env/1 cannot express.

What this check deliberately does NOT catch

The same bug also wears a second syntax: prior = Application.get_env(app, key) followed by Application.put_env(app, key, prior), which CREATES the key holding nil when it was absent, rather than removing it. That is not statically decidable — whether prior can be nil depends on runtime config — so this check does not guess at it. The narrow, always- wrong idiom is caught here; the general case is a review concern, and with_app_env!/2 is the answer to both.

Check-Specific Parameters

Use the following parameters to configure this check:

:allowed_modules

Modules explicitly allowed to reference Application.put_all_env/1 directly.

This parameter defaults to [Mailglass.TestSupport.SandboxOwnership, Mailglass.TestSupport.SandboxOwnershipTest].

:included_path_prefixes

Only files in these path prefixes are linted.

This parameter defaults to ["test/", "mailglass_inbound/test/"].

General Parameters

Like with all checks, general params can be applied.

Parameters can be configured via the .credo.exs config file.