From 1bf4f561450fa477ea1f629ec4cba879d19409af Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 1 Dec 2024 23:17:32 -0500 Subject: [PATCH 01/18] Add `Typing/MacroCallVarTypeRestriction` --- spec/ameba/base_spec.cr | 1 + .../macro_call_var_type_restriction_spec.cr | 51 +++++++++++ .../typing/macro_call_var_type_restriction.cr | 87 +++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr create mode 100644 src/ameba/rule/typing/macro_call_var_type_restriction.cr diff --git a/spec/ameba/base_spec.cr b/spec/ameba/base_spec.cr index 81fe11453..2502a0592 100644 --- a/spec/ameba/base_spec.cr +++ b/spec/ameba/base_spec.cr @@ -19,6 +19,7 @@ module Ameba::Rule Naming Performance Style + Typing ] end end diff --git a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr new file mode 100644 index 000000000..fe80b684a --- /dev/null +++ b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr @@ -0,0 +1,51 @@ +require "../../../spec_helper" + +module Ameba::Rule::Typing + subject = MacroCallVarTypeRestriction.new + + it "passes if macro call args have type restrictions" do + expect_no_issues subject, <<-CRYSTAL + class Greeter + getter name : String? + class_getter age : Int32 = 0 + setter tasks : Array(String) = [] of String + class_setter queue : Array(Int32)? + property task_mutex : Mutex = Mutex.new + class_property asdf : String + + record Task, + var1 : String, + var2 : String = "asdf" + end + CRYSTAL + end + + it "fails if a macro call arg doesn't have a type restriction" do + expect_issue subject, <<-CRYSTAL + class Greeter + getter name + # ^^^^ error: Variable arguments to `getter` should have a type restriction + end + CRYSTAL + end + + it "fails if a record call arg doesn't have a type restriction" do + expect_issue subject, <<-CRYSTAL + class Greeter + record Task, + var1 : String, + var2 = "asdf" + # ^^^^ error: Variable arguments to `record` should have a type restriction + end + CRYSTAL + end + + it "fails if a top-level record call arg doesn't have a type restriction" do + expect_issue subject, <<-CRYSTAL + record Task, + var1 : String, + var2 = "asdf" + # ^^^^ error: Variable arguments to `record` should have a type restriction + CRYSTAL + end +end diff --git a/src/ameba/rule/typing/macro_call_var_type_restriction.cr b/src/ameba/rule/typing/macro_call_var_type_restriction.cr new file mode 100644 index 000000000..1394370ed --- /dev/null +++ b/src/ameba/rule/typing/macro_call_var_type_restriction.cr @@ -0,0 +1,87 @@ +module Ameba::Rule::Typing + # A rule that enforces variable arguments to certain macros have a type restriction. + # + # For example, these are considered valid: + # + # ``` + # class Greeter + # getter name : String? + # class_getter age : Int32 = 0 + # setter tasks : Array(String) = [] of String + # class_setter queue : Array(Int32)? + # property task_mutex : Mutex = Mutex.new + # class_property asdf : String + + # record Task, + # var1 : String, + # var2 : String = "asdf" + # end + # ``` + # + # And these are considered invalid: + # + # ``` + # class Greeter + # getter name + # + # record Task, + # var1 : String, + # var2 = "asdf" + # end + # ``` + # + # YAML configuration example: + # + # ``` + # Typing/MethodParamTypeRestriction: + # Enabled: false + # MacroNames: + # - getter + # - getter? + # - getter! + # - class_getter + # - class_getter? + # - class_getter! + # - setter + # - setter? + # - setter! + # - class_setter + # - class_setter? + # - class_setter! + # - property + # - property? + # - property! + # - class_property + # - class_property? + # - class_property! + # - record + # ``` + class MacroCallVarTypeRestriction < Base + properties do + description "Recommends that variable args to certain macros have type restrictions" + enabled false + macro_names %w[ + getter getter? getter! class_getter class_getter? class_getter! + setter setter? setter! class_setter class_setter? class_setter! + property property? property! class_property class_property? class_property! + record + ] + end + + MSG = "Variable arguments to `%s` should have a type restriction" + + def test(source, node : Crystal::Call) + return unless node.name.in?(macro_names) + + node.args.each do |arg| + case arg + when Crystal::Assign + issue_for arg.target, MSG % {node.name}, prefer_name_location: true + when Crystal::Path, Crystal::TypeDeclaration # Allowed + else + issue_for arg, MSG % {node.name}, prefer_name_location: true + end + end + end + end +end From 616a9a4e6fdf4197bbae340a2161f3e7a9e217c9 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sat, 28 Dec 2024 09:08:05 -0500 Subject: [PATCH 02/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- src/ameba/rule/typing/macro_call_var_type_restriction.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ameba/rule/typing/macro_call_var_type_restriction.cr b/src/ameba/rule/typing/macro_call_var_type_restriction.cr index 1394370ed..eac759f59 100644 --- a/src/ameba/rule/typing/macro_call_var_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_var_type_restriction.cr @@ -68,7 +68,7 @@ module Ameba::Rule::Typing ] end - MSG = "Variable arguments to `%s` should have a type restriction" + MSG = "Argument should have a type restriction" def test(source, node : Crystal::Call) return unless node.name.in?(macro_names) @@ -76,10 +76,10 @@ module Ameba::Rule::Typing node.args.each do |arg| case arg when Crystal::Assign - issue_for arg.target, MSG % {node.name}, prefer_name_location: true + issue_for arg.target, MSG % node.name, prefer_name_location: true when Crystal::Path, Crystal::TypeDeclaration # Allowed else - issue_for arg, MSG % {node.name}, prefer_name_location: true + issue_for arg, MSG % node.name, prefer_name_location: true end end end From b61f85ac02115d1f1e774100386fb78d25d51b53 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sat, 28 Dec 2024 09:49:28 -0500 Subject: [PATCH 03/18] Fix specs --- .../rule/typing/macro_call_var_type_restriction_spec.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr index fe80b684a..bd6daa6b0 100644 --- a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr @@ -24,7 +24,7 @@ module Ameba::Rule::Typing expect_issue subject, <<-CRYSTAL class Greeter getter name - # ^^^^ error: Variable arguments to `getter` should have a type restriction + # ^^^^ error: Argument should have a type restriction end CRYSTAL end @@ -35,7 +35,7 @@ module Ameba::Rule::Typing record Task, var1 : String, var2 = "asdf" - # ^^^^ error: Variable arguments to `record` should have a type restriction + # ^^^^ error: Argument should have a type restriction end CRYSTAL end @@ -45,7 +45,7 @@ module Ameba::Rule::Typing record Task, var1 : String, var2 = "asdf" - # ^^^^ error: Variable arguments to `record` should have a type restriction + # ^^^^ error: Argument should have a type restriction CRYSTAL end end From f7c89692ab7292a81b4701c7ec337194a667846f Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sat, 28 Dec 2024 09:57:07 -0500 Subject: [PATCH 04/18] Add DefaultValue config option --- .../macro_call_var_type_restriction_spec.cr | 21 +++++++++++++++++++ .../typing/macro_call_var_type_restriction.cr | 7 +++++++ 2 files changed, 28 insertions(+) diff --git a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr index bd6daa6b0..a0d9b9f7c 100644 --- a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr @@ -48,4 +48,25 @@ module Ameba::Rule::Typing # ^^^^ error: Argument should have a type restriction CRYSTAL end + + context "#default_value" do + rule = MacroCallVarTypeRestriction.new + rule.default_value = false + + it "passes if a macro call arg with a default value doesn't have a type restriction" do + expect_no_issues rule, <<-CRYSTAL + class Greeter + getter name = "Kenobi" + end + CRYSTAL + end + + it "passes if a top-level record call arg with default value doesn't have a type restriction" do + expect_no_issues rule, <<-CRYSTAL + record Task, + var1 : String, + var2 = "asdf" + CRYSTAL + end + end end diff --git a/src/ameba/rule/typing/macro_call_var_type_restriction.cr b/src/ameba/rule/typing/macro_call_var_type_restriction.cr index eac759f59..524682e3e 100644 --- a/src/ameba/rule/typing/macro_call_var_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_var_type_restriction.cr @@ -30,11 +30,15 @@ module Ameba::Rule::Typing # end # ``` # + # The `DefaultValue` configuration option controls whether this rule applies to + # call arguments that have a default value. + # # YAML configuration example: # # ``` # Typing/MethodParamTypeRestriction: # Enabled: false + # DefaultValue: true # MacroNames: # - getter # - getter? @@ -60,6 +64,7 @@ module Ameba::Rule::Typing properties do description "Recommends that variable args to certain macros have type restrictions" enabled false + default_value true macro_names %w[ getter getter? getter! class_getter class_getter? class_getter! setter setter? setter! class_setter class_setter? class_setter! @@ -76,6 +81,8 @@ module Ameba::Rule::Typing node.args.each do |arg| case arg when Crystal::Assign + next unless default_value? + issue_for arg.target, MSG % node.name, prefer_name_location: true when Crystal::Path, Crystal::TypeDeclaration # Allowed else From 27f6d4638e6a5a94cd9ec50afd7a4b178133b8f3 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sat, 28 Dec 2024 19:28:22 -0500 Subject: [PATCH 05/18] Update docs --- .../typing/macro_call_var_type_restriction.cr | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/ameba/rule/typing/macro_call_var_type_restriction.cr b/src/ameba/rule/typing/macro_call_var_type_restriction.cr index 524682e3e..d082ef39c 100644 --- a/src/ameba/rule/typing/macro_call_var_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_var_type_restriction.cr @@ -1,32 +1,32 @@ module Ameba::Rule::Typing # A rule that enforces variable arguments to certain macros have a type restriction. # - # For example, these are considered valid: + # For example, these are considered invalid: # # ``` # class Greeter - # getter name : String? - # class_getter age : Int32 = 0 - # setter tasks : Array(String) = [] of String - # class_setter queue : Array(Int32)? - # property task_mutex : Mutex = Mutex.new - # class_property asdf : String - + # getter name + # # record Task, # var1 : String, - # var2 : String = "asdf" + # var2 = "asdf" # end # ``` # - # And these are considered invalid: + # And these are considered valid: # # ``` # class Greeter - # getter name - # + # getter name : String? + # class_getter age : Int32 = 0 + # setter tasks : Array(String) = [] of String + # class_setter queue : Array(Int32)? + # property task_mutex : Mutex = Mutex.new + # class_property asdf : String + # record Task, # var1 : String, - # var2 = "asdf" + # var2 : String = "asdf" # end # ``` # From ba747082ef3efa02fc47cc74c9217161aa100613 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 12:38:33 -0500 Subject: [PATCH 06/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr | 1 + src/ameba/rule/typing/macro_call_var_type_restriction.cr | 1 + 2 files changed, 2 insertions(+) diff --git a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr index a0d9b9f7c..376799ab1 100644 --- a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr @@ -1,6 +1,7 @@ require "../../../spec_helper" module Ameba::Rule::Typing + describe MacroCallVarTypeRestriction do subject = MacroCallVarTypeRestriction.new it "passes if macro call args have type restrictions" do diff --git a/src/ameba/rule/typing/macro_call_var_type_restriction.cr b/src/ameba/rule/typing/macro_call_var_type_restriction.cr index d082ef39c..ef25d2002 100644 --- a/src/ameba/rule/typing/macro_call_var_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_var_type_restriction.cr @@ -62,6 +62,7 @@ module Ameba::Rule::Typing # ``` class MacroCallVarTypeRestriction < Base properties do + since_version "1.7.0" description "Recommends that variable args to certain macros have type restrictions" enabled false default_value true From 597d59a66c4469407eb0b9adc39604ac8ba03f7f Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 13:33:06 -0500 Subject: [PATCH 07/18] Fix specs --- .../macro_call_var_type_restriction_spec.cr | 107 +++++++++--------- 1 file changed, 54 insertions(+), 53 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr index 376799ab1..928de648d 100644 --- a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr @@ -2,72 +2,73 @@ require "../../../spec_helper" module Ameba::Rule::Typing describe MacroCallVarTypeRestriction do - subject = MacroCallVarTypeRestriction.new + subject = MacroCallVarTypeRestriction.new - it "passes if macro call args have type restrictions" do - expect_no_issues subject, <<-CRYSTAL - class Greeter - getter name : String? - class_getter age : Int32 = 0 - setter tasks : Array(String) = [] of String - class_setter queue : Array(Int32)? - property task_mutex : Mutex = Mutex.new - class_property asdf : String - - record Task, - var1 : String, - var2 : String = "asdf" - end - CRYSTAL - end - - it "fails if a macro call arg doesn't have a type restriction" do - expect_issue subject, <<-CRYSTAL - class Greeter - getter name - # ^^^^ error: Argument should have a type restriction - end - CRYSTAL - end - - it "fails if a record call arg doesn't have a type restriction" do - expect_issue subject, <<-CRYSTAL - class Greeter - record Task, - var1 : String, - var2 = "asdf" - # ^^^^ error: Argument should have a type restriction - end - CRYSTAL - end + it "passes if macro call args have type restrictions" do + expect_no_issues subject, <<-CRYSTAL + class Greeter + getter name : String? + class_getter age : Int32 = 0 + setter tasks : Array(String) = [] of String + class_setter queue : Array(Int32)? + property task_mutex : Mutex = Mutex.new + class_property asdf : String - it "fails if a top-level record call arg doesn't have a type restriction" do - expect_issue subject, <<-CRYSTAL - record Task, - var1 : String, - var2 = "asdf" - # ^^^^ error: Argument should have a type restriction - CRYSTAL - end + record Task, + var1 : String, + var2 : String = "asdf" + end + CRYSTAL + end - context "#default_value" do - rule = MacroCallVarTypeRestriction.new - rule.default_value = false + it "fails if a macro call arg doesn't have a type restriction" do + expect_issue subject, <<-CRYSTAL + class Greeter + getter name + # ^^^^ error: Argument should have a type restriction + end + CRYSTAL + end - it "passes if a macro call arg with a default value doesn't have a type restriction" do - expect_no_issues rule, <<-CRYSTAL + it "fails if a record call arg doesn't have a type restriction" do + expect_issue subject, <<-CRYSTAL class Greeter - getter name = "Kenobi" + record Task, + var1 : String, + var2 = "asdf" + # ^^^^ error: Argument should have a type restriction end CRYSTAL end - it "passes if a top-level record call arg with default value doesn't have a type restriction" do - expect_no_issues rule, <<-CRYSTAL + it "fails if a top-level record call arg doesn't have a type restriction" do + expect_issue subject, <<-CRYSTAL record Task, var1 : String, var2 = "asdf" + # ^^^^ error: Argument should have a type restriction CRYSTAL end + + context "#default_value" do + rule = MacroCallVarTypeRestriction.new + rule.default_value = false + + it "passes if a macro call arg with a default value doesn't have a type restriction" do + expect_no_issues rule, <<-CRYSTAL + class Greeter + getter name = "Kenobi" + end + CRYSTAL + end + + it "passes if a top-level record call arg with default value doesn't have a type restriction" do + expect_no_issues rule, <<-CRYSTAL + record Task, + var1 : String, + var2 = "asdf" + CRYSTAL + end + end end end From a5dc5c61cdd8f5eaca0ecf62c669bde949913f6d Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 13:34:43 -0500 Subject: [PATCH 08/18] MacroCallVarTypeRestriction -> MacroCallArgumentTypeRestriction --- ...spec.cr => macro_call_argument_type_restriction_spec.cr} | 6 +++--- ...striction.cr => macro_call_argument_type_restriction.cr} | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) rename spec/ameba/rule/typing/{macro_call_var_type_restriction_spec.cr => macro_call_argument_type_restriction_spec.cr} (93%) rename src/ameba/rule/typing/{macro_call_var_type_restriction.cr => macro_call_argument_type_restriction.cr} (94%) diff --git a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr similarity index 93% rename from spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr rename to spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index 928de648d..9ba342cff 100644 --- a/spec/ameba/rule/typing/macro_call_var_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -1,8 +1,8 @@ require "../../../spec_helper" module Ameba::Rule::Typing - describe MacroCallVarTypeRestriction do - subject = MacroCallVarTypeRestriction.new + describe MacroCallArgumentTypeRestriction do + subject = MacroCallArgumentTypeRestriction.new it "passes if macro call args have type restrictions" do expect_no_issues subject, <<-CRYSTAL @@ -51,7 +51,7 @@ module Ameba::Rule::Typing end context "#default_value" do - rule = MacroCallVarTypeRestriction.new + rule = MacroCallArgumentTypeRestriction.new rule.default_value = false it "passes if a macro call arg with a default value doesn't have a type restriction" do diff --git a/src/ameba/rule/typing/macro_call_var_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr similarity index 94% rename from src/ameba/rule/typing/macro_call_var_type_restriction.cr rename to src/ameba/rule/typing/macro_call_argument_type_restriction.cr index ef25d2002..168331216 100644 --- a/src/ameba/rule/typing/macro_call_var_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -1,5 +1,5 @@ module Ameba::Rule::Typing - # A rule that enforces variable arguments to certain macros have a type restriction. + # A rule that enforces variable arguments to specific macros have a type restriction. # # For example, these are considered invalid: # @@ -60,7 +60,7 @@ module Ameba::Rule::Typing # - class_property! # - record # ``` - class MacroCallVarTypeRestriction < Base + class MacroCallArgumentTypeRestriction < Base properties do since_version "1.7.0" description "Recommends that variable args to certain macros have type restrictions" From 22b8545ba0446c3482e48b1fbf0a5612f1818873 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 13:36:28 -0500 Subject: [PATCH 09/18] Be specific about what arg types the macro call arg type restriction rule applies to --- src/ameba/rule/typing/macro_call_argument_type_restriction.cr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 168331216..2343ff903 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -85,8 +85,7 @@ module Ameba::Rule::Typing next unless default_value? issue_for arg.target, MSG % node.name, prefer_name_location: true - when Crystal::Path, Crystal::TypeDeclaration # Allowed - else + when Crystal::Var, Crystal::Call issue_for arg, MSG % node.name, prefer_name_location: true end end From 10d84ff62ae220e2d605e5b842652d3196598f40 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 17:13:09 -0500 Subject: [PATCH 10/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- ...cro_call_argument_type_restriction_spec.cr | 3 ++- .../macro_call_argument_type_restriction.cr | 22 ++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index 9ba342cff..1d939ef9f 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -50,7 +50,8 @@ module Ameba::Rule::Typing CRYSTAL end - context "#default_value" do + context "properties" do + context "#default_value" do rule = MacroCallArgumentTypeRestriction.new rule.default_value = false diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 2343ff903..38196bbf3 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -6,11 +6,11 @@ module Ameba::Rule::Typing # ``` # class Greeter # getter name - # - # record Task, - # var1 : String, - # var2 = "asdf" # end + # + # record Task, + # cmd : String, + # args = %w[] # ``` # # And these are considered valid: @@ -19,15 +19,11 @@ module Ameba::Rule::Typing # class Greeter # getter name : String? # class_getter age : Int32 = 0 - # setter tasks : Array(String) = [] of String - # class_setter queue : Array(Int32)? - # property task_mutex : Mutex = Mutex.new - # class_property asdf : String - - # record Task, - # var1 : String, - # var2 : String = "asdf" # end + # + # record Task, + # cmd : String, + # args : Array(String) = %w[] # ``` # # The `DefaultValue` configuration option controls whether this rule applies to @@ -36,7 +32,7 @@ module Ameba::Rule::Typing # YAML configuration example: # # ``` - # Typing/MethodParamTypeRestriction: + # Typing/MacroCallArgumentTypeRestriction: # Enabled: false # DefaultValue: true # MacroNames: From 3473a224238067ffde69b4d768bc20bf1fa06f96 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 17:49:13 -0500 Subject: [PATCH 11/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- .../typing/macro_call_argument_type_restriction.cr | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 38196bbf3..6aa7ac943 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -1,15 +1,17 @@ module Ameba::Rule::Typing # A rule that enforces variable arguments to specific macros have a type restriction. + # By default these macros are: `(class_)getter/setter/property(?/!)` and `record`. # # For example, these are considered invalid: # # ``` # class Greeter # getter name + # getter age = 0.days # end # # record Task, - # cmd : String, + # cmd = "", # args = %w[] # ``` # @@ -18,11 +20,11 @@ module Ameba::Rule::Typing # ``` # class Greeter # getter name : String? - # class_getter age : Int32 = 0 + # getter age : Time::Span = 0.days # end # # record Task, - # cmd : String, + # cmd : String = "", # args : Array(String) = %w[] # ``` # @@ -59,7 +61,7 @@ module Ameba::Rule::Typing class MacroCallArgumentTypeRestriction < Base properties do since_version "1.7.0" - description "Recommends that variable args to certain macros have type restrictions" + description "Recommends that call arguments to certain macros have type restrictions" enabled false default_value true macro_names %w[ @@ -80,9 +82,9 @@ module Ameba::Rule::Typing when Crystal::Assign next unless default_value? - issue_for arg.target, MSG % node.name, prefer_name_location: true + issue_for arg.target, MSG, prefer_name_location: true when Crystal::Var, Crystal::Call - issue_for arg, MSG % node.name, prefer_name_location: true + issue_for arg, MSG, prefer_name_location: true end end end From 7a52546d303dac194197cf95140c139a07b2df55 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 17:50:10 -0500 Subject: [PATCH 12/18] Fix specs --- .../macro_call_argument_type_restriction_spec.cr | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index 1d939ef9f..dc0caa213 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -52,23 +52,24 @@ module Ameba::Rule::Typing context "properties" do context "#default_value" do - rule = MacroCallArgumentTypeRestriction.new - rule.default_value = false + rule = MacroCallArgumentTypeRestriction.new + rule.default_value = false - it "passes if a macro call arg with a default value doesn't have a type restriction" do - expect_no_issues rule, <<-CRYSTAL + it "passes if a macro call arg with a default value doesn't have a type restriction" do + expect_no_issues rule, <<-CRYSTAL class Greeter getter name = "Kenobi" end CRYSTAL - end + end - it "passes if a top-level record call arg with default value doesn't have a type restriction" do - expect_no_issues rule, <<-CRYSTAL + it "passes if a top-level record call arg with default value doesn't have a type restriction" do + expect_no_issues rule, <<-CRYSTAL record Task, var1 : String, var2 = "asdf" CRYSTAL + end end end end From 13a7713af3cea1fff2a2188fe2e3a8f2e1915136 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 17:52:38 -0500 Subject: [PATCH 13/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- .../macro_call_argument_type_restriction_spec.cr | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index dc0caa213..5383ef730 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -57,18 +57,18 @@ module Ameba::Rule::Typing it "passes if a macro call arg with a default value doesn't have a type restriction" do expect_no_issues rule, <<-CRYSTAL - class Greeter - getter name = "Kenobi" - end - CRYSTAL + class Greeter + getter name = "Kenobi" + end + CRYSTAL end it "passes if a top-level record call arg with default value doesn't have a type restriction" do expect_no_issues rule, <<-CRYSTAL - record Task, - var1 : String, - var2 = "asdf" - CRYSTAL + record Task, + var1 : String, + var2 = "asdf" + CRYSTAL end end end From e8267166e65e4210eb862f2965309c317cef3004 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 17:58:29 -0500 Subject: [PATCH 14/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- ...cro_call_argument_type_restriction_spec.cr | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index 5383ef730..f7d471e4f 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -13,11 +13,11 @@ module Ameba::Rule::Typing class_setter queue : Array(Int32)? property task_mutex : Mutex = Mutex.new class_property asdf : String - - record Task, - var1 : String, - var2 : String = "asdf" end + + record Task, + cmd : String, + args : Array(String) = %w[] CRYSTAL end @@ -31,21 +31,10 @@ module Ameba::Rule::Typing end it "fails if a record call arg doesn't have a type restriction" do - expect_issue subject, <<-CRYSTAL - class Greeter - record Task, - var1 : String, - var2 = "asdf" - # ^^^^ error: Argument should have a type restriction - end - CRYSTAL - end - - it "fails if a top-level record call arg doesn't have a type restriction" do expect_issue subject, <<-CRYSTAL record Task, - var1 : String, - var2 = "asdf" + cmd : String, + args = %[] # ^^^^ error: Argument should have a type restriction CRYSTAL end From 4f105790c388a1575add89615801c614a9f7f3a9 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 18:00:29 -0500 Subject: [PATCH 15/18] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- .../typing/macro_call_argument_type_restriction_spec.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index f7d471e4f..14704fd60 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -52,11 +52,11 @@ module Ameba::Rule::Typing CRYSTAL end - it "passes if a top-level record call arg with default value doesn't have a type restriction" do + it "passes if a record call arg with default value doesn't have a type restriction" do expect_no_issues rule, <<-CRYSTAL record Task, - var1 : String, - var2 = "asdf" + cmd : String, + args = %[] CRYSTAL end end From c98597f29dbccce6481ea6215135f92ad74603f8 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 18:04:03 -0500 Subject: [PATCH 16/18] Update src/ameba/rule/typing/macro_call_argument_type_restriction.cr Co-authored-by: Sijawusz Pur Rahnama --- src/ameba/rule/typing/macro_call_argument_type_restriction.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 6aa7ac943..6d711222e 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -1,5 +1,5 @@ module Ameba::Rule::Typing - # A rule that enforces variable arguments to specific macros have a type restriction. + # A rule that enforces call arguments to specific macros have a type restriction. # By default these macros are: `(class_)getter/setter/property(?/!)` and `record`. # # For example, these are considered invalid: From afb7914dca8622fe435ceefdc064f2e3881ef426 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 29 Dec 2024 19:47:37 -0500 Subject: [PATCH 17/18] Extend to string / symbol literals in macro call args --- .../rule/typing/macro_call_argument_type_restriction_spec.cr | 4 ++++ src/ameba/rule/typing/macro_call_argument_type_restriction.cr | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index 14704fd60..84bbae51a 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -26,6 +26,10 @@ module Ameba::Rule::Typing class Greeter getter name # ^^^^ error: Argument should have a type restriction + getter :age + # ^^^^ error: Argument should have a type restriction + getter "height" + # ^^^^^^^^ error: Argument should have a type restriction end CRYSTAL end diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 6d711222e..68af45eeb 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -8,6 +8,7 @@ module Ameba::Rule::Typing # class Greeter # getter name # getter age = 0.days + # getter :height # end # # record Task, @@ -21,6 +22,7 @@ module Ameba::Rule::Typing # class Greeter # getter name : String? # getter age : Time::Span = 0.days + # getter height : Float64? # end # # record Task, @@ -83,7 +85,7 @@ module Ameba::Rule::Typing next unless default_value? issue_for arg.target, MSG, prefer_name_location: true - when Crystal::Var, Crystal::Call + when Crystal::Var, Crystal::Call, Crystal::StringLiteral, Crystal::SymbolLiteral issue_for arg, MSG, prefer_name_location: true end end From b0a924f3383c99520d5277faba035723902428f2 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Thu, 9 Jan 2025 10:07:17 -0500 Subject: [PATCH 18/18] Typing/MacroCallArgumentTypeRestriction default_value should be false ootb --- ...macro_call_argument_type_restriction_spec.cr | 17 +++++++++-------- .../macro_call_argument_type_restriction.cr | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr index 84bbae51a..47a8f92e9 100644 --- a/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr +++ b/spec/ameba/rule/typing/macro_call_argument_type_restriction_spec.cr @@ -34,33 +34,34 @@ module Ameba::Rule::Typing CRYSTAL end - it "fails if a record call arg doesn't have a type restriction" do - expect_issue subject, <<-CRYSTAL + it "passes if a record call arg with a default value doesn't have a type restriction" do + expect_no_issues subject, <<-CRYSTAL record Task, cmd : String, args = %[] - # ^^^^ error: Argument should have a type restriction CRYSTAL end context "properties" do context "#default_value" do rule = MacroCallArgumentTypeRestriction.new - rule.default_value = false + rule.default_value = true - it "passes if a macro call arg with a default value doesn't have a type restriction" do - expect_no_issues rule, <<-CRYSTAL + it "fails if a macro call arg with a default value doesn't have a type restriction" do + expect_issue rule, <<-CRYSTAL class Greeter getter name = "Kenobi" + # ^^^^ error: Argument should have a type restriction end CRYSTAL end - it "passes if a record call arg with default value doesn't have a type restriction" do - expect_no_issues rule, <<-CRYSTAL + it "fails if a record call arg with default value doesn't have a type restriction" do + expect_issue rule, <<-CRYSTAL record Task, cmd : String, args = %[] + # ^^^^ error: Argument should have a type restriction CRYSTAL end end diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 68af45eeb..fab1e2d82 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -38,7 +38,7 @@ module Ameba::Rule::Typing # ``` # Typing/MacroCallArgumentTypeRestriction: # Enabled: false - # DefaultValue: true + # DefaultValue: false # MacroNames: # - getter # - getter? @@ -65,7 +65,7 @@ module Ameba::Rule::Typing since_version "1.7.0" description "Recommends that call arguments to certain macros have type restrictions" enabled false - default_value true + default_value false macro_names %w[ getter getter? getter! class_getter class_getter? class_getter! setter setter? setter! class_setter class_setter? class_setter!