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

PromotedConstructorPropertyFixer suggestions #917

Open
zanderwar opened this issue Sep 28, 2023 · 2 comments
Open

PromotedConstructorPropertyFixer suggestions #917

zanderwar opened this issue Sep 28, 2023 · 2 comments
Labels

Comments

@zanderwar
Copy link

If the existing class property has a doc block, then the positioning of it when it gets moved to the __constructor is a little whack

Adding the following to the bottom of updateParameterSignature resulted in perfect positioning

foreach ($tokensToInsert as $key => $token) {
    if ($token->isGivenKind(\T_DOC_COMMENT)) {
        $tokensToInsert[$key] = new Token([\T_DOC_COMMENT, PHP_EOL.'    '.preg_replace('/^/m', '    ', $token->getContent())]);
    }
}

$this->tokensToInsert[$propertyStartIndex] = $tokensToInsert;

A few additional options would be nice:

  • keep_class_property_docblocks defaults to true
  • padding defaults to 4spaces

Currently you only put 1 whitespace in front of the promoted property which doesn't follow standards I believe.

@kubawerlos
Copy link
Owner

Hi @zanderwar,

it seems like a good improvement, would you like to raise the PR? Do we need and option for keep_class_property_docblocks or should it always work as it is true?

Currently you only put 1 whitespace in front of the promoted property which doesn't follow standards I believe.

I don't understand, how is it breaking standards? It keeps the padding for what was before.

@windaishi
Copy link

windaishi commented Dec 8, 2023

I think the problem is that the current fixer works like this:
image
I'd like to either remove the docblock or convert it to @param ones.

I currently see no case on why we should keep the doc comment

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

No branches or pull requests

3 participants