Add declare(strict_types=1) to all Functional JavaScript tests

Blocker is in. I reviewed the MR changes. Although it's a huge diff, it's very scannable, since it's all the identical change. I didn't spot any other changes. Bot is happy. Title and summary are accurate. Nothing else to complain about.

I think the more condensed version used in the MR (declare(strict_types=1);) is fine.

Thanks!
-Derek

As per #3401994-7: Add declare(strict_types=1) to all test traits → .

I agree that in this instance the large scope is acceptable; it's an example of a totally scannable change that doesn't require any extra decisions or context.

Oops, forgot to change the status and finish my sentence. I meant that as per my comment on the other issue, we should try to add a coding standards check to ensure that any new JS tests folks add get strict typing enabled, so that this work does not regress.

So should this be postponed till the rule gets merged?

Yep, I think it makes sense to postpone it on the trait issue. That one is nice and short, so a good place to work out the exact specifics.

Simplified the steps to not require rector

I'm just calling out that this issue and the seemingly related https://www.drupal.org/project/drupal/issues/3401994 📌 Add declare(strict_types=1) to all test traits RTBC are making opposite decisions on spaces and enforcement.

The trait issue has this rule:
And put all declarations like this: declare(strict_types = 1);

But this JS issue has this rule:
And put all declarations like this: declare(strict_types=1);

No spaces seems to be the decision made: see https://www.drupal.org/project/drupal/issues/3401994#comment-15330621 📌 Add declare(strict_types=1) to all test traits RTBC for references.

Unpostponing while we resolve merge conflicts and make sure no additional ones have crept back in.

Merged the rules section again.

In the case of this issue, there was also a test removed since it was created:

core/profiles/standard/tests/src/FunctionalJavascript/NoJavaScriptAnonymousTest.php

I removed that in our version too.

Three new tests missing it here as well:

Time: 10.25 secs; Memory: 6MB
PHP CODE SNIFFER REPORT SUMMARY
----------------------------------------------------------------------
FILE                                                  ERRORS  WARNINGS
----------------------------------------------------------------------
...c/FunctionalJavascript/DefaultValueWidgetTest.php  1       0
.../FunctionalJavascript/StandardPerformanceTest.php  1       0
...Drupal/FunctionalJavascriptTests/AjaxWaitTest.php  1       0
----------------------------------------------------------------------
A TOTAL OF 3 ERRORS AND 0 WARNINGS WERE FOUND IN 3 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 3 OF THESE SNIFF VIOLATIONS AUTOMATICALLY

Again, this can either be postponed on the traits issue or fixed concurrently. Thanks!

The phpcs rule will probably conflict with the one in 📌 Add declare(strict_types=1) to all test traits RTBC , but otherwise this issue looks good to go to me.

Postponing on the traits issue since the ruleset will need another change after it lands.

OK, the other is in.

Looks great, the phpcs file is updated to include the traits as well as this and phpcs passes successfully, back to rtbc.

Committed to 11.x

Asking release managers if its ok to backport this to 10.2 during the 10.2.0 window or if it needs to wait until after.

larowlan → closed merge request !5421

larowlan → credited longwave → .

@longwave confirmed this was fine to backport to 10.2.x

It doesn't apply to 10.1.x so marking as fixed as that branch is security only.

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