Merge pull request #355 from bormilan/feature/prefer-unquoted-atoms

feat(#297): New rule: Prefer Unquoted Atoms

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
 

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, #{}}
+```

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
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

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'.

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'].
+

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),