Inner classes with short syntax #5
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a new RFC in PHP that proposes a concise syntax for class, enum, and inner class definitions. The RFC details a simplified way to declare public properties using constructor-like parentheses, along with new rules for inner classes (including visibility and access through the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as "PHP Code"
participant Outer as "Outer Class"
participant Inner as "Inner Class"
Caller->>Outer: Access inner class via Outer::Inner
Outer->>Inner: Validate visibility and short syntax rules
Inner-->>Caller: Return new inner class instance
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
published/short-class.ptxtis excluded by!**/*.ptxtpublished/template.ptxtis excluded by!**/*.ptxt
📒 Files selected for processing (2)
drafts/short-class.md(1 hunks)drafts/template.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`drafts/*.md`: These are PHP RFC's to change the PHP languag...
drafts/*.md: These are PHP RFC's to change the PHP language. Do not just consider the grammar of the text, but consider how it might change the language. For example, if a new feature is being added, consider how it might be used, and propose better ideas if you have them.
drafts/template.mddrafts/short-class.md
🪛 LanguageTool
drafts/template.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] * Status: Draft (or Under Discussion or Accepted or Declined) * First Publis...
(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
drafts/short-class.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: ..., [email protected] * Status: Draft (or Under Discussion or Accepted or Declined) * First Publis...
(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~11-~11: This is not the usual sequence for adjectives that have no special emphasis.
Context: ...> ## Introduction This RFC proposes a new short syntax for class/enum definitions in PH...
(EN_ADJ_ORDER)
[style] ~115-~115: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nnot access private class Foo::Baz ``` Inner classes have scope similar to propertie...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~123-~123: Loose punctuation mark.
Context: ...re are some intuitive rules: - public, private, and protected apply to the...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~124-~124: Loose punctuation mark.
Context: ... the visibility of the class. - static, final, and readonly apply to the cl...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~252-~252: Consider adding a comma.
Context: ...aults If there are any php.ini settings then list: * hardcoded default values * ph...
(IF_THEN_COMMA)
[locale-violation] ~268-~268: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...eas where the feature might be improved in future, but that are not currently proposed in...
(IN_FUTURE)
🔇 Additional comments (7)
drafts/short-class.md (7)
9-12: Introduction Section Clarity:
The introduction succinctly establishes the purpose—proposing a short syntax for class/enum definitions and embedding inner classes. The content reads clearly and effectively sets the context.🧰 Tools
🪛 LanguageTool
[style] ~11-~11: This is not the usual sequence for adjectives that have no special emphasis.
Context: ...> ## Introduction This RFC proposes a new short syntax for class/enum definitions in PH...(EN_ADJ_ORDER)
28-30: Short Class Declaration Example:
The exampleclass Point(int $x, int $y);perfectly illustrates the proposed short syntax. Its brevity and clarity make the concept immediately understandable.
32-38: Full Class Equivalence Example:
The expanded example showing the equivalent full class definition (including an explicit constructor) effectively demonstrates what the short syntax translates into. This reinforces understanding for developers new to the concept.
60-67: Empty Classes Example:
The demonstration of an empty short class (class Point() extends BasePoint use PointTrait;) is consistent with the RFC’s intent. Just ensure that documentation makes it clear when parentheses may be omitted versus when they are required.
76-94: Short Enums Syntax:
The short enum example, along with its full enum equivalence, is well presented and provides a clear comparison. This section is complete and self-explanatory.
129-144: Visibility of Inner Classes:
The example provided effectively demonstrates the scope limitations of a private inner class and the resulting runtime error when attempting to access it externally. This clarity is beneficial for understanding the RFC’s rules.
146-164: Accessing Inner Classes from Various Contexts:
The multiple instantiation examples (usingFoo::Bar,self::Bar, andstatic::Bar) comprehensively cover how inner classes can be accessed. It would be wise to confirm that the proposed syntax does not conflict with existing PHP operator precedence rules.
| * Version: 0.9 | ||
| * Date: 2013-02-24 (use today's date here) | ||
| * Author: Your Name, [email protected] | ||
| * Status: Draft (or Under Discussion or Accepted or Declined) | ||
| * First Published at: <http://wiki.php.net/rfc/your_rfc_name> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Metadata Formatting Consistency:
The metadata block is neatly restructured, improving overall readability. However, consider refining the status line by replacing “Under Discussion” with a shorter, more concise alternative (e.g. “Draft” or “Proposed”) to better align with clear, succinct language.
🧰 Tools
🪛 LanguageTool
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] * Status: Draft (or Under Discussion or Accepted or Declined) * First Publis...
(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
drafts/short-class.md
Outdated
| # PHP RFC: Inner Classes with Short Syntax | ||
|
|
||
| * Version: 0.1 | ||
| * Date: 2025-02-08 | ||
| * Author: Rob Landers, [email protected] | ||
| * Status: Draft (or Under Discussion or Accepted or Declined) | ||
| * First Published at: <http://wiki.php.net/rfc/short-class> | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
RFC Metadata Header:
The header clearly presents the RFC title, version, date, author, status, and publication link. As noted in the static analysis hints, you might consider shortening “Under Discussion” to a briefer alternative for clarity.
🧰 Tools
🪛 LanguageTool
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: ..., [email protected] * Status: Draft (or Under Discussion or Accepted or Declined) * First Publis...
(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
drafts/short-class.md
Outdated
| ### Short Class Syntax | ||
|
|
||
| The proposed syntax for a short class definition is as follows: a keyword `class`, | ||
| followed by the class name, then a list of public properties enclosed in parentheses. | ||
| Optionally, a list of traits, interfaces, and a parent class may be defined. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Short Class Syntax Explanation:
The description clearly explains that a short class definition uses a single-line syntax with public properties in parentheses. For further clarity, consider mentioning the required order when mixing traits, interfaces, and parent classes.
drafts/short-class.md
Outdated
| #### Inheritance and Behavior | ||
|
|
||
| With class short syntax, no behavior may be defined, yet it can still utilize traits, interfaces, and other classes. | ||
|
|
||
| ```php | ||
| class Point(int $x, int $y) extends BasePoint implements JsonSerializable use PointTrait, Evolvable; | ||
| ``` | ||
|
|
||
| Note that the original constructor from any parent class is overridden and not called by the short syntax. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Inheritance and Behavior Section:
This section explains that while behavior isn’t defined within the short syntax, inheritance, interfaces, and trait usage are supported. The combined example using extends, implements, and use is innovative; however, clarifying the ordering or any special syntax rules (especially for trait usage) would help prevent ambiguity.
drafts/short-class.md
Outdated
| #### Attributes | ||
|
|
||
| Attributes may also be used with short classes: | ||
|
|
||
| ```php | ||
| class Password(#[SensitiveParameter] string $password); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Attributes in Short Classes:
Including attributes directly within the short class declaration is a forward-thinking addition. It would be beneficial to note any PHP version requirements for attribute support to set proper expectations for adopters.
drafts/short-class.md
Outdated
| #### Modifiers | ||
|
|
||
| Properties support modifiers such as `public`, `protected`, and `private` as well as `static`, `final` and `readonly`. | ||
| When using these as modifiers on an inner class, there are some intuitive rules: | ||
|
|
||
| - `public`, `private`, and `protected` apply to the visibility of the class. | ||
| - `static`, `final`, and `readonly` apply to the class itself. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Modifiers for Inner Classes:
The modifiers section clearly distinguishes between visibility modifiers and those affecting the class behavior (static, final, readonly). A slight improvement in punctuation in the bullet list may aid readability.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~123-~123: Loose punctuation mark.
Context: ...re are some intuitive rules: - public, private, and protected apply to the...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~124-~124: Loose punctuation mark.
Context: ... the visibility of the class. - static, final, and readonly apply to the cl...
(UNLIKELY_OPENING_PUNCTUATION)
drafts/short-class.md
Outdated
| Note that inner classes effectively "shadow" outer classes of the same name: | ||
|
|
||
| ```php | ||
| readonly class Vect(int $x, int $y); | ||
|
|
||
| class Foo { | ||
| class Vect(int $x, int $y, int $z); | ||
|
|
||
| // Vect is Foo::Vect not \Vect | ||
| public function __construct(public Vect $vect) {} | ||
| } | ||
| ``` | ||
|
|
||
| #### Names | ||
|
|
||
| Inner classes may not have any name that conflicts with a constant or static property of the same name. | ||
|
|
||
| ```php | ||
| class Foo { | ||
| const Bar = 'bar'; | ||
| class Bar(public string $message); | ||
|
|
||
| // Fatal error: Uncaught Error: Cannot redeclare Foo::Bar | ||
| } | ||
|
|
||
| class Foo { | ||
| static $Bar = 'bar'; | ||
| class Bar(public string $message); | ||
|
|
||
| // Fatal error: Uncaught Error: Cannot redeclare Foo::Bar | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Naming Conventions and Conflicts:
The examples illustrate how inner classes can shadow outer classes and demonstrate conflicts with constants or static properties. Including some best-practice guidelines or naming recommendations could help developers avoid inadvertent ambiguities in their implementations.
drafts/short-class.md
Outdated
|
|
||
| These rules are to prevent developer confusion because these instantiations all look similar, | ||
| however, the following all result in the same inner class being instantiated: | ||
|
|
||
| ```php | ||
| new (Foo::Bar); | ||
| new (Foo::$Bar); | ||
| new Foo::Bar(); | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Instantiating Inner Classes – Potential Ambiguity:
The demonstration that different instantiation styles (e.g., new (Foo::Bar), new (Foo::$Bar), new Foo::Bar()) yield the same result is informative. Consider adding a brief note on best practices to ensure code clarity when utilizing these various styles.
drafts/short-class.md
Outdated
| ## Backward Incompatible Changes | ||
|
|
||
| Creating a new instance from an existing static member is now allowed: | ||
|
|
||
| ```php | ||
| class Foo { | ||
| public const Bar = 'bar'; | ||
| } | ||
|
|
||
| new Foo::Bar(); // previously this is a syntax error, but now results in creating a new "bar" object. | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Backward Incompatible Change Clarification:
Allowing instantiation from an existing static member is a significant change. While the provided example is clear, a short explanation of the rationale behind this decision could help developers understand its broader implications on legacy code.
drafts/short-class.md
Outdated
| ## Future Scope | ||
|
|
||
| This section details areas where the feature might be improved in | ||
| future, but that are not currently proposed in this RFC. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Future Scope Section – Minor Language Update:
For consistency with standard American English, consider changing “in future” on line 268 to “in the future.”
🧰 Tools
🪛 LanguageTool
[locale-violation] ~268-~268: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...eas where the feature might be improved in future, but that are not currently proposed in...
(IN_FUTURE)
Summary by CodeRabbit
New Features
Documentation