Skip to content

Modified Keywords code sample contains parse errors #73

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

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented May 4, 2023

  • The static and readonly modifiers for properties are, at the time of writing, mutually exclusive, which makes the first property example a parse error.
  • final properties are not a thing in PHP, which makes the second property example a parse error.

This commit fixes both code examples.

While I can appreciate that the code sample intends to show an example where all possible modifier keywords + type declarations are used, I do not believe that showing this via code which isn't valid PHP to begin with, is helpful.

Introduced in #19.

* The `static` and `readonly` modifiers for properties are, at the time of writing, mutually exclusive, which makes the first property example a parse error.
* `final` properties are not a thing in PHP, which makes the second property example a parse error.

This commit fixes both code examples.

While I can appreciate that the code sample _intends_ to show an example where all possible modifier keywords + type declarations are used, I do not believe that showing this via code which isn't valid PHP to begin with, is helpful.

Introduced in 19.
@samdark samdark added the bug Something isn't working label May 4, 2023
@Crell
Copy link
Collaborator

Crell commented May 4, 2023

A valid point, but to me that means we shouldn't just change the examples, but add more. Like, "public static string $foo", "public readonly string $bar", etc. Let's show several different valid permutations. (Valid both by PHP and the PER.) Can you update?

@jrfnl
Copy link
Contributor Author

jrfnl commented May 5, 2023

we shouldn't just change the examples, but add more. Like, "public static string $foo", "public readonly string $bar", etc. Let's show several different valid permutations.

It already does and is also, why I changed protected in the second example to private.

The combined examples in that whole section now show all possible modifiers in use within valid PHP code.

@Crell
Copy link
Collaborator

Crell commented May 5, 2023

Ah, that's what I get for not expanding the diff... 😛 Thanks!

@Crell Crell merged commit a34e2ca into php-fig:master May 5, 2023
@jrfnl jrfnl deleted the feature/4.6-modifier-keywords-fix-code-sample branch May 5, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants