Remove deprecated method `className()` for branch `2.2`. #19894

terabytesoftw · 2023-07-14T21:52:36Z

Q	A
Is bugfix?	✔️
New feature?	❌
Breaks BC?	❌

what-the-diff · 2023-07-14T21:54:31Z

PR Summary

This Pull Request primarily focuses on updating the codebase to utilize the ::class constant instead of the ::className() method across multiple files, which enhances readability and code quality. Here are the main areas of improvement:

Replacement of className() Method Calls
In numerous files, references to object classes have been updated from className() method calls to direct references using ::class. This has advantages in terms of clarity and potential error reduction by providing a simpler, more direct way of referring to a class.
Update to yii\base\BaseObject Method Call
The className() method call in the yii\base\BaseObject::className() line was replaced with yii\base\BaseObject::class.
Changes in Various Controllers
Several controller files migrated from using className() to class. For instance, in files like Controller.php, CacheController.php, FixtureController.php, and MigrationController.php.
Changes to the Database Classes
References to various database connection and schema-building classes, such as Connection, QueryBuilder, ColumnSchemaBuilder, were updated in numerous files, including BaseDataProvider.php, Schema.php, and DbDependency.php.
Changes to Authorization and Access Control Classes
Class references in files related to authorization, like AccessControl.php and PhpManager.php, were revised from className() method calls to class constant.
Changes in View Files
Changes were made to achieve consistency in class referencing in files such as GridView.php and BaseMailer.php.
Updates in Test Files
Several unit test-related files were updated, notably replacing className() method calls with class in files like ActiveFixtureTest.php, ExistValidatorTest.php, and ValidatorTest.php.

In short, this PR modernizes the codebase by adopting the ::class constant for class references, making the codebase easier to understand and maintain.

codecov · 2023-07-14T21:54:37Z

Codecov Report

Patch coverage: 88.33% and no project coverage change.

Comparison is base (9a5e2a5) 66.38% compared to head (d7740c3) 66.38%.

Additional details and impacted files

@@            Coverage Diff            @@
##                2.2   #19894   +/-   ##
=========================================
  Coverage     66.38%   66.38%           
+ Complexity    11338    11337    -1     
=========================================
  Files           429      429           
  Lines         35934    35934           
=========================================
  Hits          23856    23856           
  Misses        12078    12078

Impacted Files	Coverage Δ
framework/base/BaseObject.php	`100.00% <ø> (ø)`
...ramework/console/controllers/FixtureController.php	`0.00% <0.00%> (ø)`
framework/db/cubrid/Schema.php	`0.00% <0.00%> (ø)`
framework/web/UrlManager.php	`95.05% <0.00%> (ø)`
...ramework/console/controllers/MigrateController.php	`22.69% <50.00%> (ø)`
framework/data/DataFilter.php	`81.77% <50.00%> (ø)`
framework/db/Schema.php	`88.23% <50.00%> (ø)`
framework/web/AssetManager.php	`72.89% <50.00%> (ø)`
framework/base/Controller.php	`85.61% <100.00%> (ø)`
framework/behaviors/SluggableBehavior.php	`96.49% <100.00%> (ø)`
... and 32 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

rhertogh · 2023-07-15T13:06:05Z

yii2-gii has the option to use the class constant or not in the model generator (https://github.com/yiisoft/yii2-gii/blob/master/src/generators/model/Generator.php#L53).
It won't cause immediate problems since the composer.json specifies "yiisoft/yii2": "~2.0.46" but this PR will brake that setting in the future.
I think it would be good to have a central place to keep track of these breaking changes. We could, for example, create an issue in the yii2-gii repo and add a Yii2.2 milestone.
@samdark What do you think about it?

terabytesoftw · 2023-07-15T13:10:00Z

yii2-gii has the option to use the class constant or not in the model generator (https://github.com/yiisoft/yii2-gii/blob/master/src/generators/model/Generator.php#L53).
It won't cause immediate problems since the composer.json specifies "yiisoft/yii2": "~2.0.46" but this PR will brake that setting in the future.
I think it would be good to have a central place to keep track of these breaking changes. We could, for example, create an issue in the yii2-gii repo and add a Yii2.2 milestone.
@samdark What do you think about it?`

If it is a good suggestion create the 2.2 branch. in all yii2 repositories, and run tests with the 2.2 branch, and update all changes.

schmunk42 · 2023-07-18T09:39:41Z

This creates also a lot of problems with 3rd party extensions, making it quite impossible to upgrade existing apps.

Could we just keep in BaseObject.php (for now):

    public static function className()
    {
        return get_called_class();
    }

Maybe with an additional Yii::info(...) call.

mtangoo · 2023-07-18T09:49:44Z

@schmunk42 if we do that, then we will have problems as there will always be extension lagging.
That change can be fixed with simple search/replace and any extension unwilling to do that shouldn't really be considered as maintained.

Best option is letting extensions update themselves for that change

schmunk42 · 2023-07-18T10:13:51Z

shouldn't really be considered as maintained.

I basically agree, but it's the reality.
There are also cases where this might be patched in an extension, but you still need an older version due to other constraints.

It's a fine line between BC and using more clean code.

But this is a huge blocker for existing projects. And makes it impossible just to test 2.2 for a lot of existing stuff.
In our case we'd have to override BaseObject in a very ugly way, since we can't fork a dozen repos just for a search and replace.

How about something like:

if (YII_ENABLE_20_BC_MODE) {
    public static function className()
    {
        return get_called_class();
    }
}

it has not to stay there forever, but would give developers the opportunity to have a more graceful way of upgrading.
PHP usually does the same for x.0 versions, hence the bigger problems with 8.1 and 8.2 atm.

terabytesoftw · 2023-07-18T10:15:57Z

shouldn't really be considered as maintained.

I basically agree, but it's the reality. There are also cases where this might be patched in an extension, but you still need an older version due to other constraints.

It's a fine line between BC and using more clean code.

But this is a huge blocker for existing projects. And makes it impossible just to test 2.2 for a lot of existing stuff. In our case we'd have to override BaseObject in a very ugly way, since we can't fork a dozen repos just for a search and replace.

How about something like:
if (YII_ENABLE_20_BC_MODE) {
    public static function className()
    {
        return get_called_class();
    }
}
it has not to stay there forever, but would give developers the opportunity to have a more graceful way of upgrading. PHP usually does the same for x.0 versions, hence the bigger problems with 8.1 and 8.2 atm.

For now we can keep without deprecated giving everyone time to change.

mtangoo · 2023-07-18T11:40:22Z

How about something like:

php if (YII_ENABLE_20_BC_MODE) {
public static function className()
{
return get_called_class();
. }
}

I think we can keep without adding the constant check. But with deprecation to warn people that it is going away!

mtangoo · 2023-07-18T11:42:12Z

shouldn't really be considered as maintained.

I basically agree, but it's the reality. There are also cases where this might be patched in an extension, but you still need an older version due to other constraints.
It's a fine line between BC and using more clean code.
But this is a huge blocker for existing projects. And makes it impossible just to test 2.2 for a lot of existing stuff. In our case we'd have to override BaseObject in a very ugly way, since we can't fork a dozen repos just for a search and replace.
How about something like:
if (YII_ENABLE_20_BC_MODE) {
    public static function className()
    {
        return get_called_class();
    }
}
it has not to stay there forever, but would give developers the opportunity to have a more graceful way of upgrading. PHP usually does the same for x.0 versions, hence the bigger problems with 8.1 and 8.2 atm.
For now we can keep without deprecated giving everyone time to change.

How are people going to notice that the function will be going away without deprecation?

terabytesoftw · 2023-07-18T11:57:05Z

shouldn't really be considered as maintained.

I basically agree, but it's the reality. There are also cases where this might be patched in an extension, but you still need an older version due to other constraints.
It's a fine line between BC and using more clean code.
But this is a huge blocker for existing projects. And makes it impossible just to test 2.2 for a lot of existing stuff. In our case we'd have to override BaseObject in a very ugly way, since we can't fork a dozen repos just for a search and replace.
How about something like:
if (YII_ENABLE_20_BC_MODE) {
    public static function className()
    {
        return get_called_class();
    }
}
it has not to stay there forever, but would give developers the opportunity to have a more graceful way of upgrading. PHP usually does the same for x.0 versions, hence the bigger problems with 8.1 and 8.2 atm.
For now we can keep without deprecated giving everyone time to change.
How are people going to notice that the function will be going away without deprecation?

You are so right.

rhertogh · 2023-07-18T18:19:23Z

@schmunk42

This creates also a lot of problems with 3rd party extensions, making it quite impossible to upgrade existing apps.

I agree with this, but could we do the following:

public static function className()
{
    return static::class;
}

Then we have at least some benefits of the newer PHP version 😅.

Remove deprecate className() method brach 2.2.

01fdf13

terabytesoftw changed the title ~~Remove deprecated className() method for branch 2.2.~~ Remove deprecated method className() for branch 2.2. Jul 14, 2023

terabytesoftw added 3 commits July 14, 2023 17:55

Fix error typo in CHANGELOG.md.

bb891cc

Update UPGRADE.md.

8981171

Fix error typo in workflow.

d7740c3

machour approved these changes Jul 15, 2023

View reviewed changes

terabytesoftw requested a review from a team July 15, 2023 12:31

terabytesoftw added the status:code review The pull request needs review. label Jul 15, 2023

rhertogh approved these changes Jul 15, 2023

View reviewed changes

Arhell approved these changes Jul 16, 2023

View reviewed changes

samdark merged commit c9c100a into yiisoft:2.2 Jul 16, 2023
30 checks passed

terabytesoftw deleted the remove-deprecated-classname-22 branch July 16, 2023 13:45

terabytesoftw mentioned this pull request Sep 22, 2023

Add method className(), don't break BC third party extensions. #19973

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove deprecated method `className()` for branch `2.2`. #19894

Remove deprecated method `className()` for branch `2.2`. #19894

terabytesoftw commented Jul 14, 2023

what-the-diff bot commented Jul 14, 2023

codecov bot commented Jul 14, 2023 •

edited

Loading

rhertogh commented Jul 15, 2023

terabytesoftw commented Jul 15, 2023 •

edited

Loading

schmunk42 commented Jul 18, 2023

mtangoo commented Jul 18, 2023

schmunk42 commented Jul 18, 2023

terabytesoftw commented Jul 18, 2023 •

edited

Loading

mtangoo commented Jul 18, 2023 •

edited

Loading

mtangoo commented Jul 18, 2023

terabytesoftw commented Jul 18, 2023

rhertogh commented Jul 18, 2023 •

edited

Loading

Remove deprecated method className() for branch 2.2. #19894

Remove deprecated method className() for branch 2.2. #19894

Conversation

terabytesoftw commented Jul 14, 2023

what-the-diff bot commented Jul 14, 2023

PR Summary

codecov bot commented Jul 14, 2023 • edited Loading

Codecov Report

rhertogh commented Jul 15, 2023

terabytesoftw commented Jul 15, 2023 • edited Loading

schmunk42 commented Jul 18, 2023

mtangoo commented Jul 18, 2023

schmunk42 commented Jul 18, 2023

terabytesoftw commented Jul 18, 2023 • edited Loading

mtangoo commented Jul 18, 2023 • edited Loading

mtangoo commented Jul 18, 2023

terabytesoftw commented Jul 18, 2023

rhertogh commented Jul 18, 2023 • edited Loading

Remove deprecated method `className()` for branch `2.2`. #19894

Remove deprecated method `className()` for branch `2.2`. #19894

codecov bot commented Jul 14, 2023 •

edited

Loading

terabytesoftw commented Jul 15, 2023 •

edited

Loading

terabytesoftw commented Jul 18, 2023 •

edited

Loading

mtangoo commented Jul 18, 2023 •

edited

Loading

rhertogh commented Jul 18, 2023 •

edited

Loading