In some cases, we pass delegates around to allow information flow from one place to the other (using properties than can be set, but need not be). However, CRR0026 ("The '$name' parameter is never used and can be deleted") still shows up in those cases (both when using a property or field). Invoking the refactoring breaks compilation, as the assignments are not implicitly possible anymore.
Example:
C#private Action<object, bool> _thisIsMarkedAsUnused;
public void Test()
{
Action<object, bool> thisIsntUnused = NotUnused;
_thisIsMarkedAsUnused = ShouldNotBeUnused;
}
private void NotUnused(object p) // no CRR0026 (which is correct)
{
Console.WriteLine("I'm not using the parameter 'p' here.");
}
private void ShouldNotBeUnused(object p) // CRR0026 (incorrect; the object parameter is required for the signature)
{
Console.WriteLine("I'm not using the parameter 'p' here either.");
}
Using a local apparently triggers a different code path than fields and properties; which might be a problem for the incorrect suggestion that the parameter can be removed.
I didn't see any issues with events at this point, but then again the only reason this isn't an event is because it returns a value and thus isn't really suited for multicast delegates.
Hi Emanuel,
Thank you for the code snippet. I agree that it's bug and we will try to fix it as soon as possible.
Not sure if you'd prefer a separate ticket for this, but I did find a problem with events (or rather, event handler).
Visual Studios CodeLens shows 1 reference, but CRR insists that the event handler is unused.
This only happens for empty event handlers and event handlers that throw exceptions (but don't do anything else).
public event EventHandler<EventArgs> EmptyHandler; public event EventHandler<EventArgs> ThrowingHandler; public void Test() { EmptyHandler += OnEmptyHandler; ThrowingHandler += OnThrowingHandler; EmptyHandler?.Invoke(this, EventArgs.Empty); ThrowingHandler?.Invoke(this, EventArgs.Empty); } private void OnEmptyHandler(object sender, EventArgs ea) // CRR0026 { // intentionally left blank } private void OnThrowingHandler(object sender, EventArgs ea) // CRR0026 { throw new NotSupportedException("This should not happen for " + sender.ToString()); }
I can get behind the first one (because doing nothing in the event handler looks strange; and in fact the only place where this happens for me has a big "TODO" next to it), but I'm not too sure about the second one. Doing anything else than just the exception in there (like a Console.WriteLine, a null-check or anything) makes the message disappear.
Use case for the second kind of event handler is a "something happened" event that should log in one place, do nothing in another but abort the action in a third location (which all use the same event raiser, but handle it differently).
It feels as if the second case should specifically check for NotImplementedException on a generated event handler that is yet to be updated with proper code; but instead triggers for any kind of exception being thrown unconditionally.
I've created a separate ticket on your behalf - "Empty Event Handler" analyzer should be implemented in CodeRush for Roslyn. It has been placed in our processing queue and will be answered shortly.