Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions doc_rules/elvis_text_style/prefer_unquoted_atoms.md
Original file line number Diff line number Diff line change
@@ -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, #{}}
```
55 changes: 53 additions & 2 deletions src/elvis_text_style.erl
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down Expand Up @@ -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
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down
11 changes: 11 additions & 0 deletions test/examples/fail_quoted_atoms.erl
Original file line number Diff line number Diff line change
@@ -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'.
11 changes: 11 additions & 0 deletions test/examples/pass_unquoted_atoms.erl
Original file line number Diff line number Diff line change
@@ -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'].

14 changes: 12 additions & 2 deletions test/style_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) ->
Expand Down Expand Up @@ -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),
Expand Down