Skip to content

PHP8: Adds support for nullsafe operator#75

Merged
cfroystad merged 1 commit intotree-sitter:masterfrom
cfroystad:nullsafe_operator
May 3, 2021
Merged

PHP8: Adds support for nullsafe operator#75
cfroystad merged 1 commit intotree-sitter:masterfrom
cfroystad:nullsafe_operator

Conversation

@cfroystad
Copy link
Collaborator

@cfroystad cfroystad commented Apr 29, 2021

RFC

Introduces support for the nullsafe operator

New nodes:

  • nullsafe_member_access_expression
  • nullsafe_member_call_expression

Example:

$country = $session?->user?->getAddress()?->country;

Is parsed into:

(expression_statement
    (assignment_expression
      (variable_name (name))
      (nullsafe_member_access_expression
        (nullsafe_member_call_expression
          (nullsafe_member_access_expression
            (variable_name (name))
            (name)
          )
          (name)
          (arguments)
        )
        (name)
      )
    )
  )

Checklist:

  • All tests pass in CI.
  • There are sufficient tests for the new fix/feature (see tests in expressions.txt)
  • Grammar rules have not been renamed unless absolutely necessary (0 rules renamed)
  • The conflicts section hasn't grown too much (0 new conflicts)
  • The parser size hasn't grown too much (master: 1664, PR: 1727)

@cfroystad cfroystad mentioned this pull request Apr 29, 2021
Closed
32 tasks
@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented May 1, 2021

Ugh, sorry that it's so much extra work for you to maintain these parallel branches because of the checked-in generated files. That's something we want to address at some point, but it's hard to find the time.

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented May 2, 2021

Are you interested in having commit access to this repo @cfroystad? You're doing great work on this grammar.

@cfroystad
Copy link
Collaborator Author

cfroystad commented May 2, 2021 •
edited
Loading

No worries, I've encountered worse 🙂 Am I right in assuming the work required to avoid checkin of generated files is more related to release prosess than this repository in particular?

I'll rebase come Monday afternoon (Norwegian time, that is).

Just to make sure I don't bite over more than I han chew: what would commit access entail?

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented May 2, 2021

what would commit access entail?

I’d propose this: still make changes in PRs, but for PRs like this one, where you can be pretty confident in your solution, just go ahead and merge it after CI finishes green. For PRs where there are new conflicts, or which add a lot of new complexity to the grammar (or a huge amount of states to the parser), keep requesting reviews as you see fit?

Of course, No need to change your workflow if you’re already happy with it. I just think for certain PRs, you should feel free to merge them yourself.

@cfroystad
Copy link
Collaborator Author

cfroystad commented May 2, 2021

That sounds like a good plan!

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented May 2, 2021

Ok, sent an invite.

@cfroystad cfroystad force-pushed the nullsafe_operator branch from a1ca138 to c0b56ec Compare May 3, 2021 16:30
@cfroystad cfroystad merged commit 194b613 into tree-sitter:master May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxbrunsfeld maxbrunsfeld maxbrunsfeld approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cfroystad @maxbrunsfeld