Unused method parameter in the base class implementation

[ttyusupov]

This bug is still relevant: https://sourceforge.net/p/codenarc/bugs/104

Ref: https://sourceforge.net/mailarchive/message.php?msg_id=28546328 For the code below, CodeNarc complains that A.foo() does not use method parameter x. In the base implementation, there is no need to use x, but the subclasses do. Discussion on the CodeNarc mailing list in the thread above confirmed that for the base class, the warning should not be raised. There is a way to suppress the warning, but it is not a good way to resolve the issue, because if in future someone will update B.foo to not use x - codenarc won't catch that (the rule doesn't check method annotated with @Override: http://codenarc.sourceforge.net/codenarc-rules-unused.html)

class A {
  protected foo(x) {
    // base implementation leaves x unused
  }
}

class B extends A {
  @Override
  protected foo(x) {
    // this overridden implementation uses x
  }
}

[chrismair]

Because of the way that CodeNarc processes the files, there is no good way (so far) to have knowledge about the existence of subclasses.

Do any of the existing configuration options help: http://codenarc.sourceforge.net/codenarc-rules-unused.html#UnusedMethodParameter e.g. calling the parameters ignore(d) or specifying your own ignoreRegex or ignoreClassRegex?

I don't think I understand your comment:

but it is not a good way to resolve the issue, because if in future someone will update B.foo to not use x - codenarc won't catch that (the rule doesn't check method annotated with @Override

Because the B.foo() method is annotated with @Override, it should not ever trigger a rule violation. But that is intended behavior -- in general, if you are overriding a method it shouldn't matter if the parameter is not used. So we don't want CodeNarc to catch that. Am I misunderstanding?

[ttyusupov]

What I am trying to achieve is to know when there are unused parameters which really can be removed from the code. If abstract class A has unused parameters - that doesn't mean they can be removed, because subclasses can use them, for example B.

I know how to suppress rule at level of abstract class. But that means the case when neither abstract class not any of subclasses are using parameter won't be caught, so I won't know that I can remove those parameters which are actually dead code which is not really used anywhere in the project.

[chrismair]

Okay, I understand now. Yeah, I don't think CodeNarc is able to reasonably determine/catch that situation.

[liviutudor]

My 2 cents here: the above example is far from ideal in an OOP world and I think your problem can be resolved if you do something like this:

abstract class A {
  protected foo(x) {
    // base implementation leaves x unused
    hookMethodForSubclasses(x)
  }
  abstract hookMethodForSubclasses(x)
}

class B extends A {
  @Override
  protected hookMethodForSubclasses(x) {
    // this overridden implementation finally uses x
  }
}

Would this not solve your problem @ttyusupov ?