Event handler declared Public, exposing it as part of the type’s API

ID

vbnet.maintainability.review_visible_event_handlers

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Api Design

Language

VB.NET

Description

Reports a Public method whose parameter list is exactly the event-handler signature - a first parameter typed Object and a second parameter whose type name ends in EventArgs, with ByVal and a System. prefix accepted. Handlers declared Protected, Private or Friend are not reported: Protected is what the WebForms designer generates and is the normal shape. A method that implements an interface member is not reported either, since its visibility is fixed by the contract - unless its body is empty, in which case the Implements clause is not visible to the check and the handler is reported. Neither is a method that takes further parameters after the event arguments, which cannot be an event handler.

Rationale

Nothing in the framework needs a handler to be public. Handles clauses, AddHandler and the designer-generated wiring all bind to Private and Protected methods, so Public is a decision about the type’s API rather than about how the event is delivered - and it makes a promise the type cannot keep. A handler assumes the state its event guarantees: the control that raised it, a page part-way through its lifecycle, view state already loaded, a specific EventArgs instance carrying the command name or the cancel flag. Once the method is public, any code in any assembly can call it directly with Nothing as sender and a freshly constructed EventArgs, and the handler runs with none of those guarantees and no compiler complaint. On an ASP.NET page or a form this is a reachable path into logic that was written for one caller, and in an assembly consumed by others it means a save, a delete or a redirect can be triggered without going through the control that was supposed to authorize it. Public visibility also freezes the wiring into the contract. Handler names carry the Control_Event shape, which is an implementation detail of how this form binds its controls; published as API, that name and signature can no longer be changed - not when the control is renamed, not when the event is re-routed - without breaking a caller. Reviewers reading the type see a member that looks like a service operation and is not one, which is how handlers end up called from ordinary code paths in the first place.

The following code illustrates the pattern detected by this rule:

Public Class OrdersPage
    Inherits Page

    ' FLAGGED: Event handler declared Public, exposing it as part of the type's API
    Public Sub btnSave_Click(sender As Object, e As EventArgs) Handles btnSave.Click
        SaveOrder()
    End Sub

Remediation

Reduce the handler to Private, or to Protected where a derived page or form is expected to override or reuse it - the Handles clause and AddHandler bind either way. When some of the handler’s work genuinely belongs to the type’s API, move that work into a separate public method with parameters of its own and have the handler call it, so callers get a meaningful entry point and the handler keeps its event-only contract.

' Before: callable by anyone, with a fabricated sender and event arguments
Public Sub btnSave_Click(sender As Object, e As EventArgs) Handles btnSave.Click
    Persist(BuildOrderFromForm())
End Sub

' After: the event path stays private, the operation is the API
Private Sub btnSave_Click(sender As Object, e As EventArgs) Handles btnSave.Click
    SaveOrder(BuildOrderFromForm())
End Sub

Public Sub SaveOrder(order As Order)
    Persist(order)
End Sub

Configuration

This detector does not need any configuration.

References