-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Open
Labels
feature requestgood first issueEasy task, suitable for newcomers to the projectEasy task, suitable for newcomers to the project
Description
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
endI.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
endIt 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 :)
grantbdev
Metadata
Metadata
Assignees
Labels
feature requestgood first issueEasy task, suitable for newcomers to the projectEasy task, suitable for newcomers to the project