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

Remove redundant parenthesis on attribute #19899

Conversation

alamirault
Copy link
Contributor

Follow #19898 on, 6.4

In symfony codebase #[Foo] is used instead of #[Foo()]

Phpstorm like also without
image

I've made also PR for 6.4, if you're agree with this rule I will made other PR.
(And maybe a new doctor-rst rule)

@carsonbot carsonbot added this to the 6.4 milestone May 20, 2024
@OskarStark
Copy link
Contributor

What about a DOCtor rule? 😅

@OskarStark
Copy link
Contributor

Thank you Antoine.

@OskarStark OskarStark merged commit fe023ab into symfony:6.4 May 21, 2024
3 checks passed
@dkarlovi
Copy link
Contributor

@OskarStark would it be possible to have DOCtor run an external task on the block?

That way, you could define a coding style for PHP CS Fixer and have it run across the docs.

@OskarStark
Copy link
Contributor

I tried this already, but not all code examples we have a complete AND valid php code in the past.

But we can try ofc, do want to give it a try? 😅

@dkarlovi
Copy link
Contributor

I thought about that, there can be two approaches:

  1. the tools get support for partial syntax (hard)
  2. the examples are actually made complete, but then the unimportant parts get hidden (either stripped fully or collapsed, like in Github / Gitlab diff view) when importing

The second approach seems like it would work on not just PHP, but YAML, XML, basically anything. It would also mean some additional processing could be done, for example Psalm could run on PHP code examples, xmllint could run on XML examples, etc.

@alamirault alamirault deleted the feature/remove-redundant-parenthesis-attribute-64 branch May 22, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants