-
Notifications
You must be signed in to change notification settings - Fork 75
Description
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 implementationsInternal 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!