Skip to content

Consider Adding final Keyword to Core Classes for API Stability #47

@butschster

Description

@butschster

Consider Adding final Keyword to Core Classes for API Stability

Hey team! 👋

I was reviewing the codebase and noticed that most of our core classes like Registry, ServerBuilder, ReferenceHandler, etc are currently extensible (not marked as final). While this seems flexible at first glance, I'm wondering if we might want to consider a more intentional approach to inheritance.

The Challenge

Right now, since these classes can be extended, we're essentially committed to maintaining backward compatibility for any internal method changes, even if they were never intended as public APIs. This could potentially limit our ability to:

  • Refactor internal implementations for performance improvements
  • Modify private/protected method signatures
  • Optimize class structures in future versions
  • Deprecate certain approaches cleanly

What I'm Thinking

For classes that represent core business logic or data structures (rather than extension points), marking them as final might actually improve the developer experience by:

  • Clearer API contracts - Making it obvious which classes are meant to be extended vs. composed
  • Safer refactoring - Allowing us to optimize internals without breaking downstream code
  • Better performance - PHP can optimize final classes more aggressively
  • Reduced maintenance burden - No need to consider inheritance when making internal changes

Potential Approach

We could evaluate each class and ask: "Is this designed to be extended, or should developers compose with it instead?"

For example:

final class Registry        // Core business logic - compose, don't extend
final class ServerBuilder   // Builder pattern - use, don't inherit
final class ToolMetadata    // Data structure - immutable by design

// Keep extensible only where inheritance makes sense
abstract class BaseTransport // Clearly designed for extension
interface ToolExecutorInterface // Contract for implementations

Internal Implementation Classes

There's also another category we might want to consider - classes that are purely internal implementation details and aren't meant to be used by developers at all. For these, we could combine final with @internal annotations to make the boundaries crystal clear:

/**
 * @internal This class is not part of the public API and may change without notice
 */
final class InternalMessageParser
{
    // Implementation details that users shouldn't depend on
}

/**
 * @internal Used internally by Registry - do not use directly
 */
final class ToolRegistrationValidator
{
    // Internal validation logic
}

This would help developers understand:

  • Public API classes - Safe to use, stable interface
  • Extensible classes - Designed for inheritance, documented extension points
  • ⚠️ Internal classes - Implementation details, may change without notice

Having this clear distinction could really help both library users and contributors understand what's safe to depend on versus what's subject to change.

Next Steps

What do you think? I'm happy to do some research on which classes might be good candidates for each category, or we could start with new classes going forward and gradually migrate existing ones.

Would love to hear your thoughts on this approach!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions