Skip to content

Commit

Permalink
🎨 Adopt Styler (#116)
Browse files Browse the repository at this point in the history
Adopt the [Styler](https://github.com/adobe/elixir-styler) library, which is an Elixir formatter plugin that imposes some opinionated formatting on the code in place of a number of Credo rules.

While I don't always prefer its opinions, I really like have it auto-fix a number of issues, and I find that it's worth the tradeoff to adopt it.
  • Loading branch information
randycoulman authored Jan 10, 2024
1 parent 7906b96 commit cf572a3
Show file tree
Hide file tree
Showing 30 changed files with 211 additions and 291 deletions.
46 changes: 24 additions & 22 deletions .credo.exs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
#
{Credo.Check.Consistency.ExceptionNames, []},
{Credo.Check.Consistency.LineEndings, []},
{Credo.Check.Consistency.ParameterPatternMatching, []},
{Credo.Check.Consistency.SpaceAroundOperators, []},
{Credo.Check.Consistency.SpaceInParentheses, []},
{Credo.Check.Consistency.TabsOrSpaces, []},
Expand All @@ -82,8 +81,7 @@
# You can customize the priority of any check
# Priority values are: `low, normal, high, higher`
#
{Credo.Check.Design.AliasUsage,
[priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
{Credo.Check.Design.AliasUsage, [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
# You can also customize the exit_status of each check.
# If you don't want TODO comments to cause `mix credo` to fail, just
# set this value to 0 (zero).
Expand All @@ -94,56 +92,37 @@
#
## Readability Checks
#
{Credo.Check.Readability.AliasOrder, []},
{Credo.Check.Readability.FunctionNames, []},
{Credo.Check.Readability.ImplTrue, []},
{Credo.Check.Readability.LargeNumbers, []},
{Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]},
{Credo.Check.Readability.ModuleAttributeNames, []},
{Credo.Check.Readability.ModuleDoc, []},
{Credo.Check.Readability.ModuleNames, []},
{Credo.Check.Readability.MultiAlias, []},
{Credo.Check.Readability.ParenthesesInCondition, []},
{Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
{Credo.Check.Readability.PipeIntoAnonymousFunctions, []},
{Credo.Check.Readability.PredicateFunctionNames, []},
{Credo.Check.Readability.PreferImplicitTry, []},
{Credo.Check.Readability.RedundantBlankLines, []},
{Credo.Check.Readability.Semicolons, []},
{Credo.Check.Readability.SingleFunctionToBlockPipe, []},
{Credo.Check.Readability.SpaceAfterCommas, []},
{Credo.Check.Readability.Specs, []},
{Credo.Check.Readability.StrictModuleLayout,
ignore: [:behaviour], order: [:shortdoc, :moduledoc, :use, :import, :alias, :require]},
{Credo.Check.Readability.StringSigils, []},
{Credo.Check.Readability.TrailingBlankLine, []},
{Credo.Check.Readability.TrailingWhiteSpace, []},
{Credo.Check.Readability.UnnecessaryAliasExpansion, []},
{Credo.Check.Readability.VariableNames, []},
{Credo.Check.Readability.WithCustomTaggedTuple, []},
{Credo.Check.Readability.WithSingleClause, []},

#
## Refactoring Opportunities
#
{Credo.Check.Refactor.Apply, []},
{Credo.Check.Refactor.CondStatements, []},
{Credo.Check.Refactor.CyclomaticComplexity, []},
{Credo.Check.Refactor.FunctionArity, []},
{Credo.Check.Refactor.IoPuts, []},
{Credo.Check.Refactor.LongQuoteBlocks, []},
{Credo.Check.Refactor.MatchInCondition, []},
{Credo.Check.Refactor.MapJoin, []},
{Credo.Check.Refactor.NegatedConditionsInUnless, []},
{Credo.Check.Refactor.NegatedConditionsWithElse, []},
{Credo.Check.Refactor.Nesting, []},
{Credo.Check.Refactor.PassAsyncInTestCases, []},
{Credo.Check.Refactor.UnlessWithElse, []},
{Credo.Check.Refactor.WithClauses, []},
{Credo.Check.Refactor.FilterCount, []},
{Credo.Check.Refactor.FilterFilter, []},
{Credo.Check.Refactor.RejectReject, []},
{Credo.Check.Refactor.RedundantWithClauseResult, []},

#
## Warnings
Expand All @@ -170,6 +149,29 @@
{Credo.Check.Warning.UnsafeExec, []}
],
disabled: [
#
# Checks that are handled by Styler
{Credo.Check.Consistency.ParameterPatternMatching, []},
{Credo.Check.Readability.AliasOrder, []},
{Credo.Check.Readability.LargeNumbers, []},
{Credo.Check.Readability.ModuleDoc, []},
{Credo.Check.Readability.MultiAlias, []},
{Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
{Credo.Check.Readability.PipeIntoAnonymousFunctions, []},
{Credo.Check.Readability.PreferImplicitTry, []},
{Credo.Check.Readability.StrictModuleLayout,
ignore: [:behaviour], order: [:shortdoc, :moduledoc, :use, :import, :alias, :require]},
{Credo.Check.Readability.UnnecessaryAliasExpansion, []},
{Credo.Check.Readability.WithSingleClause, []},
{Credo.Check.Refactor.CondStatements, []},
{Credo.Check.Refactor.FilterCount, []},
{Credo.Check.Refactor.MapJoin, []},
{Credo.Check.Refactor.NegatedConditionsInUnless, []},
{Credo.Check.Refactor.NegatedConditionsWithElse, []},
{Credo.Check.Refactor.RedundantWithClauseResult, []},
{Credo.Check.Refactor.UnlessWithElse, []},
{Credo.Check.Refactor.WithClauses, []},

#
# Checks scheduled for next check update (opt-in for now, just replace `false` with `[]`)

Expand Down
3 changes: 2 additions & 1 deletion .formatter.exs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by "mix format"
[
import_deps: [:typed_struct],
inputs: ["{mix,.credo,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"]
inputs: ["{mix,.credo,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"],
plugins: [Styler]
]
5 changes: 3 additions & 2 deletions lib/config_cat/cache.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ defmodule ConfigCat.Cache do
alias ConfigCat.ConfigEntry
alias ConfigCat.Hooks

require ConfigCat.Constants, as: Constants
require ConfigCat.ConfigCatLogger, as: ConfigCatLogger
require ConfigCat.Constants, as: Constants

defmodule State do
@moduledoc false
Expand Down Expand Up @@ -53,7 +53,8 @@ defmodule ConfigCat.Cache do
def generate_key(sdk_key) do
key = "#{sdk_key}_#{Constants.config_filename()}_#{Constants.serialization_format_version()}"

:crypto.hash(:sha, key)
:sha
|> :crypto.hash(key)
|> Base.encode16(case: :lower)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/config_cat/cache_policy.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ defmodule ConfigCat.CachePolicy do
See `manual/0` below for details.
"""

@behaviour ConfigCat.CachePolicy.Behaviour

alias ConfigCat.CachePolicy.Auto
alias ConfigCat.CachePolicy.Behaviour
alias ConfigCat.CachePolicy.Lazy
alias ConfigCat.CachePolicy.Manual

require ConfigCat.Constants, as: Constants

@behaviour Behaviour

@typedoc "Options for auto-polling mode."
@type auto_options :: [
{:max_init_wait_time_seconds, non_neg_integer()}
Expand Down
17 changes: 9 additions & 8 deletions lib/config_cat/cache_policy/auto.ex
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ defmodule ConfigCat.CachePolicy.Auto do
@default_poll_interval_seconds 60

typedstruct enforce: true do
field :max_init_wait_time_ms, non_neg_integer(),
default: @default_max_init_wait_time_seconds * 1000
field :max_init_wait_time_ms, non_neg_integer(), default: @default_max_init_wait_time_seconds * 1000

field :mode, String.t(), default: "a"
field :poll_interval_ms, pos_integer(), default: @default_poll_interval_seconds * 1000
Expand All @@ -56,11 +55,13 @@ defmodule ConfigCat.CachePolicy.Auto do
Keyword.pop(options, :poll_interval_seconds, @default_poll_interval_seconds)

options =
[
max_init_wait_time_ms: rounded_ms(max_init_wait_time_seconds, 0),
poll_interval_ms: rounded_ms(poll_interval_seconds, 1)
]
|> Keyword.merge(options)
Keyword.merge(
[
max_init_wait_time_ms: rounded_ms(max_init_wait_time_seconds, 0),
poll_interval_ms: rounded_ms(poll_interval_seconds, 1)
],
options
)

struct(__MODULE__, options)
end
Expand All @@ -69,7 +70,7 @@ defmodule ConfigCat.CachePolicy.Auto do
seconds
|> max(min_value)
|> Kernel.*(1000)
|> round
|> round()
end

@impl GenServer
Expand Down
6 changes: 2 additions & 4 deletions lib/config_cat/cache_policy/null.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ defmodule ConfigCat.CachePolicy.Null do
@moduledoc false

# The CachePolicy that gets used in :local_only mode
@behaviour ConfigCat.CachePolicy.Behaviour

alias ConfigCat.CachePolicy.Behaviour

require ConfigCat.ConfigCatLogger, as: ConfigCatLogger

@behaviour Behaviour

@impl Behaviour
def get(_instance_id) do
# Should never be called
Expand Down Expand Up @@ -37,7 +36,6 @@ defmodule ConfigCat.CachePolicy.Null do

@impl Behaviour
def force_refresh(_instance_id) do
{:error,
"The SDK uses the `:local_only` override behavior which prevents making HTTP requests."}
{:error, "The SDK uses the `:local_only` override behavior which prevents making HTTP requests."}
end
end
26 changes: 13 additions & 13 deletions lib/config_cat/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ defmodule ConfigCat.Client do
alias ConfigCat.Rollout
alias ConfigCat.User

require ConfigCat.Constants, as: Constants
require ConfigCat.ConfigCatLogger, as: ConfigCatLogger
require ConfigCat.Constants, as: Constants

defmodule State do
@moduledoc false
Expand Down Expand Up @@ -96,17 +96,19 @@ defmodule ConfigCat.Client do

@impl GenServer
def handle_call({:get_key_and_value, variation_id}, _from, %State{} = state) do
with {:ok, settings, _fetch_time_ms} <- cached_settings(state),
result <- Enum.find_value(settings, nil, &entry_matching(&1, variation_id)) do
if is_nil(result) do
ConfigCatLogger.error(
"Could not find the setting for the specified variation ID: '#{variation_id}'",
event_id: 2011
)
end
case cached_settings(state) do
{:ok, settings, _fetch_time_ms} ->
result = Enum.find_value(settings, nil, &entry_matching(&1, variation_id))

if is_nil(result) do
ConfigCatLogger.error(
"Could not find the setting for the specified variation ID: '#{variation_id}'",
event_id: 2011
)
end

{:reply, result, state}

{:reply, result, state}
else
_ ->
ConfigCatLogger.error(
"Config JSON is not present. Returning nil.",
Expand Down Expand Up @@ -207,8 +209,6 @@ defmodule ConfigCat.Client do
defp value_matching(key, value, variation_id) do
if Map.get(value, Constants.variation_id(), nil) == variation_id do
{key, Map.get(value, Constants.value())}
else
nil
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/config_cat/config_cat_logger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ defmodule ConfigCat.ConfigCatLogger do

Logger.error(fn -> module.formatted_message(message, metadata) end)

instance_id = Logger.metadata() |> Keyword.get(:instance_id)
instance_id = Keyword.get(Logger.metadata(), :instance_id)

if instance_id do
ConfigCat.Hooks.invoke_on_error(instance_id, message)
Expand Down
28 changes: 9 additions & 19 deletions lib/config_cat/config_fetcher.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ end
defmodule ConfigCat.CacheControlConfigFetcher do
@moduledoc false

@behaviour ConfigCat.ConfigFetcher

use GenServer

alias ConfigCat.Config
Expand All @@ -42,8 +44,8 @@ defmodule ConfigCat.CacheControlConfigFetcher do
alias ConfigCat.ConfigFetcher.FetchError
alias HTTPoison.Response

require ConfigCat.Constants, as: Constants
require ConfigCat.ConfigCatLogger, as: ConfigCatLogger
require ConfigCat.Constants, as: Constants
require ConfigCat.RedirectMode, as: RedirectMode

defmodule State do
Expand Down Expand Up @@ -108,8 +110,6 @@ defmodule ConfigCat.CacheControlConfigFetcher do
| {:sdk_key, String.t()}
@type options :: [option]

@behaviour ConfigFetcher

@spec start_link(options()) :: GenServer.on_start()
def start_link(options) do
instance_id = Keyword.fetch!(options, :instance_id)
Expand Down Expand Up @@ -194,7 +194,7 @@ defmodule ConfigCat.CacheControlConfigFetcher do
end

defp base_headers(%State{mode: mode}) do
version = Application.spec(:configcat, :vsn) |> to_string()
version = :configcat |> Application.spec(:vsn) |> to_string()
user_agent = "ConfigCat-Elixir/#{mode}-#{version}"

[
Expand All @@ -221,18 +221,12 @@ defmodule ConfigCat.CacheControlConfigFetcher do
# This function is slightly complex, but still reasonably understandable.
# Breaking it up doesn't seem like it will help much.
# credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity
defp handle_response(
%Response{status_code: code, body: raw_config, headers: headers},
%State{} = state,
etag
)
defp handle_response(%Response{status_code: code, body: raw_config, headers: headers}, %State{} = state, etag)
when code >= 200 and code < 300 do
ConfigCatLogger.debug(
"ConfigCat configuration json fetch response code: #{code} Cached: #{extract_etag(headers)}"
)
ConfigCatLogger.debug("ConfigCat configuration json fetch response code: #{code} Cached: #{extract_etag(headers)}")

with {:ok, config} <- Jason.decode(raw_config),
new_etag <- extract_etag(headers),
new_etag = extract_etag(headers),
%{base_url: new_base_url, custom_endpoint?: custom_endpoint?, redirects: redirects} <-
state,
{base_url, redirect_mode} <- Config.preferences(config) do
Expand Down Expand Up @@ -288,8 +282,7 @@ defmodule ConfigCat.CacheControlConfigFetcher do
{:ok, :unchanged, state}
end

defp handle_response(%Response{status_code: status} = response, %State{} = state, _etag)
when status in [403, 404] do
defp handle_response(%Response{status_code: status} = response, %State{} = state, _etag) when status in [403, 404] do
ConfigCatLogger.error(
"Your SDK Key seems to be wrong. You can find the valid SDKKey at https://app.configcat.com/sdkkey. Received unexpected response: #{inspect(response)}",
event_id: 1100
Expand All @@ -311,10 +304,7 @@ defmodule ConfigCat.CacheControlConfigFetcher do
{:error, error, state}
end

defp handle_error(
{:error, %HTTPoison.Error{reason: :checkout_timeout} = error},
%State{} = state
) do
defp handle_error({:error, %HTTPoison.Error{reason: :checkout_timeout} = error}, %State{} = state) do
ConfigCatLogger.error(
"Request timed out while trying to fetch config JSON. Timeout values: [connect: #{state.connect_timeout_milliseconds}ms, read: #{state.read_timeout_milliseconds}ms]",
event_id: 1102
Expand Down
2 changes: 1 addition & 1 deletion lib/config_cat/fetch_time.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule ConfigCat.FetchTime do
@type t :: non_neg_integer()

@spec now_ms :: t()
def now_ms, do: DateTime.utc_now() |> DateTime.to_unix(:millisecond)
def now_ms, do: DateTime.to_unix(DateTime.utc_now(), :millisecond)

@spec to_datetime(t()) :: {:ok, DateTime.t()} | {:error, atom()}
def to_datetime(ms) do
Expand Down
4 changes: 2 additions & 2 deletions lib/config_cat/in_memory_cache.ex
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
defmodule ConfigCat.InMemoryCache do
@moduledoc false

@behaviour ConfigCat.ConfigCache

use GenServer

alias ConfigCat.ConfigCache

@behaviour ConfigCache

@spec start_link(keyword()) :: GenServer.on_start()
def start_link(_options) do
GenServer.start_link(__MODULE__, %{}, name: __MODULE__)
Expand Down
4 changes: 2 additions & 2 deletions lib/config_cat/local_file_data_source.ex
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ defmodule ConfigCat.LocalFileDataSource do
with {:ok, %{mtime: timestamp}} <- File.stat(filename, time: :posix) do
unless FileCache.cached_timestamp(cache) == timestamp do
with {:ok, contents} <- File.read(filename),
{:ok, data} <- Jason.decode(contents),
settings <- normalize(data) do
{:ok, data} <- Jason.decode(contents) do
settings = normalize(data)
FileCache.update(cache, settings, timestamp)
else
error ->
Expand Down
Loading

0 comments on commit cf572a3

Please sign in to comment.