Skip to content

Conversation

@khaledalam
Copy link

@khaledalam khaledalam commented Jan 11, 2026

Description

Fix #19557

This PR adds the missing inNamespace() method to ReflectionConstant to achieve full parity with ReflectionClass and ReflectionFunctionAbstract for API consistency.

Background

ReflectionConstant already has:

  • getShortName() - returns the short name without namespace
  • getNamespaceName() - returns the namespace name

But was missing:

  • inNamespace() - returns boolean indicating if in a namespace

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @khaledalam! There were some recent policy changes:

https://github.com/php/policies/blob/main/release-process.rst#feature-selection-and-development

A core developer MAY also request that the feature be discussed on the internals mailing list, in which case an additional two-week period MUST pass without objection or RFC request before the feature can be merged.

Trivial new functions should briefly be discussed on the internals mailing list. If nobody objects after two weeks, this should be ok to merge.

@iluuu1994
Copy link
Member

Correction, I missed this part:

Internal API changes (those that do not affect the user-facing API), as well as user-facing features in extensions and SAPIs, do not require an RFC unless a core developer (someone with commit access to php-src) raises an objection or requests an RFC within one month of the implementation pull request being opened.

So I suppose no discussion is needed. (I don't object to this function). So @DanielEScherzer, please merge if you see appropriate.

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is an obvious addition that follows the existing design and thus there is nothing to discuss about.

@DanielEScherzer
Copy link
Member

Correction, I missed this part:

Internal API changes (those that do not affect the user-facing API), as well as user-facing features in extensions and SAPIs, do not require an RFC unless a core developer (someone with commit access to php-src) raises an objection or requests an RFC within one month of the implementation pull request being opened.

So I suppose no discussion is needed. (I don't object to this function). So @DanielEScherzer, please merge if you see appropriate.

I interpreted this to mean that the PR needs to be open for a month to allow someone to raise an objection or request an RFC, i.e. that this shouldn't be merged until February 10
Code-wise, this looks good, and the addition makes sense, so I would support it at that point
This will also need a NEWS/UPGRADING entry and a fix for the merge conflict, but that should probably wait until closer to February 10 to avoid needing to fix them again later

Copy link
Member

@DanielEScherzer DanielEScherzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I support this, but marking as "request changes" to make sure it doesn't get merged before February 10

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ext/reflection: Add missing ReflectionConstant::inNamespace() method

4 participants