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

NoSpacesAfterFunctionNameFixer - introduce fix_special_constructs option #4998

Closed
wants to merge 5 commits into from
Closed

NoSpacesAfterFunctionNameFixer - introduce fix_special_constructs option #4998

wants to merge 5 commits into from

Conversation

guilliamxavier
Copy link
Contributor

Closes #4817
Opt-in version of #4837
Alternative to (but not incompatible with) #4893

(The first commits are just refactoring and optimization, the actual change is the last commit)

This option is enabled by default (to keep BC) but disabled in the @Symfony ruleset (cf. symfony/symfony#35588 (comment))

@guilliamxavier
Copy link
Contributor Author

I probably should ask @nicolas-grekas what they think about the @Symfony ruleset part?

@nicolas-grekas
Copy link

In the Symfony CS, a space after echo is always desired (but no space after array).

@guilliamxavier
Copy link
Contributor Author

Thanks. Well, with this PR, php-cs-fixer fix --rules=@Symfony would no longer remove spaces after echo (but would still remove them after array) 🙂 (To also have it add a space after echo (but not array) if there isn't one already, we would need #4435)

@@ -1196,6 +1196,11 @@ Choose from the list of available rules:
When making a method or function call, there MUST NOT be a space between
the method or function name and the opening parenthesis.

Configuration options:

- ``fix_special_constructs`` (``bool``): whether to fix ``echo``, ``print``,
Copy link
Contributor

Choose a reason for hiding this comment

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

The name fix_special_constructs is really confusing - why these 6 are "special" and other from $tokenKinds are not?

Can we have tokens option like in no_extra_blank_lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

array, empty etc. are language constructs (dedicated tokens, cannot be called using variable functions) but require parentheses around their argument(s) (just like regular functions), whereas
echo, print etc. are "special" [language] constructs in that they don't expect parentheses around their argument(s) (like return or throw).
That's maybe confusing (and I could even add "why not clone?") but that's how the fixer was created (and see #1266) and my attempts to fix it so far have been blocked or ignored (at least that's how it feels like) 😕

Thanks for the review, I shall try to do like NoExtraBlankLinesFixer when I get more time...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants