Php modernisation by corneliuhoffman · Pull Request #529 · opengrep/opengrep

corneliuhoffman · 2026-01-08T15:55:50Z

PHP 7.1 - 8.4 Syntax Support

Adds comprehensive PHP syntax support to the Menhir parser. Closes #297.

b0529bc - added tests

Initial test files for PHP 8.x features.

d3da661 - readonly fix

PHP 8.2: Readonly Classes

  readonly class ImmutablePoint {
      public int $x;
      public int $y;
  }

18aa6ce - Union and Intersection DNF

PHP 8.2: DNF Types (Disjunctive Normal Form)

  function process((Countable&Iterator)|null $input): (A&B)|C {
      // ...
  }

b59447d - property hooks

PHP 8.4: Property Hooks

  class User {
      // Block syntax
      public string $name {
          get { return $this->name; }
          set { $this->name = strtoupper($value); }
      }

      // Arrow syntax
      public int $age {
          get => $this->age;
          set => max(0, $value);
      }
  }

aaed1fb - asymmetric visibility private(set) and protected(set)

PHP 8.4: Asymmetric Visibility

  class Counter {
      public private(set) int $count = 0;
      public protected(set) string $name = "";
  }

19e3d19 - few others

PHP 7.1: Class Constant Visibility

  class Config {
      public const PUBLIC_CONST = 1;
      protected const PROTECTED_CONST = 2;
      private const PRIVATE_CONST = 3;
  }

PHP 7.1: Multi-Catch Exception Handling

  try {
      riskyOperation();
  } catch (InvalidArgumentException | TypeError | ValueError $e) {
      handleError($e);
  }

PHP 8.3: Dynamic Class Constant Fetch

  $name = 'VERSION';
  echo Config::{$name};

e09be54 - first-class callable syntax

PHP 8.1: First-Class Callable Syntax

  $fn = strlen(...);           // → Closure::fromCallable('strlen')
  $fn = $obj->method(...);     // → Closure::fromCallable([$obj, 'method'])
  $fn = Foo::bar(...);         // → Closure::fromCallable([Foo::class, 'bar'])
```  In semgrep patterns, ... retains its ellipsis meaning.

languages/php/ast/ast_php.ml

languages/php/menhir/ast_php_build.ml

tests/parsing/php/php81_first_class_callable.php

dimitris-m · 2026-01-08T18:20:40Z

tests/parsing/php/php8_property_hooks.php

+    public string $name {
+        get {
+            return $this->name;
+        }
+        set {
+            $this->name = strtoupper($value);
+        }


I recommend to add a tainting test also for this material. Where $value is tainted for example.

In particular I'm wondering how it relates to #516 and #525.

cc @maciejpirog

dimitris-m · 2026-01-08T18:25:56Z

tests/rules/php_first_class_callable.yaml

+  message: Detected first-class callable (desugared to Closure::fromCallable)
+  languages: [php]
+  severity: INFO
+  pattern: Closure::fromCallable(...)


We should document that this is the only way to match such constructs.

Maybe in Wiki?

will add a php upgrade in wiki

tests/rules/php_ellipsis_args.php

maciejpirog · 2026-01-08T22:23:56Z

Property hooks don't produce correct AST. Example from https://www.php.net/manual/en/language.oop5.property-hooks.php

<?php
class Example
{
    private bool $modified = false;

    public string $foo = 'default value' {
        get {
            if ($this->modified) {
                return $this->foo . ' (modified)';
            }
            return $this->foo;
        }
        set(string $value) {
            $this->foo = strtolower($value);
            $this->modified = true;
        }
    }
}

$example = new Example();
$example->foo = 'changed';
print $example->foo;
?>

The AST for $foo is:

DefStmt(
({
    name=EN(
            Id(("$foo", ()),
            {id_flags=Ref(0); 
            id_resolved_alternative=Ref([]); 
            id_resolved=Ref(Some((EnclosedVar, 2))); 
            id_type=Ref(Some({t_attrs=[]; 
                                t=TyN(
                                    Id(("string", ()),
                                    {id_flags=Ref(
                                        2); 
                                        id_resolved_alternative=Ref(
                                        []); 
                                        id_resolved=Ref(
                                        None); 
                                        id_type=Ref(
                                        None); 
                                        id_svalue=Ref(
                                        None); }));
                                }));
            id_svalue=Ref(None); }));
    attrs=[KeywordAttr((Public, ()))]; tparams=None; },
    VarDef(
    {
    vinit=Some(ArrayAccess(
                    L(String(("default value", ()))),
                    ArrayAccess(
                    Call(
                        ArrayAccess(
                        N(
                            Id(("get", ()),
                            {id_flags=Ref(2); 
                            id_resolved_alternative=Ref(
                            []); id_resolved=Ref(
                            None); id_type=Ref(None); 
                            id_svalue=Ref(None); })),
                        DotAccess(
                            ArrayAccess(
                            Call(
                                N(
                                Id(("if", ()),
                                    {id_flags=Ref(2); 
                                    id_resolved_alternative=Ref(
                                    []); 
                                    id_resolved=Ref(
                                    None); 
                                    id_type=Ref(None); 
                                    id_svalue=Ref(None); })),
                                [Arg(
                                DotAccess(
                                    IdSpecial((This, ())),
                                    (),
                                    FN(
                                    Id(("modified", ()),
                                        {id_flags=Ref(
                                        0); 
                                        id_resolved_alternative=Ref(
                                        []); 
                                        id_resolved=Ref(
                                        None); 
                                        id_type=Ref(
                                        None); 
                                        id_svalue=Ref(
                                        None); }))))]),
                            Call(
                                IdSpecial((Op(Concat), ())),
                                [Arg(
                                DotAccess(
                                    IdSpecial((This, ())),
                                    (),
                                    FN(
                                    Id(("foo", ()),
                                        {id_flags=Ref(
                                        0); 
                                        id_resolved_alternative=Ref(
                                        []); 
                                        id_resolved=Ref(
                                        None); 
                                        id_type=Ref(
                                        None); 
                                        id_svalue=Ref(
                                        None); }))));
                                Arg(
                                L(
                                    String(
                                    (" (modified)", ()))))])),
                            (),
                            FN(
                            Id(("foo", ()),
                                {id_flags=Ref(0); 
                                id_resolved_alternative=Ref(
                                []); id_resolved=Ref(
                                None); id_type=Ref(
                                None); id_svalue=Ref(
                                None); })))),
                        [Arg(
                        N(
                            Id(("$value", ()),
                            {id_flags=Ref(2); 
                                id_resolved_alternative=Ref(
                                []); id_resolved=Ref(
                                None); id_type=Ref(None); 
                                id_svalue=Ref(None); })))]),
                    Assign(
                        DotAccess(IdSpecial((This, ())), (),
                        FN(
                            Id(("modified", ()),
                            {id_flags=Ref(0); 
                            id_resolved_alternative=Ref(
                            []); id_resolved=Ref(
                            None); id_type=Ref(None); 
                            id_svalue=Ref(None); }))),
                        (), L(Bool((true, ())))))));
    vtype=Some({t_attrs=[]; 
                t=TyN(
                    Id(("string", ()),
                        {id_flags=Ref(2); 
                        id_resolved_alternative=Ref(
                        []); id_resolved=Ref(None); 
                        id_type=Ref(None); 
                        id_svalue=Ref(None); }));
                });
    vtok=None; })))

It seems that things in { ... } are not even parsed as statements (and we get weird things like Call(N(Id(("if", ()),...))))

corneliuhoffman · 2026-01-09T09:22:25Z

Property hooks don't produce correct AST. Example from https://www.php.net/manual/en/language.oop5.property-hooks.php
whoa, thanks

corneliuhoffman · 2026-01-09T12:00:32Z

For some reason I cannot comment on @maciejpirog comment above. Now the thing parses fine and you can see a tainting test as well

dimitris-m · 2026-01-09T15:57:26Z

For some reason I cannot comment on @maciejpirog comment above. Now the thing parses fine and you can see a tainting test as well

I think because it was his review comment, but it's possible to "quote reply".

languages/php/generic/php_to_generic.ml

see #297

class constants visibility multicatch dynamic constant types

tests/parsing/php/php71_multi_catch.php

tests/parsing/php/php8_readonly.php

tests/parsing/php/php8_static_return.php

tests/parsing/php/php8_readonly.php

dimitris-m

LGTM - modulo small comments

maciejpirog · 2026-01-09T17:55:56Z

The AST looks good now! One thing to consider in future is the naming of the functions that represent setters and getters. They are now always called get and set, so there could be some confusion if we have more than one property.

I don't think it's urgent, because, as I understand it, at the moment we don't use properties in intrafile tainting, and whenever we have obj->prop = 2, it is treated as a field rather than being translated to a property call. But to support this would require a more global refactor, so not needed for this PR.

dimitris-m · 2026-01-09T18:00:37Z

The AST looks good now! One thing to consider in future is the naming of the functions that represent setters and getters. They are now always called get and set, so there could be some confusion if we have more than one property.

I don't think it's urgent, because, as I understand it, at the moment we don't use properties in intrafile tainting, and whenever we have obj->prop = 2, it is treated as a field rather than being translated to a property call. But to support this would require a more global refactor, so not needed for this PR.

Maybe open an issue for this? So we don't forget.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [opengrep/opengrep](https://github.com/opengrep/opengrep) | minor | `v1.14.1` → `v1.15.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>opengrep/opengrep (opengrep/opengrep)</summary> ### [`v1.15.1`](https://github.com/opengrep/opengrep/releases/tag/v1.15.1): Opengrep 1.15.1 [Compare Source](opengrep/opengrep@v1.15.0...v1.15.1) #### Bug fixes - Clojure translation improvements by [@dimitris-m](https://github.com/dimitris-m) in [#534](opengrep/opengrep#534) **Full Changelog**: <opengrep/opengrep@v1.15.0...v1.15.1> ### [`v1.15.0`](https://github.com/opengrep/opengrep/releases/tag/v1.15.0): Opengrep 1.15.0 [Compare Source](opengrep/opengrep@v1.14.1...v1.15.0) #### What's Changed - Clojure translation part III by [@dimitris-m](https://github.com/dimitris-m) in [#527](opengrep/opengrep#527) - Php modernisation by [@corneliuhoffman](https://github.com/corneliuhoffman) in [#529](opengrep/opengrep#529) - Intrafile tainting with variadic functions by [@maciejpirog](https://github.com/maciejpirog) in [#538](opengrep/opengrep#538) - C#: The `field` implicit parameter can be skipped in a pattern by [@maciejpirog](https://github.com/maciejpirog) in [#525](opengrep/opengrep#525) - C#: Add conditional array access (`?[...]`) to l-values by [@maciejpirog](https://github.com/maciejpirog) in [#535](opengrep/opengrep#535) - C#: Collection expressions vs attributes with targets (parser fix) by [@maciejpirog](https://github.com/maciejpirog) in [#539](opengrep/opengrep#539) - Add `noopengrep` to the default nosem patterns by [@dimitris-m](https://github.com/dimitris-m) in [#533](opengrep/opengrep#533) **Full Changelog**: <opengrep/opengrep@v1.14.1...v1.15.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

corneliuhoffman requested review from dimitris-m, maciejpirog and willem-delbare as code owners January 8, 2026 15:55

dimitris-m removed the request for review from willem-delbare January 8, 2026 15:59

corneliuhoffman force-pushed the php-modernisation branch from e09be54 to 2895794 Compare January 8, 2026 16:00

dimitris-m reviewed Jan 8, 2026

View reviewed changes

languages/php/ast/ast_php.ml Outdated Show resolved Hide resolved

dimitris-m reviewed Jan 8, 2026

View reviewed changes

languages/php/menhir/ast_php_build.ml Outdated Show resolved Hide resolved

dimitris-m reviewed Jan 8, 2026

View reviewed changes

tests/parsing/php/php81_first_class_callable.php Show resolved Hide resolved

dimitris-m reviewed Jan 8, 2026

View reviewed changes

tests/rules/php_ellipsis_args.php Show resolved Hide resolved

dimitris-m reviewed Jan 8, 2026

View reviewed changes

tests/rules/php_ellipsis_args.php Outdated Show resolved Hide resolved

corneliuhoffman force-pushed the php-modernisation branch 2 times, most recently from a9f9554 to abec309 Compare January 9, 2026 11:57

dimitris-m reviewed Jan 9, 2026

View reviewed changes

languages/php/generic/php_to_generic.ml Outdated Show resolved Hide resolved

dimitris-m reviewed Jan 9, 2026

View reviewed changes

languages/php/generic/php_to_generic.ml Outdated Show resolved Hide resolved

dimitris-m reviewed Jan 9, 2026

View reviewed changes

languages/php/generic/php_to_generic.ml Outdated Show resolved Hide resolved

Corneliu Hoffman added 9 commits January 9, 2026 17:19

added tests

e4c7e7d

see #297

readonly fix

5ffb0ae

Union and Intersection DNF

7449599

property hooks

1734ffc

asymmetric visibility private(set) and protected(set)

e1b13b7

few others

94459de

class constants visibility multicatch dynamic constant types

First-Class Callable Syntax

5794775

HintIntersection cleanup

1c2032e

inlined member modifier to fix the lookahead

1e11582

Corneliu Hoffman added 2 commits January 9, 2026 17:19

added proper static_scalar and fixed PHExpr to contain the ;

7d5424e

tests added

65ff86d

corneliuhoffman force-pushed the php-modernisation branch from 7bf6402 to ffca1c0 Compare January 9, 2026 17:20

dimitris-m reviewed Jan 9, 2026

View reviewed changes

tests/parsing/php/php71_multi_catch.php Show resolved Hide resolved

dimitris-m reviewed Jan 9, 2026

View reviewed changes

tests/parsing/php/php8_readonly.php Outdated Show resolved Hide resolved

dimitris-m reviewed Jan 9, 2026

View reviewed changes

tests/parsing/php/php8_static_return.php Outdated Show resolved Hide resolved

dimitris-m reviewed Jan 9, 2026

View reviewed changes

tests/parsing/php/php8_readonly.php Outdated Show resolved Hide resolved

dimitris-m approved these changes Jan 9, 2026

View reviewed changes

comments fixes

7ab6373

corneliuhoffman force-pushed the php-modernisation branch from ffca1c0 to 7ab6373 Compare January 9, 2026 17:33

maciejpirog approved these changes Jan 9, 2026

View reviewed changes

dimitris-m merged commit 0e2e4e9 into main Jan 9, 2026
6 checks passed

dimitris-m deleted the php-modernisation branch January 9, 2026 18:01

dimitris-m added the lang Add or improve language support label Jan 11, 2026

maciejpirog mentioned this pull request Jan 14, 2026

Release v1.15.0 #541

Merged

Conversation

corneliuhoffman commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dimitris-m Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

dimitris-m Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimitris-m Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

dimitris-m Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

dimitris-m Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maciejpirog commented Jan 8, 2026

Uh oh!

corneliuhoffman commented Jan 9, 2026

Uh oh!

corneliuhoffman commented Jan 9, 2026

Uh oh!

dimitris-m commented Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dimitris-m left a comment

Choose a reason for hiding this comment

Uh oh!

maciejpirog commented Jan 9, 2026

Uh oh!

dimitris-m commented Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

corneliuhoffman commented Jan 8, 2026 •

edited

Loading

dimitris-m Jan 8, 2026 •

edited

Loading