RuboCop Ruby Style Guide - Rev: 2022-08-24 58f7c09

# bad - four spaces
def some_method
    do_something
end

# good
def some_method
  do_something
end

# bad
puts 'foobar'; # superfluous semicolon

# good
puts 'foobar'

# bad
puts 'foo'; puts 'bar' # two expressions on the same line

# good
puts 'foo'
puts 'bar'

puts 'foo', 'bar' # this applies to puts in particular

# bad
sum=1+2
a,b=1,2
class FooError<StandardError;end

# good
sum = 1 + 2
a, b = 1, 2
class FooError < StandardError; end

# bad
e = M * c ** 2

# good
e = M * c**2

# bad
o_scale = 1 / 48r

# good
o_scale = 1/48r

# bad
foo &. bar
foo &.bar
foo&. bar

# good
foo&.bar

# bad
some( arg ).other
[ 1, 2, 3 ].each{|e| puts e}

# good
some(arg).other
[1, 2, 3].each { |e| puts e }

# good - space after { and before }
{ one: 1, two: 2 }

# good - no space after { and before }
{one: 1, two: 2}

# bad
"From: #{ user.first_name }, #{ user.last_name }"

# good
"From: #{user.first_name}, #{user.last_name}"

# bad
! something

# good
!something

# bad
1 .. 3
'a' ... 'z'

# good
1..3
'a'...'z'

# bad
case
  when song.name == 'Misty'
    puts 'Not again!'
  when song.duration > 120
    puts 'Too long!'
  when Time.now.hour > 21
    puts "It's too late"
  else
    song.play
end

# good
case
when song.name == 'Misty'
  puts 'Not again!'
when song.duration > 120
  puts 'Too long!'
when Time.now.hour > 21
  puts "It's too late"
else
  song.play
end

# bad - pretty convoluted
kind = case year
when 1850..1889 then 'Blues'
when 1890..1909 then 'Ragtime'
when 1910..1929 then 'New Orleans Jazz'
when 1930..1939 then 'Swing'
when 1940..1950 then 'Bebop'
else 'Jazz'
end

result = if some_cond
  calc_something
else
  calc_something_else
end

# good - it's apparent what's going on
kind = case year
       when 1850..1889 then 'Blues'
       when 1890..1909 then 'Ragtime'
       when 1910..1929 then 'New Orleans Jazz'
       when 1930..1939 then 'Swing'
       when 1940..1950 then 'Bebop'
       else 'Jazz'
       end

result = if some_cond
           calc_something
         else
           calc_something_else
         end

# good (and a bit more width efficient)
kind =
  case year
  when 1850..1889 then 'Blues'
  when 1890..1909 then 'Ragtime'
  when 1910..1929 then 'New Orleans Jazz'
  when 1930..1939 then 'Swing'
  when 1940..1950 then 'Bebop'
  else 'Jazz'
  end

result =
  if some_cond
    calc_something
  else
    calc_something_else
  end

# bad
def some_method
  data = initialize(options)
  data.manipulate!
  data.result
end
def some_other_method
  result
end

# good
def some_method
  data = initialize(options)

  data.manipulate!

  data.result
end

def some_other_method
  result
end

# bad - It has two empty lines.
some_method


some_method

# good
some_method

some_method

# bad
class Foo
  attr_reader :foo
  def foo
    # do something...
  end
end

# good
class Foo
  attr_reader :foo

  def foo
    # do something...
  end
end

# bad
class Foo
  def bar; end
  private
  def baz; end
end

# good
class Foo
  def bar; end

  private

  def baz; end
end

# bad
class Foo

  def foo

    begin

      do_something do

        something

      end

    rescue

      something

    end

    true

  end

end

# good
class Foo
  def foo
    begin
      do_something do
        something
      end
    rescue
      something
    end
  end
end

# bad - easier to move/add/remove parameters, but still not preferred
some_method(
  size,
  count,
  color,
)

# bad
some_method(size, count, color, )

# good
some_method(size, count, color)

# bad
def some_method(arg1=:default, arg2=nil, arg3=[])
  # do something...
end

# good
def some_method(arg1 = :default, arg2 = nil, arg3 = [])
  # do something...
end

# bad (\ is not needed here)
result = 1 - \
         2

# bad (\ is required, but still ugly as hell)
result = 1 \
         - 2

# good
result = 1 -
         2

long_string = 'First part of the long string' \
              ' and second part of the long string'

# starting point (line is too long)
def send_mail(source)
  Mailer.deliver(to: 'bob@example.com', from: 'us@example.com', subject: 'Important message', body: source.text)
end

# bad (double indent)
def send_mail(source)
  Mailer.deliver(
      to: 'bob@example.com',
      from: 'us@example.com',
      subject: 'Important message',
      body: source.text)
end

# good
def send_mail(source)
  Mailer.deliver(to: 'bob@example.com',
                 from: 'us@example.com',
                 subject: 'Important message',
                 body: source.text)
end

# good (normal indent)
def send_mail(source)
  Mailer.deliver(
    to: 'bob@example.com',
    from: 'us@example.com',
    subject: 'Important message',
    body: source.text
  )
end

# bad
user.set({ name: 'John', age: 45, permissions: { read: true } })

# good
user.set(name: 'John', age: 45, permissions: { read: true })

class Person < ActiveRecord::Base
  # bad
  validates(:name, { presence: true, length: { within: 1..10 } })

  # good
  validates :name, presence: true, length: { within: 1..10 }
end

# bad
puts (x + y)

# good
puts(x + y)

# bad
collection [index_or_key]

# good
collection[index_or_key]

# bad - single indent
menu_item = %w[Spam Spam Spam Spam Spam Spam Spam Spam
  Baked beans Spam Spam Spam Spam Spam]

# good
menu_item = %w[
  Spam Spam Spam Spam Spam Spam Spam Spam
  Baked beans Spam Spam Spam Spam Spam
]

# good
menu_item =
  %w[Spam Spam Spam Spam Spam Spam Spam Spam
     Baked beans Spam Spam Spam Spam Spam]

# bad - identifier is a Bulgarian word, using non-ascii (Cyrillic) characters
заплата = 1_000

# bad - identifier is a Bulgarian word, written with Latin letters (instead of Cyrillic)
zaplata = 1_000

# good
salary = 1_000

# bad
:'some symbol'
:SomeSymbol
:someSymbol

someVar = 5

def someMethod
  # some code
end

def SomeMethod
  # some code
end

# good
:some_symbol

some_var = 5

def some_method
  # some code
end

# bad
:some_sym_1

some_var_1 = 1

var_10 = 10

def some_method_1
  # some code
end

# good
:some_sym1

some_var1 = 1

var10 = 10

def some_method1
  # some code
end

# bad
class Someclass
  # some code
end

class Some_Class
  # some code
end

class SomeXml
  # some code
end

class XmlSomething
  # some code
end

# good
class SomeClass
  # some code
end

class SomeXML
  # some code
end

class XMLSomething
  # some code
end

# bad
SomeConst = 5

# good
SOME_CONST = 5

# bad
def even(value)
end

# good
def even?(value)
end

# bad
class Person
  def is_tall?
    true
  end

  def can_play_basketball?
    false
  end

  def does_like_candy?
    true
  end
end

# good
class Person
  def tall?
    true
  end

  def basketball_player?
    false
  end

  def likes_candy?
    true
  end
end

# bad - there is no matching 'safe' method
class Person
  def update!
  end
end

# good
class Person
  def update
  end
end

# good
class Person
  def update!
  end

  def update
  end
end

class Array
  def flatten_once!
    res = []

    each do |e|
      [*e].each { |f| res << f }
    end

    replace(res)
  end

  def flatten_once
    dup.flatten_once!
  end
end

# bad
result = hash.map { |k, v| v + 1 }

def something(x)
  unused_var, used_var = something_else(x)
  # some code
end

# good
result = hash.map { |_k, v| v + 1 }

def something(x)
  _unused_var, used_var = something_else(x)
  # some code
end

# good
result = hash.map { |_, v| v + 1 }

def something(x)
  _, used_var = something_else(x)
  # some code
end

# good
def +(other)
  # body omitted
end

# bad
def <<(other)
  @internal << other
end

# good
def <<(item)
  @internal << item
end

# bad
# Returns some string multiplied `other` times
def *(other)
  # body omitted
end

# good
# Returns some string multiplied `num` times
def *(num)
  # body omitted
end

arr = [1, 2, 3]

# bad
for elem in arr do
  puts elem
end

# note that elem is accessible outside of the for loop
elem # => 3

# good
arr.each { |elem| puts elem }

# elem is not accessible outside each block
elem # => NameError: undefined local variable or method `elem'

# bad
if some_condition then
  # body omitted
end

# bad
case foo
when bar then
  # body omitted
end

# bad
case expression
in pattern then
  # body omitted
end

# good
if some_condition
  # body omitted
end

# good
case foo
when bar
  # body omitted
end

# good
case expression
in pattern
  # body omitted
end

# bad
if
  some_condition
  do_something
  do_something_else
end

# good
if some_condition
  do_something
  do_something_else
end

# bad
result = if some_condition then something else something_else end

# good
result = some_condition ? something : something_else

# bad
some_condition ? (nested_condition ? nested_something : nested_something_else) : something_else

# good
if some_condition
  nested_condition ? nested_something : nested_something_else
else
  something_else
end

# bad
result = if some_condition; something else something_else end

# good
result = some_condition ? something : something_else

# bad
if status == :active
  perform_action
elsif status == :inactive || status == :hibernating
  check_timeout
else
  final_action
end

# good
case status
when :active
  perform_action
when :inactive, :hibernating
  check_timeout
else
  final_action
end

# bad
if condition
  result = x
else
  result = y
end

# good
result =
  if condition
    x
  else
    y
  end

# bad
case expression
in pattern; do_something
end

# good
case expression
in pattern then do_something
end

# bad - parentheses are required because of op precedence
x = (not something)

# good
x = !something

# bad
x = 'test'
# obscure nil check
if !!x
  # body omitted
end

# good
x = 'test'
if x
  # body omitted
end

# good
def named?
  !name.nil?
end

# good
def banned?
  !!banned_until&.future?
end

module Utilities
  class Queue
  end
end

# bad
class Utilities::Store
  Module.nesting # => [Utilities::Store]

  def initialize
    # Refers to the top level ::Queue class because Utilities isn't in the
    # current nesting chain.
    @queue = Queue.new
  end
end

# good
module Utilities
  class WaitingList
    Module.nesting # => [Utilities::WaitingList, Utilities]

    def initialize
      @queue = Queue.new # Refers to Utilities::Queue
    end
  end
end

# bad
class SomeClass
  def self.some_method
    # body omitted
  end

  def self.some_other_method
    # body omitted
  end
end

# good
module SomeModule
  module_function

  def some_method
    # body omitted
  end

  def some_other_method
    # body omitted
  end
end

# bad
module Utilities
  extend self

  def parse_something(string)
    # do stuff here
  end

  def other_utility_method(number, string)
    # do some more stuff
  end
end

# good
module Utilities
  module_function

  def parse_something(string)
    # do stuff here
  end

  def other_utility_method(number, string)
    # do some more stuff
  end
end

class Person
  attr_reader :first_name, :last_name

  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end

  def to_s
    "#{first_name} #{last_name}"
  end
end

# bad
class Person
  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end

  def first_name
    @first_name
  end

  def last_name
    @last_name
  end
end

# good
class Person
  attr_reader :first_name, :last_name

  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end
end

# bad
class Person
  def get_name
    "#{@first_name} #{@last_name}"
  end

  def set_name(name)
    @first_name, @last_name = name.split(' ')
  end
end

# good
class Person
  def name
    "#{@first_name} #{@last_name}"
  end

  def name=(name)
    @first_name, @last_name = name.split(' ')
  end
end

# bad - creates a single attribute accessor (deprecated in Ruby 1.9)
attr :something, true
attr :one, :two, :three # behaves as attr_reader

# good
attr_accessor :something
attr_reader :one, :two, :three

# good
class Person
  attr_accessor :first_name, :last_name

  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end
end

# better
Person = Struct.new(:first_name, :last_name) do
end

# bad
class Person < Struct.new(:first_name, :last_name)
end

# good
Person = Struct.new(:first_name, :last_name)

# bad
class Animal
  # abstract method
  def speak
  end
end

# extend superclass
class Duck < Animal
  def speak
    puts 'Quack! Quack'
  end
end

# extend superclass
class Dog < Animal
  def speak
    puts 'Bau! Bau!'
  end
end

# good
class Duck
  def speak
    puts 'Quack! Quack'
  end
end

class Dog
  def speak
    puts 'Bau! Bau!'
  end
end

class Parent
  @@class_var = 'parent'

  def self.print_class_var
    puts @@class_var
  end
end

class Child < Parent
  @@class_var = 'child'
end

Parent.print_class_var # => will print 'child'

# good
class SomeClass
  def public_method
    # some code
  end

  private

  def private_method
    # some code
  end

  def another_private_method
    # some code
  end
end

class TestClass
  # bad
  def TestClass.some_method
    # body omitted
  end

  # good
  def self.some_other_method
    # body omitted
  end

  # Also possible and convenient when you
  # have to define many class methods.
  class << self
    def first_method
      # body omitted
    end

    def second_method_etc
      # body omitted
    end
  end
end

class Westerner
  def first_name
    @names.first
  end

  alias given_name first_name
end

class Fugitive < Westerner
  def first_name
    'Nobody'
  end
end

class Fugitive < Westerner
  def first_name
    'Nobody'
  end

  alias given_name first_name
end

module Mononymous
  def self.included(other)
    other.class_eval { alias_method :full_name, :given_name }
  end
end

class Sting < Westerner
  include Mononymous
end

class TestClass
  # bad -- more work when class renamed/method moved
  def self.call(param1, param2)
    TestClass.new(param1).call(param2)
  end

  # bad -- more verbose than necessary
  def self.call(param1, param2)
    self.new(param1).call(param2)
  end

  # good
  def self.call(param1, param2)
    new(param1).call(param2)
  end

  # ...other methods...
end

# bad - FILES_TO_LINT is now defined globally
task :lint do
  FILES_TO_LINT = Dir['lib/*.rb']
  # ...
end

# good - files_to_lint is only defined inside the block
task :lint do
  files_to_lint = Dir['lib/*.rb']
  # ...
end

class Person
  def self.create(options_hash)
    # body omitted
  end
end

# bad
def initialize
  @x ||= 1
end

# good
def initialize
  @x = 1
end

# bad

x = BuggyClass.something.dup

def compute_dependency_graph
  ...30 lines of recursive graph merging...
end

# good

# BuggyClass returns an internal object, so we have to dup it to modify it.
x = BuggyClass.something.dup

# This is algorithm 6.4(a) from Worf & Yar's _Amazing Graph Algorithms_ (2243).
def compute_dependency_graph
  ...30 lines of recursive graph merging...
end

# bad
counter += 1 # Increments counter by one.

# bad
def bar
  baz(:quux) # FIXME: This has crashed occasionally since v3.2.1.
end

# good
def bar
  # FIXME: This has crashed occasionally since v3.2.1.
  baz(:quux)
end

# bad
def bar
  # FIXME This has crashed occasionally since v3.2.1.
  baz(:quux)
end

# good
def bar
  # FIXME: This has crashed occasionally since v3.2.1.
  baz(:quux)
end

def bar
  # FIXME: This has crashed occasionally since v3.2.1. It may
  #   be related to the BarBazUtil upgrade.
  baz(:quux)
end

def bar
  sleep 100 # OPTIMIZE
end

# bad
# Some documentation about Person

# frozen_string_literal: true
class Person
end

# good
# frozen_string_literal: true

# Some documentation about Person
class Person
end

# bad
# frozen_string_literal: true
#!/usr/bin/env ruby

App.parse(ARGV)

# good
#!/usr/bin/env ruby
# frozen_string_literal: true

App.parse(ARGV)

# bad
# -*- frozen_string_literal: true; encoding: ascii-8bit -*-

# good
# frozen_string_literal: true
# encoding: ascii-8bit

# bad
# frozen_string_literal: true
# Some documentation for Person
class Person
  # Some code
end

# good
# frozen_string_literal: true

# Some documentation for Person
class Person
  # Some code
end

# bad
arr = Array.new
hash = Hash.new

# good
arr = []
arr = Array.new(10)
hash = {}
hash = Hash.new(0)

# bad
STATES = ['draft', 'open', 'closed']

# good
STATES = %w[draft open closed]

# bad
STATES = [:draft, :open, :closed]

# good
STATES = %i[draft open closed]

# bad - easier to move/add/remove items, but still not preferred
VALUES = [
           1001,
           2020,
           3333,
         ]

# bad
VALUES = [1001, 2020, 3333, ]

# good
VALUES = [1001, 2020, 3333]

arr = []
arr[100] = 1 # now you have an array with lots of nils

# bad
hash = { 'one' => 1, 'two' => 2, 'three' => 3 }

# good
hash = { one: 1, two: 2, three: 3 }

# bad
hash = { :one => 1, :two => 2, :three => 3 }

# good
hash = { one: 1, two: 2, three: 3 }

# bad
hash = { one: one, two: two, three: three }

# good
hash = { one:, two:, three: }

# bad
[1, 2, one: 1, two: 2]

# good
[1, 2, { one: 1, two: 2 }]

# bad
{ a: 1, 'b' => 2 }

# good
{ :a => 1, 'b' => 2 }

# bad
Hash[ary]
Hash[a, b, c, d]

# good
ary.to_h
{a => b, c => d}

# bad
hash.has_key?(:test)
hash.has_value?(value)

# good
hash.key?(:test)
hash.value?(value)

# bad
hash.keys.each { |k| p k }
hash.values.each { |v| p v }
hash.each { |k, _v| p k }
hash.each { |_k, v| p v }

# good
hash.each_key { |k| p k }
hash.each_value { |v| p v }

heroes = { batman: 'Bruce Wayne', superman: 'Clark Kent' }
# bad - if we make a mistake we might not spot it right away
heroes[:batman] # => 'Bruce Wayne'
heroes[:supermann] # => nil

# good - fetch raises a KeyError making the problem obvious
heroes.fetch(:supermann)

batman = { name: 'Bruce Wayne', is_evil: false }

# bad - if we just use || operator with falsey value we won't get the expected result
batman[:is_evil] || true # => true

# good - fetch works correctly with falsey values
batman.fetch(:is_evil, true) # => false