Skip to content

[Rector] Remove @var from class constant #4766

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
merged 1 commit into from
Jun 2, 2021

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Jun 2, 2021

I created new custom rector rule RemoveVarTagFromClassConstantRector to remove @var from class const per this discussion #4759 (comment)

Checklist:

  • Securely signed commits

@samsonasik samsonasik force-pushed the remove-var-from-const branch from 9d9c184 to 3e4e414 Compare June 2, 2021 03:48
@samsonasik samsonasik requested a review from lonnieezell June 2, 2021 03:49
@MGatner MGatner merged commit 6a2d5d6 into codeigniter4:develop Jun 2, 2021
@samsonasik samsonasik deleted the remove-var-from-const branch June 2, 2021 14:51
@kenjis
Copy link
Member

kenjis commented Nov 20, 2023

Why don't we remove this Rector?
I don't see the need to delete @var that someone wrote.

The @var tag defines which type of data is represented by a value of a Constant, Property or Variable.
https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc-tags.md#518-var

@MGatner
Copy link
Member

MGatner commented Nov 20, 2023

I don't have a strong opinion. My original point was that the @var tag is unnecessary on constants because their type is apparent at runtime. If it's helpful for devs to indicate that explicitly on the code then a comment could always be used without the tag.

I suppose an argument could be made for non-native types, like:

/**
 * @var class-string<Model>
 */
public const $model = Widget::class;

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.

4 participants