fix(i18n): anchor the literal guard's exclusions so they stop swallowing copy (#2288)

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
João Salavisa
2026-08-05 19:36:27 +02:00
committed by GitHub
parent fef76fda57
commit 24c3b594ce
15 changed files with 260 additions and 5 deletions
+14 -2
View File
@@ -79,6 +79,9 @@ function RemoteDockerFields({
type="password"
value={gitToken}
onChange={(event) => onGitTokenChange(event.target.value)}
// GitHub's own personal-access-token prefix. An identifier the user
// matches against the token they paste, not copy.
// eslint-disable-next-line i18next/no-literal-string -- PAT prefix
placeholder="ghp_..."
/>
<p className="text-xs text-muted-foreground">
@@ -164,8 +167,17 @@ function ExecutorFormCard({
onChange={(event) => onDockerHostChange(event.target.value)}
placeholder={
isRemoteDocker
? "tcp://remote:2376 or ssh://user@host"
: "unix:///var/run/docker.sock"
? // Two sample host URLs joined by a connector. The URLs are values
// Docker parses, so they interpolate; the "or" between them is
// copy and goes through the catalog.
t("executors:dockerHostRemotePlaceholder", {
tcp: "tcp://remote:2376",
ssh: "ssh://user@host",
})
: // The default Docker socket path. A filesystem path the daemon
// owns, not copy.
// eslint-disable-next-line i18next/no-literal-string -- Docker socket path
"unix:///var/run/docker.sock"
}
/>
<p className="text-xs text-muted-foreground">
@@ -68,6 +68,7 @@ export function GitHubCIConfig({ config, onUpdate }: GitHubCIConfigProps) {
value={checkNames}
onChange={(e) => setCheckNames(e.target.value)}
onBlur={handleCheckNamesBlur}
// eslint-disable-next-line i18next/no-literal-string -- example check names, see above
placeholder="build, test, lint"
/>
</div>
@@ -83,6 +83,7 @@ export function GitHubPRConfig({ config, onUpdate }: GitHubPRConfigProps) {
value={branches}
onChange={(e) => setBranches(e.target.value)}
onBlur={handleBranchesBlur}
// eslint-disable-next-line i18next/no-literal-string -- example branch names, see above
placeholder="main, develop"
/>
</div>
@@ -93,6 +94,7 @@ export function GitHubPRConfig({ config, onUpdate }: GitHubPRConfigProps) {
value={authors}
onChange={(e) => setAuthors(e.target.value)}
onBlur={handleAuthorsBlur}
// eslint-disable-next-line i18next/no-literal-string -- example usernames, see above
placeholder="alice, bob"
/>
</div>
@@ -35,6 +35,7 @@ export function GitHubPushConfig({ config, onUpdate }: GitHubPushConfigProps) {
value={branches}
onChange={(e) => setBranches(e.target.value)}
onBlur={handleBlur}
// eslint-disable-next-line i18next/no-literal-string -- example branch globs, see above
placeholder="main, release/*"
/>
<p className="text-xs text-muted-foreground">{t("automations:pushTriggerHelp")}</p>
@@ -180,6 +180,10 @@ function RepositoryScopeFields({
data-settings-dirty={orgs !== baseline.orgs}
onChange={(event) => onOrgsChange(event.target.value)}
disabled={loading || mode !== "orgs"}
// Sample GitHub organization logins, i.e. the shape of the value
// this field accepts. A translated `example-org` would stop being
// a usable example.
// eslint-disable-next-line i18next/no-literal-string -- example org logins
placeholder="kdlbs, example-org"
data-testid="github-scope-orgs-input"
/>
@@ -195,6 +199,9 @@ function RepositoryScopeFields({
onChange={(event) => onReposChange(event.target.value)}
disabled={loading || mode !== "repos"}
aria-invalid={invalidRepos}
// Sample `owner/repo` slugs — the format the sibling validation
// message describes, not copy.
// eslint-disable-next-line i18next/no-literal-string -- example repo slugs
placeholder="kdlbs/kandev, example/api"
data-testid="github-scope-repos-input"
/>
@@ -396,6 +396,9 @@ function ReviewWatchSection({ workspaceId }: { workspaceId: string }) {
}}
onUpdate={async (id, req) => {
const watch = watches.find((item) => item.id === id);
// Unreachable-invariant guard: the dialog only ever calls onUpdate for
// a row it was opened from. A developer diagnostic, not user copy.
// eslint-disable-next-line i18next/no-literal-string -- invariant message
if (!watch) throw new Error("review watch not found");
await update(id, watch.workspace_id, req);
toast({ description: t("github:reviewWatchUpdated"), variant: "success" });
@@ -499,6 +502,8 @@ function IssueWatchSection({ workspaceId }: { workspaceId: string }) {
}}
onUpdate={async (id, req) => {
const watch = issueActions.watches.find((item) => item.id === id);
// Unreachable-invariant guard, as above. Developer diagnostic.
// eslint-disable-next-line i18next/no-literal-string -- invariant message
if (!watch) throw new Error("issue watch not found");
await issueActions.update(id, watch.workspace_id, req);
toast({ description: t("github:issueWatchUpdated"), variant: "success" });
@@ -129,6 +129,7 @@ function FilterFields({ kind, form, setForm }: FormFieldsProps) {
// Sample namespace/project paths, i.e. the shape of the GitLab data the
// field accepts. Not copy — a translated "group" would stop being a
// usable example.
// eslint-disable-next-line i18next/no-literal-string -- example project paths
placeholder="group/api, group/web"
/>
</div>
@@ -144,6 +145,7 @@ function FilterFields({ kind, form, setForm }: FormFieldsProps) {
onChange={(event) => setForm((current) => ({ ...current, labels: event.target.value }))}
// Sample GitLab label names — user data, like the labels the user
// will actually type here.
// eslint-disable-next-line i18next/no-literal-string -- example label names
placeholder="bug, priority::high"
/>
</div>
@@ -168,6 +170,7 @@ function FilterFields({ kind, form, setForm }: FormFieldsProps) {
}
// A literal GitLab API query parameter. It is submitted to GitLab
// verbatim, so it is protocol, not copy.
// eslint-disable-next-line i18next/no-literal-string -- GitLab API query parameter
placeholder="state=opened"
className="font-mono text-xs"
/>
@@ -236,6 +236,7 @@ function JQLField({
// A sample JQL query. JQL is a query language, not copy: `project`,
// `status` and `PROJ` are the field names and project key Jira parses,
// so a translated (or pseudo-transliterated) example would not run.
// eslint-disable-next-line i18next/no-literal-string -- sample JQL query
placeholder='project = PROJ AND status = "Open"'
rows={3}
className="font-mono text-xs resize-y"
@@ -220,6 +220,9 @@ export function JiraIssueWatchersSection() {
onCreate={actions.create}
onUpdate={(id, req) => {
const w = editing;
// Unreachable-invariant guard: onUpdate only fires while a row is
// being edited. A developer diagnostic, not user copy.
// eslint-disable-next-line i18next/no-literal-string -- invariant message
if (!w) throw new Error("update without editing watch");
return actions.update(id, req, w.workspaceId);
}}
@@ -213,6 +213,9 @@ export function LinearIssueWatchersSection() {
onCreate={actions.create}
onUpdate={(id, req) => {
const w = editing;
// Unreachable-invariant guard: onUpdate only fires while a row is
// being edited. A developer diagnostic, not user copy.
// eslint-disable-next-line i18next/no-literal-string -- invariant message
if (!w) throw new Error("update without editing watch");
return actions.update(id, req, w.workspaceId);
}}
@@ -168,7 +168,15 @@ function IdentitySourceField({
</SelectTrigger>
<SelectContent>
<SelectItem value="agent" data-testid="ssh-input-identity-source-agent">
ssh-agent (SSH_AUTH_SOCK)
{/* The OpenSSH agent program and the environment variable it
publishes its socket on are identifiers the user finds under
those names on their own machine, so both travel as interpolated
values and the pseudo-locale leaves them intact. Only the
bracketing around them is copy. */}
{t("executors:sshIdentitySourceAgent", {
program: "ssh-agent",
envVar: "SSH_AUTH_SOCK",
})}
</SelectItem>
<SelectItem value="file" data-testid="ssh-input-identity-source-file">
{t("executors:sshIdentitySourceFile")}
+31 -2
View File
@@ -77,7 +77,16 @@ export const noLiteralStringOptions = {
// separator, which the token patterns above do not cover. Copy chunks
// ("Select task ", " tasks, over WIP limit") carry a capital, a space or
// punctuation and so still get flagged.
"[a-z0-9]+(?:[-_][a-z0-9]*)*|[-_][a-z0-9]+(?:[-_][a-z0-9]*)*",
//
// The outer `(?:…)` is load-bearing. `^` and `$` bind tighter than `|`, so
// the ungrouped form compiled to `(^A)|(B$)` — a first branch with no end
// anchor, which matched ANY string starting with a lowercase letter or a
// digit. That swallowed every `<Trans>` split fragment that happens to
// begin lowercase ("open pull requests assigned to you") in silence, for
// the whole repo, for as long as the pattern has existed. See the compiled
// -pattern test in `eslint.i18n.options.test.ts`, which fails without the
// group.
"(?:[a-z0-9]+(?:[-_][a-z0-9]*)*|[-_][a-z0-9]+(?:[-_][a-z0-9]*)*)",
// Single lowercase/camel/kebab tokens are prop enum values,
// classnames, and identifiers (variant="ghost", side="top",
// value="work-items") — never display copy, which is capitalized
@@ -92,7 +101,19 @@ export const noLiteralStringOptions = {
"^(noopener|noreferrer)( (noopener|noreferrer))*$",
"^__[a-z_]+__$",
"^/[\\w/\\-\\[\\]:.]*(\\?[\\w=&%.\\-]*)?$",
"(?:-?[a-z0-9]+(?:[:/-][a-z0-9.]+)*\\s+)*-?[a-z0-9]+(?:[:/-][a-z0-9.]+)*",
// Whitespace-separated token lists: Tailwind class lists that reach the
// guard as an object property (`{ className: "h-4 w-4" }`) rather than as
// a `className` JSX attribute, plus `owner/repo` and `ns:key` values.
//
// At least ONE token must carry a `-`, `:` or `/` (the middle branch).
// Without that requirement the pattern was "any run of lowercase words
// separated by spaces", which is a description of TYPOGRAPHY, not of
// syntax — and so it silently excluded every lowercase English sentence
// that happens to carry no punctuation ("open pull requests assigned to
// you"). Separators are what make a token a class name or a path rather
// than a word; English copy that needs one is rare, English copy that
// needs none is most of it.
"(?:-?[a-z0-9]+(?:[:/-][a-z0-9.]+)*\\s+)*-?[a-z0-9]+(?:[:/-][a-z0-9.]+)+(?:\\s+-?[a-z0-9]+(?:[:/-][a-z0-9.]+)*)*",
],
},
"jsx-attributes": {
@@ -120,6 +141,14 @@ export const noLiteralStringOptions = {
// Identifiers and prefixes the caller composes into ids/testids.
"id",
"k",
// A prop whose NAME ends in `Key` carries a catalog key, not copy:
// `titleKey`, `descriptionKey`, `labelKey`, `i18nKey`. Migrating a surface
// that resolves its copy at render (`SystemRouteShell`, the preset-icon
// catalogs) produces exactly this shape, and the value is by construction
// `namespace:someKey` — already-migrated copy, flagged only for being a
// string. This keys off the prop NAME, a syntactic category, not off what
// the value happens to look like.
".*[Kk]ey$",
// Option/badge values are data the app compares and submits.
"value",
"cmd",
+176
View File
@@ -0,0 +1,176 @@
import { createRequire } from "node:module";
import { describe, expect, it } from "vitest";
import { noLiteralStringOptions } from "./eslint.i18n.options.mjs";
/**
* `words.exclude` decides what `i18next/no-literal-string` is allowed to stay
* quiet about. A pattern there that is broader than its author intended does not
* fail anything — it just stops reporting, for the whole repo, until somebody
* notices copy shipping untranslated.
*
* That is not hypothetical. The id-fragment entry used to read
*
* "[a-z0-9]+(?:[-_][a-z0-9]*)*|[-_][a-z0-9]+(?:[-_][a-z0-9]*)*"
*
* and the plugin wraps every entry as `^…$`. `|` binds looser than the anchors,
* so that compiled to `(^A)|(B$)` — a first branch with NO end anchor, matching
* any string that merely *starts* with a lowercase letter or digit. And the
* token-list entry required nothing but lowercase words separated by spaces, so
* it swallowed any unpunctuated lowercase sentence.
*
* Both holes are invisible from the rule's output, which is why they lasted. The
* corpus below is the check the output cannot give us: real copy, asserted to
* survive every pattern.
*/
const require_ = createRequire(import.meta.url);
/** The plugin's own wrapper, imported rather than reimplemented so this test
* cannot drift from what ESLint actually compiles. */
const generateFullMatchRegExp = require_(
"eslint-plugin-i18next/lib/helper/generateFullMatchRegExp",
) as (source: string) => RegExp;
const wordExcludes = noLiteralStringOptions.words.exclude;
/** Copy that MUST reach the developer as a finding. */
const COPY = [
// The three verbatim regressions this test exists for. The first two were
// measured SKIPPED on main; the third was flagged only because it happens to
// start with a capital.
"query to the sidebar for quick access later.",
"open pull requests assigned to you",
"Save this query",
// The template chunks the id-fragment entry's own comment claims are still
// flagged because they "carry a capital, a space or punctuation".
"Select task ",
" tasks, over WIP limit",
// More `<Trans>` split fragments: sentence-splitting produces this shape
// constantly, and lowercase continuations are the majority of it.
"and reopen it later",
"to a different workspace",
"in the settings above",
// Lowercase sentences with no punctuation at all — the shape the token-list
// entry used to accept wholesale.
"no repositories match this filter",
"this cannot be undone",
];
/** Non-copy the exclusions exist for. Listed so tightening a pattern cannot
* quietly re-flag the identifiers the guard is supposed to ignore. */
const NOT_COPY = [
// Static chunks of an interpolated DOM id (`startup-page-${v}-label`).
"startup-page-",
"-label",
"task-card-",
// Prop enum values, classnames, identifiers.
"ghost",
"top",
"work-items",
// Tailwind class lists that reach the guard as an object property rather than
// as a `className` JSX attribute.
"h-4 w-4",
"flex items-center gap-2",
"text-xs text-muted-foreground",
// Values and identifiers.
"owner/repo",
"ssh-host.example.com",
"settings-page:editors",
"https://kandev.ai/docs",
];
function matchingPatterns(value: string): string[] {
return wordExcludes.filter((pattern) => generateFullMatchRegExp(pattern).test(value));
}
/**
* True when `pattern` has a `|` that is not inside a group or character class.
*
* Such a branch escapes the `^`/`$` the plugin adds, so one side of the
* alternation ends up unanchored and matches far more than it reads as. Wrapping
* the alternation in `(?:…)` is the whole fix.
*/
function hasTopLevelAlternation(pattern: string): boolean {
let depth = 0;
let inClass = false;
for (let i = 0; i < pattern.length; i += 1) {
const char = pattern[i];
if (char === "\\") {
i += 1;
continue;
}
if (inClass) {
if (char === "]") inClass = false;
continue;
}
if (char === "[") inClass = true;
else if (char === "(") depth += 1;
else if (char === ")") depth -= 1;
else if (char === "|" && depth === 0) return true;
}
return false;
}
describe("words.exclude", () => {
it.each(COPY)("does not exclude %j", (copy) => {
expect(matchingPatterns(copy)).toEqual([]);
});
it.each(NOT_COPY)("still excludes %j", (value) => {
expect(matchingPatterns(value).length).toBeGreaterThan(0);
});
it("has no alternation outside a group", () => {
// `^`/`$` bind tighter than `|`, so a top-level alternation leaves one
// branch unanchored and silently matching prefixes or suffixes.
const unanchored = wordExcludes.filter(hasTopLevelAlternation);
expect(unanchored).toEqual([]);
});
it("compiles every pattern", () => {
for (const pattern of wordExcludes) {
expect(() => generateFullMatchRegExp(pattern)).not.toThrow();
}
});
});
describe("hasTopLevelAlternation", () => {
it("finds the bug this test was written for", () => {
expect(hasTopLevelAlternation("[a-z0-9]+(?:[-_][a-z0-9]*)*|[-_][a-z0-9]+")).toBe(true);
});
it("accepts the grouped form", () => {
expect(hasTopLevelAlternation("(?:[a-z0-9]+(?:[-_][a-z0-9]*)*|[-_][a-z0-9]+)")).toBe(false);
});
it("ignores a pipe inside a character class or escaped", () => {
expect(hasTopLevelAlternation("^[·+\\-|/(),.:\\s]+$")).toBe(false);
expect(hasTopLevelAlternation("a\\|b")).toBe(false);
});
});
describe("jsx-attributes.exclude", () => {
const attributeExcludes = noLiteralStringOptions["jsx-attributes"].exclude;
function excludesAttribute(name: string): boolean {
return attributeExcludes.some((pattern) => generateFullMatchRegExp(pattern).test(name));
}
it.each(["titleKey", "descriptionKey", "labelKey", "i18nKey", "key"])(
"excludes the catalog-key prop %j",
(name) => {
expect(excludesAttribute(name)).toBe(true);
},
);
it.each(["title", "placeholder", "aria-label", "label", "description", "tooltip"])(
"still checks the display prop %j",
(name) => {
expect(excludesAttribute(name)).toBe(false);
},
);
});
+2
View File
@@ -64,6 +64,7 @@
"dockerContainers": "Docker Containers",
"dockerContainersCreatedByThisProfile": "Docker containers created by this profile.",
"dockerHost": "Docker host",
"dockerHostRemotePlaceholder": "{{tcp}} or {{ssh}}",
"dockerfile": "Dockerfile",
"dockerfileContent": "Dockerfile Content",
"domain": "Domain",
@@ -206,6 +207,7 @@
"sshIdentityFileHint": "Passphrase-protected keys must be loaded into ssh-agent first.",
"sshIdentityFilePath": "Identity file path",
"sshIdentitySource": "Identity source",
"sshIdentitySourceAgent": "{{program}} ({{envVar}})",
"sshIdentitySourceFile": "Identity file (private key path)",
"sshInstallHint": "Install hint",
"sshInstallHintCopied": "Install hint copied",
@@ -64,6 +64,7 @@
"dockerContainers": "Ďōćķēŕ Ćōńţàĩńēŕś",
"dockerContainersCreatedByThisProfile": "Ďōćķēŕ ćōńţàĩńēŕś ćŕēàţēď ƀŷ ţĥĩś ƥŕōƒĩĺē.",
"dockerHost": "Ďōćķēŕ ĥōśţ",
"dockerHostRemotePlaceholder": "{{tcp}} ōŕ {{ssh}}",
"dockerfile": "Ďōćķēŕƒĩĺē",
"dockerfileContent": "Ďōćķēŕƒĩĺē Ćōńţēńţ",
"domain": "Ďōḿàĩń",
@@ -206,6 +207,7 @@
"sshIdentityFileHint": "Ƥàśśƥĥŕàśē-ƥŕōţēćţēď ķēŷś ḿũśţ ƀē ĺōàďēď ĩńţō śśĥ-àĝēńţ ƒĩŕśţ.",
"sshIdentityFilePath": "Ĩďēńţĩţŷ ƒĩĺē ƥàţĥ",
"sshIdentitySource": "Ĩďēńţĩţŷ śōũŕćē",
"sshIdentitySourceAgent": "{{program}} ({{envVar}})",
"sshIdentitySourceFile": "Ĩďēńţĩţŷ ƒĩĺē (ƥŕĩvàţē ķēŷ ƥàţĥ)",
"sshInstallHint": "Ĩńśţàĺĺ ĥĩńţ",
"sshInstallHintCopied": "Ĩńśţàĺĺ ĥĩńţ ćōƥĩēď",