alexocode/knigge

An opinionated way of dealing with behaviours

knigge's Issues

Using Knigge in a package results in compilation errors when using `otp_app: ...`

In a nutshell: the config from config/config.exs is not available for packages, which makes configuring Knigge via it not possible for packages.

A potential solution might be to provide a "default" implementation, which can be overridden via the config.

Workaround is something like this:

  knigge_opts =
    if Mix.env() == :prod do
      [implementation: MyProdImplementation]
    else
      [otp_app: :my_app]
    end

  use Knigge, knigge_opts

Add a .formattter.exs which exports defdefault as locals_without_parens

Currently the formatter adds parens around a one-line defdefault; this could be disabled by adding defdefault: 2 as an exported locals_without_parents.

Utilize the `Logger` module for logging purposes

Currently, there are three areas, the Knigge.Warnings and Knigge.Options modules using the native IO module, and another in the Knigge.CLI.Output module using the third party Bunt library.

I think it would be better to utilize the Logger module directly to ensure logs go through to whatever logging backend the users utilize, at least (especially?) in the non CLI modules.

What do you think @sascha-wolf?

Introduce testing helpers

There are cases where it's useful to swap an implantation out for a single test. As such an easy to use API for this might be useful to add.

Handle argument defaults in defdefault

Currently argument defaults in defdefault are a bit wonky. Knigge should either explicitly allow default values or forbid them.

Compilition fails if callbacks in behaviour are defined without brackets

When a @callback in the behaviour is defined without brackets (as is valid if the function doesn't require any arguments), compilition fails. Example Behaviour:

defmodule MyApp.Rules.Parser do
  use Knigge, otp_app: :my_app

  @callback parse! :: %{String.t() => Dataset.t()}
end

This results in the following error at compiletime:

== Compilation error in file lib/my_app/rules/parser.ex ==
** (ArgumentError) argument error
    :erlang.length(nil)
    (knigge 1.2.0) lib/knigge/ast.ex:13: Knigge.AST.function_spec_from_callback/1
    (elixir 1.11.1) lib/enum.ex:1399: Enum."-map/2-lists^map/1-0-"/2
    (knigge 1.2.0) lib/knigge/behaviour.ex:33: Knigge.Behaviour.callbacks_from_attribute/1
    (knigge 1.2.0) lib/knigge/code.ex:42: Knigge.Code.generate/2
    (knigge 1.2.0) expanding macro: Knigge.Code.__before_compile__/1
    lib/authorizer/rules/parser.ex:1: Authorizer.Rules.Parser (module)
    (elixir 1.11.1) lib/kernel/parallel_compiler.ex:314: anonymous fn/4 in Kernel.ParallelCompiler.spawn_workers/7

Changing the callback to parse!() will fix the error.

Compilation fails because it checks if the implementation exists

In the following example:

defmodule MyGreatcheckBehaviour do
  use Knigge, otp_app: :my_application

  @callback my_great_callback(my_argument :: any()) :: any()
end

defmodule MyGreatImplementation do
  @behaviour MyGreatBehaviour
  @impl true
  def my_great_callback(_), do: true
end

# config.exs
config :my_application, MyGreatBehaviour, MyGreatImplementation

It doesn't work in dev or prod because MyGreatImplementation depends on MyGreatBehaviour on compilation time, and Knigge tries to Code.load?(MyGreatImplementation) when compiling MyGreatBehaviour.

Now, I'm using the check_if_exists?: false option, however, it is not explained in the README and this example doesn't work without it. Another solution would be to use a third module for the behaviour, but if the main purpose is to avoid boilerplate code I don't think it is a great idea.

I don't know if it would be possible to try to check if the implementation exists in the @after_compile hook using the Code.ensure_compile(implementation_module)

Thank you!

Must set `check_if_exists?` to false when using Mox

Just a heads up. In order to get my project to compile when using this library in combination with Mox, I had to set check_if_exists? option to false.

I have my mock defined in /test/support/mocks.ex which is included in my elixirc_paths, as recommended in the Mox documentation to provide compile-time support, but the project wouldn't compile when I use Knigge unless I set that option.

That option isn't documented in the Knigge.Options docs and I only found out about it looking at the tests you wrote for issue #2.

Ignore duplicate `@callback` definitions

When specifying multiple "versions" for a single @callback - for example with differing typespecs - Knigge also generates multiple clauses which in turn generates a compiler warning.

Exclude Bunt dependency from the runtime

Hello.

I'm facing some issue to integrate Knigge.
Bunt is a dependency used for the CLI, however, when you have to do some release upgrades, SASL release_handler crash because there is no top supervisor for the bunt application.
(See other discussion regarding this also: bitwalker/exrm#183)

An idea would be to tag the bunt dependency only in dev or exclude it from the runtime.

Warn when an unmatched `defdefault` has been passed

This could for example with a typo happen.

Typo in `Options` module

There is a typo in the Options module which prevents this library from working properly.

https://github.com/sascha-wolf/knigge/blob/1fb2f223b3818a1a674878ae4552a7e413d3bbf9/lib/knigge/options.ex#L83

You are trying to read opt_app from the options list instead of otp_app.

Add additional action to publish docs of main

Currently docs only get published on release.

It would be great if we could publish the docs for main on each push/merge.

Questions

• Which version would these docs have?

Disable missing module warnings in test

When configuring a module in test which only gets compiled during the tests the compiler will emit a bunch of warnings that the module can’t be found.

While there are some workarounds for this it would be nice if we could find a way to disable these warnings completely.

Elixir 1.10 will introduce a @compile {:no_warn_undefined, ...} option but that will obviously only help for 1.10.

Possible Solutions

1. delegate_at: :runtime per default in :test

This would require some changes into how we configure delegate_at. Maybe deprecate it in favour of a delegate_at_runtime option which can receive some environment specs like check_if_exists can?

defdefault might fail when the implementation is not yet being compiled