fix(mobile): close localization value-flow review gaps

This commit is contained in:
Brennan Benson
2026-08-01 11:49:47 -07:00
parent 2e0cc20e8e
commit 1cedf2d7aa
5 changed files with 106 additions and 23 deletions
@@ -218,6 +218,9 @@ function ancestorJsxAttributeName(node) {
ts.isJsxExpression(current) ||
ts.isConditionalExpression(current) ||
ts.isParenthesizedExpression(current) ||
ts.isAsExpression(current) ||
ts.isSatisfiesExpression(current) ||
ts.isNonNullExpression(current) ||
ts.isBinaryExpression(current)
) {
current = current.parent
@@ -133,12 +133,12 @@ export function Example() {
expect(candidates.map((candidate) => candidate.text)).toEqual(['Actually rendered value'])
})
it('finds literals assigned to variables rendered through user-visible JSX attributes', () => {
it('finds direct and assigned literals in user-visible JSX attributes', () => {
const source = `
export function Example() {
const copy = 'Save changes'
const className = 'not-visible-copy'
return <Button title={copy} className={className} />
return <><Button title={copy} className={className} /><Button title={'Direct save'!} /><Button title={('Open menu' as string)} /><Button title={('Choose host' satisfies string)} /></>
}
`
const candidates = collectLocalizationCandidates(
@@ -147,7 +147,12 @@ export function Example() {
'/repo'
)
expect(candidates.map((candidate) => candidate.text)).toEqual(['Save changes'])
expect(candidates.map((candidate) => candidate.text)).toEqual([
'Save changes',
'Direct save',
'Open menu',
'Choose host'
])
})
it('finds unlocalized success-toast object copy', () => {
@@ -437,7 +442,17 @@ const box = { tr: t }
box.tr = (value) => value
let logical = t
logical &&= (value) => value
export const label = <><Text>{box.tr('Member raw copy')}</Text><Text>{logical('Logical raw copy')}</Text></>
let closure = t
const closureBox = { tr: t }
function Later() { return <><Text>{closure('Scalar closure raw')}</Text><Text>{closureBox.tr('Member closure raw')}</Text></> }
closure = (value) => value
closureBox.tr = (value) => value
const a = { tr: t }
const b = { tr: (value) => value }
let y = a
y &&= b
a.tr = y.tr
export const label = <><Text>{box.tr('Member raw copy')}</Text><Text>{logical('Logical raw copy')}</Text><Text>{a.tr('Ambiguous property raw')}</Text><Later /></>
`
const candidates = collectLocalizationCandidates(
'/repo/mobile/src/components/Example.tsx',
@@ -446,8 +461,11 @@ export const label = <><Text>{box.tr('Member raw copy')}</Text><Text>{logical('L
)
expect(candidates.map((candidate) => candidate.text)).toEqual([
'Scalar closure raw',
'Member closure raw',
'Member raw copy',
'Logical raw copy'
'Logical raw copy',
'Ambiguous property raw'
])
})
@@ -463,9 +481,12 @@ const alias = box
alias.tr = raw
const holder = { inner: { tr: t } }
holder.inner.tr = raw
let objectSource = { tr: raw }
const objectSnapshot = objectSource
objectSource = { tr: t }
const visible = 'Visible non-null copy'
function render() { return 'Helper non-null copy' }
export const label = <><Text>{snapshot('Raw snapshot copy')}</Text><Text>{box.tr('Raw alias mutation')}</Text><Text>{holder.inner.tr('Raw nested mutation')}</Text><Text>{visible!}</Text><Text>{'Direct non-null copy'!}</Text><Text>{render!()}</Text></>
export const label = <><Text>{snapshot('Raw snapshot copy')}</Text><Text>{box.tr('Raw alias mutation')}</Text><Text>{holder.inner.tr('Raw nested mutation')}</Text><Text>{objectSnapshot.tr('Raw object snapshot')}</Text><Text>{visible!}</Text><Text>{'Direct non-null copy'!}</Text><Text>{render!()}</Text></>
`
const candidates = collectLocalizationCandidates(
'/repo/mobile/src/components/Example.tsx',
@@ -479,6 +500,7 @@ export const label = <><Text>{snapshot('Raw snapshot copy')}</Text><Text>{box.tr
'Raw snapshot copy',
'Raw alias mutation',
'Raw nested mutation',
'Raw object snapshot',
'Direct non-null copy'
])
})
@@ -224,9 +224,13 @@ function expressionTargetsFunction(node, owner, bindings, valueFlow, seen = new
if (!object || !propertyName) {
return false
}
if (seen.has(expression)) {
return false
}
const nextSeen = new Set(seen).add(expression)
const values = valueFlow.propertyValues(object, propertyName, node)
return Boolean(
values?.some((value) => expressionTargetsFunction(value, owner, bindings, valueFlow, seen))
values?.some((value) => expressionTargetsFunction(value, owner, bindings, valueFlow, nextSeen))
)
}
@@ -94,9 +94,13 @@ source = t
const box = { tr: raw }
const { tr: memberSnapshot } = box
box.tr = t
let objectSource = { tr: raw }
const objectSnapshot = objectSource
objectSource = { tr: t }
export const labels = [
snapshot('Raw binding snapshot'),
memberSnapshot('Raw member snapshot'),
objectSnapshot.tr('Raw object snapshot'),
source('example.binding'),
box.tr('example.member')
]
@@ -106,6 +110,24 @@ export const labels = [
expect(calls.map((call) => call.keys)).toEqual([['example.binding'], ['example.member']])
})
it('includes later outer writes that can reach closure calls', () => {
const sourceText = `
import { t } from '@/i18n/mobile-i18n'
const raw = (value) => value
let closure = t
const box = { tr: t }
function render() {
return [closure('Raw scalar closure'), box.tr('Raw member closure')]
}
closure = raw
box.tr = raw
export const labels = render()
`
const calls = collectMobileTranslationCalls('/repo/mobile/app/Example.tsx', sourceText, '/repo')
expect(calls).toEqual([])
})
it('tracks member and logical assignments at each call', () => {
const sourceText = `
import { t } from '@/i18n/mobile-i18n'
@@ -216,7 +238,12 @@ export const labels = next.map((value) => value.label)
import { t } from '@/i18n/mobile-i18n'
const holder = { current: t }
holder.current = holder.current
export const label = holder.current('example.selfReference')
const a = { tr: t }
const b = { tr: (value) => value }
let y = a
y &&= b
a.tr = y.tr
export const labels = [holder.current('example.selfReference'), a.tr('Raw ambiguous write')]
`
const calls = collectMobileTranslationCalls('/repo/mobile/app/Example.tsx', sourceText, '/repo')
@@ -92,6 +92,25 @@ function isDefiniteBefore(write, use) {
return !write.conditional && isOrderedBefore(write, use)
}
function functionOwner(node) {
let current = node.parent
while (current && !ts.isSourceFile(current)) {
if (ts.isFunctionLike(current)) {
return current
}
current = current.parent
}
return undefined
}
function canReachUse(write, use, sourceFile) {
if (write.position < use.getStart(sourceFile)) {
return true
}
const useOwner = functionOwner(use)
return Boolean(useOwner && functionOwner(write.node) !== useOwner)
}
function destructuredTargets(name) {
const target = unwrapExpression(name)
if (ts.isObjectBindingPattern(target)) {
@@ -256,8 +275,7 @@ export function createMobileLocalizationValueFlow(sourceFile, bindings) {
if (all || (binding && isConstBinding(binding))) {
return entries
}
const usePosition = use.getStart(sourceFile)
const available = entries.filter((entry) => entry.position < usePosition)
const available = entries.filter((entry) => canReachUse(entry, use, sourceFile))
let selected = []
for (const entry of available) {
const action = entry.conditional
@@ -348,8 +366,9 @@ export function createMobileLocalizationValueFlow(sourceFile, bindings) {
: []
const rightKeys = new Set(rightLocations.map(locationKey))
const selfAssignment =
targetLocations.length > 0 &&
targetLocations.every((location) => rightKeys.has(locationKey(location)))
targetLocations.length === 1 &&
rightLocations.length === 1 &&
rightKeys.has(locationKey(targetLocations[0]))
const indexed = { ...entry, selfAssignment }
for (const location of targetLocations) {
indexedPropertyWrites(location).push(indexed)
@@ -362,14 +381,15 @@ export function createMobileLocalizationValueFlow(sourceFile, bindings) {
}
function propertyWriteEntries(node, propertyName, use, all) {
const usePosition = use.getStart(sourceFile)
const entries = objectLocations(node, use).flatMap((location) => {
const key = [...location.path, propertyName].join('\0')
return propertyWrites.get(location.binding)?.get(key) ?? []
})
return [
...new Set(
entries.filter((entry) => !entry.selfAssignment && (all || entry.position < usePosition))
entries.filter(
(entry) => !entry.selfAssignment && (all || canReachUse(entry, use, sourceFile))
)
)
].sort((left, right) => left.position - right.position)
}
@@ -407,17 +427,24 @@ export function createMobileLocalizationValueFlow(sourceFile, bindings) {
return undefined
}
const nextSeen = new Set(seen).add(binding)
const values = all
? allValueExpressions(expression, use, nextSeen)
: valueExpressions(expression, use, nextSeen)
const sources = valueSources(expression, use, all)
const resolved = []
let known = values !== undefined
for (const value of values ?? []) {
const matches = propertyValues(value, propertyName, use, nextSeen, all)
if (matches === undefined) {
let known = true
for (const source of sources) {
const values = source.propertyName
? propertyValues(source.expression, source.propertyName, source.node, nextSeen, all)
: [source.expression]
if (values === undefined) {
known = false
} else {
resolved.push(...matches)
continue
}
for (const value of values) {
const matches = propertyValues(value, propertyName, source.node, nextSeen, all)
if (matches === undefined) {
known = false
} else {
resolved.push(...matches)
}
}
}
known = applyPropertyWrites(resolved, known, expression, propertyName, use, all)