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

Add Naming/AccessorMethodName rule #415

Merged
merged 1 commit into from
Nov 9, 2023
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
93 changes: 93 additions & 0 deletions spec/ameba/rule/naming/accessor_method_name_spec.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
require "../../../spec_helper"

module Ameba::Rule::Naming
subject = AccessorMethodName.new

describe AccessorMethodName do
it "passes if accessor method name is correct" do
expect_no_issues subject, <<-CRYSTAL
class Foo
def self.instance
end

def self.instance=(value)
end

def user
end

def user=(user)
end
end
CRYSTAL
end

it "passes if accessor method is defined in top-level scope" do
expect_no_issues subject, <<-CRYSTAL
def get_user
end

def set_user(user)
end
CRYSTAL
end

it "fails if accessor method is defined with receiver in top-level scope" do
expect_issue subject, <<-CRYSTAL
def Foo.get_user
# ^^^^^^^^ error: Favour method name 'user' over 'get_user'
end

def Foo.set_user(user)
# ^^^^^^^^ error: Favour method name 'user=' over 'set_user'
end
CRYSTAL
end

it "fails if accessor method name is wrong" do
expect_issue subject, <<-CRYSTAL
class Foo
def self.get_instance
# ^^^^^^^^^^^^ error: Favour method name 'instance' over 'get_instance'
end

def self.set_instance(value)
# ^^^^^^^^^^^^ error: Favour method name 'instance=' over 'set_instance'
end

def get_user
# ^^^^^^^^ error: Favour method name 'user' over 'get_user'
end

def set_user(user)
# ^^^^^^^^ error: Favour method name 'user=' over 'set_user'
end
end
CRYSTAL
end

it "ignores if alternative name isn't valid syntax" do
expect_no_issues subject, <<-CRYSTAL
class Foo
def get_404
end

def set_404(value)
end
end
CRYSTAL
end

it "ignores if the method has unexpected arity" do
expect_no_issues subject, <<-CRYSTAL
class Foo
def get_user(type)
end

def set_user(user, type)
end
end
CRYSTAL
end
end
end
90 changes: 90 additions & 0 deletions src/ameba/rule/naming/accessor_method_name.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
module Ameba::Rule::Naming
# A rule that makes sure that accessor methods are named properly.
#
# Favour this:
#
# ```
# class Foo
# def user
# @user
# end
#
# def user=(value)
# @user = value
# end
# end
# ```
#
# Over this:
#
# ```
# class Foo
# def get_user
# @user
# end
#
# def set_user(value)
# @user = value
# end
# end
# ```
#
# YAML configuration example:
#
# ```
# Naming/AccessorMethodName:
# Enabled: true
# ```
class AccessorMethodName < Base
include AST::Util

properties do
description "Makes sure that accessor methods are named properly"
end

MSG = "Favour method name '%s' over '%s'"

def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
defs =
case body = node.body
when Crystal::Def
[body]
when Crystal::Expressions
body.expressions.select(Crystal::Def)
end

defs.try &.each do |def_node|
# skip defs with explicit receiver, as they'll be handled
# by the `test(source, node : Crystal::Def)` overload
check_issue(source, def_node) unless def_node.receiver
end
end

def test(source, node : Crystal::Def)
# check only defs with explicit receiver (`def self.foo`)
check_issue(source, node) if node.receiver
end

private def check_issue(source, node : Crystal::Def)
location = name_location(node)
end_location = name_end_location(node)

case node.name
when /^get_([a-z]\w*)$/
return unless node.args.empty?
if location && end_location
issue_for location, end_location, MSG % {$1, node.name}
else
issue_for node, MSG % {$1, node.name}
end
when /^set_([a-z]\w*)$/
return unless node.args.size == 1
if location && end_location
issue_for location, end_location, MSG % {"#{$1}=", node.name}
else
issue_for node, MSG % {"#{$1}=", node.name}
end
end
end
end
end