search

seattlerb / debride

Analyze code for potentially uncalled / dead methods, now with auto-removal.
https://www.zenspider.com/projects/debride.html
720 stars 19 forks source link

Characters before methods are throwing debride off #3

Closed ianlotinsky closed 9 years ago

ianlotinsky commented 9 years ago

debride would not consider the following examples uses of some_method:

before_filter :some_method
some_variable =some_method
zenspider commented 9 years ago

:some_method is not a call, it is a symbol. So it isn't throwing debride off because debride doesn't look for symbols.

The =some_method one should NOT be throwing it off. Can you provide a reproduction?

ianlotinsky commented 9 years ago

(I'm really sorry for being so terse with my first comment. That wasn't helpful. My mom taught me better!)

Yes, the first example is a symbol, but much of Ruby meta programming relies on passing-around method names as symbols: ActiveModel::Callbacks, ActionController::Filters::ClassMethods, alias, alias_method, etc. It looks like you've recognized this by supporting alias_method_chain.

How would you envision a patch for this sort of support?

Ignore the second example. That was in a HAML template ;)

Thanks.

zenspider commented 9 years ago

I'm open to it, but I'm pretty certain there's got to be a better way than just assuming all symbols are methods.

I was thinking I'd provide a monkeypatch to wrap up __send__ and have it log all messages to a file. Then a human can come along and review the list and make a whitelist. Debride can then take a whitelist that prepopulates the called set. This is error prone too, but there is no perfect solution for a language like ruby.

ianlotinsky commented 9 years ago

Sorry, to be clear: I was envisioning a whitelist of method names that take method-name-symbols as params. We would start with a static list with such known methods for starters.

On Mar 17, 2015, at 8:51 PM, Ryan Davis notifications@github.com wrote:

I'm open to it, but I'm pretty certain there's got to be a better way that just assuming all symbols are methods.

I was thinking I'd provide a monkeypatch to wrap up send and have it log all messages to a file. Then a human can come along and review the list and make a whitelist. Debride can then take a whitelist that prepopulates the called set. This is error prone too, but there is no perfect solution for a language like ruby.

— Reply to this email directly or view it on GitHub.

zenspider commented 9 years ago

Whitelisting has been added. If that covers your issues, please close.

ianlotinsky commented 9 years ago

I'll close this issue since it's a misnomer and join the discussion on https://github.com/seattlerb/debride/issues/5