diff --git a/RULES.md b/RULES.md index da5561e..34ce892 100644 --- a/RULES.md +++ b/RULES.md @@ -61,6 +61,7 @@ identified with `(since ...)` for convenience purposes. - [State Record and Type](doc_rules/elvis_style/state_record_and_type.md) - [Used Ignored Variable](doc_rules/elvis_style/used_ignored_variable.md) - [Variable Naming Convention](doc_rules/elvis_style/variable_naming_convention.md) +- [Prefer Unquoted Atoms](doc_rules/elvis_text_style/prefer_unquoted_atoms.md) ## Project rules diff --git a/doc_rules/elvis_text_style/prefer_unquoted_atoms.md b/doc_rules/elvis_text_style/prefer_unquoted_atoms.md new file mode 100644 index 0000000..1f545c6 --- /dev/null +++ b/doc_rules/elvis_text_style/prefer_unquoted_atoms.md @@ -0,0 +1,15 @@ +# Prefer unquoted atoms + +Do not use quotes on atoms that don't need to be quoted. + +> Works on `.beam` file? No. + +## Options + +- None. + +## Example + +```erlang +{elvis_style, prefer_unquoted_atoms, #{}} +``` diff --git a/src/elvis_text_style.erl b/src/elvis_text_style.erl index c560d16..eaeb44b 100644 --- a/src/elvis_text_style.erl +++ b/src/elvis_text_style.erl @@ -1,19 +1,26 @@ -module(elvis_text_style). --export([default/1, line_length/3, no_tabs/3, no_trailing_whitespace/3]). +-export([default/1, line_length/3, no_tabs/3, no_trailing_whitespace/3, + prefer_unquoted_atoms/3]). -export_type([line_length_config/0, no_trailing_whitespace_config/0]). -define(LINE_LENGTH_MSG, "Line ~p is too long. It has ~p characters."). -define(NO_TABS_MSG, "Line ~p has a tab at column ~p."). -define(NO_TRAILING_WHITESPACE_MSG, "Line ~b has ~b trailing whitespace characters."). +-define(ATOM_PREFERRED_QUOTES_MSG, + "Atom ~p on line ~p is quoted " + "but quotes are not needed."). % These are part of a non-declared "behaviour" % The reason why we don't try to handle them with different arity is % that arguments are ignored in different positions (1 and 3) so that'd % probably be messier than to ignore the warning -hank([{unnecessary_function_arguments, - [{no_trailing_whitespace, 3}, {no_tabs, 3}, {line_length, 3}]}]). + [{no_trailing_whitespace, 3}, + {no_tabs, 3}, + {line_length, 3}, + {prefer_unquoted_atoms, 3}]}]). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Default values @@ -71,6 +78,37 @@ no_trailing_whitespace(_Config, Target, RuleConfig) -> end, RuleConfig). +-spec prefer_unquoted_atoms(elvis_config:config(), + elvis_file:file(), + elvis_style:empty_rule_config()) -> + [elvis_result:item()]. +prefer_unquoted_atoms(_Config, Target, _RuleConfig) -> + {Content, #{encoding := _Encoding}} = elvis_file:src(Target), + Tree = ktn_code:parse_tree(Content), + AtomNodes = elvis_code:find(fun is_atom_node/1, Tree, #{traverse => all, mode => node}), + check_atom_quotes(AtomNodes, []). + +%% @private +check_atom_quotes([] = _AtomNodes, Acc) -> + Acc; +check_atom_quotes([AtomNode | RemainingAtomNodes], AccIn) -> + AtomName = ktn_code:attr(text, AtomNode), + + IsException = is_exception_prefer_quoted(AtomName), + + AccOut = + case unicode:characters_to_list(AtomName, unicode) of + [$' | _] when not IsException -> + Msg = ?ATOM_PREFERRED_QUOTES_MSG, + {Line, _} = ktn_code:attr(location, AtomNode), + Info = [AtomName, Line], + Result = elvis_result:new(item, Msg, Info, Line), + AccIn ++ [Result]; + _ -> + AccIn + end, + check_atom_quotes(RemainingAtomNodes, AccOut). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Private %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -164,6 +202,19 @@ check_no_trailing_whitespace(Line, Num, IgnoreEmptyLines) -> {ok, Result} end. +%% @private +is_atom_node(MaybeAtom) -> + ktn_code:type(MaybeAtom) =:= atom. + +%% @private +is_exception_prefer_quoted(Elem) -> + KeyWords = + ["'after'", "'and'", "'andalso'", "'band'", "'begin'", "'bnot'", "'bor'", "'bsl'", + "'bsr'", "'bxor'", "'case'", "'catch'", "'cond'", "'div'", "'end'", "'fun'", "'if'", + "'let'", "'not'", "'of'", "'or'", "'orelse'", "'receive'", "'rem'", "'try'", "'when'", + "'xor'", "'maybe'"], + lists:member(Elem, KeyWords). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% Internal Function Definitions %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/test/examples/fail_quoted_atoms.erl b/test/examples/fail_quoted_atoms.erl new file mode 100644 index 0000000..aa56755 --- /dev/null +++ b/test/examples/fail_quoted_atoms.erl @@ -0,0 +1,11 @@ +-module(fail_quoted_atoms). + +-export([test/1, test/2]). + +-define(TEST(), test(1, default)). + +test(_Test) -> {ok, test}. + +test(_A, 'ugly_atom_name') -> 'why_use_quotes_here'; +test(_A, default) -> ?TEST(); +test(_A, _B) -> 'and'. diff --git a/test/examples/pass_unquoted_atoms.erl b/test/examples/pass_unquoted_atoms.erl new file mode 100644 index 0000000..48ddb76 --- /dev/null +++ b/test/examples/pass_unquoted_atoms.erl @@ -0,0 +1,11 @@ +-module(pass_unquoted_atoms). + +-export([test/1, test/2]). + +test(_Test) -> ok. + +test(_A, nice_atom_name) -> perfect_atomname; +test(_Reserved, _Words) -> ['after', 'and', 'andalso', 'band', 'begin', 'bnot', 'bor', 'bsl', 'bsr', 'bxor', 'case', + 'catch', 'cond', 'div', 'end', 'fun', 'if', 'let', 'not', 'of', 'or', 'orelse', 'receive', + 'rem', 'try', 'when', 'xor', 'maybe']. + diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl index c59f94e..8ebfaea 100644 --- a/test/style_SUITE.erl +++ b/test/style_SUITE.erl @@ -25,7 +25,7 @@ verify_always_shortcircuit/1, verify_consistent_generic_type/1, verify_no_types/1, verify_no_specs/1, verify_export_used_types/1, verify_consistent_variable_casing/1, verify_no_match_in_condition/1, verify_param_pattern_matching/1, - verify_private_data_types/1]). + verify_private_data_types/1, verify_unquoted_atoms/1]). %% -elvis attribute -export([verify_elvis_attr_atom_naming_convention/1, verify_elvis_attr_numeric_format/1, verify_elvis_attr_dont_repeat_yourself/1, verify_elvis_attr_function_naming_convention/1, @@ -82,7 +82,7 @@ groups() -> verify_always_shortcircuit, verify_no_catch_expressions, verify_no_single_clause_case, verify_no_macros, verify_export_used_types, verify_max_anonymous_function_arity, verify_max_function_arity, verify_no_match_in_condition, verify_behaviour_spelling, - verify_param_pattern_matching, verify_private_data_types]}]. + verify_param_pattern_matching, verify_private_data_types, verify_unquoted_atoms]}]. -spec init_per_suite(config()) -> config(). init_per_suite(Config) -> @@ -1297,6 +1297,16 @@ verify_no_successive_maps(_Config) -> -endif. +-spec verify_unquoted_atoms(config()) -> any(). +verify_unquoted_atoms(Config) -> + PassPath = "pass_unquoted_atoms." ++ "erl", + [] = + elvis_core_apply_rule(Config, elvis_text_style, prefer_unquoted_atoms, #{}, PassPath), + + FailPath = "fail_quoted_atoms." ++ "erl", + [_, _] = + elvis_core_apply_rule(Config, elvis_text_style, prefer_unquoted_atoms, #{}, FailPath). + -spec verify_atom_naming_convention(config()) -> any(). verify_atom_naming_convention(Config) -> Group = proplists:get_value(group, Config, erl_files),