New sniff to enforce using the Lite textdomain for translations#2877
Conversation
📝 WalkthroughWalkthroughA new PHP_CodeSniffer sniff ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.php`:
- Around line 87-94: The domain index mapping for the plural translation
functions is off: in the UseSharedTranslationDomainSniff mapping for '_n' and
'_nx' update the 'domain' values to the correct 0-based positions (set '_n' =>
['domain' => 3] and '_nx' => ['domain' => 4]) so the sniff checks the correct
argument for the domain when analyzing _n() and _nx() calls.
- Around line 365-407: In getStringValue, ensure the argument is a pure literal
string and unescape its contents: scan from $arg['start'] to $arg['end'] to find
a single T_CONSTANT_ENCAPSED_STRING token (the $stringToken) but do not return
early on the first non-string token — instead verify that all tokens before and
after the found string are only T_WHITESPACE (reject if any other token exists,
which prevents allowing concatenation like 'foo' . $bar); once you have the
string content and the quote character, remove the surrounding quotes and
unescape properly: for single-quoted strings use str_replace to convert \' and
\\ to their characters, and for double-quoted strings use stripcslashes to
interpret C-style escapes, then return the unescaped content.
- Around line 270-302: The current loadPotFile() uses
preg_match_all('/^msgid\s+"(.*)"/m', ...) which fails to capture multi-line
msgid entries; update loadPotFile() to handle msgid "" followed by one or more
continuation lines by either: (a) using a regex that captures the full msgid
block including following quoted continuation lines (match msgid "..." OR msgid
"" followed by subsequent lines starting with quote marks) and then join the
captured pieces, or (b) parse $content line-by-line inside loadPotFile() to
detect a msgid start (msgid "..." or msgid "") and, if empty, accumulate
subsequent lines that begin with a quoted string until a non-quoted line
appears; after assembling the full message, call unescapePotString() on the
concatenated string and add it to $strings. Ensure the code still skips the
header empty msgid and that preg_match_all or the parser targets only msgid (not
msgstr) occurrences.
| '_n' => array( | ||
| 'string' => 0, | ||
| 'domain' => 4, | ||
| ), | ||
| '_nx' => array( | ||
| 'string' => 0, | ||
| 'domain' => 5, | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WordPress _n() function signature parameter order domain
💡 Result:
In WordPress, the _n() signature (and parameter order) is:
_n( string $single, string $plural, int $number, string $domain = 'default' ): string
So the text domain is the 4th parameter ($domain), optional, defaulting to 'default'. [1]
🌐 Web query:
WordPress _nx() function signature parameter order domain
💡 Result:
In WordPress, the _nx() function signature (and parameter order, including the text domain last) is:
_nx( string $single, string $plural, int $number, string $context, string $domain = 'default' ): string [1]
So the domain parameter is 5th (last), optional, defaulting to 'default'. [1]
Fix domain argument indexes for _n and _nx.
Both functions use 0‑based positions. In the WordPress _n() function, the domain is the 4th parameter (0-based index 3). In _nx(), the domain is the 5th parameter (0-based index 4). The current mapping (4 and 5) will cause the sniff to fail when checking these translation calls.
🛠️ Proposed fix
- '_n' => array(
- 'string' => 0,
- 'domain' => 4,
- ),
- '_nx' => array(
- 'string' => 0,
- 'domain' => 5,
- ),
+ '_n' => array(
+ 'string' => 0,
+ 'domain' => 3,
+ ),
+ '_nx' => array(
+ 'string' => 0,
+ 'domain' => 4,
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| '_n' => array( | |
| 'string' => 0, | |
| 'domain' => 4, | |
| ), | |
| '_nx' => array( | |
| 'string' => 0, | |
| 'domain' => 5, | |
| ), | |
| '_n' => array( | |
| 'string' => 0, | |
| 'domain' => 3, | |
| ), | |
| '_nx' => array( | |
| 'string' => 0, | |
| 'domain' => 4, | |
| ), |
🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.php`
around lines 87 - 94, The domain index mapping for the plural translation
functions is off: in the UseSharedTranslationDomainSniff mapping for '_n' and
'_nx' update the 'domain' values to the correct 0-based positions (set '_n' =>
['domain' => 3] and '_nx' => ['domain' => 4]) so the sniff checks the correct
argument for the domain when analyzing _n() and _nx() calls.
| private function loadPotFile() { | ||
| if ( empty( $this->potFilePath ) || ! file_exists( $this->potFilePath ) ) { | ||
| return array(); | ||
| } | ||
|
|
||
| $content = file_get_contents( $this->potFilePath ); | ||
|
|
||
| if ( false === $content ) { | ||
| return array(); | ||
| } | ||
|
|
||
| $strings = array(); | ||
|
|
||
| // Parse msgid entries from the .pot file. | ||
| // Match msgid "string" or msgid "" followed by continuation strings. | ||
| preg_match_all( '/^msgid\s+"(.*)"/m', $content, $matches ); | ||
|
|
||
| if ( ! empty( $matches[1] ) ) { | ||
| foreach ( $matches[1] as $string ) { | ||
| // Skip empty strings (header). | ||
| if ( '' === $string ) { | ||
| continue; | ||
| } | ||
|
|
||
| // Unescape the string. | ||
| $string = $this->unescapePotString( $string ); | ||
|
|
||
| $strings[ $string ] = true; | ||
| } | ||
| } | ||
|
|
||
| return $strings; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GNU gettext .pot file format multiline msgid continuation lines
💡 Result:
In GNU gettext .pot (same syntax as .po), multi-line msgid/msgstr values are written as multiple adjacent quoted C-strings, which gettext concatenates—they are not continued with backslashes or “escaped newlines” in the file itself. The canonical style is:
msgid ""
"First line\n"
"Second line\n"
"Third line"Key points:
msgid(andmsgstr) must be followed by a quoted string; usingmsgid ""is a common way to keep subsequent lines neatly aligned, and the empty string just concatenates to nothing. [1]- Each quoted line uses C string escaping (e.g.,
\n,\",\\), and the PO file’s physical newlines outside quotes do not become part of the string—only\ninside quotes does. [1] - This same concatenation rule applies to other string fields like
msgid_pluralandmsgstr[0],msgstr[1], etc. (they can also be split across multiple quoted lines). [2]
Sources: GNU gettext manual on multi-line strings and PO file entry structure. [1] [2]
🏁 Script executed:
# First, locate and examine the actual file mentioned in the review
fd "UseSharedTranslationDomainSniff.php" --type fRepository: Strategy11/formidable-forms
Length of output: 150
🏁 Script executed:
sed -n '260,310p' phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1131
🏁 Script executed:
# Also check if there's an unescapePotString method
grep -n "unescapePotString" phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.phpRepository: Strategy11/formidable-forms
Length of output: 178
🏁 Script executed:
# Check the full implementation of unescapePotString
sed -n '311,350p' phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1422
🏁 Script executed:
# Also check how the pot file content is typically structured by looking for test files
fd -e pot -e po --type f | head -20Repository: Strategy11/formidable-forms
Length of output: 245
🏁 Script executed:
# Examine the actual .pot file to see if it contains multi-line msgid entries
head -100 languages/formidable.potRepository: Strategy11/formidable-forms
Length of output: 3124
🏁 Script executed:
# Search for msgid "" pattern which would indicate multi-line entries
grep -A 3 'msgid ""' languages/formidable.pot | head -40Repository: Strategy11/formidable-forms
Length of output: 209
🏁 Script executed:
# Search for msgid entries with continuation lines (multi-line format)
# This would show msgid "" or msgid "something" followed by "..." on next lines
grep -E '(^msgid |^msgstr )' languages/formidable.pot | head -50Repository: Strategy11/formidable-forms
Length of output: 1151
🏁 Script executed:
# More targeted search: look for lines with just quotes after msgid/msgstr
awk '/^msgid ""/,/^[a-z]/ {print NR": "$0}' languages/formidable.pot | head -100Repository: Strategy11/formidable-forms
Length of output: 82
🏁 Script executed:
# Check if there are any actual multi-line msgid entries in the .pot file
# (where msgid is followed by continuation lines like "...")
grep -B1 -A5 'msgid "' languages/formidable.pot | grep -A3 'msgid ""' | tail -20Repository: Strategy11/formidable-forms
Length of output: 209
🏁 Script executed:
# Look for the pattern of msgid "" (empty) followed by quoted continuation strings
awk '/^msgid ""$/ {found=1; print NR": "$0; next} found {print NR": "$0; if (/^msgid/ || /^msgstr/ || /^#/) {if (!/^msgid ""/) found=0}}' languages/formidable.pot | head -50Repository: Strategy11/formidable-forms
Length of output: 95
🏁 Script executed:
# Let me check more carefully for any multi-line string entries
python3 << 'EOF'
with open('languages/formidable.pot', 'r') as f:
lines = f.readlines()
in_msgid = False
msgid_start_line = 0
continuation_count = 0
for i, line in enumerate(lines[:200], 1):
if line.startswith('msgid '):
if continuation_count > 0:
print(f"Lines {msgid_start_line}-{i-1}: Found multi-line msgid with {continuation_count} continuations")
in_msgid = True
msgid_start_line = i
continuation_count = 0
print(f"{i}: {line.rstrip()}")
elif in_msgid and line.startswith('"') and not line.startswith('msgid'):
continuation_count += 1
print(f"{i}: {line.rstrip()} [continuation {continuation_count}]")
elif in_msgid and not line.startswith('"'):
in_msgid = False
EOFRepository: Strategy11/formidable-forms
Length of output: 157
🏁 Script executed:
# Let me search more carefully for msgid entries with continuation lines
# by looking for the specific pattern of msgid "" followed by quoted strings
grep -n 'msgid ""' languages/formidable.potRepository: Strategy11/formidable-forms
Length of output: 81
🏁 Script executed:
# Check if any msgid (not msgstr) entries span multiple lines
# by looking for msgid followed by a closing quote and then continuation lines
sed -n '1,500p' languages/formidable.pot | grep -E '(^msgid |^msgstr )' | grep -A1 'msgid ""'Repository: Strategy11/formidable-forms
Length of output: 89
🏁 Script executed:
# Let me sample different parts of the file to find if any msgid entries have continuations
tail -500 languages/formidable.pot | head -100Repository: Strategy11/formidable-forms
Length of output: 1974
🏁 Script executed:
# Check the entire .pot file to see if ANY msgid entries have multi-line format
# (where a msgid is split across multiple lines with continuation strings)
wc -l languages/formidable.potRepository: Strategy11/formidable-forms
Length of output: 100
🏁 Script executed:
# Sample more of the file to look for the pattern of msgid with continuations
# Look for msgid followed by a line that starts with a quote
awk '/^msgid / {msgid_line=NR; msgid=$0} /^"/ && NR == msgid_line+1 {print "Found continuation at line " NR ": " msgid " -> " $0}' languages/formidable.pot | head -20Repository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
# More robust check: look for msgid entries where the line doesn't end with a closing quote
grep -n '^msgid ' languages/formidable.pot | grep -v '"$' | head -20Repository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
# Final verification: check for msgid "" pattern (the standard for multi-line entries)
# and see if there are any continuation lines after it (other than msgstr)
grep -n '^msgid ""$' languages/formidable.potRepository: Strategy11/formidable-forms
Length of output: 81
🏁 Script executed:
# Also check the actual .po files to see if they have multi-line entries
head -50 languages/formidable-hu_HU.poRepository: Strategy11/formidable-forms
Length of output: 1783
🏁 Script executed:
# Search specifically for the pattern where msgid is followed by a continuation line
# (a line starting with " that's not msgstr)
awk '/^msgid ".*"$/ {next} /^msgid ""$/ {multiline=1; next} multiline && /^"/ && !/^msgstr/ {print "Found continuation line"; multiline=0} multiline && !/^"/ {multiline=0}' languages/formidable.potRepository: Strategy11/formidable-forms
Length of output: 53
Multi-line msgid support is limited in current implementation, though not triggered by existing .pot files.
The regex /^msgid\s+"(.*)"/m does not capture multi-line msgid entries per the GNU gettext .pot format (where msgid "" can be followed by continuation lines like "part1\n" "part2\n"). However, inspection of languages/formidable.pot and languages/formidable-hu_HU.po reveals that the codebase uses only single-line msgid entries throughout, meaning the limitation does not cause false negatives in practice.
Nevertheless, the code should be hardened to properly handle the documented .pot format for future compatibility and robustness.
🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.php`
around lines 270 - 302, The current loadPotFile() uses
preg_match_all('/^msgid\s+"(.*)"/m', ...) which fails to capture multi-line
msgid entries; update loadPotFile() to handle msgid "" followed by one or more
continuation lines by either: (a) using a regex that captures the full msgid
block including following quoted continuation lines (match msgid "..." OR msgid
"" followed by subsequent lines starting with quote marks) and then join the
captured pieces, or (b) parse $content line-by-line inside loadPotFile() to
detect a msgid start (msgid "..." or msgid "") and, if empty, accumulate
subsequent lines that begin with a quoted string until a non-quoted line
appears; after assembling the full message, call unescapePotString() on the
concatenated string and add it to $strings. Ensure the code still skips the
header empty msgid and that preg_match_all or the parser targets only msgid (not
msgstr) occurrences.
| /** | ||
| * Get the string value from an argument. | ||
| * | ||
| * @param File $phpcsFile The file being scanned. | ||
| * @param array{start: int, end: int} $arg The argument info. | ||
| * | ||
| * @return string|null The string value or null if not a simple string. | ||
| */ | ||
| private function getStringValue( File $phpcsFile, $arg ) { | ||
| $tokens = $phpcsFile->getTokens(); | ||
|
|
||
| // Find the first non-whitespace token in the argument. | ||
| $stringToken = null; | ||
|
|
||
| for ( $i = $arg['start']; $i <= $arg['end']; $i++ ) { | ||
| if ( $tokens[ $i ]['code'] === T_WHITESPACE ) { | ||
| continue; | ||
| } | ||
|
|
||
| if ( $tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) { | ||
| $stringToken = $i; | ||
| break; | ||
| } | ||
|
|
||
| // Not a simple string literal. | ||
| return null; | ||
| } | ||
|
|
||
| if ( null === $stringToken ) { | ||
| return null; | ||
| } | ||
|
|
||
| $content = $tokens[ $stringToken ]['content']; | ||
|
|
||
| // Remove quotes. | ||
| $quote = substr( $content, 0, 1 ); | ||
|
|
||
| if ( $quote === '"' || $quote === "'" ) { | ||
| $content = substr( $content, 1, -1 ); | ||
| } | ||
|
|
||
| return $content; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "UseSharedTranslationDomainSniff.php" | head -5Repository: Strategy11/formidable-forms
Length of output: 152
🌐 Web query:
PHP string literal escaping rules single-quoted double-quoted strings stripcslashes
💡 Result:
PHP string literal escaping rules
Single-quoted strings ('...')
- Only two escape sequences are recognized:
\'→ a literal single quote\\→ a literal backslash
- Any other backslash sequence is not interpreted (e.g.
'\n'is two characters: backslash +n), and variables are not expanded. [1]
Double-quoted strings ("...")
- PHP interprets common C-style escapes such as
\n,\r,\t,\\,\",\$, plus octal (\123), hex (\x7F), and Unicode codepoint escapes (\u{...}), and it also performs variable interpolation. [1]
stripcslashes()
stripcslashes($s)un-escapes C-style backslash sequences (the kind produced byaddcslashes()), converting sequences like\ninto an actual newline character, etc. [2]- Example from the manual:
stripcslashes('I\'d have a coffee.\nNot a problem.')becomes a string containing a real newline between the sentences. [2]
Key practical point: if you write '\n' (single-quoted), PHP does not turn it into a newline at parse time—but stripcslashes('\n') will turn it into an actual newline at runtime. [1][2]
Sources: PHP Strings manual [1]; stripcslashes() manual [2].
🏁 Script executed:
cat -n ./phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.php | sed -n '365,407p'Repository: Strategy11/formidable-forms
Length of output: 1388
🏁 Script executed:
grep -n "getStringValue\|T_CONSTANT_ENCAPSED_STRING" ./phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.php | head -20Repository: Strategy11/formidable-forms
Length of output: 412
Ensure the argument is only a literal string and unescape escape sequences for reliable matching.
The current implementation has two issues:
-
Concatenation not rejected: The loop breaks immediately after finding the first string token, without validating that only whitespace follows. This allows expressions like
'foo' . $barto be treated as literal strings. -
Escape sequences not unescaped: The code removes quotes but does not unescape escape sequences. For double-quoted strings, sequences like
\nremain literal; for single-quoted strings,\'and\\are not converted to their actual characters. This causes mismatches against .pot files.
The proposed fix adds validation to reject any non-whitespace tokens after the string literal, and properly unescapes the content: using str_replace() for single-quoted strings (which only recognize \' and \\), and stripcslashes() for double-quoted strings (which support C-style escape sequences).
🛠️ Proposed fix
- // Find the first non-whitespace token in the argument.
+ // Find the first non-whitespace token in the argument.
$stringToken = null;
for ( $i = $arg['start']; $i <= $arg['end']; $i++ ) {
if ( $tokens[ $i ]['code'] === T_WHITESPACE ) {
continue;
}
if ( $tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) {
$stringToken = $i;
break;
}
// Not a simple string literal.
return null;
}
if ( null === $stringToken ) {
return null;
}
+
+ // Ensure the argument is only a single string literal (no concatenation, etc.).
+ for ( $i = $stringToken + 1; $i <= $arg['end']; $i++ ) {
+ if ( $tokens[ $i ]['code'] === T_WHITESPACE ) {
+ continue;
+ }
+ return null;
+ }
$content = $tokens[ $stringToken ]['content'];
// Remove quotes.
$quote = substr( $content, 0, 1 );
if ( $quote === '"' || $quote === "'" ) {
$content = substr( $content, 1, -1 );
}
- return $content;
+ if ( "'" === $quote ) {
+ return str_replace( array( '\\\\', "\\'" ), array( '\\', "'" ), $content );
+ }
+
+ return stripcslashes( $content );🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/Translations/UseSharedTranslationDomainSniff.php`
around lines 365 - 407, In getStringValue, ensure the argument is a pure literal
string and unescape its contents: scan from $arg['start'] to $arg['end'] to find
a single T_CONSTANT_ENCAPSED_STRING token (the $stringToken) but do not return
early on the first non-string token — instead verify that all tokens before and
after the found string are only T_WHITESPACE (reject if any other token exists,
which prevents allowing concatenation like 'foo' . $bar); once you have the
string content and the quote character, remove the surrounding quotes and
unescape properly: for single-quoted strings use str_replace to convert \' and
\\ to their characters, and for double-quoted strings use stripcslashes to
interpret C-style escapes, then return the unescaped content.
1 participant
This shouldn't do anything really in this repo, but in other repositories it can help us remove string translations that are already handled in Lite.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.