From 6eb6dd05a310709470358270a670a75ac7657a87 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sat, 5 Jul 2025 01:58:45 +0100 Subject: [PATCH 01/67] Set newer expectations --- README.md | 32 +++++++++++++++++++++----------- RULES.md | 4 ---- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 8a9f7141..1d53ea5b 100644 --- a/README.md +++ b/README.md @@ -37,19 +37,28 @@ Once this is done you can apply the style rules in the following ways. #### Loading configuration from a file ```shell -1> ElvisConfig = elvis_config:from_file("elvis.config"). +1> ElvisConfig = elvis_config:config(). 2> elvis_core:rock(ElvisConfig). +Loading src/elvis_code.erl +# src/elvis_code.erl [OK] +Loading src/elvis_config.erl +# src/elvis_config.erl [OK] +Loading src/elvis_core.erl # src/elvis_core.erl [OK] -# src/elvis_result.erl [OK] -# src/elvis_style.erl [OK] -# src/elvis_utils.erl [OK] +Loading src/elvis_file.erl +# src/elvis_file.erl [OK] +... ok 3> ``` This will load the [configuration](#configuration), specified in file `elvis.config`, from the -current directory. If no configuration is found `{invalid_config, _}` is thrown. +current directory. + +If `elvis.config` is not present, the application will fall back to searching for configuration +parameters in `rebar.config`. If `rebar.config` is also unavailable, the application proceeds to +perform a tertiary lookup within the `app/sys.config` file for the required settings. #### Providing configuration as a value @@ -60,14 +69,15 @@ an argument to `elvis_core:rock/1`: 1> ElvisConfig = [#{dirs => ["src"], filter => "*.erl", rules => []}]. [#{dirs => ["src"],filter => "*.erl",rules => []}] 2> elvis_core:rock(ElvisConfig). +Loading src/elvis_code.erl +# src/elvis_code.erl [OK] +Loading src/elvis_config.erl +# src/elvis_config.erl [OK] Loading src/elvis_core.erl # src/elvis_core.erl [OK] -Loading src/elvis_result.erl -# src/elvis_result.erl [OK] -Loading src/elvis_style.erl -# src/elvis_style.erl [OK] -Loading src/elvis_utils.erl -# src/elvis_utils.erl [OK] +Loading src/elvis_file.erl +# src/elvis_file.erl [OK] +... ok 3> ``` diff --git a/RULES.md b/RULES.md index 39cca9cd..3e0885aa 100644 --- a/RULES.md +++ b/RULES.md @@ -222,10 +222,6 @@ for `.hrl` files. , filter => "*.hrl" , ruleset => hrl_files } - , #{ dirs => ["."] - , filter => "Makefile" - , ruleset => makefiles - , rules => [] } , #{ dirs => ["."] , filter => "rebar.config" , ruleset => rebar_config From da75963badad3198c3b39ea4c8669ff272f77a1d Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sat, 5 Jul 2025 02:46:26 +0100 Subject: [PATCH 02/67] Expect to move keys (and default handling) to elvis_config --- src/elvis_core.erl | 4 ++-- src/elvis_result.erl | 2 +- src/elvis_utils.erl | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index d3307645..cee64042 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -76,7 +76,7 @@ rock_this(Path, ElvisConfig) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. do_parallel_rock(ElvisConfig0) -> - Parallel = elvis_config:from_application_or_config(parallel, 1), + Parallel = elvis_config:parallel(), ElvisConfig = elvis_config:resolve_files(ElvisConfig0), Files = elvis_config:files(ElvisConfig), @@ -126,7 +126,7 @@ load_file_data(ElvisConfig, File) -> main([]) -> ok = application:load(elvis_core), {module, _} = code:ensure_loaded(elvis_style), - case rock(elvis_config:from_file("elvis.config")) of + case rock(elvis_config:config()) of ok -> true; _ -> halt(1) end. diff --git a/src/elvis_result.erl b/src/elvis_result.erl index c2cc10c1..71440e67 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -167,7 +167,7 @@ get_line_num(#{line_num := LineNum}) -> -spec print_results(file() | [elvis_warn()]) -> ok. print_results(Results) -> - Format = elvis_config:from_application_or_config(output_format, colors), + Format = elvis_config:output_format(), print(Format, Results). -spec print(plain | colors | parsable, [file()] | file()) -> ok. diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 079a3e30..ad6803a1 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -61,7 +61,7 @@ warn_prn(Message, Args) -> -spec print_info(string(), [term()]) -> ok. print_info(Message, Args) -> - case elvis_config:from_application_or_config(verbose, false) of + case elvis_config:verbose() of true -> print(Message, Args); false -> @@ -70,7 +70,7 @@ print_info(Message, Args) -> -spec print(string(), [term()]) -> ok. print(Message, Args) -> - case elvis_config:from_application_or_config(no_output, false) of + case elvis_config:no_output() of true -> ok; _ -> @@ -93,7 +93,7 @@ parse_colors(Message) -> "reset" => "\e[0m" }, Opts = [global, {return, list}], - case elvis_config:from_application_or_config(output_format, colors) of + case elvis_config:output_format() of P when P =:= plain; P =:= parsable -> re:replace(Message, "{{.*?}}", "", Opts); colors -> From 9d596a44a87299ca82fe8e55a28cd9955e187399 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sat, 5 Jul 2025 03:15:25 +0100 Subject: [PATCH 03/67] Approach our expectations on continuing validation --- src/elvis_config.erl | 136 +++++++++++++++++++++++++++++-------------- src/elvis_core.erl | 23 ++++++-- 2 files changed, 110 insertions(+), 49 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index baf7871c..80947d49 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -2,12 +2,7 @@ -feature(maybe_expr, enable). --export([ - from_rebar/1, - from_file/1, - from_application_or_config/2, - validate/1 -]). +-export([from_rebar/1, from_file/1, validate/1]). %% Geters -export([dirs/1, ignore/1, filter/1, files/1, rules/1]). %% Files @@ -15,63 +10,114 @@ %% Rules -export([merge_rules/2]). --export_type([t/0]). +%% Options +-export([config/0, output_format/0, verbose/0, no_output/0, parallel/0]). -type t() :: map(). +-export_type([t/0]). + +-type validation_error() :: empty_config. +-export_type([validation_error/0]). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% --spec from_rebar(string()) -> [t()]. -from_rebar(Path) -> - case file:consult(Path) of - {ok, AppConfig} -> - load(config, load_initial(AppConfig), []); - {error, Reason} -> - throw(Reason) - end. +load_elvis(AppConfig) -> + ElvisConfig = proplists:get_value(elvis, AppConfig, default(elvis)), + elvis_ruleset:set_rulesets(proplists:get_value(rulesets, ElvisConfig, default(rulesets))), + ElvisConfig. --spec from_file(string()) -> [t()]. -from_file(Path) -> - from_file(Path, config, []). +config() -> + for(config). --spec from_file(string(), atom(), term()) -> [t()]. -from_file(Path, Key, Default) -> - case file:consult(Path) of - {ok, [AppConfig]} -> - load(Key, load_initial(AppConfig), Default); - {error, {_Line, _Mod, _Term} = Reason} -> - throw(Reason); - {error, _Reason} -> - Default - end. +output_format() -> + for(output_format). + +verbose() -> + for(verbose). + +no_output() -> + for(no_output). --spec from_application_or_config(atom(), term()) -> term(). -from_application_or_config(Key, Default) -> +parallel() -> + for(parallel). + +default(Key) -> case application:get_env(elvis_core, Key) of + undefined -> + default_for(Key); {ok, Value} -> - Value; - _ -> - from_file("elvis.config", Key, Default) + Value end. --spec load(atom(), term(), term()) -> [t()]. -load(Key, ElvisConfig, Default) -> - proplists:get_value(Key, ElvisConfig, Default). +for(Key) -> + AppConfig = + case consult_elvis_config(default) of + [] -> + consult_rebar_config(default); + AppConfig0 -> + AppConfig0 + end, + TopLevelCfg = load_elvis(AppConfig), + proplists:get_value(Key, TopLevelCfg, default(Key)). + +consult_elvis_config(File0) -> + File = + case File0 of + default -> + "elvis.config"; + _ -> + File0 + end, + case file:consult(File) of + {ok, [AppConfig]} -> + AppConfig; + _ -> + [] + end. --spec load_initial(term()) -> [term()]. -load_initial(AppConfig) -> - ElvisConfig = proplists:get_value(elvis, AppConfig, []), - RulesetsConfig = proplists:get_value(rulesets, ElvisConfig, #{}), - elvis_ruleset:set_rulesets(RulesetsConfig), - ElvisConfig. +consult_rebar_config(File0) -> + File = + case File0 of + default -> + "rebar.config"; + _ -> + File0 + end, + case file:consult(File) of + {ok, AppConfig0} -> + AppConfig0; + _ -> + [] + end. --spec validate(Config :: [t()]) -> ok. +from_rebar(File) -> + RebarConfig = consult_rebar_config(File), + TopLevelCfg = load_elvis(RebarConfig), + Key = config, + proplists:get_value(Key, TopLevelCfg, default(Key)). + +from_file(File) -> + FileConfig = consult_elvis_config(File), + TopLevelCfg = load_elvis(FileConfig), + Key = config, + proplists:get_value(Key, TopLevelCfg, default(Key)). + +default_for(elvis) -> []; +default_for(config) -> []; +default_for(output_format) -> colors; +default_for(verbose) -> false; +default_for(no_output) -> false; +default_for(parallel) -> 1; +default_for(rulesets) -> #{}. + +-spec validate(Config :: [t()]) -> [validation_error()]. validate([]) -> - throw({invalid_config, empty_config}); + [empty_config]; validate(Config) -> - lists:foreach(fun do_validate/1, Config). + lists:foreach(fun do_validate/1, Config), + []. do_validate(RuleGroup) -> maybe diff --git a/src/elvis_core.erl b/src/elvis_core.erl index cee64042..899d9902 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -31,20 +31,35 @@ start() -> ok. -spec rock([elvis_config:t()]) -> - ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. + ok | {fail, [FailThrow | FailFile | FailRules | FailInvalid]} +when + FailThrow :: {throw, term()}, + FailFile :: elvis_result:file(), + FailRules :: elvis_result:rule(), + FailInvalid :: {elvis_config, [elvis_config:validation_error()]}. rock(ElvisConfig) -> - ok = elvis_config:validate(ElvisConfig), + _IsValid = elvis_config:validate(ElvisConfig), + continue_to_rock({_StopOnErrors = false, []}, ElvisConfig). + +continue_to_rock({false = _StopOnErrors, _Errors}, ElvisConfig) -> Results = lists:map(fun do_parallel_rock/1, ElvisConfig), lists:foldl(fun combine_results/2, ok, Results). -spec rock_this(target(), [elvis_config:t()]) -> - ok | {fail, [elvis_result:file() | elvis_result:rule()]}. + ok | {fail, [FailFile | FailRule | FailInvalid]} +when + FailFile :: elvis_result:file(), + FailRule :: elvis_result:rule(), + FailInvalid :: {throw, {elvis_config, [elvis_config:validation_error()]}}. rock_this(Module, ElvisConfig) when is_atom(Module) -> ModuleInfo = Module:module_info(compile), Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> - elvis_config:validate(ElvisConfig), + _IsValid = elvis_config:validate(ElvisConfig), + continue_to_rock_this({_StopOnErrors = false, []}, Path, ElvisConfig). + +continue_to_rock_this({false = _StopOnErrors, _Errors}, Path, ElvisConfig) -> Dirname = filename:dirname(Path), Filename = filename:basename(Path), File = From 38f0ae4ba3f7579858ff7b75bd06e3b70c267f43 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sat, 5 Jul 2025 03:32:02 +0100 Subject: [PATCH 04/67] Implement mechanism to prevent re-validation (since we have no other state to work with, at the moment) --- src/elvis_config.erl | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 80947d49..ed67b858 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -343,3 +343,36 @@ is_rule_override(Rule, UserRules) -> end, UserRules ). + +flag_validated(Option) -> + Table = table(), + _ = create_table(Table), + Obj = validated_flag(Option), + ets:insert(Table, Obj). + +is_validated(Option) -> + Table = table(), + case table_exists(Table) of + false -> + false; + _ -> + Obj = validated_flag(Option), + ets:lookup(Table, Option) =:= [Obj] + end. + +validated_flag(Option) -> + {Option, validated}. + +create_table(Table) -> + case table_exists(Table) of + false -> + _ = ets:new(Table, [public, named_table]); + _ -> + ok + end. + +table() -> + ?MODULE. + +table_exists(Table) -> + ets:info(Table) =/= undefined. From 60d2d0b6ab427fe710c4b7932f3d8f0c1d9defec Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sat, 5 Jul 2025 11:54:20 +0100 Subject: [PATCH 05/67] Go back to elvis_core as is: leave validation to elvis_config ... and consumers --- src/elvis_core.erl | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 899d9902..cee64042 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -31,35 +31,20 @@ start() -> ok. -spec rock([elvis_config:t()]) -> - ok | {fail, [FailThrow | FailFile | FailRules | FailInvalid]} -when - FailThrow :: {throw, term()}, - FailFile :: elvis_result:file(), - FailRules :: elvis_result:rule(), - FailInvalid :: {elvis_config, [elvis_config:validation_error()]}. + ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> - _IsValid = elvis_config:validate(ElvisConfig), - continue_to_rock({_StopOnErrors = false, []}, ElvisConfig). - -continue_to_rock({false = _StopOnErrors, _Errors}, ElvisConfig) -> + ok = elvis_config:validate(ElvisConfig), Results = lists:map(fun do_parallel_rock/1, ElvisConfig), lists:foldl(fun combine_results/2, ok, Results). -spec rock_this(target(), [elvis_config:t()]) -> - ok | {fail, [FailFile | FailRule | FailInvalid]} -when - FailFile :: elvis_result:file(), - FailRule :: elvis_result:rule(), - FailInvalid :: {throw, {elvis_config, [elvis_config:validation_error()]}}. + ok | {fail, [elvis_result:file() | elvis_result:rule()]}. rock_this(Module, ElvisConfig) when is_atom(Module) -> ModuleInfo = Module:module_info(compile), Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> - _IsValid = elvis_config:validate(ElvisConfig), - continue_to_rock_this({_StopOnErrors = false, []}, Path, ElvisConfig). - -continue_to_rock_this({false = _StopOnErrors, _Errors}, Path, ElvisConfig) -> + elvis_config:validate(ElvisConfig), Dirname = filename:dirname(Path), Filename = filename:basename(Path), File = From 6bf4ceb73289b47a10d437e922524d0ce3cd0fb9 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sat, 5 Jul 2025 13:49:39 +0100 Subject: [PATCH 06/67] Make it easier to debug and formulate newer implementation --- src/elvis_config.erl | 190 ++++++++++++++++++++++++++---------------- src/elvis_core.erl | 2 - src/elvis_ruleset.erl | 62 +++++++++----- src/elvis_utils.erl | 14 ++++ 4 files changed, 172 insertions(+), 96 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index ed67b858..ddcaf599 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -23,11 +23,26 @@ %%% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -load_elvis(AppConfig) -> - ElvisConfig = proplists:get_value(elvis, AppConfig, default(elvis)), - elvis_ruleset:set_rulesets(proplists:get_value(rulesets, ElvisConfig, default(rulesets))), +fetch_elvis_config(AppConfig) -> + ElvisConfig = from_static(elvis, {app, AppConfig}), + elvis_ruleset:load(from_static(rulesets, {elvis, ElvisConfig})), ElvisConfig. +from_static(Key, {Type, Config}) -> + elvis_utils:output(debug, "fetching key ~p from ~p config.", [Key, Type]), + case proplists:get_value(Key, Config) of + undefined -> + elvis_utils:output( + debug, "no value for config. key ~p found in ~p config.; going with default", [ + Key, Type + ] + ), + default(Key); + Value -> + elvis_utils:output(debug, "value for config. key ~p found in ~p config.", [Key, Type]), + Value + end. + config() -> for(config). @@ -46,71 +61,98 @@ parallel() -> default(Key) -> case application:get_env(elvis_core, Key) of undefined -> + elvis_utils:output( + debug, + "no value for config. key ~p found in application env.; going with default", + [Key] + ), default_for(Key); {ok, Value} -> + elvis_utils:output(debug, "value for config. key ~p found in application env.", [Key]), Value end. for(Key) -> + ElvisDefault = default_for(elvis), AppConfig = - case consult_elvis_config(default) of - [] -> - consult_rebar_config(default); + case consult_elvis_config("elvis.config") of + ElvisDefault -> + % This might happen whether we fail to parse the fail or it actually is [] + elvis_utils:output( + debug, "elvis.config unusable; falling back to rebar.config", [] + ), + consult_rebar_config("rebar.config"); AppConfig0 -> AppConfig0 end, - TopLevelCfg = load_elvis(AppConfig), - proplists:get_value(Key, TopLevelCfg, default(Key)). - -consult_elvis_config(File0) -> - File = - case File0 of - default -> - "elvis.config"; - _ -> - File0 - end, + ElvisConfig = fetch_elvis_config(AppConfig), + from_static(Key, {elvis, ElvisConfig}). + +consult_elvis_config(File) -> case file:consult(File) of {ok, [AppConfig]} -> + elvis_utils:output(debug, "elvis.config usable; using it", []), AppConfig; _ -> - [] + elvis_utils:output(debug, "elvis.config unusable", []), + default_for(elvis) end. -consult_rebar_config(File0) -> - File = - case File0 of - default -> - "rebar.config"; - _ -> - File0 - end, +consult_rebar_config(File) -> case file:consult(File) of {ok, AppConfig0} -> + elvis_utils:output(debug, "rebar.config usable; using it", []), AppConfig0; _ -> - [] + elvis_utils:output(debug, "rebar.config unusable", []), + default_for(elvis) end. from_rebar(File) -> RebarConfig = consult_rebar_config(File), - TopLevelCfg = load_elvis(RebarConfig), - Key = config, - proplists:get_value(Key, TopLevelCfg, default(Key)). + ElvisConfig = fetch_elvis_config(RebarConfig), + from_static(config, {elvis, ElvisConfig}). from_file(File) -> FileConfig = consult_elvis_config(File), - TopLevelCfg = load_elvis(FileConfig), - Key = config, - proplists:get_value(Key, TopLevelCfg, default(Key)). - -default_for(elvis) -> []; -default_for(config) -> []; -default_for(output_format) -> colors; -default_for(verbose) -> false; -default_for(no_output) -> false; -default_for(parallel) -> 1; -default_for(rulesets) -> #{}. + ElvisConfig = fetch_elvis_config(FileConfig), + from_static(config, {elvis, ElvisConfig}). + +default_for(elvis) -> + []; +default_for(config) -> + [ + #{ + dirs => ["apps/*/src/**", "src/**"], + filter => "*.erl", + ruleset => erl_files + }, + #{ + dirs => ["."], + filter => "rebar.config", + ruleset => rebar_config + }, + #{ + dirs => ["."], + filter => ".gitignore", + ruleset => gitignore + }, + #{ + dirs => ["."], + filter => "elvis.config", + ruleset => elvis_config + } + ]; +default_for(output_format) -> + colors; +default_for(verbose) -> + false; +default_for(no_output) -> + false; +default_for(parallel) -> + 1; +default_for(rulesets) -> + #{}. -spec validate(Config :: [t()]) -> [validation_error()]. validate([]) -> @@ -344,35 +386,37 @@ is_rule_override(Rule, UserRules) -> UserRules ). -flag_validated(Option) -> - Table = table(), - _ = create_table(Table), - Obj = validated_flag(Option), - ets:insert(Table, Obj). - -is_validated(Option) -> - Table = table(), - case table_exists(Table) of - false -> - false; - _ -> - Obj = validated_flag(Option), - ets:lookup(Table, Option) =:= [Obj] - end. - -validated_flag(Option) -> - {Option, validated}. - -create_table(Table) -> - case table_exists(Table) of - false -> - _ = ets:new(Table, [public, named_table]); - _ -> - ok - end. - -table() -> - ?MODULE. - -table_exists(Table) -> - ets:info(Table) =/= undefined. +%flag_validated(Option) -> +% Table = table(), +% _ = create_table(Table), +% Obj = validated_flag(Option), +% ets:insert(Table, Obj). +% +%is_validated(Option) -> +% Table = table(), +% case table_exists(Table) of +% false -> +% false; +% _ -> +% Obj = validated_flag(Option), +% ets:lookup(Table, Option) =:= [Obj] +% end. +% +%validated_flag(Option) -> +% {Option, validated}. +% +%create_table(Table) -> +% case table_exists(Table) of +% false -> +% _ = ets:new(Table, [public, named_table]); +% _ -> +% ok +% end. +% +%table() -> +% ?MODULE. +% +%table_exists(Table) -> +% ets:info(Table) =/= undefined. +% +%-spec default_config() -> [t()]. diff --git a/src/elvis_core.erl b/src/elvis_core.erl index cee64042..9018edff 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -33,7 +33,6 @@ start() -> -spec rock([elvis_config:t()]) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> - ok = elvis_config:validate(ElvisConfig), Results = lists:map(fun do_parallel_rock/1, ElvisConfig), lists:foldl(fun combine_results/2, ok, Results). @@ -44,7 +43,6 @@ rock_this(Module, ElvisConfig) when is_atom(Module) -> Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> - elvis_config:validate(ElvisConfig), Dirname = filename:dirname(Path), Filename = filename:basename(Path), File = diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index c201fa45..8761e479 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -2,18 +2,28 @@ -format(#{inline_items => none}). --export([rules/1, set_rulesets/1]). - --spec set_rulesets(#{atom() => list()}) -> ok. -set_rulesets(Rulesets) -> - Tid = ensure_clean_table(), - lists:foreach( - fun({Ruleset, NSNameDefs}) -> - Rules = [elvis_rule:from_tuple(NSNameDef) || NSNameDef <- NSNameDefs], - true = ets:insert(Tid, {Ruleset, Rules}) - end, - maps:to_list(Rulesets) - ). +-export([rules/1, load/1]). + +-ifdef(TEST). +-export([drop_custom/0]). +-endif. + +-spec load(#{atom() => list()}) -> ok. +load(Rulesets) -> + {Existed, Tid} = ensure_table(), + case Existed of + false -> + elvis_utils:output(debug, "loading custom rulesets into state", []), + lists:foreach( + fun({Ruleset, NSNameDefs}) -> + Rules = [elvis_rule:from_tuple(NSNameDef) || NSNameDef <- NSNameDefs], + true = ets:insert(Tid, {Ruleset, Rules}) + end, + maps:to_list(Rulesets) + ); + true -> + ok + end. -spec rules(Group :: atom()) -> [elvis_rule:t()]. rules(gitignore) -> @@ -35,22 +45,32 @@ rules(rebar_config) -> rules(erl_files_test) -> erl_files_test_rules(); rules(Group) -> - try - ets:lookup_element(?MODULE, Group, 2) - catch - error:badarg -> + Table = table(), + case ets:lookup(Table, Group) of + [{Group, Rules}] -> + Rules; + _ -> [] end. -ensure_clean_table() -> - case ets:info(?MODULE) of +ensure_table() -> + Table = table(), + case ets:info(Table) of undefined -> - ets:new(?MODULE, [set, named_table, {keypos, 1}, public]); + {false, ets:new(Table, [set, named_table, {keypos, 1}, public])}; _ -> - true = ets:delete_all_objects(?MODULE), - ?MODULE + {true, Table} end. +table() -> + ?MODULE. + +-ifdef(TEST). +drop_custom() -> + Table = table(), + ets:delete(Table). +-endif. + gitignore_rules() -> [ elvis_rule:new(elvis_gitignore, forbidden_patterns), diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index ad6803a1..050f033c 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -6,6 +6,8 @@ -export([erlang_halt/1, to_str/1, split_all_lines/1, split_all_lines/2]). %% Output -export([info/2, notice/2, error/2, error_prn/2, warn_prn/2]). +%% rebar3 +-export([output/3, abort/2]). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Public @@ -111,3 +113,15 @@ escape_format_str(String) -> Result = re:replace(Binary, "[^~]~", "~~", [global]), ResultBin = iolist_to_binary(Result), binary_to_list(ResultBin). + +-dialyzer({nowarn_function, output/3}). +-spec output(debug | info, Format :: io:format(), Data :: [term()]) -> ok. +output(debug, Format, Data) -> + rebar_api:debug("Elvis: " ++ Format, Data); +output(info, Format, Data) -> + rebar_api:info("Elvis: " ++ Format, Data). + +-dialyzer({nowarn_function, abort/2}). +-spec abort(Format :: io:format(), Data :: [term()]) -> no_return(). +abort(Format, Data) -> + rebar_api:abort("Elvis: " ++ Format, [Data]). From 2f5b2b734fec71426460fc4729a894a38acc71ca Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 18:08:11 +0100 Subject: [PATCH 07/67] Doc. it for consumers --- src/elvis_core.erl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 9018edff..5a8af73f 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -30,6 +30,8 @@ start() -> {ok, _} = application:ensure_all_started(elvis_core), ok. +%% In this context, `throw` means an error, e.g., validation or internal, not an actual +%% call to `erlang:throw/1`. -spec rock([elvis_config:t()]) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> @@ -70,6 +72,8 @@ rock_this(Path, ElvisConfig) -> elvis_result_status(Results) end. +%% In this context, `throw` means an error, e.g., validation or internal, not an actual +%% call to `erlang:throw/1`. -spec do_parallel_rock(elvis_config:t()) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. From 1658690a8a411c9251828cd01f1e2584520c17ac Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 18:08:33 +0100 Subject: [PATCH 08/67] Prepare for further validation --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index ddcaf599..232eb5b5 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -169,7 +169,7 @@ do_validate(RuleGroup) -> ok ?= maybe_invalid_rules(RuleGroup) else {error, Error} -> - throw({invalid_config, Error}) + {error, {invalid_config, Error}} end. maybe_missing_dirs(RuleGroup) -> From 16e475b811b54cfae81f4a5b1f332d5c3dd2feb4 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 22:44:16 +0100 Subject: [PATCH 09/67] Trim to useful API --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 232eb5b5..15db396f 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -2,7 +2,7 @@ -feature(maybe_expr, enable). --export([from_rebar/1, from_file/1, validate/1]). +-export([from_rebar/1, from_file/1]). %% Geters -export([dirs/1, ignore/1, filter/1, files/1, rules/1]). %% Files From d11c2f89aefb9e811a406f896b55933047784fcf Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 22:44:32 +0100 Subject: [PATCH 10/67] Doc. it a little better --- src/elvis_config.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 15db396f..8b816fea 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -13,6 +13,7 @@ %% Options -export([config/0, output_format/0, verbose/0, no_output/0, parallel/0]). +% Corresponds to the 'config' key. -type t() :: map(). -export_type([t/0]). From 65e98c5e7868c499008ad8ba50792a38bd2df69f Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 22:45:19 +0100 Subject: [PATCH 11/67] Export used type --- src/elvis_config.erl | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 8b816fea..a00280d2 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -17,6 +17,9 @@ -type t() :: map(). -export_type([t/0]). +-type elvis() :: proplists:proplist(). +-export_type([elvis/0]). + -type validation_error() :: empty_config. -export_type([validation_error/0]). @@ -27,7 +30,12 @@ fetch_elvis_config(AppConfig) -> ElvisConfig = from_static(elvis, {app, AppConfig}), elvis_ruleset:load(from_static(rulesets, {elvis, ElvisConfig})), - ElvisConfig. + case validate(elvis) of + {error, _Errors} = E -> + E; + ok -> + {ok, ElvisConfig} + end. from_static(Key, {Type, Config}) -> elvis_utils:output(debug, "fetching key ~p from ~p config.", [Key, Type]), @@ -74,10 +82,10 @@ default(Key) -> end. for(Key) -> - ElvisDefault = default_for(elvis), + AppDefault = default_for(app), AppConfig = case consult_elvis_config("elvis.config") of - ElvisDefault -> + AppDefault -> % This might happen whether we fail to parse the fail or it actually is [] elvis_utils:output( debug, "elvis.config unusable; falling back to rebar.config", [] @@ -86,7 +94,8 @@ for(Key) -> AppConfig0 -> AppConfig0 end, - ElvisConfig = fetch_elvis_config(AppConfig), + % If we got this far, the config. is valid... + {ok, ElvisConfig} = fetch_elvis_config(AppConfig), from_static(Key, {elvis, ElvisConfig}). consult_elvis_config(File) -> @@ -96,7 +105,7 @@ consult_elvis_config(File) -> AppConfig; _ -> elvis_utils:output(debug, "elvis.config unusable", []), - default_for(elvis) + default_for(app) end. consult_rebar_config(File) -> @@ -106,19 +115,30 @@ consult_rebar_config(File) -> AppConfig0; _ -> elvis_utils:output(debug, "rebar.config unusable", []), - default_for(elvis) + default_for(app) end. +-spec from_rebar(string()) -> {ok, elvis()} | {error, [validation_error()]}. from_rebar(File) -> - RebarConfig = consult_rebar_config(File), - ElvisConfig = fetch_elvis_config(RebarConfig), - from_static(config, {elvis, ElvisConfig}). + AppConfig = consult_rebar_config(File), + fetch_elvis_config_from(AppConfig). +-spec from_file(string()) -> {ok, elvis()} | {error, [validation_error()]}. from_file(File) -> FileConfig = consult_elvis_config(File), - ElvisConfig = fetch_elvis_config(FileConfig), - from_static(config, {elvis, ElvisConfig}). + fetch_elvis_config_from(FileConfig). + +fetch_elvis_config_from(AppConfig) -> + case fetch_elvis_config(AppConfig) of + {error, _Errors} = E -> + E; + {ok, ElvisConfig} -> + {ok, from_static(config, {elvis, ElvisConfig})} + end. +default_for(app) -> + % This is the top-level element, before 'elvis' + []; default_for(elvis) -> []; default_for(config) -> From 4183e7466681c1d13762de040c48c7170c5ac081 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 22:45:51 +0100 Subject: [PATCH 12/67] Respect current constraints --- src/elvis_utils.erl | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 050f033c..64cd7abf 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -115,13 +115,36 @@ escape_format_str(String) -> binary_to_list(ResultBin). -dialyzer({nowarn_function, output/3}). --spec output(debug | info, Format :: io:format(), Data :: [term()]) -> ok. +-spec output(debug | info | error, Format :: io:format(), Data :: [term()]) -> ok. output(debug, Format, Data) -> - rebar_api:debug("Elvis: " ++ Format, Data); + case erlang:function_exported(rebar_api, debug, 2) of + true -> + rebar_api:debug("Elvis: " ++ Format, Data); + false -> + ok + end; output(info, Format, Data) -> - rebar_api:info("Elvis: " ++ Format, Data). + case erlang:function_exported(rebar_api, info, 2) of + true -> + rebar_api:info("Elvis: " ++ Format, Data); + false -> + ok + end; +output(error, Format, Data) -> + case erlang:function_exported(rebar_api, error, 2) of + true -> + rebar_api:error("Elvis: " ++ Format, Data); + false -> + ok + end. -dialyzer({nowarn_function, abort/2}). -spec abort(Format :: io:format(), Data :: [term()]) -> no_return(). abort(Format, Data) -> - rebar_api:abort("Elvis: " ++ Format, [Data]). + case erlang:function_exported(rebar_api, abort, 2) of + true -> + rebar_api:abort("Elvis: " ++ Format, [Data]); + false -> + output(error, Format, Data), + throw(elvis_abort) + end. From 94bf88b6b778d240a33ea86ce996fa2057617071 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 10 Jul 2025 22:52:15 +0100 Subject: [PATCH 13/67] Adapt to current API --- src/elvis_config.erl | 2 +- src/elvis_core.erl | 2 ++ src/elvis_ruleset.erl | 15 ++++++++++++--- test/elvis_SUITE.erl | 14 +++++++------- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index a00280d2..f569bf24 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -29,7 +29,7 @@ fetch_elvis_config(AppConfig) -> ElvisConfig = from_static(elvis, {app, AppConfig}), - elvis_ruleset:load(from_static(rulesets, {elvis, ElvisConfig})), + elvis_ruleset:load_custom(from_static(rulesets, {elvis, ElvisConfig})), case validate(elvis) of {error, _Errors} = E -> E; diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 5a8af73f..2843aefc 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -35,6 +35,7 @@ start() -> -spec rock([elvis_config:t()]) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> + elvis_ruleset:dump_custom(), Results = lists:map(fun do_parallel_rock/1, ElvisConfig), lists:foldl(fun combine_results/2, ok, Results). @@ -45,6 +46,7 @@ rock_this(Module, ElvisConfig) when is_atom(Module) -> Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> + elvis_ruleset:dump_custom(), Dirname = filename:dirname(Path), Filename = filename:basename(Path), File = diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index 8761e479..5d979413 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -2,14 +2,14 @@ -format(#{inline_items => none}). --export([rules/1, load/1]). +-export([rules/1, load_custom/1, dump_custom/0]). -ifdef(TEST). -export([drop_custom/0]). -endif. --spec load(#{atom() => list()}) -> ok. -load(Rulesets) -> +-spec load_custom(#{atom() => list()}) -> ok. +load_custom(Rulesets) -> {Existed, Tid} = ensure_table(), case Existed of false -> @@ -25,6 +25,15 @@ load(Rulesets) -> ok end. +dump_custom() -> + Table = table(), + case ets:info(Table) of + undefined -> + ok; + _ -> + ets:delete(Table) + end. + -spec rules(Group :: atom()) -> [elvis_rule:t()]. rules(gitignore) -> gitignore_rules(); diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 6a410097..028bade7 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -111,7 +111,7 @@ rock_with_list_config(_Config) -> rock_with_file_config(_Config) -> ConfigPath = "../../config/elvis.config", - ElvisConfig = elvis_config:from_file(ConfigPath), + {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = "# \\.\\./\\.\\./_build/test/lib/elvis_core/test/" ++ "examples/.*\\.erl.*FAIL", @@ -120,7 +120,7 @@ rock_with_file_config(_Config) -> rock_with_rebar_default_config(_Config) -> {ok, _} = file:copy("../../config/rebar.config", "rebar.config"), - ElvisConfig = elvis_config:from_rebar("rebar.config"), + {ok, ElvisConfig} = elvis_config:from_rebar("rebar.config"), [#{name := line_length}] = try {fail, Results} = elvis_core:rock(ElvisConfig), @@ -205,7 +205,7 @@ rock_with_errors_has_output(_Config) -> rock_without_errors_has_no_output(_Config) -> ConfigPath = "../../config/test.pass.config", - ElvisConfig = elvis_config:from_file(ConfigPath), + {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Output = get_output(Fun), %% This is related to the test case `rock_with_non_parsable_file`, @@ -318,7 +318,7 @@ rock_this_skipping_files(_Config) -> [File] = elvis_file:find_files(Dirs, "small.erl"), Path = elvis_file:path(File), ConfigPath = "../../config/elvis-test-pa.config", - ElvisConfig = elvis_config:from_file(ConfigPath), + {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), ok = elvis_core:rock_this(Path, ElvisConfig), 0 = meck:num_calls(elvis_file, load_file_data, '_'), meck:unload(elvis_file), @@ -337,7 +337,7 @@ rock_this_not_skipping_files(_Config) -> rock_with_umbrella_apps(_Config) -> ElvisUmbrellaConfigFile = "../../config/elvis-umbrella.config", - ElvisConfig = elvis_config:from_file(ElvisUmbrellaConfigFile), + {ok, ElvisConfig} = elvis_config:from_file(ElvisUmbrellaConfigFile), {fail, [ #{file := "../../_build/test/lib/elvis_core/test/dirs/test/dir_test.erl"}, #{file := "../../_build/test/lib/elvis_core/test/dirs/src/dirs_src.erl"}, @@ -384,7 +384,7 @@ rock_with_invalid_rules(_Config) -> custom_ruleset(_Config) -> ConfigPath = "../../config/elvis-test-custom-ruleset.config", - ElvisConfig = elvis_config:from_file(ConfigPath), + {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), NoTabs = elvis_rule:new(elvis_text_style, no_tabs), [[NoTabs]] = elvis_config:rules(ElvisConfig), @@ -397,7 +397,7 @@ custom_ruleset(_Config) -> hrl_ruleset(_Config) -> ConfigPath = "../../config/elvis-test-hrl-files.config", - ElvisConfig = elvis_config:from_file(ConfigPath), + {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), {fail, [ #{file := "../../_build/test/lib/elvis_core/test/examples/test_good.hrl", rules := []}, #{ From bf33731766a1d58cbb7b3ef4d5ff365b576141f9 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sun, 13 Jul 2025 17:58:50 +0100 Subject: [PATCH 14/67] Expect test results to remain the same (adapt to current implementation) --- test/elvis_SUITE.erl | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 028bade7..6a410097 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -111,7 +111,7 @@ rock_with_list_config(_Config) -> rock_with_file_config(_Config) -> ConfigPath = "../../config/elvis.config", - {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), + ElvisConfig = elvis_config:from_file(ConfigPath), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = "# \\.\\./\\.\\./_build/test/lib/elvis_core/test/" ++ "examples/.*\\.erl.*FAIL", @@ -120,7 +120,7 @@ rock_with_file_config(_Config) -> rock_with_rebar_default_config(_Config) -> {ok, _} = file:copy("../../config/rebar.config", "rebar.config"), - {ok, ElvisConfig} = elvis_config:from_rebar("rebar.config"), + ElvisConfig = elvis_config:from_rebar("rebar.config"), [#{name := line_length}] = try {fail, Results} = elvis_core:rock(ElvisConfig), @@ -205,7 +205,7 @@ rock_with_errors_has_output(_Config) -> rock_without_errors_has_no_output(_Config) -> ConfigPath = "../../config/test.pass.config", - {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), + ElvisConfig = elvis_config:from_file(ConfigPath), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Output = get_output(Fun), %% This is related to the test case `rock_with_non_parsable_file`, @@ -318,7 +318,7 @@ rock_this_skipping_files(_Config) -> [File] = elvis_file:find_files(Dirs, "small.erl"), Path = elvis_file:path(File), ConfigPath = "../../config/elvis-test-pa.config", - {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), + ElvisConfig = elvis_config:from_file(ConfigPath), ok = elvis_core:rock_this(Path, ElvisConfig), 0 = meck:num_calls(elvis_file, load_file_data, '_'), meck:unload(elvis_file), @@ -337,7 +337,7 @@ rock_this_not_skipping_files(_Config) -> rock_with_umbrella_apps(_Config) -> ElvisUmbrellaConfigFile = "../../config/elvis-umbrella.config", - {ok, ElvisConfig} = elvis_config:from_file(ElvisUmbrellaConfigFile), + ElvisConfig = elvis_config:from_file(ElvisUmbrellaConfigFile), {fail, [ #{file := "../../_build/test/lib/elvis_core/test/dirs/test/dir_test.erl"}, #{file := "../../_build/test/lib/elvis_core/test/dirs/src/dirs_src.erl"}, @@ -384,7 +384,7 @@ rock_with_invalid_rules(_Config) -> custom_ruleset(_Config) -> ConfigPath = "../../config/elvis-test-custom-ruleset.config", - {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), + ElvisConfig = elvis_config:from_file(ConfigPath), NoTabs = elvis_rule:new(elvis_text_style, no_tabs), [[NoTabs]] = elvis_config:rules(ElvisConfig), @@ -397,7 +397,7 @@ custom_ruleset(_Config) -> hrl_ruleset(_Config) -> ConfigPath = "../../config/elvis-test-hrl-files.config", - {ok, ElvisConfig} = elvis_config:from_file(ConfigPath), + ElvisConfig = elvis_config:from_file(ConfigPath), {fail, [ #{file := "../../_build/test/lib/elvis_core/test/examples/test_good.hrl", rules := []}, #{ From 33926906b37d66757d7f3b821ed1543db42757c0 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sun, 13 Jul 2025 17:59:15 +0100 Subject: [PATCH 15/67] Allow elvis_rule to validate an input 2/3-tuple --- src/elvis_rule.erl | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/elvis_rule.erl b/src/elvis_rule.erl index d00394df..d90425ab 100644 --- a/src/elvis_rule.erl +++ b/src/elvis_rule.erl @@ -3,6 +3,7 @@ -export([ new/2, new/3, from_tuple/1, + is_valid_from_tuple/1, ns/1, name/1, def/1, @@ -49,7 +50,7 @@ new(NS, Name, Def) -> ignores = maps:get(ignore, Def, []) }. --spec from_tuple(Rule | NSName | NSNameDef) -> t() when +-spec from_tuple(Rule | NSName | NSNameDef) -> t() | invalid_tuple when Rule :: t(), NSName :: {NS :: module(), Name :: atom()}, NSNameDef :: {NS :: module(), Name :: atom(), Def :: disable | map()}. @@ -57,7 +58,7 @@ from_tuple(Rule) when is_record(Rule, rule) -> Rule; from_tuple({NS, Name}) -> from_tuple({NS, Name, #{}}); -from_tuple({NS, Name, Def0}) -> +from_tuple({NS, Name, Def0}) when is_map(Def0) orelse Def0 =:= disable -> {Def, Disable} = case Def0 of disable -> @@ -71,6 +72,36 @@ from_tuple({NS, Name, Def0}) -> disable(Rule); false -> Rule + end; +from_tuple(_) -> + invalid_tuple. + +is_valid_from_tuple(Tuple) -> + case from_tuple(Tuple) of + invalid_tuple -> + {false, "got an invalid tuple (is def. a map or 'disable'?)."}; + Rule -> + NS = ns(Rule), + case is_atom(NS) of + true -> + _ = code:ensure_loaded(NS); + false -> + ok + end, + Name = name(Rule), + ArityForExecute = 2, + case + is_atom(NS) andalso is_atom(Name) andalso + erlang:function_exported(NS, Name, ArityForExecute) + of + true -> + {true, Rule}; + _ -> + {false, + io_lib:format("got an unexpected/invalid ~p:~p/~p combo.", [ + NS, Name, ArityForExecute + ])} + end end. -spec ns(t()) -> module(). From 49018f60b9d5d8bc6e56846569d0304be31d80fe Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sun, 13 Jul 2025 18:55:22 +0100 Subject: [PATCH 16/67] Allow elvis_rule to validate an input ignorable --- src/elvis_rule.erl | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/elvis_rule.erl b/src/elvis_rule.erl index d90425ab..e03bf122 100644 --- a/src/elvis_rule.erl +++ b/src/elvis_rule.erl @@ -4,6 +4,7 @@ new/2, new/3, from_tuple/1, is_valid_from_tuple/1, + is_ignorable/1, ns/1, name/1, def/1, @@ -104,6 +105,42 @@ is_valid_from_tuple(Tuple) -> end end. +% Module - invalid type +is_ignorable(Module) when not is_tuple(Module) andalso not is_atom(Module) -> + false; +% Module - test if valid +is_ignorable(Module) when is_atom(Module) -> + case code:ensure_loaded(Module) of + {module, _} -> + true; + _ -> + false + end; +% {Module, Function} - invalid type +is_ignorable({Module, Function}) when not is_atom(Module) orelse not is_atom(Function) -> + false; +% {Module, Function} - test if valid +is_ignorable({Module, Function}) -> + case is_ignorable(Module) of + true -> + Exports = Module:module_info(exports), + proplists:get_value(Function, Exports) =/= undefined; + false -> + false + end; +% {Module, Function, Arity} - invalid type +is_ignorable({Module, Function, Arity}) when not is_atom(Module) orelse not is_atom(Function) orelse not is_integer(Arity) orelse Arity < 0 -> + false; +% {Module, Function, Arity} - test if valid +is_ignorable({Module, Function, Arity}) -> + case is_ignorable(Module) of + true -> + Exports = Module:module_info(exports), + proplists:get_value(Function, Exports) =:= Arity; + false -> + false + end. + -spec ns(t()) -> module(). ns(Rule) -> Rule#rule.ns. From 85517d6cbdb237f22bac12e2fb69dea9fbbb887b Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sun, 13 Jul 2025 18:55:38 +0100 Subject: [PATCH 17/67] Have notion of definition belong to appropriate namespace --- src/elvis_ruleset.erl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index 5d979413..4ea57f92 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -2,7 +2,7 @@ -format(#{inline_items => none}). --export([rules/1, load_custom/1, dump_custom/0]). +-export([rules/1, load_custom/1, dump_custom/0, is_defined/1]). -ifdef(TEST). -export([drop_custom/0]). @@ -34,6 +34,9 @@ dump_custom() -> ets:delete(Table) end. +is_defined(Ruleset) -> + elvis_ruleset:rules(Ruleset) =/= []. + -spec rules(Group :: atom()) -> [elvis_rule:t()]. rules(gitignore) -> gitignore_rules(); From 642f85d69bdb0c654241a584cef1638f084a0389 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Sun, 13 Jul 2025 22:02:56 +0100 Subject: [PATCH 18/67] Allow for further config. validation --- src/elvis_rule.erl | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/elvis_rule.erl b/src/elvis_rule.erl index e03bf122..1445b632 100644 --- a/src/elvis_rule.erl +++ b/src/elvis_rule.erl @@ -15,6 +15,7 @@ execute/2, option/2, defmap/1, + defkeys/1, ignorable/1, same/2 ]). @@ -83,12 +84,7 @@ is_valid_from_tuple(Tuple) -> {false, "got an invalid tuple (is def. a map or 'disable'?)."}; Rule -> NS = ns(Rule), - case is_atom(NS) of - true -> - _ = code:ensure_loaded(NS); - false -> - ok - end, + _ = maybe_ensure_loaded(NS), Name = name(Rule), ArityForExecute = 2, case @@ -105,12 +101,17 @@ is_valid_from_tuple(Tuple) -> end end. +maybe_ensure_loaded(NS) when not is_atom(NS) -> + ok; +maybe_ensure_loaded(NS) -> + code:ensure_loaded(NS). + % Module - invalid type is_ignorable(Module) when not is_tuple(Module) andalso not is_atom(Module) -> false; % Module - test if valid is_ignorable(Module) when is_atom(Module) -> - case code:ensure_loaded(Module) of + case maybe_ensure_loaded(Module) of {module, _} -> true; _ -> @@ -129,7 +130,9 @@ is_ignorable({Module, Function}) -> false end; % {Module, Function, Arity} - invalid type -is_ignorable({Module, Function, Arity}) when not is_atom(Module) orelse not is_atom(Function) orelse not is_integer(Arity) orelse Arity < 0 -> +is_ignorable({Module, Function, Arity}) when + not is_atom(Module) orelse not is_atom(Function) orelse not is_integer(Arity) orelse Arity < 0 +-> false; % {Module, Function, Arity} - test if valid is_ignorable({Module, Function, Arity}) -> @@ -206,12 +209,24 @@ default(Rule) -> -spec default(NS :: module(), Name :: atom()) -> def(). default(NS, Name) -> - NS:default(Name). + _ = maybe_ensure_loaded(NS), + ArityForDefault = 1, + case erlang:function_exported(NS, default, ArityForDefault) of + false -> + #{}; + true -> + NS:default(Name) + end. -spec defmap(map()) -> def(). defmap(Map) -> Map. +-spec defkeys(t()) -> [atom()]. +defkeys(Rule) -> + Def = def(Rule), + maps:keys(Def). + -spec ignorable(module() | {module(), atom()} | {module(), atom(), arity()}) -> ignorable(). ignorable(Ignorable) -> Ignorable. From 27005e610d17c44f16031b0a29b75a6216ee6006 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 14 Jul 2025 22:12:26 +0100 Subject: [PATCH 19/67] Validate the configuration --- elvis.config | 14 +- src/elvis_code.erl | 2 +- src/elvis_config.erl | 556 ++++++++++++++++++++++++++++++++----------- src/elvis_core.erl | 6 +- src/elvis_result.erl | 13 +- src/elvis_utils.erl | 27 +-- 6 files changed, 447 insertions(+), 171 deletions(-) diff --git a/elvis.config b/elvis.config index e90c5bed..310c50f1 100644 --- a/elvis.config +++ b/elvis.config @@ -7,7 +7,10 @@ rules => [ {elvis_style, no_invalid_dynamic_calls, #{ - ignore => [{elvis_core, rock_this, 2}] + ignore => [ + {elvis_core, rock_this, 2}, + {elvis_config, check_rule_for_options, 2} + ] }}, {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}, {elvis_style, no_debug_call, #{ @@ -17,7 +20,7 @@ {elvis_style, no_throw, disable}, {elvis_style, max_function_length, disable}, {elvis_style, max_function_clause_length, disable}, - {elvis_style, max_module_length, #{ignore => [elvis_style]}}, + {elvis_style, max_module_length, #{ignore => [elvis_style, elvis_config]}}, {elvis_style, no_common_caveats_call, #{ ignore => [ {elvis_file, module, 1}, @@ -34,7 +37,9 @@ filter => "*.beam", rules => [ - {elvis_style, no_invalid_dynamic_calls, #{ignore => [elvis_core]}}, + {elvis_style, no_invalid_dynamic_calls, #{ + ignore => [elvis_core, elvis_config] + }}, {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}, {elvis_style, no_debug_call, #{ignore => [elvis_result, elvis_utils]}}, {elvis_style, no_god_modules, #{ignore => [elvis_style]}}, @@ -65,6 +70,7 @@ {elvis_style, atom_naming_convention, #{ignore => [style_SUITE]}} ] } - ]} + ]}, + {no_output, true} ]} ]. diff --git a/src/elvis_code.erl b/src/elvis_code.erl index 09c0701b..8eba2a45 100644 --- a/src/elvis_code.erl +++ b/src/elvis_code.erl @@ -186,6 +186,6 @@ print_node(#{type := Type} = Node, CurrentLevel) -> Indentation = lists:duplicate(CurrentLevel * 4, $\s), Content = ktn_code:content(Node), - ok = elvis_utils:info("~s - [~p] ~p~n", [Indentation, CurrentLevel, Type]), + ok = elvis_utils:output(info, "~s - [~p] ~p~n", [Indentation, CurrentLevel, Type]), _ = lists:map(fun(Child) -> print_node(Child, CurrentLevel + 1) end, Content), ok. diff --git a/src/elvis_config.erl b/src/elvis_config.erl index f569bf24..c1c58c2c 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -20,22 +20,15 @@ -type elvis() :: proplists:proplist(). -export_type([elvis/0]). --type validation_error() :: empty_config. --export_type([validation_error/0]). - %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% fetch_elvis_config(AppConfig) -> ElvisConfig = from_static(elvis, {app, AppConfig}), - elvis_ruleset:load_custom(from_static(rulesets, {elvis, ElvisConfig})), - case validate(elvis) of - {error, _Errors} = E -> - E; - ok -> - {ok, ElvisConfig} - end. + _ = validate({elvis, ElvisConfig}), + _ = elvis_ruleset:load_custom(from_static(rulesets, {elvis, ElvisConfig})), + ElvisConfig. from_static(Key, {Type, Config}) -> elvis_utils:output(debug, "fetching key ~p from ~p config.", [Key, Type]), @@ -95,7 +88,7 @@ for(Key) -> AppConfig0 end, % If we got this far, the config. is valid... - {ok, ElvisConfig} = fetch_elvis_config(AppConfig), + ElvisConfig = fetch_elvis_config(AppConfig), from_static(Key, {elvis, ElvisConfig}). consult_elvis_config(File) -> @@ -118,23 +111,17 @@ consult_rebar_config(File) -> default_for(app) end. --spec from_rebar(string()) -> {ok, elvis()} | {error, [validation_error()]}. from_rebar(File) -> AppConfig = consult_rebar_config(File), fetch_elvis_config_from(AppConfig). --spec from_file(string()) -> {ok, elvis()} | {error, [validation_error()]}. from_file(File) -> FileConfig = consult_elvis_config(File), fetch_elvis_config_from(FileConfig). fetch_elvis_config_from(AppConfig) -> - case fetch_elvis_config(AppConfig) of - {error, _Errors} = E -> - E; - {ok, ElvisConfig} -> - {ok, from_static(config, {elvis, ElvisConfig})} - end. + ElvisConfig = fetch_elvis_config(AppConfig), + from_static(config, {elvis, ElvisConfig}). default_for(app) -> % This is the top-level element, before 'elvis' @@ -144,10 +131,23 @@ default_for(elvis) -> default_for(config) -> [ #{ - dirs => ["apps/*/src/**", "src/**"], + dirs => [ + "apps/**/src/**", + "src/**" + ], filter => "*.erl", ruleset => erl_files }, + #{ + dirs => [ + "apps/**/src/**", + "src/**", + "apps/**/include/**", + "include/**" + ], + filter => "*.erl", + ruleset => hrl_files + }, #{ dirs => ["."], filter => "rebar.config", @@ -157,11 +157,6 @@ default_for(config) -> dirs => ["."], filter => ".gitignore", ruleset => gitignore - }, - #{ - dirs => ["."], - filter => "elvis.config", - ruleset => elvis_config } ]; default_for(output_format) -> @@ -173,74 +168,18 @@ default_for(no_output) -> default_for(parallel) -> 1; default_for(rulesets) -> - #{}. - --spec validate(Config :: [t()]) -> [validation_error()]. -validate([]) -> - [empty_config]; -validate(Config) -> - lists:foreach(fun do_validate/1, Config), + #{}; +default_for([config, dirs]) -> + []; +default_for([config, filter]) -> + ""; +default_for([config, ignore]) -> + []; +default_for([config, ruleset]) -> + undefined; +default_for([config, rules]) -> []. -do_validate(RuleGroup) -> - maybe - ok ?= maybe_missing_dirs(RuleGroup), - ok ?= maybe_missing_filter(RuleGroup), - ok ?= maybe_missing_rules(RuleGroup), - ok ?= maybe_invalid_rules(RuleGroup) - else - {error, Error} -> - {error, {invalid_config, Error}} - end. - -maybe_missing_dirs(RuleGroup) -> - maybe_boolean_wrapper( - not (maps:is_key(dirs, RuleGroup) andalso not maps:is_key(filter, RuleGroup)), missing_dir - ). - -maybe_missing_filter(RuleGroup) -> - maybe_boolean_wrapper( - maps:is_key(dirs, RuleGroup), missing_filter - ). - -maybe_missing_rules(RuleGroup) -> - maybe_boolean_wrapper( - maps:is_key(rules, RuleGroup) orelse maps:is_key(ruleset, RuleGroup), missing_rules - ). - -maybe_boolean_wrapper(true, _Flag) -> ok; -maybe_boolean_wrapper(false, Flag) -> {error, Flag}. - -maybe_invalid_rules(#{rules := Rules}) -> - case invalid_rules(Rules) of - [] -> ok; - InvalidRules -> {error, {invalid_rules, InvalidRules}} - end; -maybe_invalid_rules(_) -> - ok. - -invalid_rules(Rules) -> - lists:filtermap(fun is_invalid_rule/1, Rules). - -is_invalid_rule({NS, Rule, _}) -> - is_invalid_rule({NS, Rule}); -is_invalid_rule({NS, Rule}) -> - maybe - {module, NS} ?= code:ensure_loaded(NS), - ExportedRules = erlang:get_module_info(NS, exports), - case lists:keymember(Rule, 1, ExportedRules) of - false -> {true, {invalid_rule, {NS, Rule}}}; - _ -> false - end - else - {error, _} -> - elvis_utils:warn_prn( - "Invalid module (~p) specified in elvis.config.~n", - [NS] - ), - {true, {invalid_rule, {NS, Rule}}} - end. - -spec dirs(Config :: [t()] | t()) -> [string()]. dirs(Config) when is_list(Config) -> lists:flatmap(fun dirs/1, Config); @@ -303,22 +242,6 @@ resolve_files(RuleGroup, Files) -> Dirs = dirs(RuleGroup), Ignore = ignore(RuleGroup), FilteredFiles = elvis_file:filter_files(Files, Dirs, Filter, Ignore), - _ = - case FilteredFiles of - [] -> - Ruleset = maps:get(ruleset, RuleGroup, undefined), - Error = - elvis_result:new( - warn, - "Searching for files in ~p, for ruleset ~p, " - "with filter ~p, yielded none. " - "Update your configuration", - [Dirs, Ruleset, Filter] - ), - ok = elvis_result:print_results([Error]); - _ -> - ok - end, RuleGroup#{files => FilteredFiles}. %% @doc Takes a configuration and finds all files according to its 'dirs' @@ -407,37 +330,388 @@ is_rule_override(Rule, UserRules) -> UserRules ). -%flag_validated(Option) -> -% Table = table(), -% _ = create_table(Table), -% Obj = validated_flag(Option), -% ets:insert(Table, Obj). -% -%is_validated(Option) -> -% Table = table(), -% case table_exists(Table) of -% false -> -% false; -% _ -> -% Obj = validated_flag(Option), -% ets:lookup(Table, Option) =:= [Obj] -% end. -% -%validated_flag(Option) -> -% {Option, validated}. -% -%create_table(Table) -> -% case table_exists(Table) of -% false -> -% _ = ets:new(Table, [public, named_table]); -% _ -> -% ok -% end. -% -%table() -> -% ?MODULE. -% -%table_exists(Table) -> -% ets:info(Table) =/= undefined. -% -%-spec default_config() -> [t()]. +validate({elvis = Option, ElvisConfig}) -> + case check_flag(validated(Option)) orelse check_flag(validation_started(Option)) of + true -> + ok; + false -> + do_validate({Option, ElvisConfig}) + end. + +get_elvis_opt(OptName, ElvisConfig) -> + proplists:get_value(OptName, ElvisConfig, default_for(OptName)). + +do_validate({elvis = Option, ElvisConfig}) -> + maybe + ok = flag(validation_started(Option)), + ok ?= is_list("elvis", ElvisConfig), + ok ?= + proplist_keys_are_in("elvis", ElvisConfig, [ + output_format, verbose, no_output, parallel, rulesets, config + ]), + OutputFormat = get_elvis_opt(output_format, ElvisConfig), + ok ?= is_one_of("elvis.output_format", OutputFormat, [colors, plain, parsable]), + Verbose = get_elvis_opt(verbose, ElvisConfig), + ok ?= is_boolean("elvis.verbose", Verbose), + NoOutput = get_elvis_opt(no_output, ElvisConfig), + ok ?= is_boolean("elvis.no_output", NoOutput), + Parallel = get_elvis_opt(parallel, ElvisConfig), + ok ?= is_pos_integer("elvis.parallel", Parallel), + Rulesets = get_elvis_opt(rulesets, ElvisConfig), + ok ?= is_valid_rulesets("elvis.rulesets", Rulesets), + Config = get_elvis_opt(config, ElvisConfig), + ok ?= is_valid_config("elvis.config", Rulesets, Config), + ok = flag(validated(Option)) + else + {error, FormatData} -> + {Format, Data} = + case is_list(FormatData) of + false -> + FormatData; + true -> + % Get only first result, for now + % If we wanna be smarter we need to start concatenating readable strings + [{Format0, Data0} | _] = FormatData, + {Format0, Data0} + end, + % on warnings_as_errors this'll throw an exception + %throw({invalid_config, io_lib:format(Format, Data)}) + elvis_utils:output(warn, Format, Data) + end. + +is_list(What, List) -> + case is_list(List) of + true -> + ok; + _ -> + {error, {"~p is expected to be a list.", [What]}} + end. + +proplist_keys_are_in(What, List, Keys) -> + Filtered = [Element || {Element, _} <- List, not lists:member(Element, Keys)], + case Filtered of + [] -> + ok; + _ -> + {error, {"in ~p, the following keys are unknown: ~p.", [What, Filtered]}} + end. + +is_one_of(What, Value, Possibilities) -> + case lists:member(Value, Possibilities) of + true -> + ok; + _ -> + {error, {"~p is expected to be one of the following: ~p.", [What, Possibilities]}} + end. + +is_boolean(What, Value) -> + case is_boolean(Value) of + true -> + ok; + _ -> + {error, {"~p is expected to be a boolean.", [What]}} + end. + +is_pos_integer(What, Value) -> + case is_integer(Value) andalso Value > 0 of + true -> + ok; + _ -> + {error, {"~p is expected to be a positive integer.", [What]}} + end. + +is_valid_rulesets(What, Rulesets) -> + maybe + ok ?= is_map(What, Rulesets), + ok ?= all_map_keys_are_atoms(What, Rulesets), + ok ?= all_rulesets_have_valid_rules(What, Rulesets), + ok ?= no_rulesets_exist(What, Rulesets) + else + {error, FormatData} -> + {error, FormatData} + end. + +is_map(What, Value) -> + case is_map(Value) of + true -> + ok; + _ -> + {error, {"~p is expected to be a map.", [What]}} + end. + +all_map_keys_are_atoms(What, Map) -> + Filtered = [Key || Key <- maps:keys(Map), is_atom(Key)], + case Filtered of + [] -> + ok; + _ -> + {error, {"in ~p, keys are expected to be atoms.", [What]}} + end. + +all_rulesets_have_valid_rules(What, Rulesets) -> + AccOut = maps:fold( + fun(Ruleset, RuleTuples, AccInO) -> + lists:foldl( + fun(RuleTuple, AccInI) -> + case elvis_rule:is_valid_from_tuple(RuleTuple) of + {true, _Rule} -> + AccInI; + {false, ValidError} -> + [{"in ~p, in ruleset ~p, " ++ ValidError, [What, Ruleset]} | AccInI] + end + end, + AccInO, + RuleTuples + ) + end, + [], + Rulesets + ), + case AccOut of + [] -> + ok; + _ -> + {error, lists:reverse(AccOut)} + end. + +no_rulesets_exist(What, Rulesets) -> + Filtered = [Ruleset || Ruleset <- maps:keys(Rulesets), not elvis_ruleset:is_defined(Ruleset)], + case Filtered of + [] -> + ok; + _ -> + {error, + { + "in ~p, the following rulesets are not expected to be " + "named after a default ruleset: ~p.", + [ + What, Filtered + ] + }} + end. + +is_valid_config(What, CustomRulesets, Configset0) -> + Configset = wrap_in_list(Configset0), + maybe + ok ?= is_list(What, Configset), + ok ?= all_configs_are_valid(What, CustomRulesets, Configset) + else + {error, FormatData} -> + {error, FormatData} + end. + +wrap_in_list(Term) when is_list(Term) -> + Term; +wrap_in_list(Term) -> + [Term]. + +all_configs_are_valid(What, CustomRulesets, Configset) -> + {_PosNumber, ValidErrors} = lists:foldl( + fun(Config, {PosNumber, AccIn}) -> + AccOut = + case config_is_valid(CustomRulesets, Config) of + ok -> + AccIn; + {error, ValidError} -> + [ + {"in ~p, at list position number ~p, " ++ ValidError, [What, PosNumber]} + | AccIn + ] + end, + {PosNumber + 1, AccOut} + end, + {1, []}, + Configset + ), + case ValidErrors of + [] -> + ok; + _ -> + {error, lists:reverse(ValidErrors)} + end. + +get_config_opt(OptName, Config) -> + maps:get(OptName, Config, default_for([config, OptName])). + +config_is_valid(CustomRulesets, Config) -> + maybe + ok ?= map_keys_are_in(Config, [dirs, filter, ignore, ruleset, rules]), + Dirs = get_config_opt(dirs, Config), + ok ?= is_nonempty_list_of_dirs(dirs, Dirs), + Filter = get_config_opt(filter, Config), + ok ?= is_nonempty_string(filter, Filter), + ok ?= all_dirs_filter_combos_are_valid(Dirs, Filter), + Ignore = get_config_opt(ignore, Config), + ok ?= is_list_of_ignorables(ignore, Ignore), + Ruleset = get_config_opt(ruleset, Config), + ok ?= ruleset_is_custom_or_default(CustomRulesets, Ruleset), + Rules = get_config_opt(rules, Config), + ok ?= all_rules_are_valid(rules, Rules) + else + {error, ValidError} -> + {error, ValidError} + end. + +map_keys_are_in(Map, Keys) -> + Filtered = [Key || Key <- maps:keys(Map), not lists:member(Key, Keys)], + case Filtered of + [] -> + ok; + _ -> + {error, io_lib:format("the following keys are unknown: ~p.", [Filtered])} + end. + +is_nonempty_list_of_dirs(What, List) when not is_list(List) -> + {error, io_lib:format("~p is expected to be a list.", [What])}; +is_nonempty_list_of_dirs(What, [] = _List) -> + {error, io_lib:format("~p is expected to be a nonempty list.", [What])}; +is_nonempty_list_of_dirs(What, List) -> + Filtered = [ + Element + || Element <- List, not io_lib:char_list(Element) orelse not filelib:is_dir(Element) + ], + case Filtered of + [] -> + ok; + _ -> + {error, + io_lib:format("in ~p, the following elements are not directories: ~p.", [ + What, Filtered + ])} + end. + +is_nonempty_string(What, String) -> + case io_lib:char_list(String) andalso length(String) > 0 of + true -> + ok; + _ -> + {error, io_lib:format("~p is expected to be a non-empty string.", [What])} + end. + +all_dirs_filter_combos_are_valid(Dirs, Filter) -> + AccOut = lists:foldl( + fun(Dir, AccIn) -> + case filelib:wildcard(filename:join(Dir, Filter)) of + [_ | _] -> + AccIn; + _ -> + [ + io_lib:format("dir + filter combo ~p + ~p yielded no files to analyse.", [ + Dir, Filter + ]) + | AccIn + ] + end + end, + [], + Dirs + ), + case AccOut of + [] -> + ok; + _ -> + {error, lists:reverse(AccOut)} + end. + +is_list_of_ignorables(What, List) when not is_list(List) -> + {error, io_lib:format("~p is expected to be a list.", [What])}; +is_list_of_ignorables(What, List) -> + Filtered = [Element || Element <- List, not elvis_rule:is_ignorable(Element)], + case Filtered of + [] -> + ok; + _ -> + {error, + io_lib:format("in ~p, the following elements are not ignorable: ~p.", [ + What, Filtered + ])} + end. + +ruleset_is_custom_or_default(CustomRulesets, Ruleset) -> + case + lists:member(Ruleset, maps:keys(CustomRulesets)) orelse elvis_ruleset:is_defined(Ruleset) + of + true -> + ok; + _ -> + {error, + io_lib:format("~p is expected to be either a custom or a default ruleset.", [ + Ruleset + ])} + end. + +all_rules_are_valid(What, RuleTuples) when not is_list(RuleTuples) -> + {error, io_lib:format("~p is expected to be a list.", [What])}; +all_rules_are_valid(What, RuleTuples) -> + AccOut = lists:foldl( + fun(RuleTuple, AccInI) -> + case elvis_rule:is_valid_from_tuple(RuleTuple) of + {true, Rule} -> + check_rule_for_options(Rule, AccInI); + {false, ValidError} -> + [io_lib:format("in ~p, " ++ ValidError, [What]) | AccInI] + end + end, + [], + RuleTuples + ), + case AccOut of + [] -> + ok; + _ -> + {error, lists:reverse(AccOut)} + end. + +check_rule_for_options(Rule, AccInI) -> + case elvis_rule:defkeys(Rule) of + [] -> + % No further validation possible. + AccInI; + DefKeysInput -> + NS = elvis_rule:ns(Rule), + Name = elvis_rule:name(Rule), + % Bypass new/ constraints. + DefKeys = maps:keys(NS:default(Name)) ++ [ignore], + case DefKeysInput -- DefKeys of + [] -> + AccInI; + Extra -> + [ + io_lib:format( + "in rule ~p/~p, the following options are unknown: ~p.", + [NS, Name, Extra] + ), + AccInI + ] + end + end. + +flag({_Option, _What} = Obj) -> + Table = table(), + _ = create_table(Table), + true = ets:insert(Table, Obj), + ok. + +check_flag({Option, _What} = Obj) -> + Table = table(), + table_exists(Table) andalso ets:lookup(Table, Option) =:= [Obj]. + +validated(Option) -> + {Option, validated}. + +validation_started(Option) -> + {Option, validation_started}. + +create_table(Table) -> + case table_exists(Table) of + false -> + _ = ets:new(Table, [public, named_table]); + _ -> + ok + end. + +table() -> + ?MODULE. + +table_exists(Table) -> + ets:info(Table) =/= undefined. diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 2843aefc..0df407a7 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -66,7 +66,7 @@ rock_this(Path, ElvisConfig) -> end, case lists:filter(FilterFun, ElvisConfig) of [] -> - elvis_utils:info("Skipping ~s", [Path]); + elvis_utils:output(info, "Skipping ~s", [Path]); FilteredElvisConfig -> LoadedFile = load_file_data(FilteredElvisConfig, File), ApplyRulesFun = fun(Cfg) -> apply_rules_and_print(Cfg, LoadedFile) end, @@ -116,13 +116,13 @@ do_rock(File, ElvisConfig) -> elvis_file:t(). load_file_data(ElvisConfig, File) -> Path = elvis_file:path(File), - elvis_utils:info("Loading ~s", [Path]), + elvis_utils:output(info, "Loading ~s", [Path]), try elvis_file:load_file_data(ElvisConfig, File) catch _:Reason -> Msg = "~p when loading file ~p.", - elvis_utils:error_prn(Msg, [Reason, Path]), + elvis_utils:output(error, Msg, [Reason, Path]), File end. diff --git a/src/elvis_result.erl b/src/elvis_result.erl index 71440e67..65817aff 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -184,9 +184,9 @@ print(Format, #{file := Path, rules := Rules}) -> _ -> case status(Rules) of ok -> - elvis_utils:notice("# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); + elvis_utils:output(warn, "# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); fail -> - elvis_utils:error("# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) + elvis_utils:output(error, "# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) end end, print_rules(Format, Path, Rules); @@ -213,7 +213,8 @@ print_rules( parsable -> ok; _ -> - elvis_utils:error( + elvis_utils:output( + error, " - ~p " "(https://github.com/inaka/elvis_core/tree/main/doc_rules/~p/~p.md)", [Name, Scope, Name] @@ -245,7 +246,7 @@ print_item( FMsg = io_lib:format(Msg, Info), io:format("~s:~p:~p:~s~n", [File, Ln, Name, FMsg]); _ -> - elvis_utils:error(" - " ++ Msg, Info) + elvis_utils:output(error, " - " ++ Msg, Info) end, print_item(Format, File, Name, Items); print_item(Format, File, Name, [Error | Items]) -> @@ -255,9 +256,9 @@ print_item(_Format, _File, _Name, []) -> ok. print_error(#{error_msg := Msg, info := Info}) -> - elvis_utils:error_prn(Msg, Info); + elvis_utils:output(error, Msg, Info); print_error(#{warn_msg := Msg, info := Info}) -> - elvis_utils:warn_prn(Msg, Info). + elvis_utils:output(warn, Msg, Info). -spec status([file() | rule()]) -> ok | fail. status([]) -> diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 64cd7abf..607e2644 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -4,9 +4,7 @@ %% General -export([erlang_halt/1, to_str/1, split_all_lines/1, split_all_lines/2]). -%% Output --export([info/2, notice/2, error/2, error_prn/2, warn_prn/2]). -%% rebar3 +%% Output / rebar3 -export([output/3, abort/2]). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -41,16 +39,6 @@ info(Message, Args) -> ColoredMessage = Message ++ "{{reset}}~n", print_info(ColoredMessage, Args). --spec notice(string(), [term()]) -> ok. -notice(Message, Args) -> - ColoredMessage = "{{white-bold}}" ++ Message ++ "{{reset}}~n", - print_info(ColoredMessage, Args). - --spec error(string(), [term()]) -> ok. -error(Message, Args) -> - ColoredMessage = "{{white-bold}}" ++ Message ++ "{{reset}}~n", - print(ColoredMessage, Args). - -spec error_prn(string(), [term()]) -> ok. error_prn(Message, Args) -> ColoredMessage = "{{red}}Error: {{reset}}" ++ Message ++ "{{reset}}~n", @@ -115,7 +103,7 @@ escape_format_str(String) -> binary_to_list(ResultBin). -dialyzer({nowarn_function, output/3}). --spec output(debug | info | error, Format :: io:format(), Data :: [term()]) -> ok. +-spec output(debug | info | warn | error, Format :: io:format(), Data :: [term()]) -> ok. output(debug, Format, Data) -> case erlang:function_exported(rebar_api, debug, 2) of true -> @@ -128,14 +116,21 @@ output(info, Format, Data) -> true -> rebar_api:info("Elvis: " ++ Format, Data); false -> - ok + info(Format, Data) + end; +output(warn, Format, Data) -> + case erlang:function_exported(rebar_api, warn, 2) of + true -> + rebar_api:warn("Elvis: " ++ Format, Data); + false -> + warn_prn(Format, Data) end; output(error, Format, Data) -> case erlang:function_exported(rebar_api, error, 2) of true -> rebar_api:error("Elvis: " ++ Format, Data); false -> - ok + error_prn(Format, Data) end. -dialyzer({nowarn_function, abort/2}). From 60bef3ff83ea7781ad99dc50a6223006bd539c68 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Tue, 15 Jul 2025 22:12:37 +0100 Subject: [PATCH 20/67] Act on test results and prepare for final touches --- config/elvis-test-custom-ruleset.config | 2 +- config/elvis-umbrella.config | 4 +- config/elvis.config | 2 +- config/test.config | 2 +- elvis.config | 2 +- src/elvis_config.erl | 312 ++++++++++--------- src/elvis_core.erl | 2 + src/elvis_result.erl | 2 +- src/elvis_ruleset.erl | 19 +- src/elvis_utils.erl | 88 ++---- test/dirs/apps/app3/include/app3_example.hrl | 0 test/dirs/apps/app3/rebar.config | 1 + test/dirs/apps/app3/src/app3_example.erl | 1 + test/elvis_SUITE.erl | 42 +-- test/examples/invalid_rules.elvis.config | 2 +- test/examples/user_defined_rules.erl | 4 +- 16 files changed, 256 insertions(+), 229 deletions(-) create mode 100644 test/dirs/apps/app3/include/app3_example.hrl create mode 100644 test/dirs/apps/app3/rebar.config create mode 100644 test/dirs/apps/app3/src/app3_example.erl diff --git a/config/elvis-test-custom-ruleset.config b/config/elvis-test-custom-ruleset.config index 0ce339a9..b14c9ba6 100644 --- a/config/elvis-test-custom-ruleset.config +++ b/config/elvis-test-custom-ruleset.config @@ -3,7 +3,7 @@ {rulesets, #{project => [{elvis_text_style, no_tabs}]}}, {config, [ #{ - dirs => ["."], + dirs => ["../../_build/test/lib/elvis_core/test/dirs/src"], filter => "*.erl", ruleset => project } diff --git a/config/elvis-umbrella.config b/config/elvis-umbrella.config index ad3f8dff..59050ea8 100644 --- a/config/elvis-umbrella.config +++ b/config/elvis-umbrella.config @@ -4,8 +4,8 @@ #{ dirs => [ - "../../_build/test/lib/elvis_core/test/dirs/src/**", - "../../_build/test/lib/elvis_core/test/dirs/test/**", + "../../_build/test/lib/elvis_core/test/dirs/src", + "../../_build/test/lib/elvis_core/test/dirs/test", "../../_build/test/lib/elvis_core/test/dirs/apps/**/src", "../../_build/test/lib/elvis_core/test/dirs/apps/**/test" ], diff --git a/config/elvis.config b/config/elvis.config index a68f4ced..693b2c52 100644 --- a/config/elvis.config +++ b/config/elvis.config @@ -22,7 +22,7 @@ ruleset => beam_files }, #{ - dirs => ["."], + dirs => ["../.."], filter => "rebar.config", ruleset => rebar_config } diff --git a/config/test.config b/config/test.config index 1cdd1dd9..155962b8 100644 --- a/config/test.config +++ b/config/test.config @@ -34,7 +34,7 @@ }, #{ dirs => ["../../_build/test/lib/elvis_core/test/examples"], - filter => "rebar.config", + filter => "rebar3.config.success", ruleset => rebar_config }, #{ diff --git a/elvis.config b/elvis.config index 310c50f1..c8e15c05 100644 --- a/elvis.config +++ b/elvis.config @@ -14,7 +14,7 @@ }}, {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}, {elvis_style, no_debug_call, #{ - ignore => [{elvis_result, print_item, 4}, {elvis_utils, print, 2}] + ignore => [{elvis_result, print_item, 4}, {elvis_utils, do_output, 2}] }}, {elvis_style, no_god_modules, #{ignore => [elvis_style]}}, {elvis_style, no_throw, disable}, diff --git a/src/elvis_config.erl b/src/elvis_config.erl index c1c58c2c..07117572 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -2,8 +2,8 @@ -feature(maybe_expr, enable). --export([from_rebar/1, from_file/1]). -%% Geters +-export([from_rebar/1, from_file/1, validate_config/1, default/0]). +%% Getters -export([dirs/1, ignore/1, filter/1, files/1, rules/1]). %% Files -export([resolve_files/1, resolve_files/2, apply_to_files/2]). @@ -24,11 +24,17 @@ %%% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -fetch_elvis_config(AppConfig) -> - ElvisConfig = from_static(elvis, {app, AppConfig}), - _ = validate({elvis, ElvisConfig}), - _ = elvis_ruleset:load_custom(from_static(rulesets, {elvis, ElvisConfig})), - ElvisConfig. +fetch_elvis_config(Key, AppConfig) -> + Elvis = from_static(elvis, {app, AppConfig}), + _ = + case lists:member(Key, elvis_control_opts()) of + true -> + ok; + _ -> + do_validate({elvis, Elvis}) + end, + _ = elvis_ruleset:load_custom(from_static(rulesets, {elvis, Elvis})), + Elvis. from_static(Key, {Type, Config}) -> elvis_utils:output(debug, "fetching key ~p from ~p config.", [Key, Type]), @@ -88,8 +94,8 @@ for(Key) -> AppConfig0 end, % If we got this far, the config. is valid... - ElvisConfig = fetch_elvis_config(AppConfig), - from_static(Key, {elvis, ElvisConfig}). + Elvis = fetch_elvis_config(Key, AppConfig), + from_static(Key, {elvis, Elvis}). consult_elvis_config(File) -> case file:consult(File) of @@ -116,12 +122,12 @@ from_rebar(File) -> fetch_elvis_config_from(AppConfig). from_file(File) -> - FileConfig = consult_elvis_config(File), - fetch_elvis_config_from(FileConfig). + AppConfig = consult_elvis_config(File), + fetch_elvis_config_from(AppConfig). fetch_elvis_config_from(AppConfig) -> - ElvisConfig = fetch_elvis_config(AppConfig), - from_static(config, {elvis, ElvisConfig}). + Elvis = fetch_elvis_config(undefined, AppConfig), + from_static(config, {elvis, Elvis}). default_for(app) -> % This is the top-level element, before 'elvis' @@ -129,6 +135,29 @@ default_for(app) -> default_for(elvis) -> []; default_for(config) -> + []; +default_for(output_format) -> + colors; +default_for(verbose) -> + false; +default_for(no_output) -> + false; +default_for(parallel) -> + 1; +default_for(rulesets) -> + #{}; +default_for([config, dirs]) -> + []; +default_for([config, filter]) -> + ""; +default_for([config, ignore]) -> + []; +default_for([config, ruleset]) -> + undefined; +default_for([config, rules]) -> + []. + +default() -> [ #{ dirs => [ @@ -158,27 +187,7 @@ default_for(config) -> filter => ".gitignore", ruleset => gitignore } - ]; -default_for(output_format) -> - colors; -default_for(verbose) -> - false; -default_for(no_output) -> - false; -default_for(parallel) -> - 1; -default_for(rulesets) -> - #{}; -default_for([config, dirs]) -> - []; -default_for([config, filter]) -> - ""; -default_for([config, ignore]) -> - []; -default_for([config, ruleset]) -> - undefined; -default_for([config, rules]) -> - []. + ]. -spec dirs(Config :: [t()] | t()) -> [string()]. dirs(Config) when is_list(Config) -> @@ -253,7 +262,9 @@ resolve_files(#{files := _Files} = RuleGroup) -> resolve_files(#{dirs := Dirs} = RuleGroup) -> Filter = filter(RuleGroup), Files = elvis_file:find_files(Dirs, Filter), - resolve_files(RuleGroup, Files). + resolve_files(RuleGroup, Files); +resolve_files(#{}) -> + []. %% @doc Takes a function and configuration and applies the function to all %% file in the configuration. @@ -290,7 +301,7 @@ merge_rules(UserRules, DefaultRules) -> % wants to override the rule. fun(Tuple) -> Rule = elvis_rule:from_tuple(Tuple), - case not is_rule_override(Rule, UserRules) of + case not is_rule_override(Rule, UserRules) andalso not elvis_rule:disabled(Rule) of true -> {true, Rule}; false -> @@ -330,63 +341,76 @@ is_rule_override(Rule, UserRules) -> UserRules ). -validate({elvis = Option, ElvisConfig}) -> - case check_flag(validated(Option)) orelse check_flag(validation_started(Option)) of - true -> - ok; - false -> - do_validate({Option, ElvisConfig}) - end. +validate_config(ElvisConfig) -> + do_validate({config, ElvisConfig}). -get_elvis_opt(OptName, ElvisConfig) -> - proplists:get_value(OptName, ElvisConfig, default_for(OptName)). +get_elvis_opt(OptName, Elvis) -> + proplists:get_value(OptName, Elvis, default_for(OptName)). -do_validate({elvis = Option, ElvisConfig}) -> - maybe - ok = flag(validation_started(Option)), - ok ?= is_list("elvis", ElvisConfig), - ok ?= - proplist_keys_are_in("elvis", ElvisConfig, [ - output_format, verbose, no_output, parallel, rulesets, config - ]), - OutputFormat = get_elvis_opt(output_format, ElvisConfig), - ok ?= is_one_of("elvis.output_format", OutputFormat, [colors, plain, parsable]), - Verbose = get_elvis_opt(verbose, ElvisConfig), - ok ?= is_boolean("elvis.verbose", Verbose), - NoOutput = get_elvis_opt(no_output, ElvisConfig), - ok ?= is_boolean("elvis.no_output", NoOutput), - Parallel = get_elvis_opt(parallel, ElvisConfig), - ok ?= is_pos_integer("elvis.parallel", Parallel), - Rulesets = get_elvis_opt(rulesets, ElvisConfig), - ok ?= is_valid_rulesets("elvis.rulesets", Rulesets), - Config = get_elvis_opt(config, ElvisConfig), - ok ?= is_valid_config("elvis.config", Rulesets, Config), - ok = flag(validated(Option)) - else - {error, FormatData} -> - {Format, Data} = - case is_list(FormatData) of - false -> - FormatData; - true -> - % Get only first result, for now - % If we wanna be smarter we need to start concatenating readable strings - [{Format0, Data0} | _] = FormatData, - {Format0, Data0} - end, - % on warnings_as_errors this'll throw an exception - %throw({invalid_config, io_lib:format(Format, Data)}) - elvis_utils:output(warn, Format, Data) - end. +elvis_control_opts() -> + [output_format, verbose, no_output, parallel]. -is_list(What, List) -> - case is_list(List) of +do_validate({elvis = Option, Elvis}) -> + case check_flag(validation_started(Option)) of + true -> + ok; + _ -> + maybe + ok = flag(validation_started(Option)), + ok ?= is_nonempty_list("elvis", Elvis), + ok ?= + proplist_keys_are_in( + "elvis", Elvis, elvis_control_opts() ++ [rulesets, config] + ), + OutputFormat = get_elvis_opt(output_format, Elvis), + ok ?= is_one_of("elvis.output_format", OutputFormat, [colors, plain, parsable]), + Verbose = get_elvis_opt(verbose, Elvis), + ok ?= is_boolean("elvis.verbose", Verbose), + NoOutput = get_elvis_opt(no_output, Elvis), + ok ?= is_boolean("elvis.no_output", NoOutput), + Parallel = get_elvis_opt(parallel, Elvis), + ok ?= is_pos_integer("elvis.parallel", Parallel), + CustomRulesets = get_elvis_opt(rulesets, Elvis), + ok ?= is_valid_rulesets("elvis.rulesets", CustomRulesets), + ElvisConfig = get_elvis_opt(config, Elvis), + ok ?= is_valid_config("elvis.config", maps:keys(CustomRulesets), ElvisConfig) + else + {error, FormatData} -> + do_validate_results(FormatData) + end + end; +do_validate({config = Option, ElvisConfig}) -> + case check_flag(validation_started(Option)) of true -> ok; _ -> - {error, {"~p is expected to be a list.", [What]}} + maybe + ok ?= is_valid_config("elvis.config", elvis_ruleset:custom_names(), ElvisConfig) + else + {error, FormatData} -> + do_validate_results(FormatData) + end end. +-spec do_validate_results(_) -> no_return(). +do_validate_results(FormatData) -> + {Format, Data} = + case is_list(FormatData) of + false -> + FormatData; + true -> + % Get only first result, for now + % If we wanna be smarter we need to start concatenating readable strings + [{Format0, Data0} | _] = FormatData, + {Format0, Data0} + end, + throw({invalid_config, io_lib:format(Format, Data)}). + +is_nonempty_list(What, List) when not is_list(List) orelse List =:= [] -> + {error, {"~p is expected to be a non-empty list.", [What]}}; +is_nonempty_list(_What, _List) -> + ok. + proplist_keys_are_in(What, List, Keys) -> Filtered = [Element || {Element, _} <- List, not lists:member(Element, Keys)], case Filtered of @@ -404,43 +428,34 @@ is_one_of(What, Value, Possibilities) -> {error, {"~p is expected to be one of the following: ~p.", [What, Possibilities]}} end. -is_boolean(What, Value) -> - case is_boolean(Value) of - true -> - ok; - _ -> - {error, {"~p is expected to be a boolean.", [What]}} - end. +is_boolean(_What, Value) when is_boolean(Value) -> + ok; +is_boolean(What, _Value) -> + {error, {"~p is expected to be a boolean.", [What]}}. -is_pos_integer(What, Value) -> - case is_integer(Value) andalso Value > 0 of - true -> - ok; - _ -> - {error, {"~p is expected to be a positive integer.", [What]}} - end. +is_pos_integer(_What, Value) when is_integer(Value) andalso Value > 0 -> + ok; +is_pos_integer(What, _Value) -> + {error, {"~p is expected to be a positive integer.", [What]}}. -is_valid_rulesets(What, Rulesets) -> +is_valid_rulesets(What, CustomRulesets) -> maybe - ok ?= is_map(What, Rulesets), - ok ?= all_map_keys_are_atoms(What, Rulesets), - ok ?= all_rulesets_have_valid_rules(What, Rulesets), - ok ?= no_rulesets_exist(What, Rulesets) + ok ?= is_map(What, CustomRulesets), + ok ?= all_map_keys_are_atoms(What, CustomRulesets), + ok ?= all_custom_rulesets_have_valid_rules(What, CustomRulesets), + ok ?= no_default_ruleset_override(What, CustomRulesets) else {error, FormatData} -> {error, FormatData} end. -is_map(What, Value) -> - case is_map(Value) of - true -> - ok; - _ -> - {error, {"~p is expected to be a map.", [What]}} - end. +is_map(_What, Value) when is_map(Value) -> + ok; +is_map(What, _Value) -> + {error, {"~p is expected to be a map.", [What]}}. all_map_keys_are_atoms(What, Map) -> - Filtered = [Key || Key <- maps:keys(Map), is_atom(Key)], + Filtered = [Key || Key <- maps:keys(Map), not is_atom(Key)], case Filtered of [] -> ok; @@ -448,16 +463,19 @@ all_map_keys_are_atoms(What, Map) -> {error, {"in ~p, keys are expected to be atoms.", [What]}} end. -all_rulesets_have_valid_rules(What, Rulesets) -> +all_custom_rulesets_have_valid_rules(What, CustomRulesets) -> AccOut = maps:fold( - fun(Ruleset, RuleTuples, AccInO) -> + fun(CustomRuleset, RuleTuples, AccInO) -> lists:foldl( fun(RuleTuple, AccInI) -> case elvis_rule:is_valid_from_tuple(RuleTuple) of {true, _Rule} -> AccInI; {false, ValidError} -> - [{"in ~p, in ruleset ~p, " ++ ValidError, [What, Ruleset]} | AccInI] + [ + {"in ~p, in ruleset ~p, " ++ ValidError, [What, CustomRuleset]} + | AccInI + ] end end, AccInO, @@ -465,7 +483,7 @@ all_rulesets_have_valid_rules(What, Rulesets) -> ) end, [], - Rulesets + CustomRulesets ), case AccOut of [] -> @@ -474,8 +492,11 @@ all_rulesets_have_valid_rules(What, Rulesets) -> {error, lists:reverse(AccOut)} end. -no_rulesets_exist(What, Rulesets) -> - Filtered = [Ruleset || Ruleset <- maps:keys(Rulesets), not elvis_ruleset:is_defined(Ruleset)], +no_default_ruleset_override(What, CustomRulesets) -> + Filtered = [ + CustomRuleset + || CustomRuleset <- maps:keys(CustomRulesets), elvis_ruleset:is_defined(CustomRuleset) + ], case Filtered of [] -> ok; @@ -490,11 +511,11 @@ no_rulesets_exist(What, Rulesets) -> }} end. -is_valid_config(What, CustomRulesets, Configset0) -> +is_valid_config(What, CustomRulesetNames, Configset0) -> Configset = wrap_in_list(Configset0), maybe - ok ?= is_list(What, Configset), - ok ?= all_configs_are_valid(What, CustomRulesets, Configset) + ok ?= is_nonempty_list(What, Configset), + ok ?= all_configs_are_valid(What, CustomRulesetNames, Configset) else {error, FormatData} -> {error, FormatData} @@ -505,11 +526,11 @@ wrap_in_list(Term) when is_list(Term) -> wrap_in_list(Term) -> [Term]. -all_configs_are_valid(What, CustomRulesets, Configset) -> +all_configs_are_valid(What, CustomRulesetNames, Configset) -> {_PosNumber, ValidErrors} = lists:foldl( fun(Config, {PosNumber, AccIn}) -> AccOut = - case config_is_valid(CustomRulesets, Config) of + case config_is_valid(CustomRulesetNames, Config) of ok -> AccIn; {error, ValidError} -> @@ -533,7 +554,7 @@ all_configs_are_valid(What, CustomRulesets, Configset) -> get_config_opt(OptName, Config) -> maps:get(OptName, Config, default_for([config, OptName])). -config_is_valid(CustomRulesets, Config) -> +config_is_valid(CustomRulesetNames, Config) -> maybe ok ?= map_keys_are_in(Config, [dirs, filter, ignore, ruleset, rules]), Dirs = get_config_opt(dirs, Config), @@ -544,9 +565,10 @@ config_is_valid(CustomRulesets, Config) -> Ignore = get_config_opt(ignore, Config), ok ?= is_list_of_ignorables(ignore, Ignore), Ruleset = get_config_opt(ruleset, Config), - ok ?= ruleset_is_custom_or_default(CustomRulesets, Ruleset), + ok ?= defined_ruleset_is_custom_or_default(CustomRulesetNames, Ruleset), Rules = get_config_opt(rules, Config), - ok ?= all_rules_are_valid(rules, Rules) + ok ?= all_rules_are_valid(rules, Rules), + ok ?= either_rules_is_nonempty_or_ruleset_is_defined(Rules, Ruleset) else {error, ValidError} -> {error, ValidError} @@ -561,25 +583,29 @@ map_keys_are_in(Map, Keys) -> {error, io_lib:format("the following keys are unknown: ~p.", [Filtered])} end. -is_nonempty_list_of_dirs(What, List) when not is_list(List) -> - {error, io_lib:format("~p is expected to be a list.", [What])}; -is_nonempty_list_of_dirs(What, [] = _List) -> - {error, io_lib:format("~p is expected to be a nonempty list.", [What])}; +is_nonempty_list_of_dirs(What, List) when not is_list(List) orelse List =:= [] -> + {error, io_lib:format("~p is expected to be a non-empty list.", [What])}; is_nonempty_list_of_dirs(What, List) -> Filtered = [ Element - || Element <- List, not io_lib:char_list(Element) orelse not filelib:is_dir(Element) + || Element <- List, not io_lib:char_list(Element) orelse not holds_dir(Element) ], case Filtered of [] -> ok; _ -> {error, - io_lib:format("in ~p, the following elements are not directories: ~p.", [ - What, Filtered - ])} + io_lib:format( + "in ~p, the following elements are not (or don't contain) directories: ~p.", + [What, Filtered] + )} end. +holds_dir(Element) -> + Dirs = filelib:wildcard(Element), + Filtered = [Dir || Dir <- Dirs, filelib:is_dir(Dir)], + Filtered =/= []. + is_nonempty_string(What, String) -> case io_lib:char_list(String) andalso length(String) > 0 of true -> @@ -627,10 +653,10 @@ is_list_of_ignorables(What, List) -> ])} end. -ruleset_is_custom_or_default(CustomRulesets, Ruleset) -> - case - lists:member(Ruleset, maps:keys(CustomRulesets)) orelse elvis_ruleset:is_defined(Ruleset) - of +defined_ruleset_is_custom_or_default(_CustomRulesetNames, undefined = _Ruleset) -> + ok; +defined_ruleset_is_custom_or_default(CustomRulesetNames, Ruleset) -> + case lists:member(Ruleset, CustomRulesetNames) orelse elvis_ruleset:is_defined(Ruleset) of true -> ok; _ -> @@ -662,6 +688,13 @@ all_rules_are_valid(What, RuleTuples) -> {error, lists:reverse(AccOut)} end. +either_rules_is_nonempty_or_ruleset_is_defined([_ | _] = _Rules, _Ruleset) -> + ok; +either_rules_is_nonempty_or_ruleset_is_defined(_Rules, Ruleset) when Ruleset =/= undefined -> + ok; +either_rules_is_nonempty_or_ruleset_is_defined(_Rules, _Ruleset) -> + io_lib:format("either rules or ruleset is expected to be defined.", []). + check_rule_for_options(Rule, AccInI) -> case elvis_rule:defkeys(Rule) of [] -> @@ -696,9 +729,6 @@ check_flag({Option, _What} = Obj) -> Table = table(), table_exists(Table) andalso ets:lookup(Table, Option) =:= [Obj]. -validated(Option) -> - {Option, validated}. - validation_started(Option) -> {Option, validation_started}. diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 0df407a7..35ad8e9f 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -35,6 +35,7 @@ start() -> -spec rock([elvis_config:t()]) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> + ok = elvis_config:validate_config(ElvisConfig), elvis_ruleset:dump_custom(), Results = lists:map(fun do_parallel_rock/1, ElvisConfig), lists:foldl(fun combine_results/2, ok, Results). @@ -46,6 +47,7 @@ rock_this(Module, ElvisConfig) when is_atom(Module) -> Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> + ok = elvis_config:validate_config(ElvisConfig), elvis_ruleset:dump_custom(), Dirname = filename:dirname(Path), Filename = filename:basename(Path), diff --git a/src/elvis_result.erl b/src/elvis_result.erl index 65817aff..eb0154db 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -184,7 +184,7 @@ print(Format, #{file := Path, rules := Rules}) -> _ -> case status(Rules) of ok -> - elvis_utils:output(warn, "# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); + elvis_utils:output(notice, "# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); fail -> elvis_utils:output(error, "# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) end diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index 4ea57f92..96fb2c2e 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -2,7 +2,7 @@ -format(#{inline_items => none}). --export([rules/1, load_custom/1, dump_custom/0, is_defined/1]). +-export([rules/1, load_custom/1, dump_custom/0, is_defined/1, custom_names/0]). -ifdef(TEST). -export([drop_custom/0]). @@ -25,6 +25,15 @@ load_custom(Rulesets) -> ok end. +custom_names() -> + Table = table(), + case ets:info(Table) of + undefined -> + []; + _ -> + proplists:get_keys(ets:tab2list(Table)) + end. + dump_custom() -> Table = table(), case ets:info(Table) of @@ -58,6 +67,14 @@ rules(erl_files_test) -> erl_files_test_rules(); rules(Group) -> Table = table(), + case ets:info(Table) of + undefined -> + []; + _ -> + lookup(Table, Group) + end. + +lookup(Table, Group) -> case ets:lookup(Table, Group) of [{Group, Rules}] -> Rules; diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 607e2644..1c4158d8 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -34,41 +34,6 @@ split_all_lines(Binary) -> split_all_lines(Binary, Opts) -> binary:split(Binary, [<<"\r\n">>, <<"\n">>], [global | Opts]). --spec info(string(), [term()]) -> ok. -info(Message, Args) -> - ColoredMessage = Message ++ "{{reset}}~n", - print_info(ColoredMessage, Args). - --spec error_prn(string(), [term()]) -> ok. -error_prn(Message, Args) -> - ColoredMessage = "{{red}}Error: {{reset}}" ++ Message ++ "{{reset}}~n", - print(ColoredMessage, Args). - --spec warn_prn(string(), [term()]) -> ok. -warn_prn(Message, Args) -> - ColoredMessage = "{{magenta}}Warning: {{reset}}" ++ Message ++ "{{reset}}~n", - print(ColoredMessage, Args). - --spec print_info(string(), [term()]) -> ok. -print_info(Message, Args) -> - case elvis_config:verbose() of - true -> - print(Message, Args); - false -> - ok - end. - --spec print(string(), [term()]) -> ok. -print(Message, Args) -> - case elvis_config:no_output() of - true -> - ok; - _ -> - Output = io_lib:format(Message, Args), - EscapedOutput = escape_format_str(Output), - io:format(parse_colors(EscapedOutput)) - end. - -spec parse_colors(string()) -> string(). parse_colors(Message) -> Colors = @@ -95,43 +60,54 @@ parse_colors(Message) -> lists:foldl(Fun, Message, maps:keys(Colors)) end. --spec escape_format_str(string()) -> string(). -escape_format_str(String) -> +-spec escape_chars(string()) -> string(). +escape_chars(String) -> Binary = list_to_binary(String), Result = re:replace(Binary, "[^~]~", "~~", [global]), ResultBin = iolist_to_binary(Result), binary_to_list(ResultBin). --dialyzer({nowarn_function, output/3}). --spec output(debug | info | warn | error, Format :: io:format(), Data :: [term()]) -> ok. -output(debug, Format, Data) -> +-spec output(debug | info | notice | warn | error, Format :: io:format(), Data :: [term()]) -> ok. +output(debug = _Type, Format, Data) -> + Chars = io_lib:format(Format, Data), + do_output(debug, Chars); +output(Type, Format, Data) -> + Chars = io_lib:format(Format, Data), + EscapedChars = escape_chars(Chars), + ColorParsedChars = parse_colors(EscapedChars), + case elvis_config:no_output() of + true -> + ok; + _ -> + do_output(Type, ColorParsedChars) + end. + +-dialyzer({nowarn_function, do_output/2}). +do_output(debug, Chars) -> case erlang:function_exported(rebar_api, debug, 2) of true -> - rebar_api:debug("Elvis: " ++ Format, Data); + rebar_api:debug("Elvis: " ++ Chars, []); false -> ok end; -output(info, Format, Data) -> - case erlang:function_exported(rebar_api, info, 2) of +do_output(info, Chars) -> + case elvis_config:verbose() of true -> - rebar_api:info("Elvis: " ++ Format, Data); + io:format(Chars ++ "{{reset}}~n"); false -> - info(Format, Data) + ok end; -output(warn, Format, Data) -> - case erlang:function_exported(rebar_api, warn, 2) of +do_output(notice, Chars) -> + case elvis_config:verbose() of true -> - rebar_api:warn("Elvis: " ++ Format, Data); + io:format("{{white-bold}}" ++ Chars ++ "{{reset}}~n"); false -> - warn_prn(Format, Data) + ok end; -output(error, Format, Data) -> - case erlang:function_exported(rebar_api, error, 2) of - true -> - rebar_api:error("Elvis: " ++ Format, Data); - false -> - error_prn(Format, Data) - end. +do_output(warn, Chars) -> + io:format("{{magenta}}Warning: {{reset}}" ++ Chars ++ "{{reset}}~n"); +do_output(error, Chars) -> + io:format("{{red}}Error: {{reset}}" ++ Chars ++ "{{reset}}~n"). -dialyzer({nowarn_function, abort/2}). -spec abort(Format :: io:format(), Data :: [term()]) -> no_return(). diff --git a/test/dirs/apps/app3/include/app3_example.hrl b/test/dirs/apps/app3/include/app3_example.hrl new file mode 100644 index 00000000..e69de29b diff --git a/test/dirs/apps/app3/rebar.config b/test/dirs/apps/app3/rebar.config new file mode 100644 index 00000000..0ec61832 --- /dev/null +++ b/test/dirs/apps/app3/rebar.config @@ -0,0 +1 @@ +{}. diff --git a/test/dirs/apps/app3/src/app3_example.erl b/test/dirs/apps/app3/src/app3_example.erl new file mode 100644 index 00000000..536991ea --- /dev/null +++ b/test/dirs/apps/app3/src/app3_example.erl @@ -0,0 +1 @@ +-module(app3_example). diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 6a410097..27a6bd35 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -100,7 +100,13 @@ rock_with_incomplete_config(_Config) -> end. rock_with_list_config(_Config) -> - ElvisConfig = [#{dirs => ["src"], rules => [], filter => "*.erl"}], + ElvisConfig = [ + #{ + dirs => ["../../test/dirs/src"], + rules => [{elvis_text_style, line_length, disable}], + filter => "*.erl" + } + ], ok = try ok = elvis_core:rock(ElvisConfig) @@ -236,22 +242,17 @@ rock_with_rule_groups(_Config) -> RulesGroupConfig = [ #{ - dirs => ["src"], + dirs => ["../../test/dirs/apps/app3/src"], filter => "*.erl", ruleset => erl_files }, #{ - dirs => ["include"], - filter => "*.erl", + dirs => ["../../test/dirs/apps/app3/include"], + filter => "*.hrl", ruleset => hrl_files }, #{ - dirs => ["_build/test/lib/elvis_core/ebin"], - filter => "*.beam", - ruleset => beam_files - }, - #{ - dirs => ["."], + dirs => ["../../test/dirs/apps/app3"], filter => "rebar.config", ruleset => rebar_config } @@ -287,7 +288,7 @@ rock_with_rule_groups(_Config) -> OverrideConfig = [ #{ - dirs => ["src"], + dirs => ["../../test/dirs/apps/app3/src"], filter => "*.erl", ruleset => erl_files, rules => @@ -299,7 +300,7 @@ rock_with_rule_groups(_Config) -> ] }, #{ - dirs => ["."], + dirs => ["../../test/dirs/apps/app3"], filter => "rebar.config", ruleset => rebar_config } @@ -318,6 +319,8 @@ rock_this_skipping_files(_Config) -> [File] = elvis_file:find_files(Dirs, "small.erl"), Path = elvis_file:path(File), ConfigPath = "../../config/elvis-test-pa.config", + {ok, user_defined_rules} = compile:file("../../test/examples/user_defined_rules.erl"), + {module, user_defined_rules} = code:ensure_loaded(user_defined_rules), ElvisConfig = elvis_config:from_file(ConfigPath), ok = elvis_core:rock_this(Path, ElvisConfig), 0 = meck:num_calls(elvis_file, load_file_data, '_'), @@ -341,6 +344,7 @@ rock_with_umbrella_apps(_Config) -> {fail, [ #{file := "../../_build/test/lib/elvis_core/test/dirs/test/dir_test.erl"}, #{file := "../../_build/test/lib/elvis_core/test/dirs/src/dirs_src.erl"}, + #{file := "../../_build/test/lib/elvis_core/test/dirs/apps/app3/src/app3_example.erl"}, #{ file := "../../_build/test/lib/elvis_core/test/dirs/apps/app2/test/dirs_apps_app2_test.erl" @@ -363,19 +367,12 @@ rock_with_umbrella_apps(_Config) -> rock_with_invalid_rules(_Config) -> ConfigPath = "../../test/examples/invalid_rules.elvis.config", - ElvisConfig = elvis_config:from_file(ConfigPath), - ExpectedErrorMessage = - {invalid_rules, [ - {invalid_rule, {elvis_style, not_existing_rule}}, - {invalid_rule, {elvis_style, what_is_this_rule}}, - {invalid_rule, {not_existing_module, dont_repeat_yourself}}, - {invalid_rule, {not_existing_module, dont_repeat_yourself}} - ]}, try + ElvisConfig = elvis_config:from_file(ConfigPath), ok = elvis_core:rock(ElvisConfig), ct:fail("Elvis should not have rocked with ~p", [ElvisConfig]) catch - {invalid_config, ExpectedErrorMessage} -> + {invalid_config, _} -> ok end. @@ -388,6 +385,9 @@ custom_ruleset(_Config) -> NoTabs = elvis_rule:new(elvis_text_style, no_tabs), [[NoTabs]] = elvis_config:rules(ElvisConfig), + %% this is also done by :rock and :rock_this + _ = elvis_ruleset:dump_custom(), + %% read unknown ruleset configuration to ensure rulesets from %% previous load do not stick around ConfigPathMissing = "../../config/elvis-test-unknown-ruleset.config", diff --git a/test/examples/invalid_rules.elvis.config b/test/examples/invalid_rules.elvis.config index 958f1fa6..c16787ae 100644 --- a/test/examples/invalid_rules.elvis.config +++ b/test/examples/invalid_rules.elvis.config @@ -2,7 +2,7 @@ {elvis, [ {config, [ #{ - dirs => ["src"], + dirs => ["../../_build/default/lib/elvis_core/src"], filter => "*.erl", rules => [ diff --git a/test/examples/user_defined_rules.erl b/test/examples/user_defined_rules.erl index 33240cbf..656cd171 100644 --- a/test/examples/user_defined_rules.erl +++ b/test/examples/user_defined_rules.erl @@ -1,6 +1,6 @@ -module(user_defined_rules). --export([rule/3]). +-export([rule/2]). -rule(_Config, _Target, _) -> +rule(_Config, _Target) -> [elvis_result:new_item("this will always FAIL", [], #{line => 10, column => 2})]. From b3dfaa3093eb9625777fd27fd1658f71f0a81da4 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Tue, 15 Jul 2025 22:45:36 +0100 Subject: [PATCH 21/67] Act on Xref results --- src/elvis_utils.erl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 1c4158d8..b8acfcec 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -7,6 +7,10 @@ %% Output / rebar3 -export([output/3, abort/2]). +% These call (but verify if exported) rebar3-specific functions. +-ignore_xref(do_output/2). +-ignore_xref(abort/2). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% From a686d634f6e857b3d3be880c4d3d6b5706d08afb Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:29:08 +0100 Subject: [PATCH 22/67] Fix for erl_files_test trim'ing --- src/elvis_ruleset.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index 96fb2c2e..ec92e99c 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -191,11 +191,11 @@ elvis_style_rules() -> erl_files_test_rules() -> trim( + elvis_style_rules(), [ elvis_rule:new(elvis_style, dont_repeat_yourself), elvis_rule:new(elvis_style, no_god_modules) - ], - elvis_style_rules() + ] ). elvis_text_style_rules() -> From 9803ffc6544127d0db285bff592b43a067c80bc0 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:31:46 +0100 Subject: [PATCH 23/67] Don't be afraid to write text as it's supposed to be read --- src/elvis_config.erl | 12 ++++++------ test/elvis_SUITE.erl | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 07117572..f18b8238 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -37,17 +37,17 @@ fetch_elvis_config(Key, AppConfig) -> Elvis. from_static(Key, {Type, Config}) -> - elvis_utils:output(debug, "fetching key ~p from ~p config.", [Key, Type]), + elvis_utils:output(debug, "fetching key ~p from ~p configuration", [Key, Type]), case proplists:get_value(Key, Config) of undefined -> elvis_utils:output( - debug, "no value for config. key ~p found in ~p config.; going with default", [ + debug, "no value for key ~p found in ~p configuration; going with default", [ Key, Type ] ), default(Key); Value -> - elvis_utils:output(debug, "value for config. key ~p found in ~p config.", [Key, Type]), + elvis_utils:output(debug, "value for key ~p found in ~p configuration", [Key, Type]), Value end. @@ -71,12 +71,12 @@ default(Key) -> undefined -> elvis_utils:output( debug, - "no value for config. key ~p found in application env.; going with default", + "no value for key ~p found in application environment; going with default", [Key] ), default_for(Key); {ok, Value} -> - elvis_utils:output(debug, "value for config. key ~p found in application env.", [Key]), + elvis_utils:output(debug, "value for key ~p found in application environment", [Key]), Value end. @@ -93,7 +93,7 @@ for(Key) -> AppConfig0 -> AppConfig0 end, - % If we got this far, the config. is valid... + % If we got this far, the configuration is valid... Elvis = fetch_elvis_config(Key, AppConfig), from_static(Key, {elvis, Elvis}). diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 27a6bd35..eb0cdae6 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -238,7 +238,7 @@ rock_without_errors_and_with_verbose_has_output(_Config) -> rock_with_rule_groups(_Config) -> % elvis_config will load default elvis_core rules for every - % rule_group in the config. + % rule_group in the configuration RulesGroupConfig = [ #{ From 42fc5ce94edea4607980d652cdcaed6eb41ce087 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:32:54 +0100 Subject: [PATCH 24/67] Just because something sounds similar... --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index f18b8238..dbeee3a1 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -85,7 +85,7 @@ for(Key) -> AppConfig = case consult_elvis_config("elvis.config") of AppDefault -> - % This might happen whether we fail to parse the fail or it actually is [] + % This might happen whether we fail to parse the file or it actually is [] elvis_utils:output( debug, "elvis.config unusable; falling back to rebar.config", [] ), From 651c6046d0f9afe9f3fd231a651d286cfa3e2e99 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:34:30 +0100 Subject: [PATCH 25/67] Care more about how it's read that correct form --- src/elvis_config.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index dbeee3a1..39eaf876 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -371,7 +371,7 @@ do_validate({elvis = Option, Elvis}) -> Parallel = get_elvis_opt(parallel, Elvis), ok ?= is_pos_integer("elvis.parallel", Parallel), CustomRulesets = get_elvis_opt(rulesets, Elvis), - ok ?= is_valid_rulesets("elvis.rulesets", CustomRulesets), + ok ?= are_valid_rulesets("elvis.rulesets", CustomRulesets), ElvisConfig = get_elvis_opt(config, Elvis), ok ?= is_valid_config("elvis.config", maps:keys(CustomRulesets), ElvisConfig) else @@ -438,7 +438,7 @@ is_pos_integer(_What, Value) when is_integer(Value) andalso Value > 0 -> is_pos_integer(What, _Value) -> {error, {"~p is expected to be a positive integer.", [What]}}. -is_valid_rulesets(What, CustomRulesets) -> +are_valid_rulesets(What, CustomRulesets) -> maybe ok ?= is_map(What, CustomRulesets), ok ?= all_map_keys_are_atoms(What, CustomRulesets), From d7da54fe829f738dfc3254d67b9cf62236614431 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:36:39 +0100 Subject: [PATCH 26/67] Be more correct --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1d53ea5b..9d05de76 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,9 @@ current directory. If `elvis.config` is not present, the application will fall back to searching for configuration parameters in `rebar.config`. If `rebar.config` is also unavailable, the application proceeds to -perform a tertiary lookup within the `app/sys.config` file for the required settings. +perform a tertiary lookup within its application environment (which can also be set via the +`app/sys.config` file, or e.g., via `application:set_env(elvis_core, Key, Value).` for the required +settings. #### Providing configuration as a value From 48be7f6756c085ff51fac92e236bae968279c0c0 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:43:12 +0100 Subject: [PATCH 27/67] Use ?MODULE less --- src/elvis_config.erl | 17 ++++++----------- src/elvis_ruleset.erl | 36 ++++++++++++++++-------------------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 39eaf876..d85e8606 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -720,28 +720,23 @@ check_rule_for_options(Rule, AccInI) -> end. flag({_Option, _What} = Obj) -> - Table = table(), - _ = create_table(Table), - true = ets:insert(Table, Obj), + _ = create_table(elvis_config), + true = ets:insert(elvis_config, Obj), ok. check_flag({Option, _What} = Obj) -> - Table = table(), - table_exists(Table) andalso ets:lookup(Table, Option) =:= [Obj]. + table_exists() andalso ets:lookup(elvis_config, Option) =:= [Obj]. validation_started(Option) -> {Option, validation_started}. create_table(Table) -> - case table_exists(Table) of + case table_exists() of false -> _ = ets:new(Table, [public, named_table]); _ -> ok end. -table() -> - ?MODULE. - -table_exists(Table) -> - ets:info(Table) =/= undefined. +table_exists() -> + ets:info(elvis_config) =/= undefined. diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index ec92e99c..9abe67bd 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -26,21 +26,19 @@ load_custom(Rulesets) -> end. custom_names() -> - Table = table(), - case ets:info(Table) of - undefined -> + case table_exists() of + false -> []; _ -> - proplists:get_keys(ets:tab2list(Table)) + proplists:get_keys(ets:tab2list(elvis_ruleset)) end. dump_custom() -> - Table = table(), - case ets:info(Table) of - undefined -> + case table_exists() of + false -> ok; _ -> - ets:delete(Table) + ets:delete(elvis_ruleset) end. is_defined(Ruleset) -> @@ -66,12 +64,11 @@ rules(rebar_config) -> rules(erl_files_test) -> erl_files_test_rules(); rules(Group) -> - Table = table(), - case ets:info(Table) of - undefined -> + case table_exists() of + false -> []; _ -> - lookup(Table, Group) + lookup(elvis_ruleset, Group) end. lookup(Table, Group) -> @@ -83,23 +80,22 @@ lookup(Table, Group) -> end. ensure_table() -> - Table = table(), - case ets:info(Table) of - undefined -> - {false, ets:new(Table, [set, named_table, {keypos, 1}, public])}; + case table_exists() of + false -> + {false, ets:new(elvis_ruleset, [set, named_table, {keypos, 1}, public])}; _ -> - {true, Table} + {true, elvis_ruleset} end. -table() -> - ?MODULE. - -ifdef(TEST). drop_custom() -> Table = table(), ets:delete(Table). -endif. +table_exists() -> + ets:info(elvis_ruleset) =/= undefined. + gitignore_rules() -> [ elvis_rule:new(elvis_gitignore, forbidden_patterns), From ee5b09387dff951ff5d381c8303d7445ad8a11d7 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 18:43:24 +0100 Subject: [PATCH 28/67] Drop unused test code --- src/elvis_ruleset.erl | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index 9abe67bd..f80f280d 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -4,10 +4,6 @@ -export([rules/1, load_custom/1, dump_custom/0, is_defined/1, custom_names/0]). --ifdef(TEST). --export([drop_custom/0]). --endif. - -spec load_custom(#{atom() => list()}) -> ok. load_custom(Rulesets) -> {Existed, Tid} = ensure_table(), @@ -87,12 +83,6 @@ ensure_table() -> {true, elvis_ruleset} end. --ifdef(TEST). -drop_custom() -> - Table = table(), - ets:delete(Table). --endif. - table_exists() -> ets:info(elvis_ruleset) =/= undefined. From 0734f2d3761de013278afbb83c517e6c3a73b8cb Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 19:11:39 +0100 Subject: [PATCH 29/67] Allow xref'ing further --- rebar.config | 17 +++++++++++++++-- src/elvis_code.erl | 3 +++ src/elvis_config.erl | 3 +++ src/elvis_core.erl | 13 ++++++++++--- src/elvis_gitignore.erl | 3 +++ src/elvis_project.erl | 3 +++ src/elvis_result.erl | 3 +++ src/elvis_rule.erl | 3 +-- src/elvis_style.erl | 3 +++ src/elvis_text_style.erl | 3 +++ src/elvis_utils.erl | 4 +++- 11 files changed, 50 insertions(+), 8 deletions(-) diff --git a/rebar.config b/rebar.config index 91aea5b4..c7757730 100644 --- a/rebar.config +++ b/rebar.config @@ -22,7 +22,18 @@ ]} ]}. -{alias, [{test, [compile, fmt, hank, xref, dialyzer, ct, cover, ex_doc]}]}. +{alias, [ + {test, [ + compile, + fmt, + hank, + {xref, "default as test xref"}, + {do, "default as test dialyzer"}, + ct, + cover, + ex_doc + ]} +]}. {shell, [{config, "config/test.config"}]}. @@ -63,6 +74,8 @@ {dialyzer, [{warnings, [no_return, unmatched_returns, error_handling, unknown]}]}. -{xref_checks, [undefined_function_calls, deprecated_function_calls, deprecated_functions]}. +{xref_checks, [ + undefined_function_calls, deprecated_function_calls, deprecated_functions, exports_not_used +]}. {xref_extra_paths, ["test/**"]}. diff --git a/src/elvis_code.erl b/src/elvis_code.erl index 8eba2a45..edfd4f7f 100644 --- a/src/elvis_code.erl +++ b/src/elvis_code.erl @@ -12,6 +12,9 @@ print_node/2 ]). +% These are local debug functions. +-ignore_xref([print_node/1, print_node/2]). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%% Public API %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_config.erl b/src/elvis_config.erl index d85e8606..95fa822f 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -20,6 +20,9 @@ -type elvis() :: proplists:proplist(). -export_type([elvis/0]). +% API exports, not consumed locally. +-ignore_xref([from_rebar/1, from_file/1, default/0, resolve_files/2, apply_to_files/2]). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 35ad8e9f..9eda80d1 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -4,8 +4,6 @@ -export([rock/1, rock_this/2]). -export([start/0]). -%% for internal use only --export([do_rock/2]). %% for eating our own dogfood -export([main/1]). @@ -13,10 +11,19 @@ -ifdef(TEST). --export([apply_rule/2]). +-export([do_rock/2, apply_rule/2]). +% For tests (we can't Xref the tests because rebar3 fails to compile some files). +-ignore_xref([do_rock/2, apply_rule/2]). -endif. +% For eating our own dogfood. +-ignore_xref([main/1]). +% For shell usage. +-ignore_xref([start/0]). +% API exports, not consumed locally. +-ignore_xref([rock/1, rock_this/2]). + -type source_filename() :: nonempty_string(). -type target() :: source_filename() | module(). diff --git a/src/elvis_gitignore.erl b/src/elvis_gitignore.erl index c2a9f16f..86414933 100644 --- a/src/elvis_gitignore.erl +++ b/src/elvis_gitignore.erl @@ -5,6 +5,9 @@ -export([required_patterns/2, forbidden_patterns/2]). +% The whole file is considered to have either callback functions or rules. +-ignore_xref(elvis_gitignore). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Default values %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_project.erl b/src/elvis_project.erl index 86084079..eac8554a 100644 --- a/src/elvis_project.erl +++ b/src/elvis_project.erl @@ -5,6 +5,9 @@ -export([no_branch_deps/2, protocol_for_deps/2]). +% The whole file is considered to have either callback functions or rules. +-ignore_xref(elvis_project). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Default values %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_result.erl b/src/elvis_result.erl index eb0154db..c9901aa4 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -16,6 +16,9 @@ %% Types -export_type([item/0, rule/0, file/0, elvis_error/0, elvis_warn/0, attrs/0]). +% API exports, not consumed locally. +-ignore_xref([get_path/1, get_rules/1, get_items/1, get_message/1, get_info/1, get_line_num/1]). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Records %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_rule.erl b/src/elvis_rule.erl index 1445b632..fb7a7ce8 100644 --- a/src/elvis_rule.erl +++ b/src/elvis_rule.erl @@ -8,7 +8,6 @@ ns/1, name/1, def/1, - ignores/1, disabled/1, file/1, file/2, ignored/2, @@ -182,7 +181,7 @@ disable(Rule) -> -spec ignored(Needle :: ignorable(), t()) -> boolean(). ignored(Needle, Rule) -> - lists:member(Needle, Rule#rule.ignores). + lists:member(Needle, ignores(Rule)). -spec execute(t(), ElvisConfig) -> Results when ElvisConfig :: elvis_config:t(), diff --git a/src/elvis_style.erl b/src/elvis_style.erl index b5f1ed2a..b56d32b2 100644 --- a/src/elvis_style.erl +++ b/src/elvis_style.erl @@ -63,6 +63,9 @@ prefer_include/2 ]). +% The whole file is considered to have either callback functions or rules. +-ignore_xref(elvis_style). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Default values %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_text_style.erl b/src/elvis_text_style.erl index c1ba7e05..2cf97618 100644 --- a/src/elvis_text_style.erl +++ b/src/elvis_text_style.erl @@ -10,6 +10,9 @@ no_redundant_blank_lines/2 ]). +% The whole file is considered to have either callback functions or rules. +-ignore_xref(elvis_text_style). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Default values %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index b8acfcec..4b234e2c 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -11,11 +11,13 @@ -ignore_xref(do_output/2). -ignore_xref(abort/2). +% API exports, not consumed locally. +-ignore_xref([erlang_halt/1]). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -%% @doc This is defined so that it can be mocked for tests. -spec erlang_halt(integer()) -> no_return(). erlang_halt(Code) -> halt(Code). From 540b064ca13f84ab426fdc992f38f91a00eae38f Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 19:13:07 +0100 Subject: [PATCH 30/67] Make it compilation error -free --- src/elvis_core.erl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 9eda80d1..425d245e 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -114,6 +114,8 @@ do_parallel_rock(ElvisConfig0) -> {fail, [{T, E}]} end. +-ifdef(TEST). + -spec do_rock(elvis_file:t(), [elvis_config:t()] | elvis_config:t()) -> {ok, elvis_result:file()}. do_rock(File, ElvisConfig) -> @@ -121,6 +123,8 @@ do_rock(File, ElvisConfig) -> Results = apply_rules(ElvisConfig, LoadedFile), {ok, Results}. +-endif. + -spec load_file_data([elvis_config:t()] | elvis_config:t(), elvis_file:t()) -> elvis_file:t(). load_file_data(ElvisConfig, File) -> From 836f4946bd788f5a5ce2f3eb7033bd35a463aa86 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 19:32:40 +0100 Subject: [PATCH 31/67] Ease it again! --- rebar.config | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/rebar.config b/rebar.config index c7757730..4b0cf02c 100644 --- a/rebar.config +++ b/rebar.config @@ -23,16 +23,7 @@ ]}. {alias, [ - {test, [ - compile, - fmt, - hank, - {xref, "default as test xref"}, - {do, "default as test dialyzer"}, - ct, - cover, - ex_doc - ]} + {test, [compile, fmt, hank, xref, dialyzer, ct, cover, ex_doc]} ]}. {shell, [{config, "config/test.config"}]}. From 5d4664fbf0936aa26ae970e2ece3dd97eb2a5227 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 19:47:20 +0100 Subject: [PATCH 32/67] Go with no-throw-but-error, by default Later, when we implement warnings_as_errors we can tweak this to return ok even if there are warnings (while they're still printed by elvis_core) It simplifies the consumption API and the tests (no more unnecessary catches) --- src/elvis_config.erl | 9 ++++- src/elvis_core.erl | 79 ++++++++++++++++++++++--------------- test/elvis_SUITE.erl | 94 +++++++------------------------------------- 3 files changed, 70 insertions(+), 112 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 95fa822f..080e082a 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -129,8 +129,13 @@ from_file(File) -> fetch_elvis_config_from(AppConfig). fetch_elvis_config_from(AppConfig) -> - Elvis = fetch_elvis_config(undefined, AppConfig), - from_static(config, {elvis, Elvis}). + try fetch_elvis_config(undefined, AppConfig) of + Elvis -> + from_static(config, {elvis, Elvis}) + catch + {invalid_config, _} = Caught -> + {fail, [{throw, Caught}]} + end. default_for(app) -> % This is the top-level element, before 'elvis' diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 425d245e..0e055c43 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -37,15 +37,28 @@ start() -> {ok, _} = application:ensure_all_started(elvis_core), ok. +validate_config_or_throw(ElvisConfig) -> + try elvis_config:validate_config(ElvisConfig) of + ok -> + ok + catch + {invalid_config, _} = Caught -> + {error, {fail, [{throw, Caught}]}} + end. + %% In this context, `throw` means an error, e.g., validation or internal, not an actual %% call to `erlang:throw/1`. -spec rock([elvis_config:t()]) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> - ok = elvis_config:validate_config(ElvisConfig), - elvis_ruleset:dump_custom(), - Results = lists:map(fun do_parallel_rock/1, ElvisConfig), - lists:foldl(fun combine_results/2, ok, Results). + case validate_config_or_throw(ElvisConfig) of + ok -> + elvis_ruleset:dump_custom(), + Results = lists:map(fun do_parallel_rock/1, ElvisConfig), + lists:foldl(fun combine_results/2, ok, Results); + {error, Error} -> + Error + end. -spec rock_this(target(), [elvis_config:t()]) -> ok | {fail, [elvis_result:file() | elvis_result:rule()]}. @@ -54,33 +67,37 @@ rock_this(Module, ElvisConfig) when is_atom(Module) -> Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> - ok = elvis_config:validate_config(ElvisConfig), - elvis_ruleset:dump_custom(), - Dirname = filename:dirname(Path), - Filename = filename:basename(Path), - File = - case elvis_file:find_files([Dirname], Filename) of - [] -> - throw({enoent, Path}); - [File0] -> - File0 - end, - - FilterFun = - fun(Cfg) -> - Filter = elvis_config:filter(Cfg), - Dirs = elvis_config:dirs(Cfg), - IgnoreList = elvis_config:ignore(Cfg), - [] =/= elvis_file:filter_files([File], Dirs, Filter, IgnoreList) - end, - case lists:filter(FilterFun, ElvisConfig) of - [] -> - elvis_utils:output(info, "Skipping ~s", [Path]); - FilteredElvisConfig -> - LoadedFile = load_file_data(FilteredElvisConfig, File), - ApplyRulesFun = fun(Cfg) -> apply_rules_and_print(Cfg, LoadedFile) end, - Results = lists:map(ApplyRulesFun, FilteredElvisConfig), - elvis_result_status(Results) + case validate_config_or_throw(ElvisConfig) of + ok -> + elvis_ruleset:dump_custom(), + Dirname = filename:dirname(Path), + Filename = filename:basename(Path), + File = + case elvis_file:find_files([Dirname], Filename) of + [] -> + throw({enoent, Path}); + [File0] -> + File0 + end, + + FilterFun = + fun(Cfg) -> + Filter = elvis_config:filter(Cfg), + Dirs = elvis_config:dirs(Cfg), + IgnoreList = elvis_config:ignore(Cfg), + [] =/= elvis_file:filter_files([File], Dirs, Filter, IgnoreList) + end, + case lists:filter(FilterFun, ElvisConfig) of + [] -> + elvis_utils:output(info, "Skipping ~s", [Path]); + FilteredElvisConfig -> + LoadedFile = load_file_data(FilteredElvisConfig, File), + ApplyRulesFun = fun(Cfg) -> apply_rules_and_print(Cfg, LoadedFile) end, + Results = lists:map(ApplyRulesFun, FilteredElvisConfig), + elvis_result_status(Results) + end; + {error, Error} -> + Error end. %% In this context, `throw` means an error, e.g., validation or internal, not an actual diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index eb0cdae6..85f77f97 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -61,43 +61,18 @@ end_per_suite(Config) -> %%% Rocking rock_with_empty_map_config(_Config) -> - ok = - try - ok = elvis_core:rock([#{}]), - fail - catch - {invalid_config, _} -> - ok - end, - ok = - try - ok = elvis_core:rock([#{} || X <- lists:seq(1, 10), X < 1]), - fail - catch - {invalid_config, _} -> - ok - end. + {fail, [{throw, {invalid_config, _}}]} = elvis_core:rock([#{}]), + {fail, [{throw, {invalid_config, _}}]} = elvis_core:rock([]), + ok. rock_with_empty_list_config(_Config) -> - ok = - try - ok = elvis_core:rock([#{}, #{}]), - fail - catch - {invalid_config, _} -> - ok - end. + {fail, [{throw, {invalid_config, _}}]} = elvis_core:rock([#{}, #{}]), + ok. rock_with_incomplete_config(_Config) -> ElvisConfig = [#{dirs => ["src"]}], - ok = - try - ok = elvis_core:rock(ElvisConfig), - fail - catch - {invalid_config, _} -> - ok - end. + {fail, [{throw, {invalid_config, _}}]} = elvis_core:rock(ElvisConfig), + ok. rock_with_list_config(_Config) -> ElvisConfig = [ @@ -107,13 +82,7 @@ rock_with_list_config(_Config) -> filter => "*.erl" } ], - ok = - try - ok = elvis_core:rock(ElvisConfig) - catch - {invalid_config, _} -> - fail - end. + ok = elvis_core:rock(ElvisConfig). rock_with_file_config(_Config) -> ConfigPath = "../../config/elvis.config", @@ -257,13 +226,7 @@ rock_with_rule_groups(_Config) -> ruleset => rebar_config } ], - ok = - try - ok = elvis_core:rock(RulesGroupConfig) - catch - {invalid_config, _} -> - fail - end, + ok = elvis_core:rock(RulesGroupConfig), % Override default elvis_core rules without ruleset should fail. OverrideFailConfig = [ @@ -276,14 +239,7 @@ rock_with_rule_groups(_Config) -> ] } ], - ok = - try - _ = elvis_core:rock(OverrideFailConfig), - fail - catch - {invalid_config, _} -> - ok - end, + {fail, [{throw, {invalid_config, _}}]} = elvis_core:rock(OverrideFailConfig), % Override default elvis_core rules. OverrideConfig = [ @@ -305,13 +261,7 @@ rock_with_rule_groups(_Config) -> ruleset => rebar_config } ], - ok = - try - ok = elvis_core:rock(OverrideConfig) - catch - {invalid_config, _} -> - fail - end. + ok = elvis_core:rock(OverrideConfig). rock_this_skipping_files(_Config) -> meck:new(elvis_file, [passthrough]), @@ -367,14 +317,8 @@ rock_with_umbrella_apps(_Config) -> rock_with_invalid_rules(_Config) -> ConfigPath = "../../test/examples/invalid_rules.elvis.config", - try - ElvisConfig = elvis_config:from_file(ConfigPath), - ok = elvis_core:rock(ElvisConfig), - ct:fail("Elvis should not have rocked with ~p", [ElvisConfig]) - catch - {invalid_config, _} -> - ok - end. + {fail, [{throw, {invalid_config, _}}]} = elvis_config:from_file(ConfigPath), + ok. %%%%%%%%%%%%%%% %%% Utils @@ -411,16 +355,8 @@ hrl_ruleset(_Config) -> throw_configuration(_Config) -> Filename = "./elvis.config", ok = file:write_file(Filename, <<"-">>), - ok = - try - _ = elvis_config:from_file(Filename), - fail - catch - _ -> - ok - after - file:delete(Filename) - end. + {fail, [{throw, {invalid_config, _}}]} = elvis_config:from_file(Filename), + _ = file:delete(Filename). find_file_and_check_src(_Config) -> Dirs = ["../../test/examples"], From b08309da69edd0cee248b290ef955afdf14f0edb Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 22:38:13 +0100 Subject: [PATCH 33/67] Have `elvis_core on elvis_core` work --- elvis.config | 10 ++++++++-- src/elvis_core.erl | 18 +++++++++--------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/elvis.config b/elvis.config index c8e15c05..741c0b39 100644 --- a/elvis.config +++ b/elvis.config @@ -14,7 +14,11 @@ }}, {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}, {elvis_style, no_debug_call, #{ - ignore => [{elvis_result, print_item, 4}, {elvis_utils, do_output, 2}] + ignore => [ + {elvis_result, print_item, 4}, + {elvis_utils, do_output, 2}, + {elvis_core, main, 1} + ] }}, {elvis_style, no_god_modules, #{ignore => [elvis_style]}}, {elvis_style, no_throw, disable}, @@ -41,7 +45,9 @@ ignore => [elvis_core, elvis_config] }}, {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}, - {elvis_style, no_debug_call, #{ignore => [elvis_result, elvis_utils]}}, + {elvis_style, no_debug_call, #{ + ignore => [elvis_result, elvis_utils, elvis_core] + }}, {elvis_style, no_god_modules, #{ignore => [elvis_style]}}, {elvis_style, no_throw, disable}, {elvis_style, no_common_caveats_call, disable} diff --git a/src/elvis_core.erl b/src/elvis_core.erl index 0e055c43..fbe79f35 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -4,6 +4,8 @@ -export([rock/1, rock_this/2]). -export([start/0]). +%% for internal use only +-export([do_rock/2]). %% for eating our own dogfood -export([main/1]). @@ -11,14 +13,16 @@ -ifdef(TEST). --export([do_rock/2, apply_rule/2]). +-export([apply_rule/2]). % For tests (we can't Xref the tests because rebar3 fails to compile some files). --ignore_xref([do_rock/2, apply_rule/2]). +-ignore_xref([apply_rule/2]). -endif. % For eating our own dogfood. -ignore_xref([main/1]). +% For internal use only +-ignore_xref([do_rock/2]). % For shell usage. -ignore_xref([start/0]). % API exports, not consumed locally. @@ -37,7 +41,7 @@ start() -> {ok, _} = application:ensure_all_started(elvis_core), ok. -validate_config_or_throw(ElvisConfig) -> +validate_config(ElvisConfig) -> try elvis_config:validate_config(ElvisConfig) of ok -> ok @@ -51,7 +55,7 @@ validate_config_or_throw(ElvisConfig) -> -spec rock([elvis_config:t()]) -> ok | {fail, [{throw, term()} | elvis_result:file() | elvis_result:rule()]}. rock(ElvisConfig) -> - case validate_config_or_throw(ElvisConfig) of + case validate_config(ElvisConfig) of ok -> elvis_ruleset:dump_custom(), Results = lists:map(fun do_parallel_rock/1, ElvisConfig), @@ -67,7 +71,7 @@ rock_this(Module, ElvisConfig) when is_atom(Module) -> Path = proplists:get_value(source, ModuleInfo), rock_this(Path, ElvisConfig); rock_this(Path, ElvisConfig) -> - case validate_config_or_throw(ElvisConfig) of + case validate_config(ElvisConfig) of ok -> elvis_ruleset:dump_custom(), Dirname = filename:dirname(Path), @@ -131,8 +135,6 @@ do_parallel_rock(ElvisConfig0) -> {fail, [{T, E}]} end. --ifdef(TEST). - -spec do_rock(elvis_file:t(), [elvis_config:t()] | elvis_config:t()) -> {ok, elvis_result:file()}. do_rock(File, ElvisConfig) -> @@ -140,8 +142,6 @@ do_rock(File, ElvisConfig) -> Results = apply_rules(ElvisConfig, LoadedFile), {ok, Results}. --endif. - -spec load_file_data([elvis_config:t()] | elvis_config:t(), elvis_file:t()) -> elvis_file:t(). load_file_data(ElvisConfig, File) -> From 76328fb6a8afe8ad440fe6feb497de4637b9f22d Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 23:43:44 +0100 Subject: [PATCH 34/67] Re-read README and act on expected output Only do color parsing after creating the expected output --- README.md | 17 +++++------------ elvis.config | 3 +-- src/elvis_code.erl | 2 +- src/elvis_config.erl | 25 +++++++++++-------------- src/elvis_core.erl | 6 +++--- src/elvis_result.erl | 13 ++++++------- src/elvis_ruleset.erl | 2 +- src/elvis_utils.erl | 27 +++++++++++++++++++++------ 8 files changed, 49 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 9d05de76..bbe9433a 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Once this is done you can apply the style rules in the following ways. ```shell 1> ElvisConfig = elvis_config:config(). -2> elvis_core:rock(ElvisConfig). +2> application:set_env(elvis_core, verbose, true), elvis_core:rock(ElvisConfig). Loading src/elvis_code.erl # src/elvis_code.erl [OK] Loading src/elvis_config.erl @@ -68,18 +68,11 @@ Another option for using `elvis_core` from the shell is to explicitly provide th an argument to `elvis_core:rock/1`: ```shell -1> ElvisConfig = [#{dirs => ["src"], filter => "*.erl", rules => []}]. +1> ElvisConfig = [#{dirs => ["src"], filter => "elvis_rule.erl", ruleset => erl_files}]. [#{dirs => ["src"],filter => "*.erl",rules => []}] -2> elvis_core:rock(ElvisConfig). -Loading src/elvis_code.erl -# src/elvis_code.erl [OK] -Loading src/elvis_config.erl -# src/elvis_config.erl [OK] -Loading src/elvis_core.erl -# src/elvis_core.erl [OK] -Loading src/elvis_file.erl -# src/elvis_file.erl [OK] -... +2> application:set_env(elvis_core, verbose, true), elvis_core:rock(ElvisConfig). +Loading src/elvis_rule.erl +# src/elvis_rule.erl [OK] ok 3> ``` diff --git a/elvis.config b/elvis.config index 741c0b39..2c3aece5 100644 --- a/elvis.config +++ b/elvis.config @@ -76,7 +76,6 @@ {elvis_style, atom_naming_convention, #{ignore => [style_SUITE]}} ] } - ]}, - {no_output, true} + ]} ]} ]. diff --git a/src/elvis_code.erl b/src/elvis_code.erl index edfd4f7f..24400769 100644 --- a/src/elvis_code.erl +++ b/src/elvis_code.erl @@ -189,6 +189,6 @@ print_node(#{type := Type} = Node, CurrentLevel) -> Indentation = lists:duplicate(CurrentLevel * 4, $\s), Content = ktn_code:content(Node), - ok = elvis_utils:output(info, "~s - [~p] ~p~n", [Indentation, CurrentLevel, Type]), + ok = elvis_utils:info("~s - [~p] ~p~n", [Indentation, CurrentLevel, Type]), _ = lists:map(fun(Child) -> print_node(Child, CurrentLevel + 1) end, Content), ok. diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 080e082a..1a135723 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -40,17 +40,17 @@ fetch_elvis_config(Key, AppConfig) -> Elvis. from_static(Key, {Type, Config}) -> - elvis_utils:output(debug, "fetching key ~p from ~p configuration", [Key, Type]), + elvis_utils:debug("fetching key ~p from ~p configuration", [Key, Type]), case proplists:get_value(Key, Config) of undefined -> - elvis_utils:output( - debug, "no value for key ~p found in ~p configuration; going with default", [ + elvis_utils:debug( + "no value for key ~p found in ~p configuration; going with default", [ Key, Type ] ), default(Key); Value -> - elvis_utils:output(debug, "value for key ~p found in ~p configuration", [Key, Type]), + elvis_utils:debug("value for key ~p found in ~p configuration", [Key, Type]), Value end. @@ -72,14 +72,13 @@ parallel() -> default(Key) -> case application:get_env(elvis_core, Key) of undefined -> - elvis_utils:output( - debug, + elvis_utils:debug( "no value for key ~p found in application environment; going with default", [Key] ), default_for(Key); {ok, Value} -> - elvis_utils:output(debug, "value for key ~p found in application environment", [Key]), + elvis_utils:debug("value for key ~p found in application environment", [Key]), Value end. @@ -89,9 +88,7 @@ for(Key) -> case consult_elvis_config("elvis.config") of AppDefault -> % This might happen whether we fail to parse the file or it actually is [] - elvis_utils:output( - debug, "elvis.config unusable; falling back to rebar.config", [] - ), + elvis_utils:debug("elvis.config unusable; falling back to rebar.config", []), consult_rebar_config("rebar.config"); AppConfig0 -> AppConfig0 @@ -103,20 +100,20 @@ for(Key) -> consult_elvis_config(File) -> case file:consult(File) of {ok, [AppConfig]} -> - elvis_utils:output(debug, "elvis.config usable; using it", []), + elvis_utils:debug("elvis.config usable; using it", []), AppConfig; _ -> - elvis_utils:output(debug, "elvis.config unusable", []), + elvis_utils:debug("elvis.config unusable", []), default_for(app) end. consult_rebar_config(File) -> case file:consult(File) of {ok, AppConfig0} -> - elvis_utils:output(debug, "rebar.config usable; using it", []), + elvis_utils:debug("rebar.config usable; using it", []), AppConfig0; _ -> - elvis_utils:output(debug, "rebar.config unusable", []), + elvis_utils:debug("rebar.config unusable", []), default_for(app) end. diff --git a/src/elvis_core.erl b/src/elvis_core.erl index fbe79f35..d42d2893 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -93,7 +93,7 @@ rock_this(Path, ElvisConfig) -> end, case lists:filter(FilterFun, ElvisConfig) of [] -> - elvis_utils:output(info, "Skipping ~s", [Path]); + elvis_utils:info("Skipping ~s", [Path]); FilteredElvisConfig -> LoadedFile = load_file_data(FilteredElvisConfig, File), ApplyRulesFun = fun(Cfg) -> apply_rules_and_print(Cfg, LoadedFile) end, @@ -146,13 +146,13 @@ do_rock(File, ElvisConfig) -> elvis_file:t(). load_file_data(ElvisConfig, File) -> Path = elvis_file:path(File), - elvis_utils:output(info, "Loading ~s", [Path]), + elvis_utils:info("Loading ~s", [Path]), try elvis_file:load_file_data(ElvisConfig, File) catch _:Reason -> Msg = "~p when loading file ~p.", - elvis_utils:output(error, Msg, [Reason, Path]), + elvis_utils:error(Msg, [Reason, Path]), File end. diff --git a/src/elvis_result.erl b/src/elvis_result.erl index c9901aa4..711ede46 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -187,9 +187,9 @@ print(Format, #{file := Path, rules := Rules}) -> _ -> case status(Rules) of ok -> - elvis_utils:output(notice, "# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); + elvis_utils:notice("# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); fail -> - elvis_utils:output(error, "# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) + elvis_utils:error("# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) end end, print_rules(Format, Path, Rules); @@ -216,8 +216,7 @@ print_rules( parsable -> ok; _ -> - elvis_utils:output( - error, + elvis_utils:error( " - ~p " "(https://github.com/inaka/elvis_core/tree/main/doc_rules/~p/~p.md)", [Name, Scope, Name] @@ -249,7 +248,7 @@ print_item( FMsg = io_lib:format(Msg, Info), io:format("~s:~p:~p:~s~n", [File, Ln, Name, FMsg]); _ -> - elvis_utils:output(error, " - " ++ Msg, Info) + elvis_utils:error(" - " ++ Msg, Info) end, print_item(Format, File, Name, Items); print_item(Format, File, Name, [Error | Items]) -> @@ -259,9 +258,9 @@ print_item(_Format, _File, _Name, []) -> ok. print_error(#{error_msg := Msg, info := Info}) -> - elvis_utils:output(error, Msg, Info); + elvis_utils:error(Msg, Info); print_error(#{warn_msg := Msg, info := Info}) -> - elvis_utils:output(warn, Msg, Info). + elvis_utils:warn(Msg, Info). -spec status([file() | rule()]) -> ok | fail. status([]) -> diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index f80f280d..2dbd0a9b 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -9,7 +9,7 @@ load_custom(Rulesets) -> {Existed, Tid} = ensure_table(), case Existed of false -> - elvis_utils:output(debug, "loading custom rulesets into state", []), + elvis_utils:debug("loading custom rulesets into state", []), lists:foreach( fun({Ruleset, NSNameDefs}) -> Rules = [elvis_rule:from_tuple(NSNameDef) || NSNameDef <- NSNameDefs], diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 4b234e2c..6ab9e8fe 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -5,7 +5,7 @@ %% General -export([erlang_halt/1, to_str/1, split_all_lines/1, split_all_lines/2]). %% Output / rebar3 --export([output/3, abort/2]). +-export([debug/2, info/2, notice/2, warn/2, error/2, abort/2]). % These call (but verify if exported) rebar3-specific functions. -ignore_xref(do_output/2). @@ -73,6 +73,21 @@ escape_chars(String) -> ResultBin = iolist_to_binary(Result), binary_to_list(ResultBin). +debug(Format, Data) -> + output(debug, "Elvis: " ++ Format, Data). + +info(Format, Data) -> + output(info, Format ++ "{{reset}}~n", Data). + +notice(Format, Data) -> + output(notice, "{{white-bold}}" ++ Format ++ "{{reset}}~n", Data). + +warn(Format, Data) -> + output(warn, "{{magenta}}Warning: {{reset}}" ++ Format ++ "{{reset}}~n", Data). + +error(Format, Data) -> + output(error, "{{red}}Error: {{reset}}" ++ Format ++ "{{reset}}~n", Data). + -spec output(debug | info | notice | warn | error, Format :: io:format(), Data :: [term()]) -> ok. output(debug = _Type, Format, Data) -> Chars = io_lib:format(Format, Data), @@ -92,28 +107,28 @@ output(Type, Format, Data) -> do_output(debug, Chars) -> case erlang:function_exported(rebar_api, debug, 2) of true -> - rebar_api:debug("Elvis: " ++ Chars, []); + rebar_api:debug(Chars, []); false -> ok end; do_output(info, Chars) -> case elvis_config:verbose() of true -> - io:format(Chars ++ "{{reset}}~n"); + io:format(Chars); false -> ok end; do_output(notice, Chars) -> case elvis_config:verbose() of true -> - io:format("{{white-bold}}" ++ Chars ++ "{{reset}}~n"); + io:format(Chars); false -> ok end; do_output(warn, Chars) -> - io:format("{{magenta}}Warning: {{reset}}" ++ Chars ++ "{{reset}}~n"); + io:format(Chars); do_output(error, Chars) -> - io:format("{{red}}Error: {{reset}}" ++ Chars ++ "{{reset}}~n"). + io:format(Chars). -dialyzer({nowarn_function, abort/2}). -spec abort(Format :: io:format(), Data :: [term()]) -> no_return(). From 2b0ba4bb7cae2e6f0703d8c4f8ae180d6857e127 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 23:50:22 +0100 Subject: [PATCH 35/67] Don't change if we don't have to --- rebar.config | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rebar.config b/rebar.config index 4b0cf02c..08cb04ee 100644 --- a/rebar.config +++ b/rebar.config @@ -22,9 +22,7 @@ ]} ]}. -{alias, [ - {test, [compile, fmt, hank, xref, dialyzer, ct, cover, ex_doc]} -]}. +{alias, [{test, [compile, fmt, hank, xref, dialyzer, ct, cover, ex_doc]}]}. {shell, [{config, "config/test.config"}]}. From 39d3cbe6eb8ace208a8fb99a6d82d652d38e3025 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 23:56:01 +0100 Subject: [PATCH 36/67] Rename it for clarity --- src/elvis_core.erl | 4 ++-- src/elvis_ruleset.erl | 4 ++-- test/elvis_SUITE.erl | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index d42d2893..a43d5125 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -57,7 +57,7 @@ validate_config(ElvisConfig) -> rock(ElvisConfig) -> case validate_config(ElvisConfig) of ok -> - elvis_ruleset:dump_custom(), + elvis_ruleset:drop_custom(), Results = lists:map(fun do_parallel_rock/1, ElvisConfig), lists:foldl(fun combine_results/2, ok, Results); {error, Error} -> @@ -73,7 +73,7 @@ rock_this(Module, ElvisConfig) when is_atom(Module) -> rock_this(Path, ElvisConfig) -> case validate_config(ElvisConfig) of ok -> - elvis_ruleset:dump_custom(), + elvis_ruleset:drop_custom(), Dirname = filename:dirname(Path), Filename = filename:basename(Path), File = diff --git a/src/elvis_ruleset.erl b/src/elvis_ruleset.erl index 2dbd0a9b..80b059b5 100644 --- a/src/elvis_ruleset.erl +++ b/src/elvis_ruleset.erl @@ -2,7 +2,7 @@ -format(#{inline_items => none}). --export([rules/1, load_custom/1, dump_custom/0, is_defined/1, custom_names/0]). +-export([rules/1, load_custom/1, drop_custom/0, is_defined/1, custom_names/0]). -spec load_custom(#{atom() => list()}) -> ok. load_custom(Rulesets) -> @@ -29,7 +29,7 @@ custom_names() -> proplists:get_keys(ets:tab2list(elvis_ruleset)) end. -dump_custom() -> +drop_custom() -> case table_exists() of false -> ok; diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 85f77f97..204df707 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -330,7 +330,7 @@ custom_ruleset(_Config) -> [[NoTabs]] = elvis_config:rules(ElvisConfig), %% this is also done by :rock and :rock_this - _ = elvis_ruleset:dump_custom(), + _ = elvis_ruleset:drop_custom(), %% read unknown ruleset configuration to ensure rulesets from %% previous load do not stick around From 85d72d7a8fe391a3c78a49785d77384c46d6e6fb Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 16 Jul 2025 23:59:34 +0100 Subject: [PATCH 37/67] Use it if available, and drop custom ignore_xref --- src/elvis_core.erl | 2 +- src/elvis_utils.erl | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index a43d5125..b177ffe6 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -162,7 +162,7 @@ main([]) -> {module, _} = code:ensure_loaded(elvis_style), case rock(elvis_config:config()) of ok -> true; - _ -> halt(1) + _ -> elvis_utils:erlang_halt(1) end. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 6ab9e8fe..080d08a9 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -11,9 +11,6 @@ -ignore_xref(do_output/2). -ignore_xref(abort/2). -% API exports, not consumed locally. --ignore_xref([erlang_halt/1]). - %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Public %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% From 7cb4cc70a254c16dcf903c906215917ae9981970 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 00:25:01 +0100 Subject: [PATCH 38/67] Spec it for consumption --- src/elvis_config.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 1a135723..86dda3ed 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -117,10 +117,12 @@ consult_rebar_config(File) -> default_for(app) end. +-spec from_rebar(File :: string()) -> t(). from_rebar(File) -> AppConfig = consult_rebar_config(File), fetch_elvis_config_from(AppConfig). +-spec from_file(File :: string()) -> t(). from_file(File) -> AppConfig = consult_elvis_config(File), fetch_elvis_config_from(AppConfig). From 961d118fc4154bf86250f824ad02c56aa0cd67cc Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 00:25:32 +0100 Subject: [PATCH 39/67] Add convenience elvis_config API --- src/elvis_config.erl | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 86dda3ed..57c756fe 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -12,6 +12,7 @@ %% Options -export([config/0, output_format/0, verbose/0, no_output/0, parallel/0]). +-export([set_output_format/1, set_verbose/1, set_no_output/1, set_parallel/1]). % Corresponds to the 'config' key. -type t() :: map(). @@ -22,6 +23,7 @@ % API exports, not consumed locally. -ignore_xref([from_rebar/1, from_file/1, default/0, resolve_files/2, apply_to_files/2]). +-ignore_xref([set_output_format/1, set_verbose/1, set_no_output/1, set_parallel/1]). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%% Public @@ -69,6 +71,21 @@ no_output() -> parallel() -> for(parallel). +set_output_format(OutputFormat) -> + set_env(output_format, OutputFormat). + +set_verbose(Verbose) -> + set_env(verbose, Verbose). + +set_no_output(NoOutput) -> + set_env(no_output, NoOutput). + +set_parallel(Parallel) -> + set_env(parallel, Parallel). + +set_env(Key, Value) -> + application:set_env(elvis_core, Key, Value). + default(Key) -> case application:get_env(elvis_core, Key) of undefined -> From 9e3c7ef38f6ae97d578f67554f18ba39844cd85e Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 00:39:34 +0100 Subject: [PATCH 40/67] Fix default configuration --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 57c756fe..9938ac77 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -198,7 +198,7 @@ default() -> "apps/**/include/**", "include/**" ], - filter => "*.erl", + filter => "*.hrl", ruleset => hrl_files }, #{ From 1767ba451fd2197fe91cdf1b64c6887af9d8cd81 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 00:43:29 +0100 Subject: [PATCH 41/67] Add a note on default configuration to the README --- README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/README.md b/README.md index bbe9433a..eaac8e7d 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,32 @@ An `elvis.config` file looks something like this: ]}]. ``` +To look at what is considered the "current default" configuration, do: + +```console +rebar3 shell +... +1> elvis_config:default(). +[#{filter => "*.erl", + dirs => ["apps/**/src/**","src/**"], + ruleset => erl_files}, + #{filter => "*.erl", + dirs => + ["apps/**/src/**","src/**","apps/**/include/**", + "include/**"], + ruleset => hrl_files}, + #{filter => "rebar.config", + dirs => ["."], + ruleset => rebar_config}, + #{filter => ".gitignore", + dirs => ["."], + ruleset => gitignore}] +2> +``` + +**Note**: this element might change with time. The above was what was generated when this +documentation was updated. + ### Files, rules and rulesets The `dirs` key is a list that tells `elvis_core` where it should look for the files that match From 797de2b1a295af3d026b72e43ee07838476bd1b0 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 02:12:34 +0100 Subject: [PATCH 42/67] Make for easier-to-follow output --- src/elvis_config.erl | 84 +++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 9938ac77..5a424fba 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -42,17 +42,17 @@ fetch_elvis_config(Key, AppConfig) -> Elvis. from_static(Key, {Type, Config}) -> - elvis_utils:debug("fetching key ~p from ~p configuration", [Key, Type]), + elvis_utils:debug("fetching key '~s' from '~s' configuration", [Key, Type]), case proplists:get_value(Key, Config) of undefined -> elvis_utils:debug( - "no value for key ~p found in ~p configuration; going with default", [ + "no value for key '~s' found in '~s' configuration; going with default", [ Key, Type ] ), default(Key); Value -> - elvis_utils:debug("value for key ~p found in ~p configuration", [Key, Type]), + elvis_utils:debug("value for key '~s' found in '~s' configuration", [Key, Type]), Value end. @@ -90,12 +90,12 @@ default(Key) -> case application:get_env(elvis_core, Key) of undefined -> elvis_utils:debug( - "no value for key ~p found in application environment; going with default", + "no value for key '~s' found in application environment; going with default", [Key] ), default_for(Key); {ok, Value} -> - elvis_utils:debug("value for key ~p found in application environment", [Key]), + elvis_utils:debug("value for key '~s' found in application environment", [Key]), Value end. @@ -134,12 +134,12 @@ consult_rebar_config(File) -> default_for(app) end. --spec from_rebar(File :: string()) -> t(). +-spec from_rebar(File :: string()) -> [t()]. from_rebar(File) -> AppConfig = consult_rebar_config(File), fetch_elvis_config_from(AppConfig). --spec from_file(File :: string()) -> t(). +-spec from_file(File :: string()) -> [t()]. from_file(File) -> AppConfig = consult_elvis_config(File), fetch_elvis_config_from(AppConfig). @@ -381,26 +381,26 @@ do_validate({elvis = Option, Elvis}) -> _ -> maybe ok = flag(validation_started(Option)), - ok ?= is_nonempty_list("elvis", Elvis), + ok ?= is_nonempty_list(elvis, Elvis), ok ?= proplist_keys_are_in( - "elvis", Elvis, elvis_control_opts() ++ [rulesets, config] + 'elvis', Elvis, elvis_control_opts() ++ [rulesets, config] ), OutputFormat = get_elvis_opt(output_format, Elvis), - ok ?= is_one_of("elvis.output_format", OutputFormat, [colors, plain, parsable]), + ok ?= is_one_of('elvis.output_format', OutputFormat, [colors, plain, parsable]), Verbose = get_elvis_opt(verbose, Elvis), - ok ?= is_boolean("elvis.verbose", Verbose), + ok ?= is_boolean('elvis.verbose', Verbose), NoOutput = get_elvis_opt(no_output, Elvis), - ok ?= is_boolean("elvis.no_output", NoOutput), + ok ?= is_boolean('elvis.no_output', NoOutput), Parallel = get_elvis_opt(parallel, Elvis), - ok ?= is_pos_integer("elvis.parallel", Parallel), + ok ?= is_pos_integer('elvis.parallel', Parallel), CustomRulesets = get_elvis_opt(rulesets, Elvis), - ok ?= are_valid_rulesets("elvis.rulesets", CustomRulesets), + ok ?= are_valid_rulesets('elvis.rulesets', CustomRulesets), ElvisConfig = get_elvis_opt(config, Elvis), - ok ?= is_valid_config("elvis.config", maps:keys(CustomRulesets), ElvisConfig) + ok ?= is_valid_config('elvis.config', maps:keys(CustomRulesets), ElvisConfig) else {error, FormatData} -> - do_validate_results(FormatData) + do_validate_throw(FormatData) end end; do_validate({config = Option, ElvisConfig}) -> @@ -409,15 +409,15 @@ do_validate({config = Option, ElvisConfig}) -> ok; _ -> maybe - ok ?= is_valid_config("elvis.config", elvis_ruleset:custom_names(), ElvisConfig) + ok ?= is_valid_config('elvis.config', elvis_ruleset:custom_names(), ElvisConfig) else {error, FormatData} -> - do_validate_results(FormatData) + do_validate_throw(FormatData) end end. --spec do_validate_results(_) -> no_return(). -do_validate_results(FormatData) -> +-spec do_validate_throw(_) -> no_return(). +do_validate_throw(FormatData) -> {Format, Data} = case is_list(FormatData) of false -> @@ -431,7 +431,7 @@ do_validate_results(FormatData) -> throw({invalid_config, io_lib:format(Format, Data)}). is_nonempty_list(What, List) when not is_list(List) orelse List =:= [] -> - {error, {"~p is expected to be a non-empty list.", [What]}}; + {error, {"'~s' is expected to be a non-empty list.", [What]}}; is_nonempty_list(_What, _List) -> ok. @@ -441,7 +441,7 @@ proplist_keys_are_in(What, List, Keys) -> [] -> ok; _ -> - {error, {"in ~p, the following keys are unknown: ~p.", [What, Filtered]}} + {error, {"in '~s', the following keys are unknown: ~p.", [What, Filtered]}} end. is_one_of(What, Value, Possibilities) -> @@ -449,18 +449,18 @@ is_one_of(What, Value, Possibilities) -> true -> ok; _ -> - {error, {"~p is expected to be one of the following: ~p.", [What, Possibilities]}} + {error, {"'~s' is expected to be one of the following: ~p.", [What, Possibilities]}} end. is_boolean(_What, Value) when is_boolean(Value) -> ok; is_boolean(What, _Value) -> - {error, {"~p is expected to be a boolean.", [What]}}. + {error, {"'~s' is expected to be a boolean.", [What]}}. is_pos_integer(_What, Value) when is_integer(Value) andalso Value > 0 -> ok; is_pos_integer(What, _Value) -> - {error, {"~p is expected to be a positive integer.", [What]}}. + {error, {"'~s' is expected to be a positive integer.", [What]}}. are_valid_rulesets(What, CustomRulesets) -> maybe @@ -476,7 +476,7 @@ are_valid_rulesets(What, CustomRulesets) -> is_map(_What, Value) when is_map(Value) -> ok; is_map(What, _Value) -> - {error, {"~p is expected to be a map.", [What]}}. + {error, {"'~s' is expected to be a map.", [What]}}. all_map_keys_are_atoms(What, Map) -> Filtered = [Key || Key <- maps:keys(Map), not is_atom(Key)], @@ -484,7 +484,7 @@ all_map_keys_are_atoms(What, Map) -> [] -> ok; _ -> - {error, {"in ~p, keys are expected to be atoms.", [What]}} + {error, {"in '~s', keys are expected to be atoms.", [What]}} end. all_custom_rulesets_have_valid_rules(What, CustomRulesets) -> @@ -497,7 +497,7 @@ all_custom_rulesets_have_valid_rules(What, CustomRulesets) -> AccInI; {false, ValidError} -> [ - {"in ~p, in ruleset ~p, " ++ ValidError, [What, CustomRuleset]} + {"in '~s', in ruleset '~s', " ++ ValidError, [What, CustomRuleset]} | AccInI ] end @@ -527,7 +527,7 @@ no_default_ruleset_override(What, CustomRulesets) -> _ -> {error, { - "in ~p, the following rulesets are not expected to be " + "in '~s', the following rulesets are not expected to be " "named after a default ruleset: ~p.", [ What, Filtered @@ -559,7 +559,9 @@ all_configs_are_valid(What, CustomRulesetNames, Configset) -> AccIn; {error, ValidError} -> [ - {"in ~p, at list position number ~p, " ++ ValidError, [What, PosNumber]} + {"in '~s', at list position number ~p, " ++ ValidError, [ + What, PosNumber + ]} | AccIn ] end, @@ -608,7 +610,7 @@ map_keys_are_in(Map, Keys) -> end. is_nonempty_list_of_dirs(What, List) when not is_list(List) orelse List =:= [] -> - {error, io_lib:format("~p is expected to be a non-empty list.", [What])}; + {error, io_lib:format("'~s' is expected to be a non-empty list.", [What])}; is_nonempty_list_of_dirs(What, List) -> Filtered = [ Element @@ -620,7 +622,7 @@ is_nonempty_list_of_dirs(What, List) -> _ -> {error, io_lib:format( - "in ~p, the following elements are not (or don't contain) directories: ~p.", + "in '~s', the following elements are not (or don't contain) directories: ~p.", [What, Filtered] )} end. @@ -635,7 +637,7 @@ is_nonempty_string(What, String) -> true -> ok; _ -> - {error, io_lib:format("~p is expected to be a non-empty string.", [What])} + {error, io_lib:format("'~s' is expected to be a non-empty string.", [What])} end. all_dirs_filter_combos_are_valid(Dirs, Filter) -> @@ -646,9 +648,11 @@ all_dirs_filter_combos_are_valid(Dirs, Filter) -> AccIn; _ -> [ - io_lib:format("dir + filter combo ~p + ~p yielded no files to analyse.", [ + io_lib:format( + "'' + '' combo '~s' + '~s' yielded no files to analyse.", [ Dir, Filter - ]) + ] + ) | AccIn ] end @@ -664,7 +668,7 @@ all_dirs_filter_combos_are_valid(Dirs, Filter) -> end. is_list_of_ignorables(What, List) when not is_list(List) -> - {error, io_lib:format("~p is expected to be a list.", [What])}; + {error, io_lib:format("'~s' is expected to be a list.", [What])}; is_list_of_ignorables(What, List) -> Filtered = [Element || Element <- List, not elvis_rule:is_ignorable(Element)], case Filtered of @@ -672,7 +676,7 @@ is_list_of_ignorables(What, List) -> ok; _ -> {error, - io_lib:format("in ~p, the following elements are not ignorable: ~p.", [ + io_lib:format("in '~s', the following elements are not ignorable: ~p.", [ What, Filtered ])} end. @@ -685,13 +689,13 @@ defined_ruleset_is_custom_or_default(CustomRulesetNames, Ruleset) -> ok; _ -> {error, - io_lib:format("~p is expected to be either a custom or a default ruleset.", [ + io_lib:format("'~s' is expected to be either a custom or a default ruleset.", [ Ruleset ])} end. all_rules_are_valid(What, RuleTuples) when not is_list(RuleTuples) -> - {error, io_lib:format("~p is expected to be a list.", [What])}; + {error, io_lib:format("'~s' is expected to be a list.", [What])}; all_rules_are_valid(What, RuleTuples) -> AccOut = lists:foldl( fun(RuleTuple, AccInI) -> @@ -699,7 +703,7 @@ all_rules_are_valid(What, RuleTuples) -> {true, Rule} -> check_rule_for_options(Rule, AccInI); {false, ValidError} -> - [io_lib:format("in ~p, " ++ ValidError, [What]) | AccInI] + [io_lib:format("in '~s', " ++ ValidError, [What]) | AccInI] end end, [], From d839029967eb2af11ce61fa8f00f31b6f4a84080 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 03:20:33 +0100 Subject: [PATCH 43/67] Adapt new entrypoint --- src/elvis_config.erl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 5a424fba..329adedc 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -57,7 +57,12 @@ from_static(Key, {Type, Config}) -> end. config() -> - for(config). + try + for(config) + catch + {invalid_config, _} = Caught -> + {fail, [{throw, Caught}]} + end. output_format() -> for(output_format). From adf86c25799caf0aeb014bb3cd661c161be161f9 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 03:20:45 +0100 Subject: [PATCH 44/67] Adapt to Elvis (temporarily?) --- src/elvis_config.erl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 329adedc..a949e3ad 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -587,7 +587,10 @@ get_config_opt(OptName, Config) -> config_is_valid(CustomRulesetNames, Config) -> maybe - ok ?= map_keys_are_in(Config, [dirs, filter, ignore, ruleset, rules]), + % We're keeping files, for the time being, because of elvis + % and the fact it knows about elvis_core's internals, but we + % should shortly revisit this + ok ?= map_keys_are_in(Config, [dirs, filter, ignore, ruleset, rules, files]), Dirs = get_config_opt(dirs, Config), ok ?= is_nonempty_list_of_dirs(dirs, Dirs), Filter = get_config_opt(filter, Config), @@ -655,7 +658,7 @@ all_dirs_filter_combos_are_valid(Dirs, Filter) -> [ io_lib:format( "'' + '' combo '~s' + '~s' yielded no files to analyse.", [ - Dir, Filter + Dir, Filter ] ) | AccIn From 7afc514b4e3a9361c82f587c8a5a687bf80d3fb2 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 03:26:27 +0100 Subject: [PATCH 45/67] Fix API per consumption --- src/elvis_config.erl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index a949e3ad..db581309 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -18,8 +18,8 @@ -type t() :: map(). -export_type([t/0]). --type elvis() :: proplists:proplist(). --export_type([elvis/0]). +-type fail_validation() :: {fail, [{throw, {invalid_config, Message :: string()}}]}. +-export_type([fail_validation/0]). % API exports, not consumed locally. -ignore_xref([from_rebar/1, from_file/1, default/0, resolve_files/2, apply_to_files/2]). @@ -56,6 +56,7 @@ from_static(Key, {Type, Config}) -> Value end. +-spec config() -> [t()] | fail_validation(). config() -> try for(config) @@ -139,12 +140,12 @@ consult_rebar_config(File) -> default_for(app) end. --spec from_rebar(File :: string()) -> [t()]. +-spec from_rebar(File :: string()) -> [t()] | fail_validation(). from_rebar(File) -> AppConfig = consult_rebar_config(File), fetch_elvis_config_from(AppConfig). --spec from_file(File :: string()) -> [t()]. +-spec from_file(File :: string()) -> [t()] | fail_validation(). from_file(File) -> AppConfig = consult_elvis_config(File), fetch_elvis_config_from(AppConfig). From a7d49d7c4aefd4c970d053fa0dc5f1385dba08ca Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 03:26:37 +0100 Subject: [PATCH 46/67] Act on dogfooding --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index db581309..8e76b2c2 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -390,7 +390,7 @@ do_validate({elvis = Option, Elvis}) -> ok ?= is_nonempty_list(elvis, Elvis), ok ?= proplist_keys_are_in( - 'elvis', Elvis, elvis_control_opts() ++ [rulesets, config] + elvis, Elvis, elvis_control_opts() ++ [rulesets, config] ), OutputFormat = get_elvis_opt(output_format, Elvis), ok ?= is_one_of('elvis.output_format', OutputFormat, [colors, plain, parsable]), From ae1bd21a79fe4971e2394ccc652f197c6cdb8f94 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 03:50:34 +0100 Subject: [PATCH 47/67] Typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index eaac8e7d..8816c27c 100644 --- a/README.md +++ b/README.md @@ -269,7 +269,7 @@ found in this repository's [RULES.md](https://github.com/inaka/elvis_core/blob/m The implementation of a new rule is a function that takes 2 arguments in the following order: 1. `t:elvis_rule:t()`: the opaque rule to implement -1. `t:elvis_config:config()`: the value of option `config` as found in the +1. `t:elvis_config:t()`: the value of each element in list `config` as found in the [configuration](#configuration), This means you can define rules of your own (user-defined rules) as long as the functions that From 3bb94bed35a99145b7aea13232947f4dd6bae4ae Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 03:50:52 +0100 Subject: [PATCH 48/67] Make it easier to follow --- src/elvis_config.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 8e76b2c2..82adac13 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -123,20 +123,20 @@ for(Key) -> consult_elvis_config(File) -> case file:consult(File) of {ok, [AppConfig]} -> - elvis_utils:debug("elvis.config usable; using it", []), + elvis_utils:debug("elvis.config consultable; using it", []), AppConfig; _ -> - elvis_utils:debug("elvis.config unusable", []), + elvis_utils:debug("elvis.config unconsultable", []), default_for(app) end. consult_rebar_config(File) -> case file:consult(File) of {ok, AppConfig0} -> - elvis_utils:debug("rebar.config usable; using it", []), + elvis_utils:debug("rebar.config consultable; using it", []), AppConfig0; _ -> - elvis_utils:debug("rebar.config unusable", []), + elvis_utils:debug("rebar.config unconsultable", []), default_for(app) end. From 39a4da277948f3b26973498840c315da5facf309 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 04:12:58 +0100 Subject: [PATCH 49/67] Add a bit more higher-level validation to proceed further --- src/elvis_config.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 82adac13..e11958b2 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -122,7 +122,7 @@ for(Key) -> consult_elvis_config(File) -> case file:consult(File) of - {ok, [AppConfig]} -> + {ok, [AppConfig]} when is_list(AppConfig) -> elvis_utils:debug("elvis.config consultable; using it", []), AppConfig; _ -> @@ -132,7 +132,7 @@ consult_elvis_config(File) -> consult_rebar_config(File) -> case file:consult(File) of - {ok, AppConfig0} -> + {ok, AppConfig0} when is_list(AppConfig0) -> elvis_utils:debug("rebar.config consultable; using it", []), AppConfig0; _ -> @@ -437,7 +437,7 @@ do_validate_throw(FormatData) -> throw({invalid_config, io_lib:format(Format, Data)}). is_nonempty_list(What, List) when not is_list(List) orelse List =:= [] -> - {error, {"'~s' is expected to be a non-empty list.", [What]}}; + {error, {"'~s' is expected to exist and be a non-empty list.", [What]}}; is_nonempty_list(_What, _List) -> ok. From 3ce87b321592ae92b73ed144b927760bb757995f Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 17 Jul 2025 23:00:57 +0100 Subject: [PATCH 50/67] Match on what we expect to --- src/elvis_core.erl | 5 ++--- src/elvis_task.erl | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/elvis_core.erl b/src/elvis_core.erl index b177ffe6..dfebc526 100644 --- a/src/elvis_core.erl +++ b/src/elvis_core.erl @@ -42,9 +42,8 @@ start() -> ok. validate_config(ElvisConfig) -> - try elvis_config:validate_config(ElvisConfig) of - ok -> - ok + try + elvis_config:validate_config(ElvisConfig) catch {invalid_config, _} = Caught -> {error, {fail, [{throw, Caught}]}} diff --git a/src/elvis_task.erl b/src/elvis_task.erl index 102fbdc6..97b36b60 100644 --- a/src/elvis_task.erl +++ b/src/elvis_task.erl @@ -50,9 +50,8 @@ do_in_parallel(FunWork, FunAcc, ExtraArgs, List, MaxW, 0, AccR, AccG) -> do_in_parallel(FunWork, FunAcc, ExtraArgs, List, MaxW, erlang:min(N, MaxW), AccR1, AccG1); do_in_parallel(FunWork, FunAcc, ExtraArgs, List, MaxW, RemainW, AccR, AccG) -> {WorkToBeDone, WorkRemain} = - try lists:split(RemainW, List) of - Res -> - Res + try + lists:split(RemainW, List) catch error:badarg -> {List, []} From 00c9d8e6fe311c8bf9df5778476e60acf221d7cf Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 16:26:54 +0100 Subject: [PATCH 51/67] Make it prettier --- src/elvis_config.erl | 24 ++++++++++++------------ src/elvis_utils.erl | 16 +++++++++++++++- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index e11958b2..c37563a0 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -447,7 +447,7 @@ proplist_keys_are_in(What, List, Keys) -> [] -> ok; _ -> - {error, {"in '~s', the following keys are unknown: ~p.", [What, Filtered]}} + {error, {"in '~s', the following keys are unknown: ~s.", [What, elvis_utils:list_to_str(Filtered)]}} end. is_one_of(What, Value, Possibilities) -> @@ -455,7 +455,7 @@ is_one_of(What, Value, Possibilities) -> true -> ok; _ -> - {error, {"'~s' is expected to be one of the following: ~p.", [What, Possibilities]}} + {error, {"'~s' is expected to be one of the following: ~s.", [What, elvis_utils:list_to_str(Possibilities)]}} end. is_boolean(_What, Value) when is_boolean(Value) -> @@ -534,9 +534,9 @@ no_default_ruleset_override(What, CustomRulesets) -> {error, { "in '~s', the following rulesets are not expected to be " - "named after a default ruleset: ~p.", + "named after a default ruleset: ~s.", [ - What, Filtered + What, elvis_utils:list_to_str(Filtered) ] }} end. @@ -565,7 +565,7 @@ all_configs_are_valid(What, CustomRulesetNames, Configset) -> AccIn; {error, ValidError} -> [ - {"in '~s', at list position number ~p, " ++ ValidError, [ + {"in '~s', at list position number ~w, " ++ ValidError, [ What, PosNumber ]} | AccIn @@ -615,7 +615,7 @@ map_keys_are_in(Map, Keys) -> [] -> ok; _ -> - {error, io_lib:format("the following keys are unknown: ~p.", [Filtered])} + {error, io_lib:format("the following keys are unknown: ~s.", [elvis_utils:list_to_str(Filtered)])} end. is_nonempty_list_of_dirs(What, List) when not is_list(List) orelse List =:= [] -> @@ -631,8 +631,8 @@ is_nonempty_list_of_dirs(What, List) -> _ -> {error, io_lib:format( - "in '~s', the following elements are not (or don't contain) directories: ~p.", - [What, Filtered] + "in '~s', the following elements are not (or don't contain) directories: ~s.", + [What, elvis_utils:list_to_str(Filtered)] )} end. @@ -685,8 +685,8 @@ is_list_of_ignorables(What, List) -> ok; _ -> {error, - io_lib:format("in '~s', the following elements are not ignorable: ~p.", [ - What, Filtered + io_lib:format("in '~s', the following elements are not ignorable: ~s.", [ + What, elvis_utils:list_to_str(Filtered) ])} end. @@ -748,8 +748,8 @@ check_rule_for_options(Rule, AccInI) -> Extra -> [ io_lib:format( - "in rule ~p/~p, the following options are unknown: ~p.", - [NS, Name, Extra] + "in rule ~w/~w, the following options are unknown: ~s.", + [NS, Name, elvis_utils:list_to_str(Extra)] ), AccInI ] diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 080d08a9..f62efe12 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -3,7 +3,7 @@ -compile({no_auto_import, [error/2]}). %% General --export([erlang_halt/1, to_str/1, split_all_lines/1, split_all_lines/2]). +-export([erlang_halt/1, list_to_str/1, to_str/1, split_all_lines/1, split_all_lines/2]). %% Output / rebar3 -export([debug/2, info/2, notice/2, warn/2, error/2, abort/2]). @@ -29,6 +29,20 @@ to_str(Arg) when is_integer(Arg) -> to_str(Arg) when is_list(Arg) -> Arg. +list_to_str(What) -> + list_to_str(What, []). + +list_to_str([], Acc) -> + "[" ++ string:join(Acc, ", ") ++ "]"; +list_to_str([H0 | T], Acc) -> + H = case is_list(H0) of + true -> + "\"" ++ H0 ++ "\""; + _ -> + H0 + end, + list_to_str(T, [to_str(H) | Acc]). + -spec split_all_lines(binary()) -> [binary(), ...]. split_all_lines(Binary) -> split_all_lines(Binary, []). From 890e739588e7b6ca588e09d3ef99a153c67370f6 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 16:36:40 +0100 Subject: [PATCH 52/67] Tweak for a "better" default (?) --- src/elvis_config.erl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index c37563a0..d114382c 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -191,18 +191,18 @@ default() -> [ #{ dirs => [ - "apps/**/src/**", - "src/**" + "apps/**/src", + "src" ], filter => "*.erl", ruleset => erl_files }, #{ dirs => [ - "apps/**/src/**", - "src/**", - "apps/**/include/**", - "include/**" + "apps/**/src", + "src", + "apps/**/include", + "include" ], filter => "*.hrl", ruleset => hrl_files From 50f18398bfce20048bd8c3f99a0867bfc9fdbf10 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 16:50:09 +0100 Subject: [PATCH 53/67] Previous errors weren't actually `error`, but `notice` --- src/elvis_result.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/elvis_result.erl b/src/elvis_result.erl index 711ede46..55e95dae 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -189,7 +189,7 @@ print(Format, #{file := Path, rules := Rules}) -> ok -> elvis_utils:notice("# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); fail -> - elvis_utils:error("# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) + elvis_utils:notice("# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) end end, print_rules(Format, Path, Rules); @@ -216,7 +216,7 @@ print_rules( parsable -> ok; _ -> - elvis_utils:error( + elvis_utils:notice( " - ~p " "(https://github.com/inaka/elvis_core/tree/main/doc_rules/~p/~p.md)", [Name, Scope, Name] @@ -248,7 +248,7 @@ print_item( FMsg = io_lib:format(Msg, Info), io:format("~s:~p:~p:~s~n", [File, Ln, Name, FMsg]); _ -> - elvis_utils:error(" - " ++ Msg, Info) + elvis_utils:notice(" - " ++ Msg, Info) end, print_item(Format, File, Name, Items); print_item(Format, File, Name, [Error | Items]) -> From d5c5ba13070c3946dc25e6cc78ded97266b442d7 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 17:48:31 +0100 Subject: [PATCH 54/67] Respect expected verbosity --- src/elvis_config.erl | 15 ++++++++++++--- src/elvis_result.erl | 5 ++++- src/elvis_utils.erl | 20 ++++++++------------ 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index d114382c..32fe8724 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -447,7 +447,10 @@ proplist_keys_are_in(What, List, Keys) -> [] -> ok; _ -> - {error, {"in '~s', the following keys are unknown: ~s.", [What, elvis_utils:list_to_str(Filtered)]}} + {error, + {"in '~s', the following keys are unknown: ~s.", [ + What, elvis_utils:list_to_str(Filtered) + ]}} end. is_one_of(What, Value, Possibilities) -> @@ -455,7 +458,10 @@ is_one_of(What, Value, Possibilities) -> true -> ok; _ -> - {error, {"'~s' is expected to be one of the following: ~s.", [What, elvis_utils:list_to_str(Possibilities)]}} + {error, + {"'~s' is expected to be one of the following: ~s.", [ + What, elvis_utils:list_to_str(Possibilities) + ]}} end. is_boolean(_What, Value) when is_boolean(Value) -> @@ -615,7 +621,10 @@ map_keys_are_in(Map, Keys) -> [] -> ok; _ -> - {error, io_lib:format("the following keys are unknown: ~s.", [elvis_utils:list_to_str(Filtered)])} + {error, + io_lib:format("the following keys are unknown: ~s.", [ + elvis_utils:list_to_str(Filtered) + ])} end. is_nonempty_list_of_dirs(What, List) when not is_list(List) orelse List =:= [] -> diff --git a/src/elvis_result.erl b/src/elvis_result.erl index 55e95dae..4b64bf11 100644 --- a/src/elvis_result.erl +++ b/src/elvis_result.erl @@ -185,9 +185,12 @@ print(Format, #{file := Path, rules := Rules}) -> parsable -> ok; _ -> + Verbose = elvis_config:verbose(), case status(Rules) of - ok -> + ok when Verbose -> elvis_utils:notice("# ~s [{{green-bold}}OK{{white-bold}}]", [Path]); + ok -> + ok; fail -> elvis_utils:notice("# ~s [{{red-bold}}FAIL{{white-bold}}]", [Path]) end diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index f62efe12..8354f919 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -35,12 +35,13 @@ list_to_str(What) -> list_to_str([], Acc) -> "[" ++ string:join(Acc, ", ") ++ "]"; list_to_str([H0 | T], Acc) -> - H = case is_list(H0) of - true -> - "\"" ++ H0 ++ "\""; - _ -> - H0 - end, + H = + case is_list(H0) of + true -> + "\"" ++ H0 ++ "\""; + _ -> + H0 + end, list_to_str(T, [to_str(H) | Acc]). -spec split_all_lines(binary()) -> [binary(), ...]. @@ -130,12 +131,7 @@ do_output(info, Chars) -> ok end; do_output(notice, Chars) -> - case elvis_config:verbose() of - true -> - io:format(Chars); - false -> - ok - end; + io:format(Chars); do_output(warn, Chars) -> io:format(Chars); do_output(error, Chars) -> From 7fd248290ec76debaac0daa21c4e02cc60f873e5 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 17:48:42 +0100 Subject: [PATCH 55/67] Fix broken rule result --- src/elvis_project.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/elvis_project.erl b/src/elvis_project.erl index eac8554a..66d9acaa 100644 --- a/src/elvis_project.erl +++ b/src/elvis_project.erl @@ -73,12 +73,12 @@ no_branch_deps(Rule, _ElvisConfig) -> true -> false; false -> - {true, [ + {true, elvis_result:new_item( "Dependency '~s' uses a branch; prefer a tag or a specific commit", [AppName] ) - ]} + } end end, BadDeps From d1b2e68878dc1b9ba198d91e72ccc613277e94c2 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 17:59:40 +0100 Subject: [PATCH 56/67] Adapt to actual usage --- src/elvis_utils.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index 8354f919..ebaf7105 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -139,10 +139,11 @@ do_output(error, Chars) -> -dialyzer({nowarn_function, abort/2}). -spec abort(Format :: io:format(), Data :: [term()]) -> no_return(). -abort(Format, Data) -> +abort(Format0, Data) -> + Format = "Elvis: " ++ Format0 ++ "~n", case erlang:function_exported(rebar_api, abort, 2) of true -> - rebar_api:abort("Elvis: " ++ Format, [Data]); + rebar_api:abort(Format, [Data]); false -> output(error, Format, Data), throw(elvis_abort) From 23a91f32bcc895b9b46fe7d409cd8e762cd9dab0 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 18:00:54 +0100 Subject: [PATCH 57/67] rebar3 fmt it! --- src/elvis_project.erl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/elvis_project.erl b/src/elvis_project.erl index 66d9acaa..a050b70a 100644 --- a/src/elvis_project.erl +++ b/src/elvis_project.erl @@ -77,8 +77,7 @@ no_branch_deps(Rule, _ElvisConfig) -> elvis_result:new_item( "Dependency '~s' uses a branch; prefer a tag or a specific commit", [AppName] - ) - } + )} end end, BadDeps From 8d4b07dd49b51620c617a85b09fc4acd2bd91f3f Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Mon, 21 Jul 2025 18:07:50 +0100 Subject: [PATCH 58/67] Remove extraneous ~n --- src/elvis_code.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_code.erl b/src/elvis_code.erl index 24400769..a8bc8fc2 100644 --- a/src/elvis_code.erl +++ b/src/elvis_code.erl @@ -189,6 +189,6 @@ print_node(#{type := Type} = Node, CurrentLevel) -> Indentation = lists:duplicate(CurrentLevel * 4, $\s), Content = ktn_code:content(Node), - ok = elvis_utils:info("~s - [~p] ~p~n", [Indentation, CurrentLevel, Type]), + ok = elvis_utils:info("~s - [~p] ~p", [Indentation, CurrentLevel, Type]), _ = lists:map(fun(Child) -> print_node(Child, CurrentLevel + 1) end, Content), ok. From 37da2b782c90d562ca432b4524bf2689912621d0 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Tue, 22 Jul 2025 18:26:05 +0100 Subject: [PATCH 59/67] Initialise the elvis_config_SUITE --- test/elvis_SUITE.erl | 66 +++++++++---------------------------- test/elvis_config_SUITE.erl | 54 ++++++++++++++++++++++++++++++ test/elvis_test_utils.erl | 25 +++++++++++++- 3 files changed, 94 insertions(+), 51 deletions(-) create mode 100644 test/elvis_config_SUITE.erl diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 204df707..3a192ebd 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -8,8 +8,6 @@ rock_with_empty_list_config/1, rock_with_incomplete_config/1, rock_with_list_config/1, - rock_with_file_config/1, - rock_with_rebar_default_config/1, rock_this/1, rock_without_colors/1, rock_with_parsable/1, @@ -24,7 +22,6 @@ rock_with_umbrella_apps/1, custom_ruleset/1, hrl_ruleset/1, - throw_configuration/1, find_file_and_check_src/1, find_file_with_ignore/1, invalid_file/1, @@ -84,27 +81,6 @@ rock_with_list_config(_Config) -> ], ok = elvis_core:rock(ElvisConfig). -rock_with_file_config(_Config) -> - ConfigPath = "../../config/elvis.config", - ElvisConfig = elvis_config:from_file(ConfigPath), - Fun = fun() -> elvis_core:rock(ElvisConfig) end, - Expected = - "# \\.\\./\\.\\./_build/test/lib/elvis_core/test/" ++ "examples/.*\\.erl.*FAIL", - [_ | _] = check_some_line_output(Fun, Expected, fun matches_regex/2), - ok. - -rock_with_rebar_default_config(_Config) -> - {ok, _} = file:copy("../../config/rebar.config", "rebar.config"), - ElvisConfig = elvis_config:from_rebar("rebar.config"), - [#{name := line_length}] = - try - {fail, Results} = elvis_core:rock(ElvisConfig), - [Rule || #{rules := [Rule]} <- Results] - after - file:delete("rebar.config") - end, - ok. - rock_this(_Config) -> ElvisConfig = elvis_test_utils:config(), ok = elvis_core:rock_this(elvis_core, ElvisConfig), @@ -127,7 +103,11 @@ rock_without_colors(_Config) -> Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = "\\e.*?m", ok = - try check_some_line_output(Fun, Expected, fun matches_regex/2) of + try + elvis_test_utils:check_some_line_output( + Fun, Expected, fun elvis_test_utils:matches_regex/2 + ) + of Result -> ct:fail("Unexpected result ~p", [Result]) catch @@ -142,7 +122,11 @@ rock_with_parsable(_Config) -> Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = ".*\\.erl:\\d:[a-zA-Z0-9_]+:.*", ok = - try check_some_line_output(Fun, Expected, fun matches_regex/2) of + try + elvis_test_utils:check_some_line_output( + Fun, Expected, fun elvis_test_utils:matches_regex/2 + ) + of Result -> io:format("~p~n", [Result]) catch @@ -175,7 +159,9 @@ rock_with_errors_has_output(_Config) -> ElvisConfig = elvis_test_utils:config(), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = "FAIL", - [_ | _] = check_some_line_output(Fun, Expected, fun matches_regex/2), + [_ | _] = elvis_test_utils:check_some_line_output( + Fun, Expected, fun elvis_test_utils:matches_regex/2 + ), ok. rock_without_errors_has_no_output(_Config) -> @@ -201,7 +187,9 @@ rock_without_errors_and_with_verbose_has_output(_Config) -> ElvisConfig = elvis_test_utils:config(), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = "OK", - [_ | _] = check_some_line_output(Fun, Expected, fun matches_regex/2), + [_ | _] = elvis_test_utils:check_some_line_output( + Fun, Expected, fun elvis_test_utils:matches_regex/2 + ), application:unset_env(elvis_core, verbose), ok. @@ -352,12 +340,6 @@ hrl_ruleset(_Config) -> elvis_core:rock(ElvisConfig), ok. -throw_configuration(_Config) -> - Filename = "./elvis.config", - ok = file:write_file(Filename, <<"-">>), - {fail, [{throw, {invalid_config, _}}]} = elvis_config:from_file(Filename), - _ = file:delete(Filename). - find_file_and_check_src(_Config) -> Dirs = ["../../test/examples"], @@ -432,24 +414,8 @@ chunk_fold_task(Elem, Multiplier) -> %%% Private %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -check_some_line_output(Fun, Expected, FilterFun) -> - _ = ct:capture_start(), - _ = Fun(), - _ = ct:capture_stop(), - Lines = ct:capture_get([]), - ListFun = fun(Line) -> FilterFun(Line, Expected) end, - [_ | _] = lists:filter(ListFun, Lines). - get_output(Fun) -> _ = ct:capture_start(), _ = Fun(), _ = ct:capture_stop(), ct:capture_get([]). - -matches_regex(Result, Regex) -> - case re:run(Result, Regex) of - {match, _} -> - true; - nomatch -> - false - end. diff --git a/test/elvis_config_SUITE.erl b/test/elvis_config_SUITE.erl new file mode 100644 index 00000000..268b7ef2 --- /dev/null +++ b/test/elvis_config_SUITE.erl @@ -0,0 +1,54 @@ +-module(elvis_config_SUITE). + +-behaviour(ct_suite). + +% ct_suite +-export([all/0, init_per_suite/1, end_per_suite/1]). + +% Tests +-export([rock_with_file_config/1]). +-export([rock_with_rebar_default_config/1]). +-export([throw_configuration/1]). + +% ct_suite +all() -> + Exports = ?MODULE:module_info(exports), + [F || {F, _} <- Exports, not lists:member(F, elvis_test_utils:excluded_funs_all())]. + +init_per_suite(Config) -> + {ok, _} = application:ensure_all_started(elvis_core), + Config. + +end_per_suite(Config) -> + ok = application:stop(elvis_core), + Config. + +% Tests +rock_with_file_config(_Config) -> + ConfigPath = "../../config/elvis.config", + ElvisConfig = elvis_config:from_file(ConfigPath), + Fun = fun() -> elvis_core:rock(ElvisConfig) end, + Expected = + "# \\.\\./\\.\\./_build/test/lib/elvis_core/test/" ++ "examples/.*\\.erl.*FAIL", + [_ | _] = elvis_test_utils:check_some_line_output( + Fun, Expected, fun elvis_test_utils:matches_regex/2 + ), + ok. + +rock_with_rebar_default_config(_Config) -> + {ok, _} = file:copy("../../config/rebar.config", "rebar.config"), + ElvisConfig = elvis_config:from_rebar("rebar.config"), + [#{name := line_length}] = + try + {fail, Results} = elvis_core:rock(ElvisConfig), + [Rule || #{rules := [Rule]} <- Results] + after + file:delete("rebar.config") + end, + ok. + +throw_configuration(_Config) -> + Filename = "./elvis.config", + ok = file:write_file(Filename, <<"-">>), + {fail, [{throw, {invalid_config, _}}]} = elvis_config:from_file(Filename), + _ = file:delete(Filename). diff --git a/test/elvis_test_utils.erl b/test/elvis_test_utils.erl index 794a62fb..a5211674 100644 --- a/test/elvis_test_utils.erl +++ b/test/elvis_test_utils.erl @@ -1,6 +1,13 @@ -module(elvis_test_utils). --export([config/0, config/1, find_file/2, elvis_core_apply_rule/5, excluded_funs_all/0]). +-export([ + config/0, config/1, + find_file/2, + elvis_core_apply_rule/5, + excluded_funs_all/0, + check_some_line_output/3, + matches_regex/2 +]). excluded_funs_all() -> [ @@ -44,3 +51,19 @@ elvis_core_apply_rule(Config, Module, Function, RuleConfig, Filename) -> #{items := Items} -> Items end. + +check_some_line_output(Fun, Expected, FilterFun) -> + _ = ct:capture_start(), + _ = Fun(), + _ = ct:capture_stop(), + Lines = ct:capture_get([]), + ListFun = fun(Line) -> FilterFun(Line, Expected) end, + [_ | _] = lists:filter(ListFun, Lines). + +matches_regex(Result, Regex) -> + case re:run(Result, Regex) of + {match, _} -> + true; + nomatch -> + false + end. From e68cd26b1b933bcf89fbdfe9525961007143cbcb Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Tue, 22 Jul 2025 19:06:35 +0100 Subject: [PATCH 60/67] Don't ignore stuff we don't generate any more --- .gitignore | 1 - config/elvis-test-custom-ruleset.config | 2 +- config/elvis-test-hrl-files.config | 2 +- config/elvis-test-pa.config | 2 +- config/elvis-test.config | 4 +- config/elvis-umbrella.config | 8 +-- config/elvis.config | 10 ++-- config/rebar.config | 2 +- config/test.config | 12 ++--- config/test.pass.config | 2 +- rebar.config | 2 +- test/elvis_SUITE.erl | 66 ++++++++++++++----------- test/elvis_config_SUITE.erl | 7 +-- 13 files changed, 63 insertions(+), 57 deletions(-) diff --git a/.gitignore b/.gitignore index c3704821..5f4b8e9e 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,6 @@ _build/ doc/ /rebar3.crashdump .rebar3/ -logs test/**/*.beam test/logs/ _checkouts/ diff --git a/config/elvis-test-custom-ruleset.config b/config/elvis-test-custom-ruleset.config index b14c9ba6..b5e084b0 100644 --- a/config/elvis-test-custom-ruleset.config +++ b/config/elvis-test-custom-ruleset.config @@ -3,7 +3,7 @@ {rulesets, #{project => [{elvis_text_style, no_tabs}]}}, {config, [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/dirs/src"], + dirs => ["../../../../_build/test/lib/elvis_core/test/dirs/src"], filter => "*.erl", ruleset => project } diff --git a/config/elvis-test-hrl-files.config b/config/elvis-test-hrl-files.config index bdf89b41..ffacb296 100644 --- a/config/elvis-test-hrl-files.config +++ b/config/elvis-test-hrl-files.config @@ -2,7 +2,7 @@ {elvis, [ {config, [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples/"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples/"], filter => "test_*.hrl", ruleset => hrl_files } diff --git a/config/elvis-test-pa.config b/config/elvis-test-pa.config index c9e0063d..f2539a2a 100644 --- a/config/elvis-test-pa.config +++ b/config/elvis-test-pa.config @@ -2,7 +2,7 @@ {elvis, [ {config, [ #{ - dirs => ["../../test/examples"], + dirs => ["../../../../test/examples"], filter => "user_defined_rules.erl", rules => [{user_defined_rules, rule}] } diff --git a/config/elvis-test.config b/config/elvis-test.config index 06623e21..339310f4 100644 --- a/config/elvis-test.config +++ b/config/elvis-test.config @@ -2,7 +2,7 @@ {elvis, [ {config, [ #{ - dirs => ["../../src"], + dirs => ["../../../../src"], filter => "*.erl", rules => [ @@ -27,7 +27,7 @@ ruleset => erl_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/ebin"], + dirs => ["../../../../_build/test/lib/elvis_core/ebin"], filter => "*.beam", rules => [ diff --git a/config/elvis-umbrella.config b/config/elvis-umbrella.config index 59050ea8..2f411f9a 100644 --- a/config/elvis-umbrella.config +++ b/config/elvis-umbrella.config @@ -4,10 +4,10 @@ #{ dirs => [ - "../../_build/test/lib/elvis_core/test/dirs/src", - "../../_build/test/lib/elvis_core/test/dirs/test", - "../../_build/test/lib/elvis_core/test/dirs/apps/**/src", - "../../_build/test/lib/elvis_core/test/dirs/apps/**/test" + "../../../../_build/test/lib/elvis_core/test/dirs/src", + "../../../../_build/test/lib/elvis_core/test/dirs/test", + "../../../../_build/test/lib/elvis_core/test/dirs/apps/**/src", + "../../../../_build/test/lib/elvis_core/test/dirs/apps/**/test" ], filter => "*.erl", ruleset => erl_files diff --git a/config/elvis.config b/config/elvis.config index 693b2c52..1d0d5da4 100644 --- a/config/elvis.config +++ b/config/elvis.config @@ -2,27 +2,27 @@ {elvis, [ {config, [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.erl", ruleset => erl_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.hrl", ruleset => hrl_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/test/non_compilable_examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/non_compilable_examples"], filter => "*.erl", ruleset => erl_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.beam", ruleset => beam_files }, #{ - dirs => ["../.."], + dirs => ["../../../.."], filter => "rebar.config", ruleset => rebar_config } diff --git a/config/rebar.config b/config/rebar.config index d8755dfe..e503ccd7 100644 --- a/config/rebar.config +++ b/config/rebar.config @@ -8,7 +8,7 @@ {elvis, [ {config, [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.erl", rules => [{elvis_text_style, line_length, #{limit => 135}}] } diff --git a/config/test.config b/config/test.config index 155962b8..3fc32768 100644 --- a/config/test.config +++ b/config/test.config @@ -2,7 +2,7 @@ {elvis_core, [ {config, [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.erl", rules => [ @@ -15,13 +15,13 @@ ruleset => erl_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.hrl", rules => [], ruleset => hrl_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.beam", rules => [ @@ -33,15 +33,15 @@ ruleset => beam_files }, #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "rebar3.config.success", ruleset => rebar_config }, #{ dirs => [ - "../../_build/test/lib/elvis_core/test/dirs/apps/app1", - "../../_build/test/lib/elvis_core/test/dirs/apps/app2" + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app1", + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app2" ], filter => ".gitignore", ruleset => gitignore diff --git a/config/test.pass.config b/config/test.pass.config index 41449693..1a428119 100644 --- a/config/test.pass.config +++ b/config/test.pass.config @@ -3,7 +3,7 @@ {elvis, [ {config, [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "*.erl", rules => [{elvis_text_style, line_length, #{limit => 800}}] } diff --git a/rebar.config b/rebar.config index 08cb04ee..c2bfdafd 100644 --- a/rebar.config +++ b/rebar.config @@ -16,7 +16,7 @@ {deps, [{meck, "0.9.2"}]}, {erl_opts, [nowarn_missing_spec, nowarn_export_all]}, {dialyzer, [{warnings, [no_return, error_handling]}, {plt_extra_apps, [common_test]}]}, - {ct_opts, [{sys_config, ["./config/test.config"]}, {logdir, "./logs"}, {verbose, true}]}, + {ct_opts, [{sys_config, ["./config/test.config"]}, {verbose, true}]}, {cover_enabled, true}, {cover_opts, [verbose]} ]} diff --git a/test/elvis_SUITE.erl b/test/elvis_SUITE.erl index 3a192ebd..377c13a1 100644 --- a/test/elvis_SUITE.erl +++ b/test/elvis_SUITE.erl @@ -74,7 +74,7 @@ rock_with_incomplete_config(_Config) -> rock_with_list_config(_Config) -> ElvisConfig = [ #{ - dirs => ["../../test/dirs/src"], + dirs => ["../../../../test/dirs/src"], rules => [{elvis_text_style, line_length, disable}], filter => "*.erl" } @@ -93,7 +93,7 @@ rock_this(_Config) -> ok end, - Path = "../../_build/test/lib/elvis_core/test/examples/fail_line_length.erl", + Path = "../../../../_build/test/lib/elvis_core/test/examples/fail_line_length.erl", {fail, _} = elvis_core:rock_this(Path, ElvisConfig), ok. @@ -139,7 +139,7 @@ rock_with_parsable(_Config) -> rock_with_non_parsable_file(_Config) -> ElvisConfig = elvis_test_utils:config(), Path = - "../../_build/test/lib/elvis_core/test/non_compilable_examples/fail_non_parsable_file.erl", + "../../../../_build/test/lib/elvis_core/test/non_compilable_examples/fail_non_parsable_file.erl", try elvis_core:rock_this(Path, ElvisConfig) catch @@ -165,7 +165,7 @@ rock_with_errors_has_output(_Config) -> ok. rock_without_errors_has_no_output(_Config) -> - ConfigPath = "../../config/test.pass.config", + ConfigPath = "../../../../config/test.pass.config", ElvisConfig = elvis_config:from_file(ConfigPath), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Output = get_output(Fun), @@ -199,17 +199,17 @@ rock_with_rule_groups(_Config) -> RulesGroupConfig = [ #{ - dirs => ["../../test/dirs/apps/app3/src"], + dirs => ["../../../../test/dirs/apps/app3/src"], filter => "*.erl", ruleset => erl_files }, #{ - dirs => ["../../test/dirs/apps/app3/include"], + dirs => ["../../../../test/dirs/apps/app3/include"], filter => "*.hrl", ruleset => hrl_files }, #{ - dirs => ["../../test/dirs/apps/app3"], + dirs => ["../../../../test/dirs/apps/app3"], filter => "rebar.config", ruleset => rebar_config } @@ -232,7 +232,7 @@ rock_with_rule_groups(_Config) -> OverrideConfig = [ #{ - dirs => ["../../test/dirs/apps/app3/src"], + dirs => ["../../../../test/dirs/apps/app3/src"], filter => "*.erl", ruleset => erl_files, rules => @@ -244,7 +244,7 @@ rock_with_rule_groups(_Config) -> ] }, #{ - dirs => ["../../test/dirs/apps/app3"], + dirs => ["../../../../test/dirs/apps/app3"], filter => "rebar.config", ruleset => rebar_config } @@ -253,11 +253,11 @@ rock_with_rule_groups(_Config) -> rock_this_skipping_files(_Config) -> meck:new(elvis_file, [passthrough]), - Dirs = ["../../_build/test/lib/elvis_core/test/examples"], + Dirs = ["../../../../_build/test/lib/elvis_core/test/examples"], [File] = elvis_file:find_files(Dirs, "small.erl"), Path = elvis_file:path(File), - ConfigPath = "../../config/elvis-test-pa.config", - {ok, user_defined_rules} = compile:file("../../test/examples/user_defined_rules.erl"), + ConfigPath = "../../../../config/elvis-test-pa.config", + {ok, user_defined_rules} = compile:file("../../../../test/examples/user_defined_rules.erl"), {module, user_defined_rules} = code:ensure_loaded(user_defined_rules), ElvisConfig = elvis_config:from_file(ConfigPath), ok = elvis_core:rock_this(Path, ElvisConfig), @@ -267,7 +267,7 @@ rock_this_skipping_files(_Config) -> rock_this_not_skipping_files(_Config) -> meck:new(elvis_file, [passthrough]), - Dirs = ["../../_build/test/lib/elvis_core/test/examples"], + Dirs = ["../../../../_build/test/lib/elvis_core/test/examples"], [File] = elvis_file:find_files(Dirs, "small.erl"), Path = elvis_file:path(File), ElvisConfig = elvis_test_utils:config(), @@ -277,34 +277,37 @@ rock_this_not_skipping_files(_Config) -> ok. rock_with_umbrella_apps(_Config) -> - ElvisUmbrellaConfigFile = "../../config/elvis-umbrella.config", + ElvisUmbrellaConfigFile = "../../../../config/elvis-umbrella.config", ElvisConfig = elvis_config:from_file(ElvisUmbrellaConfigFile), {fail, [ - #{file := "../../_build/test/lib/elvis_core/test/dirs/test/dir_test.erl"}, - #{file := "../../_build/test/lib/elvis_core/test/dirs/src/dirs_src.erl"}, - #{file := "../../_build/test/lib/elvis_core/test/dirs/apps/app3/src/app3_example.erl"}, + #{file := "../../../../_build/test/lib/elvis_core/test/dirs/test/dir_test.erl"}, + #{file := "../../../../_build/test/lib/elvis_core/test/dirs/src/dirs_src.erl"}, #{ file := - "../../_build/test/lib/elvis_core/test/dirs/apps/app2/test/dirs_apps_app2_test.erl" + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app3/src/app3_example.erl" }, #{ file := - "../../_build/test/lib/elvis_core/test/dirs/apps/app2/src/dirs_apps_app2_src.erl" + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app2/test/dirs_apps_app2_test.erl" }, #{ file := - "../../_build/test/lib/elvis_core/test/dirs/apps/app1/test/dirs_apps_app1_test.erl" + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app2/src/dirs_apps_app2_src.erl" }, #{ file := - "../../_build/test/lib/elvis_core/test/dirs/apps/app1/src/dirs_apps_app1_src.erl" + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app1/test/dirs_apps_app1_test.erl" + }, + #{ + file := + "../../../../_build/test/lib/elvis_core/test/dirs/apps/app1/src/dirs_apps_app1_src.erl" } ]} = elvis_core:rock(ElvisConfig), ok. rock_with_invalid_rules(_Config) -> - ConfigPath = "../../test/examples/invalid_rules.elvis.config", + ConfigPath = "../../../../test/examples/invalid_rules.elvis.config", {fail, [{throw, {invalid_config, _}}]} = elvis_config:from_file(ConfigPath), ok. @@ -312,7 +315,7 @@ rock_with_invalid_rules(_Config) -> %%% Utils custom_ruleset(_Config) -> - ConfigPath = "../../config/elvis-test-custom-ruleset.config", + ConfigPath = "../../../../config/elvis-test-custom-ruleset.config", ElvisConfig = elvis_config:from_file(ConfigPath), NoTabs = elvis_rule:new(elvis_text_style, no_tabs), [[NoTabs]] = elvis_config:rules(ElvisConfig), @@ -322,18 +325,21 @@ custom_ruleset(_Config) -> %% read unknown ruleset configuration to ensure rulesets from %% previous load do not stick around - ConfigPathMissing = "../../config/elvis-test-unknown-ruleset.config", + ConfigPathMissing = "../../../../config/elvis-test-unknown-ruleset.config", ElvisConfigMissing = elvis_config:from_file(ConfigPathMissing), [[]] = elvis_config:rules(ElvisConfigMissing), ok. hrl_ruleset(_Config) -> - ConfigPath = "../../config/elvis-test-hrl-files.config", + ConfigPath = "../../../../config/elvis-test-hrl-files.config", ElvisConfig = elvis_config:from_file(ConfigPath), {fail, [ - #{file := "../../_build/test/lib/elvis_core/test/examples/test_good.hrl", rules := []}, #{ - file := "../../_build/test/lib/elvis_core/test/examples/test_bad.hrl", + file := "../../../../_build/test/lib/elvis_core/test/examples/test_good.hrl", + rules := [] + }, + #{ + file := "../../../../_build/test/lib/elvis_core/test/examples/test_bad.hrl", rules := [#{name := line_length}] } ]} = @@ -341,7 +347,7 @@ hrl_ruleset(_Config) -> ok. find_file_and_check_src(_Config) -> - Dirs = ["../../test/examples"], + Dirs = ["../../../../test/examples"], [] = elvis_file:find_files(Dirs, "doesnt_exist.erl"), [File] = elvis_file:find_files(Dirs, "small.erl"), @@ -357,12 +363,12 @@ find_file_and_check_src(_Config) -> {error, enoent} = elvis_file:src(#{path => "doesnt_exist.erl"}). find_file_with_ignore(_Config) -> - Dirs = ["../../test/examples"], + Dirs = ["../../../../test/examples"], Filter = "find_test*.erl", Ignore = elvis_config:ignore(#{ignore => [find_test1]}), Files = [_, _] = elvis_file:find_files(Dirs, Filter), [_, _] = elvis_file:filter_files(Files, Dirs, Filter, []), - [#{path := "../../test/examples/find_test2.erl"}] = + [#{path := "../../../../test/examples/find_test2.erl"}] = elvis_file:filter_files(Files, Dirs, Filter, Ignore). invalid_file(_Config) -> diff --git a/test/elvis_config_SUITE.erl b/test/elvis_config_SUITE.erl index 268b7ef2..df5ba507 100644 --- a/test/elvis_config_SUITE.erl +++ b/test/elvis_config_SUITE.erl @@ -25,18 +25,19 @@ end_per_suite(Config) -> % Tests rock_with_file_config(_Config) -> - ConfigPath = "../../config/elvis.config", + ConfigPath = "../../../../config/elvis.config", ElvisConfig = elvis_config:from_file(ConfigPath), Fun = fun() -> elvis_core:rock(ElvisConfig) end, Expected = - "# \\.\\./\\.\\./_build/test/lib/elvis_core/test/" ++ "examples/.*\\.erl.*FAIL", + "# \\.\\./\\.\\./\\.\\./\\.\\./_build/test/lib/elvis_core/test/" ++ + "examples/.*\\.erl.*FAIL", [_ | _] = elvis_test_utils:check_some_line_output( Fun, Expected, fun elvis_test_utils:matches_regex/2 ), ok. rock_with_rebar_default_config(_Config) -> - {ok, _} = file:copy("../../config/rebar.config", "rebar.config"), + {ok, _} = file:copy("../../../../config/rebar.config", "rebar.config"), ElvisConfig = elvis_config:from_rebar("rebar.config"), [#{name := line_length}] = try From c41293d1db33df6a020d0dbaac074790f1ca5609 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 24 Jul 2025 18:35:42 +0100 Subject: [PATCH 61/67] Further minor tweaks on output --- src/elvis_utils.erl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/elvis_utils.erl b/src/elvis_utils.erl index ebaf7105..350ba137 100644 --- a/src/elvis_utils.erl +++ b/src/elvis_utils.erl @@ -36,13 +36,15 @@ list_to_str([], Acc) -> "[" ++ string:join(Acc, ", ") ++ "]"; list_to_str([H0 | T], Acc) -> H = - case is_list(H0) of - true -> + case H0 of + H0 when is_list(H0) -> "\"" ++ H0 ++ "\""; + H0 when is_binary(H0) -> + "<<\"" ++ to_str(H0) ++ "\">>"; _ -> - H0 + to_str(H0) end, - list_to_str(T, [to_str(H) | Acc]). + list_to_str(T, [H | Acc]). -spec split_all_lines(binary()) -> [binary(), ...]. split_all_lines(Binary) -> From 9710c29b20c435f70362015d7d33521316f8741e Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 24 Jul 2025 18:39:04 +0100 Subject: [PATCH 62/67] Fix for test results --- src/elvis_config.erl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 32fe8724..95f495d3 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -155,8 +155,8 @@ fetch_elvis_config_from(AppConfig) -> Elvis -> from_static(config, {elvis, Elvis}) catch - {invalid_config, _} = Caught -> - {fail, [{throw, Caught}]} + {invalid_config, Message} -> + {fail, [{throw, {invalid_config, lists:flatten(Message)}}]} end. default_for(app) -> @@ -548,9 +548,9 @@ no_default_ruleset_override(What, CustomRulesets) -> end. is_valid_config(What, CustomRulesetNames, Configset0) -> - Configset = wrap_in_list(Configset0), maybe - ok ?= is_nonempty_list(What, Configset), + ok ?= is_nonempty_list(What, Configset0), + Configset = wrap_in_list(Configset0), ok ?= all_configs_are_valid(What, CustomRulesetNames, Configset) else {error, FormatData} -> @@ -615,6 +615,8 @@ config_is_valid(CustomRulesetNames, Config) -> {error, ValidError} end. +map_keys_are_in(Map, _Keys) when not is_map(Map) -> + {error, "element is expected to be a map."}; map_keys_are_in(Map, Keys) -> Filtered = [Key || Key <- maps:keys(Map), not lists:member(Key, Keys)], case Filtered of From 63d135588d03a625b32a9586bc3f150c43b93cc6 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 24 Jul 2025 18:51:51 +0100 Subject: [PATCH 63/67] Fix for test results --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 95f495d3..285fcf8f 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -741,7 +741,7 @@ either_rules_is_nonempty_or_ruleset_is_defined([_ | _] = _Rules, _Ruleset) -> either_rules_is_nonempty_or_ruleset_is_defined(_Rules, Ruleset) when Ruleset =/= undefined -> ok; either_rules_is_nonempty_or_ruleset_is_defined(_Rules, _Ruleset) -> - io_lib:format("either rules or ruleset is expected to be defined.", []). + {error, io_lib:format("either rules or ruleset is expected to be defined.", [])}. check_rule_for_options(Rule, AccInI) -> case elvis_rule:defkeys(Rule) of From 558855100e7af8d89b107f01dbb5275ebc3b3f50 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Tue, 29 Jul 2025 09:12:59 +0100 Subject: [PATCH 64/67] Act on review comment --- test/elvis_test_utils.erl | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/elvis_test_utils.erl b/test/elvis_test_utils.erl index a5211674..d66c6355 100644 --- a/test/elvis_test_utils.erl +++ b/test/elvis_test_utils.erl @@ -61,9 +61,4 @@ check_some_line_output(Fun, Expected, FilterFun) -> [_ | _] = lists:filter(ListFun, Lines). matches_regex(Result, Regex) -> - case re:run(Result, Regex) of - {match, _} -> - true; - nomatch -> - false - end. + nomatch =/= re:run(Result, Regex). From 6e862cc065b3053860e2eed27c12d49897f3a350 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 30 Jul 2025 15:45:42 +0100 Subject: [PATCH 65/67] Fix it --- test/style_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl index 62cbe9c0..bf14548e 100644 --- a/test/style_SUITE.erl +++ b/test/style_SUITE.erl @@ -2864,7 +2864,7 @@ oddities(_Config) -> ElvisConfig = [ #{ - dirs => ["../../_build/test/lib/elvis_core/test/examples"], + dirs => ["../../../../_build/test/lib/elvis_core/test/examples"], filter => "odditiesß.erl", ruleset => erl_files, rules => [{elvis_style, no_god_modules, #{limit => 0}}] From 2bf6711fe97141e17ed079ef2fc46b0ad0ddebf3 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 30 Jul 2025 16:57:20 +0100 Subject: [PATCH 66/67] Minor tweak for test results I can't simply validate rules as being non-empty because it is, by default, so what I state is "you either provide a non-empty list or a valid ruleset" --- src/elvis_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 285fcf8f..0e29d1aa 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -741,7 +741,7 @@ either_rules_is_nonempty_or_ruleset_is_defined([_ | _] = _Rules, _Ruleset) -> either_rules_is_nonempty_or_ruleset_is_defined(_Rules, Ruleset) when Ruleset =/= undefined -> ok; either_rules_is_nonempty_or_ruleset_is_defined(_Rules, _Ruleset) -> - {error, io_lib:format("either rules or ruleset is expected to be defined.", [])}. + {error, io_lib:format("either 'rules' is a non-empty list or 'ruleset' is defined.", [])}. check_rule_for_options(Rule, AccInI) -> case elvis_rule:defkeys(Rule) of From 1fa87910883c75b25aebb2533408648d7e612555 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Wed, 30 Jul 2025 16:57:42 +0100 Subject: [PATCH 67/67] Throw earlier when elvis.config exists but is invalid Erlang --- src/elvis_config.erl | 9 +++++++++ test/elvis_config_SUITE.erl | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/elvis_config.erl b/src/elvis_config.erl index 0e29d1aa..c909846b 100644 --- a/src/elvis_config.erl +++ b/src/elvis_config.erl @@ -125,6 +125,15 @@ consult_elvis_config(File) -> {ok, [AppConfig]} when is_list(AppConfig) -> elvis_utils:debug("elvis.config consultable; using it", []), AppConfig; + {error, {Line, Mod, Term}} -> + % In this very specific case we prefer to throw, since we make efforts + % to provide a valid config., but we also need to make sure the file + % is readable + exit( + lists:flatten( + io_lib:format("elvis.config unconsultable: ~p, ~p, ~p", [Line, Mod, Term]) + ) + ); _ -> elvis_utils:debug("elvis.config unconsultable", []), default_for(app) diff --git a/test/elvis_config_SUITE.erl b/test/elvis_config_SUITE.erl index df5ba507..e6f6a492 100644 --- a/test/elvis_config_SUITE.erl +++ b/test/elvis_config_SUITE.erl @@ -51,5 +51,12 @@ rock_with_rebar_default_config(_Config) -> throw_configuration(_Config) -> Filename = "./elvis.config", ok = file:write_file(Filename, <<"-">>), - {fail, [{throw, {invalid_config, _}}]} = elvis_config:from_file(Filename), + ok = + try + elvis_config:from_file(Filename), + fail + catch + exit:"elvis.config unconsultable: 1, erl_parse, [\"syntax error before: \",[]]" -> + ok + end, _ = file:delete(Filename).