False internationalization positive for extra leading/trailing spaces

Attached patch should fix this issue.

I could have changed the regex to:

-      '#value' => '[\s\(]t\s*\(\s*[\'"](\s+|.*\s+[\'"]\s*[,\)])',
+      '#value' => '[\s\(]t\s*\(\s*[\'"](\s+|[^\'"]*\s+[\'"]\s*[,\)])',

However this would then miss trailing spaces in the string if the string itself contained a single or double quote. I've tried to over come this scenario by changing the rule to:

     array(
       '#type' => 'regex',
       '#value' => '[\s\(]t\s*\(\s*[\'"](\s+|.*\s+[\'"]\s*[,\)])',
+      '#never' => '[\s\(]t\s*\(\s*[\'"](\s+|([\'"]\s+[^,\)])*.*[^\s][\'"][,\)])',
       '#source' => 'allphp',
       '#warning_callback' => '_coder_i18n_space_starts_or_ends_t',
     ),

This should do the current match, but should then exclude matches where there is a quote followed by a space and something which is not a comma/bracket and which is also terminated by a quote and a comma/bracket which isn't prepended by a space. Hope that makes sense.

So the following checks should now be alerted upon:

  $var = t('This isn't a false positive ');
  $var = t('This isn\'t a "false" positive ');

but won't alert on:

  $var = t('This is a false positive') . implode(', ', $array); // This is the one that is currently happening, frequently.
  $var = t('This is a \'false\" positive');
  $var = t('This is a "false" positive');

I'm pretty confident this works, but I'd like it if someone could test it and confirm.

Cheers,
Stella

Ok, here's a slightly modified one. It catches all 6 tests.

Cheers,
Stella

Hmmm, this might need more work. It's now not catching lines like these:

$description[] = t('!days', array('!days' => implode(t(' and '), $results)));
$description[] = t('until !until ', array('!until' => date_format_date($until, 'custom', $format)));
$description[] = t('except !dates ', array('!dates' => implode(', ', $values)));

The first patch has already been committed, so this one is in addition to the above one. It should now catch most of the above scenarios. It catches all but the first one, where there is a nested t(). I'm not sure how to change the code to catch this, or even is something we should be trying to handle. Are nested t()s really valid? I'm inclined to think not.

The other scenario where it may not work is:

$var = t('Your rating') . t('(fail) ');

i.e. where there is more than one t() call on the same line, and the last one has a trailing space and the last one also contains a closing ). The chances of this are probably slim enough, though it could probably be fixed with a bit more effort. I do think this patch is an improvement in any case - just try some of the new test cases with the old rule to see.

Oh it also checks for leading/trailing spaces in get_t() and st() too.

Cheers,
Stella