I am not sure what to do about these cases. Should we make the args nullable in all classes that have OnMouseWheel instead?

By convention e can't be null, all event handlers are annotated as (object? sender, EventArgs e).
I've searched the codebase for protected override void On.*(\wEventArgs .*) regex pattern, and we have 322 methods that consume derived eventargs. I haven't checked all 322 instances, but those I checked - don't null-check e (e.g., painting, keyboard or mouse handling).

This said, I'm a bit split about this, as removing the null-check is a technically a breaking change. I'm going to take this to the team discussion.

Any more thoughts about this? Perhaps the safest thing to do for now is just keep the null forgiving operators we have in the PR in order to avoid any behavioral changes?

Apologies, it looks like I have failed to respond after this was discussed at a team meeting, and now I'm scratching my head trying to page back the decision.

If I recall correctly, we agreed on the following:

1. We can't assume something we know may be a null as "not null".
2. We can't throw a ANE here as it would be a behaviour breaking change.
3. We should throw a ANE in the base, as it would be within the "acceptable" breaking change rules - i.e., throwing a ANE instead of a NRE:
   

   winforms/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs

   Lines 8582 to 8585 in d121a04

   	protected virtual void OnMouseWheel(MouseEventArgs e)
   	{
   	((MouseEventHandler?)Events[s_mouseWheelEvent])?.Invoke(this, e);
   	}

Though, testing it locally it doesn't appear OnMouseWheel(null!) throws anything. 🤔

@JeremyKuhne am I misremembering something?

You have this right, @RussKie. We generally don't want to introduce ANE in existing APIs with the specific exception of when we know we're about to throw NRE (in the same method).

Hmm, so what should be the next step? If I change this to be nullable, I suspect that there will be some places where we should add null checks because we override this in classes that inherit from Control. If I change it to throw an ANE, I suspect that there will be changes in tests and therefore behavior. I still think that using ! is probably the safest option for now.

This one is a headscratcher, isn't it? :)

I think it's probably acceptable to use ! here with an added comment (like we've done in other instances) stating that this has been like this for a very long time, and we believe this should never be null. If/when we get a customer feedback we can review this, if required.

@RussKie I have added the comments. Perhaps we could merge this now?