diff --git a/doc_rules/elvis_style/atom_naming_convention.md b/doc_rules/elvis_style/atom_naming_convention.md index 2e494e2..e4bbee4 100644 --- a/doc_rules/elvis_style/atom_naming_convention.md +++ b/doc_rules/elvis_style/atom_naming_convention.md @@ -2,9 +2,12 @@ (since [1.0.0](https://github.com/inaka/elvis_core/releases/tag/1.0.0)) -All atoms should be named according to the regular expression provided. Atoms enclosed in -apostrophes have special meaning and are thus handled by a different configuration option (use -`same` if you want the same value as `regex`). +All atoms should be named according to the provided regular expression, +except if they match with a defined `forbidden_regex`. +Atoms enclosed in apostrophes have special meaning and are thus handled +by a different configuration option (use `same` if you want the same value as `regex`). +To define forbidden enclosed atoms (like the ones in `forbidden_regex` apply for `regex`), +use `forbidden_enclosed_regex`(use `same` if you want the same value as `forbidden_regex`). > Works on `.beam` file? Yes! @@ -12,8 +15,12 @@ apostrophes have special meaning and are thus handled by a different configurati - `regex :: string()`. - default: `"^([a-z][a-z0-9]*_?)*(_SUITE)?$"`. -- `enclosed_atoms :: string()`. +- `enclosed_atoms :: string() | same`. - default: `".*"`. +- `forbidden_regex :: string() | undefined`. + - default: `undefined`. +- `forbidden_enclosed_regex :: string() | undefined | same`. + - default: `undefined`. ## Example diff --git a/doc_rules/elvis_style/function_naming_convention.md b/doc_rules/elvis_style/function_naming_convention.md index af91372..1f53e73 100644 --- a/doc_rules/elvis_style/function_naming_convention.md +++ b/doc_rules/elvis_style/function_naming_convention.md @@ -1,6 +1,7 @@ # Function Naming Convention -All functions should be named according to the regular expression provided. +All functions should be named according to the provided regular expression, +except if they match with a defined `forbidden_regex`. > Works on `.beam` file? Yes! @@ -8,6 +9,8 @@ All functions should be named according to the regular expression provided. - `regex :: string()`. - default: `"^[a-z](_?[a-z0-9]+)*$"`. +- `forbidden_regex :: string() | undefined`. + - default: `undefined`. ## Example diff --git a/doc_rules/elvis_style/module_naming_convention.md b/doc_rules/elvis_style/module_naming_convention.md index 3949ed8..d9340e6 100644 --- a/doc_rules/elvis_style/module_naming_convention.md +++ b/doc_rules/elvis_style/module_naming_convention.md @@ -1,6 +1,7 @@ # Module Naming Convention -All modules should be named according to the regular expression provided. +All modules should be named according to the provided regular expression, +except if they match with a defined `forbidden_regex`. > Works on `.beam` file? Yes! @@ -8,6 +9,8 @@ All modules should be named according to the regular expression provided. - `regex :: string()`. - default: `"^[a-z](_?[a-z0-9]+)*(_SUITE)?$"`. +- `forbidden_regex :: string() | undefined`. + - default: `undefined`. ## Example diff --git a/doc_rules/elvis_style/variable_naming_convention.md b/doc_rules/elvis_style/variable_naming_convention.md index 40c9f2f..2e393d6 100644 --- a/doc_rules/elvis_style/variable_naming_convention.md +++ b/doc_rules/elvis_style/variable_naming_convention.md @@ -1,6 +1,7 @@ # Variable Naming Convention -All variables should be named according to the regular expression provided. +All variables should be named according to the provided regular expression, +except if they match with a defined `forbidden_regex`. > Works on `.beam` file? Yes! @@ -8,6 +9,8 @@ All variables should be named according to the regular expression provided. - `regex :: string()`. - default: `"^_?([A-Z][0-9a-zA-Z]*)$"`. +- `forbidden_regex :: string() | undefined`. + - default: `undefined`. ## Example diff --git a/src/elvis_style.erl b/src/elvis_style.erl index 36f274e..312f8d4 100644 --- a/src/elvis_style.erl +++ b/src/elvis_style.erl @@ -70,15 +70,24 @@ "Use the '-callback' attribute instead of 'behavior_info/1' " "on line ~p."). -define(FUNCTION_NAMING_CONVENTION_MSG, - "The function ~p does not respect the format defined by the " + "The function ~p's name does not respect the format defined by the " "regular expression '~p'."). +-define(FORBIDDEN_FUNCTION_NAMING_CONVENTION_MSG, + "The function ~p's name is written in a forbidden format" + "defined by the regular expression '~p'."). -define(VARIABLE_NAMING_CONVENTION_MSG, - "The variable ~p on line ~p does not respect the format " + "The variable ~p's name, on line ~p does not respect the format " + "defined by the regular expression '~p'."). +-define(FORBIDDEN_VARIABLE_NAMING_CONVENTION_MSG, + "The variable ~p's name on line ~p is written in a forbidden the format " "defined by the regular expression '~p'."). -define(CONSISTENT_VARIABLE_CASING_MSG, "Variable ~ts (first used in line ~p) is written in different ways within the module: ~p."). -define(MODULE_NAMING_CONVENTION_MSG, - "The module ~p does not respect the format defined by the " + "The module ~p's name does not respect the format defined by the " + "regular expression '~p'."). +-define(FORBIDDEN_MODULE_NAMING_CONVENTION_MSG, + "The module ~p's name is written in a forbidden format defined by the " "regular expression '~p'."). -define(STATE_RECORD_MISSING_MSG, "This module implements an OTP behavior but is missing " @@ -114,7 +123,10 @@ -define(NO_SUCCESSIVE_MAPS_MSG, "Found map update after map construction/update at line ~p."). -define(ATOM_NAMING_CONVENTION_MSG, - "Atom ~p on line ~p does not respect the format " + "Atom ~p's name, on line ~p does not respect the format " + "defined by the regular expression '~p'."). +-define(FORBIDDEN_ATOM_NAMING_CONVENTION_MSG, + "Atom ~p on line ~p's name is written in a forbidden format " "defined by the regular expression '~p'."). -define(NO_THROW_MSG, "Usage of throw/1 on line ~p is not recommended"). -define(NO_DOLLAR_SPACE_MSG, @@ -183,11 +195,11 @@ default(nesting_level) -> default(god_modules) -> #{limit => 25}; default(function_naming_convention) -> - #{regex => "^[a-z](_?[a-z0-9]+)*$"}; + #{regex => "^[a-z](_?[a-z0-9]+)*$", forbidden_regex => undefined}; default(variable_naming_convention) -> - #{regex => "^_?([A-Z][0-9a-zA-Z]*)$"}; + #{regex => "^_?([A-Z][0-9a-zA-Z]*)$", forbidden_regex => undefined}; default(module_naming_convention) -> - #{regex => "^[a-z](_?[a-z0-9]+)*(_SUITE)?$"}; + #{regex => "^[a-z](_?[a-z0-9]+)*(_SUITE)?$", forbidden_regex => undefined}; default(dont_repeat_yourself) -> #{min_complexity => 10}; default(max_module_length) -> @@ -225,7 +237,10 @@ default(no_common_caveats_call) -> {timer, send_interval, 3}, {erlang, size, 1}]}; default(atom_naming_convention) -> - #{regex => "^[a-z](_?[a-z0-9]+)*(_SUITE)?$", enclosed_atoms => ".*"}; + #{regex => "^[a-z](_?[a-z0-9]+)*(_SUITE)?$", + enclosed_atoms => ".*", + forbidden_regex => undefined, + forbidden_enclosed_regex => undefined}; %% Not restrictive. Those who want more restrictions can set it like "^[^_]*$" default(numeric_format) -> #{regex => ".*", @@ -292,22 +307,38 @@ default(RuleWithEmptyDefault) [elvis_result:item()]. function_naming_convention(Config, Target, RuleConfig) -> Regex = option(regex, RuleConfig, function_naming_convention), + ForbiddenRegex = option(forbidden_regex, RuleConfig, function_naming_convention), Root = get_root(Config, Target, RuleConfig), FunctionNames0 = elvis_code:function_names(Root), - errors_for_function_names(Regex, FunctionNames0). + errors_for_function_names(Regex, ForbiddenRegex, FunctionNames0). -errors_for_function_names(_Regex, []) -> +errors_for_function_names(_Regex, _ForbiddenRegex, []) -> []; -errors_for_function_names(Regex, [FunctionName | RemainingFuncNames]) -> +errors_for_function_names(Regex, ForbiddenRegex, [FunctionName | RemainingFuncNames]) -> FunctionNameStr = unicode:characters_to_list(atom_to_list(FunctionName), unicode), case re:run(FunctionNameStr, Regex, [unicode]) of nomatch -> Msg = ?FUNCTION_NAMING_CONVENTION_MSG, Info = [FunctionNameStr, Regex], Result = elvis_result:new(item, Msg, Info, 1), - [Result | errors_for_function_names(Regex, RemainingFuncNames)]; + [Result | errors_for_function_names(Regex, ForbiddenRegex, RemainingFuncNames)]; {match, _} -> - errors_for_function_names(Regex, RemainingFuncNames) + case ForbiddenRegex of + undefined -> + errors_for_function_names(Regex, ForbiddenRegex, RemainingFuncNames); + ForbiddenRegex -> + case re:run(FunctionNameStr, ForbiddenRegex, [unicode]) of + {match, _} -> + Msg = ?FORBIDDEN_FUNCTION_NAMING_CONVENTION_MSG, + Info = [FunctionNameStr, Regex], + Result = elvis_result:new(item, Msg, Info, 1), + [Result | errors_for_function_names(Regex, + ForbiddenRegex, + RemainingFuncNames)]; + nomatch -> + errors_for_function_names(Regex, ForbiddenRegex, RemainingFuncNames) + end + end end. -type consistent_variable_casing_config() :: #{ignore => [ignorable()]}. @@ -362,9 +393,10 @@ check_variable_casing_consistency({_, [elvis_result:item()]. variable_naming_convention(Config, Target, RuleConfig) -> Regex = option(regex, RuleConfig, variable_naming_convention), + ForbiddenRegex = option(forbidden_regex, RuleConfig, variable_naming_convention), Root = get_root(Config, Target, RuleConfig), Vars = elvis_code:find(fun is_var/1, Root, #{traverse => all, mode => zipper}), - check_variables_name(Regex, Vars). + check_variables_name(Regex, ForbiddenRegex, Vars). -type macro_names_config() :: #{ignore => [ignorable()], regex => string()}. @@ -724,6 +756,7 @@ no_behavior_info(Config, Target, RuleConfig) -> [elvis_result:item()]. module_naming_convention(Config, Target, RuleConfig) -> Regex = option(regex, RuleConfig, module_naming_convention), + ForbiddenRegex = option(forbidden_regex, RuleConfig, module_naming_convention), IgnoreModules = option(ignore, RuleConfig, module_naming_convention), Root = get_root(Config, Target, RuleConfig), @@ -739,12 +772,30 @@ module_naming_convention(Config, Target, RuleConfig) -> Result = elvis_result:new(item, Msg, Info, 1), [Result]; {match, _} -> - [] + case ForbiddenRegex of + undefined -> + []; + ForbiddenRegex -> + is_forbidden_module_name(ModuleNameStr, + ForbiddenRegex, + ?FORBIDDEN_MODULE_NAMING_CONVENTION_MSG) + end end; true -> [] end. +is_forbidden_module_name(Target, Regex, Message) -> + case re:run(Target, Regex, [unicode]) of + {match, _} -> + Msg = Message, + Info = [Target, Regex], + Result = elvis_result:new(item, Msg, Info, 1), + [Result]; + nomatch -> + [] + end. + -spec state_record_and_type(elvis_config:config(), elvis_file:file(), empty_rule_config()) -> @@ -1118,10 +1169,19 @@ no_successive_maps(Config, Target, RuleConfig) -> atom_naming_convention(Config, Target, RuleConfig) -> Root = get_root(Config, Target, RuleConfig), Regex = option(regex, RuleConfig, atom_naming_convention), + ForbiddenRegex = option(forbidden_regex, RuleConfig, atom_naming_convention), RegexEnclosed = specific_or_default(option(enclosed_atoms, RuleConfig, atom_naming_convention), Regex), + ForbiddenEnclosedRegex = + specific_or_default(option(forbidden_enclosed_regex, RuleConfig, atom_naming_convention), + ForbiddenRegex), AtomNodes = elvis_code:find(fun is_atom_node/1, Root, #{traverse => all, mode => node}), - check_atom_names(Regex, RegexEnclosed, AtomNodes, []). + check_atom_names(Regex, + ForbiddenRegex, + RegexEnclosed, + ForbiddenEnclosedRegex, + AtomNodes, + []). -type no_init_lists_config() :: #{behaviours => [atom()]}. @@ -1659,14 +1719,26 @@ is_exception_or_non_reversible(_) -> false. %% @private -check_atom_names(_Regex, _RegexEnclosed, [] = _AtomNodes, Acc) -> +check_atom_names(_Regex, _, _RegexEnclosed, _, [] = _AtomNodes, Acc) -> Acc; -check_atom_names(Regex, RegexEnclosed, [AtomNode | RemainingAtomNodes], AccIn) -> +check_atom_names(Regex, + ForbiddenRegexNormal, + RegexEnclosed, + ForbiddenRegexEnclosed, + [AtomNode | RemainingAtomNodes], + AccIn) -> AtomName0 = ktn_code:attr(text, AtomNode), ValueAtomName = ktn_code:attr(value, AtomNode), {IsEnclosed, AtomName} = string_strip_enclosed(AtomName0), IsExceptionClass = is_exception_or_non_reversible(ValueAtomName), RE = re_compile_for_atom_type(IsEnclosed, Regex, RegexEnclosed), + ForbiddenRegex = + case IsEnclosed of + true -> + ForbiddenRegexEnclosed; + false -> + ForbiddenRegexNormal + end, AccOut = case re:run( unicode:characters_to_list(AtomName, unicode), RE) @@ -1686,9 +1758,27 @@ check_atom_names(Regex, RegexEnclosed, [AtomNode | RemainingAtomNodes], AccIn) - Result = elvis_result:new(item, Msg, Info, Line), AccIn ++ [Result]; {match, _Captured} -> - AccIn + case ForbiddenRegex of + undefined -> + AccIn; + ForbiddenRegex -> + case re:run(AtomName, ForbiddenRegex, [unicode]) of + {match, _} -> + Msg = ?FORBIDDEN_ATOM_NAMING_CONVENTION_MSG, + Info = [AtomName, Regex], + Result = elvis_result:new(item, Msg, Info, 1), + AccIn ++ [Result]; + nomatch -> + AccIn + end + end end, - check_atom_names(Regex, RegexEnclosed, RemainingAtomNodes, AccOut). + check_atom_names(Regex, + ForbiddenRegexNormal, + RegexEnclosed, + ForbiddenRegexEnclosed, + RemainingAtomNodes, + AccOut). %% @private string_strip_enclosed([$' | Rest]) -> @@ -1714,21 +1804,34 @@ is_atom_node(MaybeAtom) -> %% Variables name %% @private -check_variables_name(_Regex, []) -> +check_variables_name(_Regex, _, []) -> []; -check_variables_name(Regex, [Variable | RemainingVars]) -> +check_variables_name(Regex, ForbiddenRegex, [Variable | RemainingVars]) -> VariableNameStr = atom_to_list(ktn_code:attr(name, Variable)), case re:run(VariableNameStr, Regex) of nomatch when VariableNameStr == "_" -> - check_variables_name(Regex, RemainingVars); + check_variables_name(Regex, ForbiddenRegex, RemainingVars); nomatch -> Msg = ?VARIABLE_NAMING_CONVENTION_MSG, {Line, _} = ktn_code:attr(location, Variable), Info = [VariableNameStr, Line, Regex], Result = elvis_result:new(item, Msg, Info, Line), - [Result | check_variables_name(Regex, RemainingVars)]; + [Result | check_variables_name(Regex, ForbiddenRegex, RemainingVars)]; {match, _} -> - check_variables_name(Regex, RemainingVars) + case ForbiddenRegex of + undefined -> + check_variables_name(Regex, ForbiddenRegex, RemainingVars); + ForbiddenRegex -> + case re:run(VariableNameStr, ForbiddenRegex, [unicode]) of + {match, _} -> + Msg = ?FORBIDDEN_VARIABLE_NAMING_CONVENTION_MSG, + Info = [VariableNameStr, Regex], + Result = elvis_result:new(item, Msg, Info, 1), + [Result | check_variables_name(Regex, ForbiddenRegex, RemainingVars)]; + nomatch -> + check_variables_name(Regex, ForbiddenRegex, RemainingVars) + end + end end. %% Result building diff --git a/test/examples/forbidden_atom_naming_convention.erl b/test/examples/forbidden_atom_naming_convention.erl new file mode 100644 index 0000000..c02341e --- /dev/null +++ b/test/examples/forbidden_atom_naming_convention.erl @@ -0,0 +1,14 @@ +-module(forbidden_atom_naming_convention). + +-export([for_test/0]). + +for_test() -> + this_is_an_ok_atom, + 'and_so_is_this', + 'and_this_1_too', + non_200, + '_', % used by ets/mnesia/etc. + non200, % valid, even without underscores + valid_200even_if_numb3rs_appear_between_letters, + blahblah_SUITE, + x. diff --git a/test/examples/forbidden_function_naming_convention.erl b/test/examples/forbidden_function_naming_convention.erl new file mode 100644 index 0000000..337ec59 --- /dev/null +++ b/test/examples/forbidden_function_naming_convention.erl @@ -0,0 +1,31 @@ +-module(forbidden_function_naming_convention). + +-dialyzer({nowarn_function, bad_names_inside/0}). + +-export([bad_names_inside/0]). + +%% Function names must use only lowercase characters. +%% Words in function names must be separated with _. + +%% Cf. https://github.com/inaka/erlang_guidelines#function-names + +bad_names_inside() -> + good_name(should, fail), + not_forbidden_but_still_bad____(should, fail), + no_numbers_1(should, fail), + fun2ms(should, fail). + +%% Private / hidden functions still checked + +good_name(Should, Fail) -> + [Should, Fail]. + +not_forbidden_but_still_bad____(Should, Fail) -> + [Should, Fail]. + +no_numbers_1(Should, Fail) -> + [Should, Fail]. + +fun2ms(Should, Fail) -> + [Should, Fail]. + diff --git a/test/examples/forbidden_module_naming_convention_12.erl b/test/examples/forbidden_module_naming_convention_12.erl new file mode 100644 index 0000000..36da17a --- /dev/null +++ b/test/examples/forbidden_module_naming_convention_12.erl @@ -0,0 +1 @@ +-module(forbidden_module_naming_convention_12). diff --git a/test/examples/forbidden_variable_naming_convention.erl b/test/examples/forbidden_variable_naming_convention.erl new file mode 100644 index 0000000..5f8f746 --- /dev/null +++ b/test/examples/forbidden_variable_naming_convention.erl @@ -0,0 +1,22 @@ +-module(forbidden_variable_naming_convention). + +-export([should_pass/5]). +-export([should_pass/0, should_pass/1]). + +%% CamelCase must be used for variables. Don’t +%% separate words in variables with _. + +%% Cf. https://github.com/inaka/erlang_guidelines#variable-names + +should_pass(Should, Pass, Way2Home, Fun1, Fun2) -> + [_IgnoredVariable, _] = ["Ignored but valid", "also valid"], + Should = "Should", + Pass = "Pass", + Way2Home = "Way to home", + Fun1 = Should ++ Pass, + Fun2 = Fun1 ++ Way2Home. + +should_pass() -> + ?MODULE_STRING. + +should_pass(_) -> ?MODULE_STRING. diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl index 6a9c3e4..317ce39 100644 --- a/test/style_SUITE.erl +++ b/test/style_SUITE.erl @@ -184,13 +184,23 @@ verify_function_naming_convention(Config) -> elvis_style, function_naming_convention, RuleConfig4, - PathIgnored). + PathIgnored), + + % forbidden + PathForbidden = "forbidden_function_naming_convention." ++ Ext, + [_, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + function_naming_convention, + #{regex => DefaultRegex, forbidden_regex => "[0-9]"}, + PathForbidden). -spec verify_variable_naming_convention(config()) -> any(). verify_variable_naming_convention(Config) -> Ext = proplists:get_value(test_file_ext, Config, "erl"), RuleConfig = #{regex => "^_?([A-Z][0-9a-zA-Z]*)$"}, + #{regex := DefaultRegex} = elvis_style:default(variable_naming_convention), PathPass = "pass_variable_naming_convention." ++ Ext, [] = @@ -210,7 +220,16 @@ verify_variable_naming_convention(Config) -> elvis_style, variable_naming_convention, RuleConfig, - PathFail). + PathFail), + + % forbidden + PathForbidden = "forbidden_variable_naming_convention." ++ Ext, + [_, _, _, _, _, _, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + variable_naming_convention, + #{regex => DefaultRegex, forbidden_regex => "[0-9]"}, + PathForbidden). -spec verify_consistent_variable_casing(config()) -> any(). verify_consistent_variable_casing(Config) -> @@ -694,7 +713,16 @@ verify_module_naming_convention(Config) -> elvis_style, module_naming_convention, RuleConfigIgnore, - PathFail). + PathFail), + + % forbidden + PathForbidden = "forbidden_module_naming_convention_12." ++ Ext, + [_] = + elvis_core_apply_rule(Config, + elvis_style, + module_naming_convention, + #{regex => DefaultRegex, forbidden_regex => "[0-9]"}, + PathForbidden). -spec verify_state_record_and_type(config()) -> any(). verify_state_record_and_type(Config) -> @@ -1437,6 +1465,8 @@ verify_atom_naming_convention(Config) -> Group = proplists:get_value(group, Config, erl_files), Ext = proplists:get_value(test_file_ext, Config, "erl"), + #{regex := DefaultRegex} = elvis_style:default(atom_naming_convention), + BaseRegex = "^[a-z](_?[a-z0-9]+)*(_SUITE)?$", % pass @@ -1564,7 +1594,35 @@ verify_atom_naming_convention(Config) -> #{regex => KeepRegex, enclosed_atoms => "^([\\\\][\-a-z0-9A-Z_' \\\\]*)$"}, FailPath) - end. + end, + + % forbidden + PathForbidden = "forbidden_atom_naming_convention." ++ Ext, + _ = case Group of + beam_files -> % 'or_THIS' getting stripped of enclosing ' + [_, _, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + atom_naming_convention, + #{regex => DefaultRegex, forbidden_regex => "[0-9]"}, + PathForbidden); + erl_files -> + [_, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + atom_naming_convention, + #{regex => DefaultRegex, forbidden_regex => "[0-9]"}, + PathForbidden) + end, + + [_, _, _, _] = + elvis_core_apply_rule(Config, + elvis_style, + atom_naming_convention, + #{regex => DefaultRegex, + forbidden_regex => "[0-9]", + forbidden_enclosed_regex => same}, + PathForbidden). -spec verify_no_init_lists(config()) -> any(). verify_no_init_lists(Config) ->