Skip to content

refactor: Change BaseAuthToken and BaseValidator to abstract - #1379

Open
samsonasik wants to merge 2 commits into
codeigniter4:developfrom
samsonasik:base-abstact
Open

refactor: Change BaseAuthToken and BaseValidator to abstract#1379
samsonasik wants to merge 2 commits into
codeigniter4:developfrom
samsonasik:base-abstact

Conversation

@samsonasik

@samsonasik samsonasik commented Sep 9, 2026

Copy link
Copy Markdown
Member

Description

Similar to what I proposed on CodeIgniter4 repo, this change class with Base prefix to abstract as never instantiated and only extended at:

detected by structarmed:

->rule('base_classes.must_be_abstract', new ExtendedClassMustBeAbstractOrInstantiatedRule('Source', '/^CodeIgniter\\\\Shield\\\\.*Base.*$/'))

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@samsonasik samsonasik added breaking change Pull requests that may break existing functionalities refactor Pull requests that refactor code labels Sep 9, 2026
@samsonasik

Copy link
Copy Markdown
Member Author

Ready for review 👍 Just let me know if it needs to target different branch as it is bc break.

@datamweb

datamweb commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

@samsonasik I consider this change logical and well aligned with object-oriented design principles. Making base classes abstract clearly communicates their intended purpose.They are designed to be extended rather than instantiated directly.

This is also consistent with our existing codebase. For example, we already have BaseModel defined as an abstract class:

abstract class BaseModel extends Model

So, in my opinion, making the other base classes abstract is a natural and consistent continuation of the same design approach already used in Shield.

Although this technically constitutes a breaking change, I don't expect it to affect many users, since these base classes are primarily intended to be extended rather than instantiated directly.

Finally, I would appreciate it if we could consider the feedback from other before merging this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Pull requests that may break existing functionalities refactor Pull requests that refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants