BasicSettingsForm pointlessly builds up a list of themes

I am looking into this.

This looks great, except we both didn't spot that with that service removed, there is no longer any reason for this class to have any DI code at all -- create() and __construct() can be completely removed, since they do the same as the parent class's methods.

Updated the MR.

That's great, thanks!

This needs work for the failing the PHPStan checks.

@sandip poddar, you can run these checks locally. It is good practice to Run core development checks → locally before submitting a change.

What removing unused code it helps to know the history. Was this used in the past? Is there an intention to use this?

Generally, yes, but here it's pretty clearly long dead code.

However, I've chased this back as far as:

- e298180f76b5e65f68c40d3c6de0761c26eee480 when the class was converted to PSR4
- e16bbd998edaa1a41624e0d8bbb0a5021da43501 when the module was moved
- 026c485a0fe01516ca7a9e82d806ab06041cc757 when the form was converted to a class

prior to that, the code is in the function views_ui_admin_settings_basic() in core/modules/views/views_ui/admin.inc and the $options array of themes is STILL not being used.

@quietone, Thank you for the feedback. The PHPStan errors are occurring because the .phpstan-baseline.php file is still expecting a return type for the create method in the BaseSettingsForm class. However, I have removed the -- create() method from this class as the ThemeHandler is no longer used in this context. This discrepancy is causing the PHPStan checks to fail. Kindly share your feedback on this.

$ignoreErrors[] = [
	// identifier: missingType.return
	'message' => '#^Method Drupal\\\\views_ui\\\\Form\\\\BasicSettingsForm\\:\\:create\\(\\) has no return type specified\\.$#',
	'count' => 1,
	'path' => __DIR__ . '/modules/views_ui/src/Form/BasicSettingsForm.php',
];

I'm don't know that much about PHPStan, but I think that we can just remove that item from the baseline?

Yes @joachim, I believe we can remove that specific item from the baseline file, as the check is no longer relevant due to the removal of the create method in the BaseSettingsForm class. This should resolve the PHPStan errors effectively.

If you agree, I will proceed to update the baseline file accordingly in the MR.

@sandip poddar, yes, remove those lines. Then when commit-code-check.sh is run, the change in the /.phpstan-baseline.php
is detected and PHPStan is run on all core files. That will take a minute locally.

There is a handbook page about working with PHPStan → , which should explain more about how it is used with Drupal.

Thank @quietone for your guidance. I am working on it.

Please review the MR. Here i also fixed some Merge conflicts.

LGTM.

The test failure is an unrelated nightwatch glitch.

After rebase unrelated Nightwatch test is also green now.

Committed/pushed to 11.x, thanks!

catch → closed merge request !10620

Automatically closed - issue fixed for 2 weeks with no activity.