Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.4 Support: Asymmetric Visibility v2 #8177

Merged
merged 5 commits into from
Jan 23, 2025

Conversation

junichi11
Copy link
Member

PHP 8.4 Support: Asymmetric Visibility v2 (Part 1)

  • Fix the lexers and the parser(grammar file)
  • Support for the final field(property)
  • Don't handle modifier errors for methods, constants, and fields in the parser. Instead, handle them in the hint error.
    Writing all valid cases for each member is very hard and complicated because the current PHP has many modifiers.
  • Fix the PHP84UnhandledError
  • Add/Fix unit tests for the navigator, lexer, and parser

Example:

class AsymmetricVisibilityClass {
    // Constructor Property Promotion
    public function __construct(
        public(set) string $field1, // implicit public
        public private(set) int $field2,
        public protected(set) readonly int $field3,
    ) {}

    // valid fields
    public(set) string $example1; // implicit public
    public private(set) int $example2 = 1;
    public protected(set) readonly int $example3;
    final private private(set) readonly string|int $example4; // final is available as of PHP 8.4

    // invalid cases but the parser doesn't handle them as errors
    // e.g.
    public public string $invalid1 = "invalid"; // multiple same modifiers
    final private private(set) string|int $invalid2; // cannot use both final and private
    public public(set) $invalid3; // need type
    private public(set) string $invalid4; // visibility(private) must not be weaker than set visibility
}

PHP 8.4

nb-php84-asymmetric-visibility-php84

nb-php84-final-property-php84

PHP 8.3

nb-php84-asymmetric-visibility-php83

nb-php84-final-property-php83

Navigator

nb-php84-asymmetric-visibility-constructor-property-promotion-navi

PHP 8.4 Support: Asymmetric Visibility v2 (Part 2)

  • Fix ModifiersCheckHintError
  • Move final modifier errors from the FinalModifierHintError to ModifiersCheckHintError
  • Add useful methods to the PhpVersion
    • hasFinalConst()
    • hasDeprecatedAttribute()
    • hasFinalField()
    • hasAsymmetricVisibility()
  • Add methods and constants for set visibility to the PhpModifiers
  • Add unit tests for hints
  • Increment spec vesions

nb-php84-asymmetric-visibility-hint1

nb-php84-asymmetric-visibility-hint2

PHP 8.4 Support: Asymmetric Visibility v2 (Part 3)

  • Fix hints
    • IncorrectConstructorPropertyPromotionHintError
    • UnusedVariableHint
  • Add/Fix unit tests

PHP 8.4 Support: Asymmetric Visibility v2 (Part 4)

  • Fix Code Completion
  • Fix/Add unit tests

Note: CC does not work correctly on the following caret position because set visibility keywords contain a brace.

class Example {
    public private(se^ // ^: caret
}

PHP 8.4 Support: Asymmetric Visibility v2 (Part 5)

  • Add unit tests for formatter
    • set visibility (private(set) int $i;)
    • final property (final public string $s = "string";)

- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/asymmetric-visibility-v2
- https://www.php.net/manual/en/language.oop5.final.php

- Fix the lexers and the parser(grammar file)
- Support for the `final` field(property)
- Don't handle modifier errors for methods, constants, and fields in the parser. Instead, handle them in the hint error.
  Writing all valid cases for each member is very hard and complicated because the current PHP has many modifiers.
- Fix the `PHP84UnhandledError`
- Add/Fix unit tests for the navigator, lexer, and parser

Example:
```php
class AsymmetricVisibilityClass {
    // Constructor Property Promotion
    public function __construct(
        public(set) string $field1, // implicit public
        public private(set) int $field2,
        public protected(set) readonly int $field3,
    ) {}

    // valid fields
    public(set) string $example1; // implicit public
    public private(set) int $example2 = 1;
    public protected(set) readonly int $example3;
    final private private(set) readonly string|int $example4; // final is available as of PHP 8.4

    // invalid cases but the parser doesn't handle them as errors
    // e.g.
    public public string $invalid1 = "invalid"; // multiple same modifiers
    final private private(set) string|int $invalid2; // cannot use both final and private
    public public(set) $invalid3; // need type
    private public(set) string $invalid4; // visibility(private) must not be weaker than set visibility
}
```
@junichi11 junichi11 added the PHP [ci] enable extra PHP tests (php/php.editor) label Jan 21, 2025
@junichi11 junichi11 added this to the NB25 milestone Jan 21, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to ModifiersCheckHintError.

Copy link
Member Author

Choose a reason for hiding this comment

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

Show errors here because the parser allows invalid modifier combinations.

;

variable_modifiers ::=
ppp_modifiers:modifier
non_empty_member_modifiers:modifier
Copy link
Member Author

Choose a reason for hiding this comment

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

The parser allows valid combinations so far. However, it's possible to maintain it no longer...because there are many modifiers.
So, the parser allows invalid combinations now.
Instead, errors are handled via ModifiersCheckHintError.

Copy link
Member

Choose a reason for hiding this comment

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

I see.

@@ -2493,6 +2505,92 @@ optional_property_modifiers ::=
result |= pModifier.intValue();
RESULT = Integer.valueOf(result);
:}

| ppp_modifiers:pppModifier ppp_set_modifiers:pppSetModifier
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, we can change here to non_empty_member_modifiers in the future.

@junichi11 junichi11 requested a review from tmysik January 21, 2025 13:39
@junichi11
Copy link
Member Author

@tmysik Could you please have a look at each commit? I've added unit tests as far as possible. So, changed files are so many. Sorry for that...

@junichi11 junichi11 mentioned this pull request Jan 21, 2025
4 tasks
Copy link
Member

@tmysik tmysik left a comment

Choose a reason for hiding this comment

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

I did a quick review, a lot of work done by @junichi11 👏

- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/asymmetric-visibility-v2
- https://www.php.net/manual/en/language.oop5.final.php

- Fix `ModifiersCheckHintError`
- Move final modifier errors from the `FinalModifierHintError` to `ModifiersCheckHintError`
- Add useful methods to the `PhpVersion`
  - `hasFinalConst()`
  - `hasDeprecatedAttribute()`
  - `hasFinalField()`
  - `hasAsymmetricVisibility()`
- Add methods and constants for set visibility to the `PhpModifiers`
- Add unit tests for hints
- Change `javac.source=1.8` to `javac.release=17`
- Increment spec vesions
- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/asymmetric-visibility-v2

- Fix hints
  - `IncorrectConstructorPropertyPromotionHintError`
  - `UnusedVariableHint`
- Add/Fix unit tests
- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/asymmetric-visibility-v2

- Fix Code Completion
- Fix/Add unit tests

Note: CC does not work correctly on the following caret position because set visibility keywords contain a brace.
```php
class Example {
    public private(se^ // ^: caret
}
```
- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/asymmetric-visibility-v2

- Add unit tests for formatter
  - set visibility (`private(set) int $i;`)
  - final property (`final public string $s = "string";`)
@junichi11 junichi11 force-pushed the php84-asymmetric-visibility-v2 branch from b024e57 to da5c988 Compare January 23, 2025 12:35
@junichi11
Copy link
Member Author

@tmysik Thank you for your time, Tomas!

@junichi11 junichi11 merged commit 1992b69 into apache:master Jan 23, 2025
32 checks passed
@junichi11 junichi11 deleted the php84-asymmetric-visibility-v2 branch January 23, 2025 16:08
@tmysik
Copy link
Member

tmysik commented Jan 23, 2025

@junichi11 nothing to thank for, really. You did all the job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants