Problem/Motivation

coder has some useful rules for preventing issues with translatable strings. Let's turn some on in the core ruleset.

Proposed resolution

Enable some of the t() semantic rules.

How to review:

$ composer require drupal/coder
$ ./vendor/bin/phpcs --config-set installed_paths /PATH/drupal/vendor/drupal/coder/coder_sniffer/
$ cd core
$ ../vendor/bin/phpcs -p

...with no errors.

Remaining tasks

User interface changes

Some broken strings are fixed. Because strings are changing this is only eligible for 8.2.0.

API changes

None

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
FileSize
7.26 KB
Mile23’s picture

Issue summary: View changes
Status: Needs review » Needs work
+++ b/core/modules/system/src/Tests/Module/InstallUninstallTest.php
@@ -203,7 +204,9 @@ public function testInstallUninstall() {
-    $this->assertText(t('@count modules have been enabled: ', array('@count' => count($all_modules))), 'Modules status has been updated.');
+    // The string been tested here is translatable but only part of it is being
+    // tested so using a translated string is impossible.
+    $this->assertText(new FormattableMarkup('@count modules have been enabled: ', array('@count' => count($all_modules))), 'Modules status has been updated.');

The patch looks good except for this one typo. Mmmm, string beans! Should be 'string being.'

alexpott’s picture

Status: Needs work » Needs review
FileSize
8.35 KB
2.03 KB

I've run the updated ruleset with #2710209: The FunctionT sniff needs to support TranslatableMarkup and TranslationWrapper too applied so adding a fix for the one thing that that found.

andypost’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
8.35 KB

reroll, looks good

remains are

     17 Drupal.Semantics.FunctionT.BackslashSingleQuote
     62 Drupal.Semantics.FunctionT.NotLiteralString
     66 Drupal.Semantics.FunctionT.ConcatString
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed b864a3a and pushed to 8.2.x. Thanks!

Unfortunately we can't have this in 8.1.x because of the string changes.

  • alexpott committed b864a3a on 8.2.x
    Issue #2710201 by alexpott, andypost: Fix Drupal.Semantics.FunctionT....

Status: Fixed » Closed (fixed)

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