Ok, here’s my pass.

This is assuming you want to pass the tuple syntax so that is what it checks for and anything else is considered and error.

It defaults to looking for modules with Worker suffixes though you can also specify a custom suffix, a namespace, or both. So use like so in your credo config:

{MyChecks, worker_suffix: "Job"}

This does NOT handle the case of importing new/2 because that’s just kinda evil Happy to add it, though.

If you specify both :worker_suffix and :worker_namespace they both must match. IE you could (for some reason) have MyApp.Workers.AWorkerThisIsNot… though not sure why you would want to do that.

EDIT: Oops left some temp stuff in for reporting… sorta fixed it, sorta (ie, "Some trigger")

defmodule MyChecks.ObanNew do
  use Credo.Check

  @opts [:worker_suffix, :worker_namespace]

  @impl true
  def run(source_file, params \\ []) do
    params =
      if not Enum.any?(@opts, &Keyword.has_key?(params, &1)) do
        Keyword.merge(params, worker_suffix: "Worker")
      else
        params
      end

    issue_meta = IssueMeta.for(source_file, params)

    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, params))
  end

  defp traverse(
         {{:., _, [{_, _, aliases}, :new]}, meta, [_, opts]} = ast,
         issues,
         issue_meta,
         params
       ) do
    if worker?(aliases, params) do
      case opts[:schedule_in] do
        nil -> {ast, issues}
        {_, _} -> {ast, issues}
        _ -> {ast, issues ++ [issue_for("Some trigger", meta[:line], issue_meta)]}
      end
    else
      {ast, issues}
    end
  end

  defp traverse(ast, issues, _issue_meta, _params) do
    {ast, issues}
  end

  defp worker?(pieces, params) do
    pieces = Enum.map(pieces, &Atom.to_string/1)

    if params[:worker_namespace] != nil and params[:worker_suffix] != nil do
      Enum.any?(pieces, &(&1 == params[:worker_namespace])) &&
        Enum.any?(pieces, &String.ends_with?(&1, params[:worker_suffix]))
    else
      Enum.any?(pieces, fn piece ->
        cond do
          params[:worker_namespace] != nil ->
            piece == params[:worker_namespace]

          params[:worker_suffix] != nil ->
            String.ends_with?(piece, params[:worker_suffix])

          true ->
            false
        end
      end)
    end
  end

  defp issue_for(trigger, line_no, issue_meta) do
    format_issue(
      issue_meta,
      message: "Please use tuple syntax for `shedule_in` option",
      trigger: trigger,
      line_no: line_no
    )
  end
end

and tests:

defmodule MyChecks.ObanNewTest do
  use Credo.Test.Case

  alias MyChecks.ObanNew

  test "schedule_in must use tuple syntax" do
    """
    defmodule CredoSampleModule do
      def foo do
        MyApp.FooWorker.new("something", schedule_in: {60, :hours})
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew)
    |> refute_issues()
  end

  test "schedule_in errors if using integer" do
    """
    defmodule CredoSampleModule do
      def foo do
        MyApp.FooWorker.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew)
    |> assert_issue()
  end

  test "ignores modules not ending with `Worker`" do
    """
    defmodule CredoSampleModule do
      def foo do
        NonWorkerModule.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew)
    |> refute_issues()
  end

  test "allows specifying custom suffix" do
    """
    defmodule CredoSampleModule do
      def foo do
        MyJob.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew, worker_suffix: "Job")
    |> assert_issue()
  end

  test "allow specifying a namespace" do
    """
    defmodule CredoSampleModule do
      def foo do
        MyJobs.Doit.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew, worker_namespace: "MyJobs")
    |> assert_issue()
  end

  test "allow specifying namespace and suffix" do
    """
    defmodule CredoSampleModule do
      def foo do
        MyJobs.SomeWorker.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew, worker_namespace: "MyJobs", worker_suffix: "Worker")
    |> assert_issue()

    """
    defmodule CredoSampleModule do
      def foo do
        MyJobs.Unrelated.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew, worker_namespace: "MyJobs", worker_suffix: "Worker")
    |> refute_issues()

    """
    defmodule CredoSampleModule do
      def foo do
        NonObanModuleEndingInWorker.new("something", schedule_in: 400 * 1000)
      end
    end
    """
    |> to_source_file()
    |> run_check(ObanNew, worker_namespace: "MyJobs", worker_suffix: "Worker")
    |> refute_issues()
  end
end

Well that’s no fun

But if you’re going to keep going… this solution is Good Enough but not perfect as it checks any new function.

Ideally you want to ensure that new belongs to a worker. One idea is to include an option to specify a worker namespace or suffix, for example {MyCheck.ObanSchdeduleIn, namespace: [:MyApp, :Workers]}.

I haven’t sorted it out yet. I’ll give @meraj_enigma’s version a try. And I wonder how we could construct a bunch of test cases to ensure that there aren’t any false positives. Maybe we could run it against a bunch of projects from GitHub/hex.pm with some that do and some that don’t use Oban.