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: Property hooks #8227

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

junichi11
Copy link
Member

@junichi11 junichi11 commented Feb 9, 2025

PHP 8.4 Support: Property hooks (Part 1)

  • Fix the lexer and parser(grammar)
  • Add PropertyHookDeclaration as an ASTNode
  • Fix PHP5ErrorHandlerImpl (handle missing tokens)
  • Fix/Add unit tests for the lexer and parser
  • Remove ArrayDimensionSyntaxSuggestionHint because this no longer works.

Example:

class PropertyHooksClass {
    public int $backed = 100 {
        get {
            return $this->backed;
        }
        set {
            $this->backed = $value;
        }
    }
    public $doubleArrow { // virtual
        get => $this->test();
        set => $this->test() . $value;
    }
    public $attributes {
        #[Attr1] get {}
        #[Attr2] set ($value) {
            $this->attributes = $value;
        }
    }
    public $reference {
        &get => $this->reference;
    }
    public $final {
        final get => $this->final;
    }

    // constructor property promotion
    public function __construct(
        public $prop {
            get {
                return $this->prop;
            }
            set {
                $this->prop = $value;
            }
        }
    ) {}
}

class Child extends PropertyHooksClass {
    public $prop {
        get => parent::$prop::get();
        set {
            parent::$prop::set($value);
        }
    }
}

interface PropertyHooksInterface {
    public string $prop1 {
        get;
    }
    public int $prop2 {
        set;
    }
    public $prop3 {
        get;
        set;
    }
    public $ref { &get; }
}

Note: Curly braces array access ({} e.g. $array{1}, $array{'key'}) can use no longer.
Because a conflict occurs in the following case:

"string"{1};
public string $prop = "string" {
    get => $this->prop;
    set {}
}

PHP 8.4

nb-php84-property-hooks-syntax-1
nb-php84-property-hooks-syntax-2

PHP 8.3

nb-php84-property-hooks-syntax-php83-1
nb-php84-property-hooks-syntax-php83-2

PHP 8.4 Support: Property hooks (Part 2)

  • Fix the indexer and the model
  • Use JSON format as a signature for property hooks
  • Current format is semicolon separated but it's hard to add hooks to a field signature without JSON format
[
    {
        "name":"set",
        "start":3651,
        "end":3690,
        "mod":1,
        "isRef":false,
        "isAttr":false,
        "hasBody":true,
        "paramSig":"$value::0::1:1:0:0:0:0::"
    }
]
  • Add interface methods

    • FieldElemnt.isHooked()
    • FieldElement.HookedFieldElemnt.isHooked()
    • FieldElement.HookedFieldElemnt.getPropertyHooks()
    • IndexScope.PHP84IndexScope.findFields()
    • TypeScope.FieldDeclarable.getDeclaredFields()
    • TypeScope.FieldDeclarable.getInheritedFields()
    • PropertyHookScope.isReference()
    • PropertyHookScope.hasBody()
    • PropertyHookScope.isAttributed()
    • PropertyHookScope.getParameterNames()
    • PropertyHookScope.getParameters()
    • PropertyHookScope.getOffsetRange()
    • PropertyHookElement.isReference()
    • PropertyHookElement.hasBody()
    • PropertyHookElement.isAttributed()
    • PropertyHookElement.getParameters()
    • PropertyHookElement.getOffsetRange()
  • Add/Fix unit tests for the index and the model

PHP 8.4 Support: Property hooks (Part 3)

  • Fix the navigator
  • Fix/Add unit tests
  • Add SVG icons for a property hook, a trait, and an enum case

nb-php84-property-hooks-navigator

nb-php84-property-hooks-navi-icons
nb-php84-property-hooks-navi-icons-dark

PHP 8.4 Support: Property hooks (Part 4)

  • Fix the folding feature
  • Add unit tests

Expanded:

nb-php84-property-hooks-folding-base

Collapsed:

nb-php84-property-hooks-folding

nb-php84-property-hooks-folding-options

PHP 8.4 Support: Property hooks (Part 5)

  • Fix the formatter and PhpTypedBreakInterceptor
  • New Formatting Options
    • Braces
      • Field Declaration (Same Line by default)
      • Property Hook (Same Line by default)
    • Blank Lines
      • Between Property Hooks (0 by default)
      • Empty Property Hook Body (Unchecked by default)
    • Spaces
      • Before Left Braces
        • Field Declaration (Checked by default)
        • Property Hook Declaration (Checked by default)
  • Add unit tests

Example:

class PropertyHookClass {
    public string $prop1 {get{} set{}}
    public string $prop2 {
        get{return $this->prop2;}
        set{$this->prop2 = $value;}
    }
}

interface PropertyHookInterface {
    public string $prop1 {get; set;}
}

Formatted:

class PropertyHookClass {

    public string $prop1 {
        get {}
        set {}
    }
    public string $prop2 {
        get {
            return $this->prop2;
        }
        set {
            $this->prop2 = $value;
        }
    }
}

interface PropertyHookInterface {

    public string $prop1 {
        get;
        set;
    }
}

- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/property-hooks

- Fix the lexer and parser(grammar)
- Add `PropertyHookDeclaration` as an `ASTNode`
- Fix `PHP5ErrorHandlerImpl` (handle missing tokens)
- Fix/Add unit tests for the lexer and parser
- Remove `ArrayDimensionSyntaxSuggestionHint` because this no longer works.

Example:
```php
class PropertyHooksClass {
    public int $backed = 100 {
        get {
            return $this->backed;
        }
        set {
            $this->backed = $value;
        }
    }
    public $doubleArrow { // virtual
        get => $this->test();
        set => $this->test() . $value;
    }
    public $attributes {
        #[Attr1] get {}
        #[Attr2] set ($value) {
            $this->attributes = $value;
        }
    }
    public $reference {
        &get => $this->reference;
    }
    public $final {
        final get => $this->final;
    }

    // constructor property promotion
    public function __construct(
        public $prop {
            get {
                return $this->prop;
            }
            set {
                $this->prop = $value;
            }
        }
    ) {}
}

class Child extends PropertyHooksClass {
    public $prop {
        get => parent::$prop::get();
        set {
            parent::$prop::set($value);
        }
    }
}

interface PropertyHooksInterface {
    public string $prop1 {
        get;
    }
    final public int $prop2 {
        set;
    }
    public $prop3 {
        get;
        set;
    }
    public $ref { &get; }
}
```

Note: Curly braces array access (`{}` e.g. `$array{1}`, `$array{'key'}`) can use no longer.
Because a conflict occurs in the following case:

```php
"string"{1};
public string $prop = "string" {
    get => $this->prop;
    set {}
}
```
@junichi11 junichi11 added the PHP [ci] enable extra PHP tests (php/php.editor) label Feb 9, 2025
@junichi11 junichi11 added this to the NB26 milestone Feb 9, 2025
@junichi11
Copy link
Member Author

@tmysik This is not done yet. I'm slowly going to add more commits to this PR. Could you please have a look at each commit when you have time?

@junichi11 junichi11 mentioned this pull request Feb 9, 2025
4 tasks
@junichi11
Copy link
Member Author

junichi11 commented Feb 10, 2025

Time out occurs in Windows tests. I'll try adding commits one by one.

  • The first commit is OK
  • The second commit has a problem. But I'm not sure...

@junichi11 junichi11 force-pushed the php84-property-hooks branch from e1d3267 to 5e2fad5 Compare February 10, 2025 02:58
@junichi11
Copy link
Member Author

@tmysik Please ignore this. I'll try to investigate this later...

@junichi11 junichi11 force-pushed the php84-property-hooks branch from 3bf0406 to 33472a7 Compare February 10, 2025 23:49
@junichi11
Copy link
Member Author

@tmysik It should be OK now. Probably, the cause is "Jackson". Instead, I used "JSON simple".

* @return property hook elements
*/
public static List<PropertyHookElement> fromSignatureItems(final List<PropertyHookSignatureItem> signatureItems, final String field, final String filenameUrl, final ElementQuery elementQuery) {
List<PropertyHookElement> elements = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
List<PropertyHookElement> elements = new ArrayList<>();
List<PropertyHookElement> elements = new ArrayList<>(signatureItems.size());

Copy link
Member Author

Choose a reason for hiding this comment

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

Will fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tmysik
Copy link
Member

tmysik commented Feb 11, 2025

I did a very quick review, and it looks good to me. Thanks for changing the code to the modern one, e.g. the switches :)

@junichi11 junichi11 force-pushed the php84-property-hooks branch from 33472a7 to b7eb3b4 Compare February 12, 2025 00:24
@junichi11
Copy link
Member Author

junichi11 commented Feb 12, 2025

Fix folding feature: 8cff930

@junichi11 junichi11 force-pushed the php84-property-hooks branch from 3b51984 to 8cff930 Compare February 13, 2025 06:04
- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/property-hooks

- Fix the indexer and the model
- Use JSON format as a signature for property hooks
- Current format is semicolon separated but it's hard to add hooks to a field signature without JSON format
- Use the "JSON simple". If we use "Jackson", problems(timeout, java.lang.NoClassDefFoundError) occurs in CI for Windows

```json
[
    {
        "name":"set",
        "start":3651,
        "end":3690,
        "mod":1,
        "isRef":false,
        "isAttr":false,
        "hasBody":true,
        "paramSig":"$value::0::1:1:0:0:0:0::"
    }
]
```

- Add interface methods
  - `FieldElemnt.isHooked()`
  - `FieldElement.HookedFieldElemnt.isHooked()`
  - `FieldElement.HookedFieldElemnt.getPropertyHooks()`
  - `IndexScope.PHP84IndexScope.findFields()`
  - `TypeScope.FieldDeclarable.getDeclaredFields()`
  - `TypeScope.FieldDeclarable.getInheritedFields()`
  - `PropertyHookScope.isReference()`
  - `PropertyHookScope.hasBody()`
  - `PropertyHookScope.isAttributed()`
  - `PropertyHookScope.getParameterNames()`
  - `PropertyHookScope.getParameters()`
  - `PropertyHookScope.getOffsetRange()`
  - `PropertyHookElement.isReference()`
  - `PropertyHookElement.hasBody()`
  - `PropertyHookElement.isAttributed()`
  - `PropertyHookElement.getParameters()`
  - `PropertyHookElement.getOffsetRange()`

- Add/Fix unit tests for the index and the model

Log:
```
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: getSignatureFromScopes() took: 0 ms
INFO [org.netbeans.ui.indexing]: Indexing finished, indexing took 14 ms.
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 1 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
FINE [org.netbeans.modules.php.editor.model.impl.PropertyHookSignatureItem]: fromSignature() took: 0 ms
```
- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/property-hooks

- Fix the navigator
- Fix/Add unit tests
- Add SVG icons for a property hook, a trait, and an enum case
- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/property-hooks

- Fix the formatter and `PhpTypedBreakInterceptor`
- New Formatting Options
  - Braces
    - Field Declaration (Same Line by default)
    - Property Hook (Same Line by default)
  - Blank Lines
    - Between Property Hooks (0 by default)
    - Empty Property Hook Body (Unchecked by default)
  - Spaces
    - Before Left Braces
      - Field Declaration (Checked by default)
      - Property Hook Declaration (Checked by default)
- Add unit tests
@junichi11 junichi11 force-pushed the php84-property-hooks branch from 8cff930 to 5348d53 Compare February 15, 2025 00:34
Comment on lines 132 to 137
for (int mode : bitmask) {
this.mod |= mode;
}
if (!Modifier.isPrivate(mod) && !Modifier.isProtected(mod) && !Modifier.isImplicitPublic(mod)) {
mod |= Modifier.PUBLIC;
if (!Modifier.isPrivate(mod) && !Modifier.isProtected(mod) && !Modifier.isPublic(mod)) {
mod |= Modifier.IMPLICIT_PUBLIC;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

IMPLICIT_PUBLIC is better if mod is 0 because the public modifier doesn't exist.

@@ -182,9 +184,11 @@ public void insert(MutableContext context) throws BadLocationException {
}
sb.append(IndentUtils.createIndentString(doc, indent));
} else {
LexUtilities.findPreviousToken(ts, Arrays.asList(PHPTokenId.PHP_CURLY_OPEN));
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a bug here:
e.g.

class C { // expected curly brace
    public function test() { // but found this brace
    }
^} // caret position

Instead, find a correct brace via the new method (findCurlyOpenOffset()).

Comment on lines +104 to +107
final get
{
return 100;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an existing bug. I didn't fix it (maybe, the priority is very low) because I don't think the New Line Indented option is important.

We can also reproduce this using a method.

class C {
    public function example() { return 100;}
}

@tmysik
Copy link
Member

tmysik commented Feb 20, 2025

@junichi11, please, let me know once this PR is ready for the review (no rush!). Thank you.

@junichi11
Copy link
Member Author

@tmysik OK. I'll do that after I fix the other features(Hints, CC, Go to Declaration, etc.) :)

@tmysik
Copy link
Member

tmysik commented Feb 20, 2025

@junichi11, as I wrote, take your time, it is a very complex feature. Thank you

- apache#8035
- https://wiki.php.net/rfc#php_84
- https://wiki.php.net/rfc/property-hooks

- Fix the indentation that is inserted when a new line is added
- Fix an existing bug for a lambda function
- Add unit tests

Existing bug:
```php
// example
test(
        function(): void {^
);
```

```php
// before
test(
        function(): void {
    ^
        }
);

// after
test(
        function(): void {
            ^
        }
);
```
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