disallow multi-lines endless methods

[elfassy]

I think we should disallow endless methods for a few reasons

def shopify_payments_has_record_of_ssn? = @legal_entity_information&.ssn.present?

# vs

def shopify_payments_has_record_of_ssn?
  @legal_entity_information&.ssn.present?
end

• changes the history of the method declaration on every update
• needs to be converted to a regular method once more code gets added
• easier to introduce bugs (for example if the def is missing it just becomes a variable assignment)
• inconsistent method declaration

cop for this https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cop/style/endless_method.rb

[sambostock]

Hmm... I'm personally a fan of endless method definitions, and find them to be a good fit for simple methods, especially in projects not using Sorbet.

# contrived example
class Service
end

class MySQL < Service
  def initialize(db_name, user, pass)
    @db_name = db_name
    @user = user
    @pass = pass
  end

  def name = "MySQL"
  def port = 3306
  def url = "mysql://localhost:#{port}"
  def connection_string = "#{url}/#{@db_name}?user=#{@user}&password=#{@pass}"
end

class Redis < Service
  def initialize(db_name, pass)
    @db_name = db_name
    @pass = pass
  end

  def name = "Redis"
  def port = 6379
  def url = "redis://localhost:#{port}"
  def connection_string = "#{url}/#{@db_name}?password=#{@pass}"
end

Are we seeing many people make mistakes with them?

[sambostock]

Specifically with regards to

if the def is missing it just becomes a variable assignment

Lint/UselessAssignment would probably catch most occurrences.

[nunosilva800]

I think the default of allow_single_line is good enough. Endless methods can be beneficial to the code readability due to this conciseness (even though personally I'm still not used to see it).

I'd like to challenge your motives:

changes the history of the method declaration on every update

If the method body is being changed often, could it be that there is some abstraction missing? Maybe the method should not be defined there in the first place? Maybe the single line of code is trying to be "too smart" instead of preferring multiple simple statements (like using guard clauses to return early), etc etc.
Point being: short method definitions like this should be simple and with low churn.

needs to be converted to a regular method once more code gets added

Sure but same can be said for attr_reader or attr_writer - sometimes you must keep the public interface, but need to change behaviour.

easier to introduce bugs (for example if the def is missing it just becomes a variable assignment)

Unit testing would surely catch this if the endless method is public, and if it's private I agree with @sambostock that there is a better linter to detect that.

inconsistent method declaration

Agree with this. It feels weird to me, but I know I need time for my eyes to get used to it.

[elfassy]

@sambostock I notice you're preferring single line methods to constants. Wouldn't we rather folks leveraging constants where they can?

class MySQL < Service
  DB_NAME = "MySQL"
  DB_PORT = 3306
  DB_URL = "mysql://localhost:#{DB_PORT}"
  
  def initialize(db_name, user, pass)
    @db_name = db_name
    @user = user
    @pass = pass
  end

  def connection_string
    "#{DB_URL}/#{@db_name}?user=#{@user}&password=#{@pass}"
  end
end

Point being: short method definitions like this should be simple and with low churn.

A lot can be done in a single line in ruby... There's a risk we'll start seeing more and more business logic wrapped in a single line example1, example2

Are we seeing many people make mistakes with them?

Not mistakes per say, but more using them in a non-organized mixed in with other methods way.

def foo
  ...
end
 
def bar = foo
 
def foobar= value
   ...
end

[elfassy]

I think the default of allow_single_line is good enough.

I agree it's a good place to start. We can set some tripwires and get more strict later as needed.

[elfassy]

This PR is ready for another look. Leaving the default EnforcedStyle: allow_single_line

[matthuhiggins]

While our team uses endless methods sparingly, and avoid multiple lines of logic, we often use this syntax:

def foo = {
  faz: "baz",
  hello: "world",
}