KEMBAR78
Naming/PredicateMethod: more detailed introspection of the method body · Issue #14256 · rubocop/rubocop · GitHub
Skip to content

Naming/PredicateMethod: more detailed introspection of the method body #14256

@zverok

Description

@zverok

Is your feature request related to a problem? Please describe.

One more idea/suggestion about Naming/PredicateName after trying to apply it to a large codebase. There is one widespread pattern that triggers the cop which feels "wrong":

def do_something
  return false unless should_be_done?

  internal_do_something

  true
end

I.e., the main thing about the method is side effects, but it also returns booleans indicating whether it has invoked something or not. It shouldn't be named like a predicate, but the cop will suggest so.

Describe the solution you'd like

How can we filter out those methods? It is not very simple but pretty straightforward:

  • starting from each return value
  • go back through statements in the method body
  • ...and check if this statement assigns some value used to calculate the return value; if not, it is not a predicate.

E.g., this is not a predicate:

def foo
  File.write('test.log', some_data) # this doesn't participate in result

  true
end

...and this is a predicate:

def foo
  data = File.read('test.log', some_data) # this is used to calculate the next line
  line = data.find { it.include?('ERROR:') } # this is used to calculate value used for result

  line.match?('not found') # start analysis from here
end

It will still miss edge cases like this (side effect is present, but doesn't look like it

def foo
  some_complicated_side_effect(lot, of, params) == :success
end

...but so will the human, so rubocop:disable in this case + appropriate comment would be helpful :)

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions