Tweak some defaults: macro_names, operator_spaces, no_space, `f…

…unction_naming_convention`, `module_naming_convention`, `no_debug_call` (#358) * Breaking: tweak macro_names' default: use ^[A-Z]+(_[A-Z0-9]+)*$ The new regex is almost as permissive as the old one, but doesn't allow for __, for example * Breaking: tweak operator_spaces' default: add {left, "!"} and {right, "!"} * Breaking: tweak no_space' default: add {right, "#"} and {right, "?"} Also test {left, ":"}, {right, ":"} by using the defaults directly * Breaking: tweak function_naming_convention's default Make it ^[a-z](_?[a-z0-9]+)*$ I also "fix" it by removing _SUITE (?) Did this ever make sense? * Breaking: tweak module_naming_convention's default Make it ^[a-z](_?[a-z0-9]+)*(_SUITE)?$ Actually, with this change a previously invalid module name (used in tests) is now valid, but I think it's something we actually want: think handler_for_ok_200; this should be allowed * Breaking: tweak no_debug_call's default: add io:put_chars/1,2 * Update documentation and migration instructions * Act on CI results: run `rebar3 test` on OTP 26.x --------- Co-authored-by: Brujo Benavides <[email protected]>
inaka · Sep 17, 2024 · 8abd0b3 · 8abd0b3
1 parent 7af61a7
commit 8abd0b3
Show file tree

Hide file tree

Showing 17 changed files with 102 additions and 42 deletions.
diff --git a/.markdownlint.yml b/.markdownlint.yml
@@ -2,3 +2,5 @@
 default: true
 MD013:
   line_length: 100
+MD024:
+  siblings_only: true
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -8,6 +8,27 @@ since these are incremental.
 This file's format is influenced by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and
 [Make a README](https://www.makeareadme.com/).
 
+## Going from `3.x` to `4.x`
+
+### Update
+
+- your `atom_naming_convention`'s options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/atom_naming_convention.md#options>
+- your `macro_names`' options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/macro_names.md#options>
+- your `operator_spaces` options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/operator_spaces.md#options>
+- your `no_space` options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/no_space.md#options>
+- your `function_naming_convention` options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/function_naming_convention.md#options>
+- your `module_naming_convention` options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/module_naming_convention.md#options>
+- your `no_debug_call` options to be the ones defined at
+<https://github.com/inaka/elvis_core/blob/3.2.5/doc_rules/elvis_style/no_debug_call.md#options>
+
+On the other hand you may choose to not implement those changes and merely adapt your code.
+
 ## Going from `0.x` to `1.x`
 
 ### Update

diff --git a/doc_rules/elvis_style/function_naming_convention.md b/doc_rules/elvis_style/function_naming_convention.md
@@ -7,7 +7,7 @@ All functions should be named according to the regular expression provided.
 ## Options
 
 - `regex :: string()`.
-  - default: `"^([a-z][a-z0-9]*_?)*(_SUITE)?$"`.
+  - default: `"^[a-z](_?[a-z0-9]+)*$"`.
 
 ## Example
 

diff --git a/doc_rules/elvis_style/macro_names.md b/doc_rules/elvis_style/macro_names.md
@@ -7,7 +7,7 @@ Macro names should only contain upper-case alphanumeric characters.
 ## Options
 
 - `regex :: string()`. (since [1.0.0](https://github.com/inaka/elvis_core/releases/tag/1.0.0))
-  - default: `"^([A-Z][A-Z_0-9]+)$"`.
+  - default: `"^[A-Z](_?[A-Z0-9]+)*$"`.
 
 ## Example
 

diff --git a/doc_rules/elvis_style/module_naming_convention.md b/doc_rules/elvis_style/module_naming_convention.md
@@ -7,7 +7,7 @@ All modules should be named according to the regular expression provided.
 ## Options
 
 - `regex :: string()`.
-  - default: `"^([a-z][a-z0-9]*_?)*(_SUITE)?$"`.
+  - default: `"^[a-z](_?[a-z0-9]+)*(_SUITE)?$"`.
 
 ## Example
 

diff --git a/doc_rules/elvis_style/no_debug_call.md b/doc_rules/elvis_style/no_debug_call.md
@@ -7,7 +7,8 @@ Don't leave debugging function calls such as `io:format` or `ct:pal` in your sou
 ## Options
 
 - `debug_functions :: [{module(), function(), arity()} | {module(), function()}]`.
-  - default: `[{ct, pal}, {ct, print}, {io, format, 1}, {io, format, 2}, {erlang, display, 1}]`
+  - default: `[{ct, pal}, {ct, print}, {io, format, 1}, {io, format, 2}, {erlang, display, 1},
+    {io, put_chars, 1}, {io, put_chars, 2}]`
   (`{erlang, display, 1}` is only included since
   [1.5.0](https://github.com/inaka/elvis_core/releases/tag/1.5.0)).
 

diff --git a/doc_rules/elvis_style/no_space.md b/doc_rules/elvis_style/no_space.md
@@ -10,7 +10,7 @@ can be any string.
 ## Options
 
 - `rules :: [{right | left, string()}].`
-  - default: `[{right, "("}, {left, ")"}, {left, ","}]`
+  - default: `[{right, "("}, {left, ")"}, {left, ","}, {right, "#"}, {right, "?"}]`
 
 ## Example
 

diff --git a/doc_rules/elvis_style/operator_spaces.md b/doc_rules/elvis_style/operator_spaces.md
@@ -21,7 +21,8 @@ operator can be any string.
            {right, "/="}, {left, "/="}, {right, "=/="}, {left, "=/="}, {right, "--"}, {left, "--"},
            {right, "=>"}, {left, "=>"}, {right, ":="}, {left, ":="}, {right, "<-"}, {left, "<-"},
            {right, "<="}, {left, "<="}, {right, "||"}, {left, "||"}, {right, "|"}, {left, "|"},
-           {right, "::"}, {left, "::"}, {right, "->"}, {left, "->"}, {right, ","}]
+           {right, "::"}, {left, "::"}, {right, "->"}, {left, "->"}, {right, ","}, {left, "!"},
+           {right, "!"}]
 ```
 
 ## Example

diff --git a/src/elvis_style.erl b/src/elvis_style.erl
@@ -145,7 +145,7 @@
 
 -spec default(Rule :: atom()) -> DefaultRuleConfig :: term().
 default(macro_names) ->
-    #{regex => "^([A-Z][A-Z_0-9]+)$"};
+    #{regex => "^[A-Z](_?[A-Z0-9]+)*$"};
 default(operator_spaces) ->
     #{rules =>
           [{right, "++"}, {left, "++"}, {right, "="}, {left, "="}, {right, "+"}, {left, "+"},
@@ -155,19 +155,27 @@ default(operator_spaces) ->
            {right, "/="}, {left, "/="}, {right, "=/="}, {left, "=/="}, {right, "--"}, {left, "--"},
            {right, "=>"}, {left, "=>"}, {right, ":="}, {left, ":="}, {right, "<-"}, {left, "<-"},
            {right, "<="}, {left, "<="}, {right, "||"}, {left, "||"}, {right, "|"}, {left, "|"},
-           {right, "::"}, {left, "::"}, {right, "->"}, {left, "->"}, {right, ","}]};
+           {right, "::"}, {left, "::"}, {right, "->"}, {left, "->"}, {right, ","}, {left, "!"},
+           {right, "!"}]};
 default(no_space) ->
-    #{rules => [{right, "("}, {left, ")"}, {left, ","}]};
+    #{rules =>
+          [{right, "("},
+           {left, ")"},
+           {left, ","},
+           {left, ":"},
+           {right, ":"},
+           {right, "#"},
+           {right, "?"}]};
 default(nesting_level) ->
     #{level => 4};
 default(god_modules) ->
     #{limit => 25};
 default(function_naming_convention) ->
-    #{regex => "^([a-z][a-z0-9]*_?)*(_SUITE)?$"};
+    #{regex => "^[a-z](_?[a-z0-9]+)*$"};
 default(variable_naming_convention) ->
     #{regex => "^_?([A-Z][0-9a-zA-Z]*)$"};
 default(module_naming_convention) ->
-    #{regex => "^([a-z][a-z0-9]*_?)*(_SUITE)?$"};
+    #{regex => "^[a-z](_?[a-z0-9]+)*(_SUITE)?$"};
 default(dont_repeat_yourself) ->
     #{min_complexity => 10};
 default(max_module_length) ->
@@ -186,7 +194,13 @@ default(no_call) ->
     #{no_call_functions => []};
 default(no_debug_call) ->
     #{debug_functions =>
-          [{ct, pal}, {ct, print}, {erlang, display, 1}, {io, format, 1}, {io, format, 2}]};
+          [{ct, pal},
+           {ct, print},
+           {erlang, display, 1},
+           {io, format, 1},
+           {io, format, 2},
+           {io, put_chars, 1},
+           {io, put_chars, 2}]};
 default(no_common_caveats_call) ->
     #{caveat_functions =>
           [{timer, send_after, 2},

diff --git a/test/examples/fail_function_naming_convention.erl b/test/examples/fail_function_naming_convention.erl
@@ -15,7 +15,8 @@ bad_names_inside() ->
     'Initial_cap'(should, fail),
     'ok-for-lisp'(should, fail),
     'no_predicates?'(should, fail),
-    user@location(should, fail).
+    user@location(should, fail),
+    before__after(should, fail).
 
 %% Private / hidden functions still checked
 
@@ -36,3 +37,6 @@ camelCase(Should, Fail) ->
 
 user@location(Should, Fail) ->
     [Should, Fail].
+
+before__after(Should, Fail) ->
+    [Should, Fail].
diff --git a/test/examples/fail_macro_names.erl b/test/examples/fail_macro_names.erl
@@ -11,6 +11,7 @@
 -define(CALLING_SOMETHING, call(1)).
 -define(  'POTENTIAL_BAD-NAME'  , nomegusta).
 -define('A,aZ', 2).
+-define (BAD__NAME, "Stumble").
 
 -export([
          define/1,

diff --git a/test/examples/fail_module_naming_1_convention_1.erl b/test/examples/fail_module_naming_1_convention_1.erl
diff --git a/test/examples/fail_module_naming_1_convention_1_.erl b/test/examples/fail_module_naming_1_convention_1_.erl
@@ -0,0 +1 @@
+-module(fail_module_naming_1_convention_1_).
diff --git a/test/examples/fail_no_debug_call.erl b/test/examples/fail_no_debug_call.erl
@@ -11,4 +11,5 @@ fail() ->
     ct:pal("Debug"),
     ct:pal("Debug ~s", ["Debug Info"]),
     ct:print("Debug"),
-    ct:print("Debug ~s", ["Debug Info"]).
+    ct:print("Debug ~s", ["Debug Info"]),
+    io:put_chars("Debug").
diff --git a/test/examples/fail_no_space.erl b/test/examples/fail_no_space.erl
@@ -17,9 +17,13 @@
         , unicode_characters/0
         , windows_newlines/0
         , this/0
-        , this/1
+        , this/2
+        , use_record/0
         ]).
 
+-define(MACRO, "Brujo loves these").
+-record(a, {one, two}).
+
 %% No space before and after coma,on a comment.
 %% ( inside a comment
 %% ( 
@@ -107,8 +111,11 @@ windows_newlines() ->
 this()
 -> should_not_crash.
 
--spec this(shouldnt_either)
+-spec this(shouldnt_either, A::integer())
 -> 32.
-this(shouldnt_either)
+this(shouldnt_either, _A)
 -> A = 1
 - 2, A, $ . 
+
+use_record() ->
+    # a{one = 1, two = ? MACRO}.
diff --git a/test/examples/fail_operator_spaces.erl b/test/examples/fail_operator_spaces.erl
@@ -20,6 +20,7 @@
         , this/1
         , pass_more_operators/0
         , fail_more_operators/0
+        , fail_no_space_excl/0
         ]).
 
 %% No space before and after coma,on a comment.
@@ -150,3 +151,6 @@ fail_more_operators()->
         <<<<$>>>||<<$>>><=<<$>>>>>,
         [X|D]
     }.
+
+fail_no_space_excl() ->
+    self()!'a'.
diff --git a/test/style_SUITE.erl b/test/style_SUITE.erl
@@ -117,17 +117,17 @@ verify_function_naming_convention(Config) ->
 
     % pass
     PathPass = "pass_function_naming_convention." ++ Ext,
+    #{regex := DefaultRegex} = elvis_style:default(function_naming_convention),
 
-    RuleConfig = #{regex => "^([a-z][a-z0-9]*_?)*$"},
+    RuleConfig = #{regex => DefaultRegex},
     [] =
         elvis_core_apply_rule(Config,
                               elvis_style,
                               function_naming_convention,
                               RuleConfig,
                               PathPass),
 
-    RuleConfig2 =
-        #{regex => "^([a-z][a-z0-9]*_?)*$", ignore => [fail_function_naming_convention]},
+    RuleConfig2 = #{regex => DefaultRegex, ignore => [fail_function_naming_convention]},
     [] =
         elvis_core_apply_rule(Config,
                               elvis_style,
@@ -142,22 +142,23 @@ verify_function_naming_convention(Config) ->
      _InitialCapError,
      _HyphenError,
      _PredError,
-     _EmailError] =
+     _EmailError,
+     _BeforeAfter] =
         elvis_core_apply_rule(Config,
                               elvis_style,
                               function_naming_convention,
                               RuleConfig,
                               PathFail),
 
     RuleConfig3 =
-        #{regex => "^([a-z][a-z0-9]*_?)*$",
+        #{regex => DefaultRegex,
           ignore =>
               [{fail_function_naming_convention, camelCase},
                {fail_function_naming_convention, 'ALL_CAPS'},
                {fail_function_naming_convention, 'Initial_cap'},
                {fail_function_naming_convention, 'ok-for-lisp'},
                {fail_function_naming_convention, 'no_predicates?'}]},
-    [_EmailError2] =
+    [_EmailError2, _BeforeAfter2] =
         elvis_core_apply_rule(Config,
                               elvis_style,
                               function_naming_convention,
@@ -168,7 +169,7 @@ verify_function_naming_convention(Config) ->
     PathIgnored = "fail_function_naming_convention_ignored_function." ++ Ext,
 
     RuleConfig4 =
-        #{regex => "^([a-z][a-z0-9]*_?)*$",
+        #{regex => DefaultRegex,
           ignore =>
               [{fail_function_naming_convention, camelCase},
                {fail_function_naming_convention, 'ALL_CAPS'},
@@ -321,7 +322,7 @@ verify_macro_names_rule(Config) ->
 
     Path = "fail_macro_names." ++ Ext,
 
-    [_, _, _, _, _] = elvis_core_apply_rule(Config, elvis_style, macro_names, #{}, Path),
+    [_, _, _, _, _, _] = elvis_core_apply_rule(Config, elvis_style, macro_names, #{}, Path),
 
     [_, _] =
         elvis_core_apply_rule(Config,
@@ -344,7 +345,7 @@ verify_macro_names_rule(Config) ->
                               #{regex => "^[A-Za-z_, \-]+$"},
                               Path),
 
-    [_, _, _, _, _, _, _, _, _, _] =
+    [_, _, _, _, _, _, _, _, _, _, _] =
         elvis_core_apply_rule(Config,
                               elvis_style,
                               macro_names,
@@ -469,7 +470,8 @@ verify_operator_spaces(Config) ->
      #{info := [left, "/=" | _]}, #{info := [right, "=/=" | _]}, #{info := [left, "=/=" | _]},
      #{info := [right, "--" | _]}, #{info := [left, "--" | _]}, #{info := [right, "||" | _]},
      #{info := [left, "||" | _]}, #{info := [right, "||" | _]}, #{info := [left, "||" | _]},
-     #{info := [right, "|" | _]}, #{info := [left, "|" | _]}] =
+     #{info := [right, "|" | _]}, #{info := [left, "|" | _]}, #{info := [left, "!" | _]},
+     #{info := [right, "!" | _]}] =
         elvis_core_apply_rule(Config, elvis_style, operator_spaces, DefaultOptions, Path).
 
 -spec verify_no_space(config()) -> any().
@@ -478,16 +480,19 @@ verify_no_space(Config) ->
 
     Path1 = "fail_no_space." ++ Ext,
     [#{info := [right, "(", 3]},
-     #{info := [right, "(", 32]},
-     #{info := [right, "(", 48]},
-     #{info := [left, ")", 48]},
-     #{info := [left, ")", 75]},
-     #{info := [right, "(", 105]},
-     #{info := [left, ")", 105]}] =
+     #{info := [right, "(", 36]},
+     #{info := [right, "(", 52]},
+     #{info := [left, ")", 52]},
+     #{info := [left, ",", 76]},
+     #{info := [left, ")", 79]},
+     #{info := [right, "(", 109]},
+     #{info := [left, ")", 109]},
+     #{info := [right, "#", 121]},
+     #{info := [right, "?", 121]}] =
         elvis_core_apply_rule(Config,
                               elvis_style,
                               no_space,
-                              #{rules => [{right, "("}, {left, ")"}]},
+                              elvis_style:default(no_space),
                               Path1).
 
 -spec verify_no_space_after_pound(config()) -> any().
@@ -659,7 +664,8 @@ verify_no_behavior_info(Config) ->
 verify_module_naming_convention(Config) ->
     Ext = proplists:get_value(test_file_ext, Config, "erl"),
 
-    RuleConfig = #{regex => "^([a-z][a-z0-9]*_?)*$", ignore => []},
+    #{regex := DefaultRegex} = elvis_style:default(module_naming_convention),
+    RuleConfig = #{regex => DefaultRegex, ignore => []},
 
     PathPass = "pass_module_naming_convention." ++ Ext,
     [] =
@@ -669,15 +675,15 @@ verify_module_naming_convention(Config) ->
                               RuleConfig,
                               PathPass),
 
-    PathFail = "fail_module_naming_1_convention_1." ++ Ext,
+    PathFail = "fail_module_naming_1_convention_1_." ++ Ext,
     [_] =
         elvis_core_apply_rule(Config,
                               elvis_style,
                               module_naming_convention,
                               RuleConfig,
                               PathFail),
 
-    RuleConfigIgnore = RuleConfig#{ignore => [fail_module_naming_1_convention_1]},
+    RuleConfigIgnore = RuleConfig#{ignore => [fail_module_naming_1_convention_1_]},
     [] =
         elvis_core_apply_rule(Config,
                               elvis_style,
@@ -1143,14 +1149,12 @@ verify_no_debug_call(Config) ->
 
     PathFail = "fail_no_debug_call." ++ Ext,
 
-    PathFail = "fail_no_debug_call." ++ Ext,
-
     _ = case Group of
             beam_files -> % io:format is preprocessed
-                [_, _, _, _, _] =
+                [_, _, _, _, _, _] =
                     elvis_core_apply_rule(Config, elvis_style, no_debug_call, #{}, PathFail);
             erl_files ->
-                [_, _, _, _, _, _, _] =
+                [_, _, _, _, _, _, _, _] =
                     elvis_core_apply_rule(Config, elvis_style, no_debug_call, #{}, PathFail)
         end,