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

Lint/UnusedMethodArgument should be able to skip empty methods #1601

Closed
zenspider opened this issue Jan 27, 2015 · 9 comments
Closed

Lint/UnusedMethodArgument should be able to skip empty methods #1601

zenspider opened this issue Jan 27, 2015 · 9 comments

Comments

@zenspider
Copy link

When defining the interface with an abstract class, you often define methods with args but with an empty body, eg:

    ##
    # Record a result and output the Runnable#result_code. Stores the
    # result of the run if the run did not pass.

    def record result
    end

The doco describes what the argument is for. If the body is empty, assume it is an abstract method and don't lint on this arg.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 28, 2015

Makes sense. We can introduce a config option for this.

On 27 January 2015 at 02:01, Ryan Davis [email protected] wrote:

When defining the interface with an abstract class, you often define
methods with args but with an empty body, eg:

##
# Record a result and output the Runnable#result_code. Stores the
# result of the run if the run did not pass.

def record result
end

The doco describes what the argument is for. If the body is empty, assume
it is an abstract method and don't lint on this arg.


Reply to this email directly or view it on GitHub
#1601.

@agrimm
Copy link
Contributor

agrimm commented Sep 22, 2015

This problem also occurs with empty blocks, especially in auto-generated configuration scripts that have lots of commented out option setting things you can do.

@alexdowad
Copy link
Contributor

So you would prefer that this would be enabled through config, rather than being the default?

I find warnings about "unused parameters" in a empty method just add noise.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 29, 2015

Well, empty or not, they are unused and sometimes this might be accidental instead of intended - e.g. you got distracted and forgot to write the body of the method. So, yes - I'm fine with supporting this, but it should be configurable. Frankly, I think this is shortcoming of Ruby's design - probably it would have been better if abstract methods could be denoted by some manner.

As for empty blocks - it seems to me that an empty block usually doesn't make much sense, right?

@alexdowad
Copy link
Contributor

Very well. I can write up a PR. Any idea what the config parameter for Lint/UnusedMethodArgument should be called?

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 29, 2015

IgnoreEmptyMethods and IgnoreEmptyBlocks.

@alexdowad
Copy link
Contributor

You got it.

@alexdowad
Copy link
Contributor

Meaning, "OK, that is fine, you will have it very soon".

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 29, 2015

10x :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants