Skip to content

Commit 2c48f83

Browse files
committed
fix(lint): check reqId-only modules and skip reassigned let messages in log-message-tag
The cheap pre-filter now admits a module whose only request id is reqId, and a let binding reassigned before it is logged is no longer followed to its initializer.
1 parent 72c8f70 commit 2c48f83

2 files changed

Lines changed: 27 additions & 2 deletions

File tree

‎scripts/check-source-conventions.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,20 @@ describe('log-message-tag', () => {
423423
expect(lines(MODULE, source, 'log-message-tag')).toEqual([5])
424424
})
425425

426+
it('checks a module whose only request id is a reqId', () => {
427+
const source = withLogger(`log.info(\`Retry for \${reqId}\`)`)
428+
expect(lines(MODULE, source, 'log-message-tag')).toEqual([3])
429+
})
430+
431+
it('stops following a let binding that is reassigned before it is logged', () => {
432+
const source = withLogger(
433+
`let message = \`[\${requestId}] Failed\``,
434+
"message = 'Started'",
435+
'logger.info(message)'
436+
)
437+
expect(hits(MODULE, source, 'log-message-tag')).toEqual([])
438+
})
439+
426440
it('does not treat other uses of request ids or loggers as a tagged message', () => {
427441
const source = withLogger(
428442
`logger.info(\`Kept \${rows.filter((row) => row.requestId).length} rows\`)`,

‎scripts/check-source-conventions.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,9 @@ type Bindings = Map<string, Node[]>
656656

657657
class MessageResolver {
658658
private readonly bindings = new Map<Node, Bindings>()
659+
/** `let` bindings by function; one reassigned anywhere in the file has no single value to follow. */
660+
private readonly lets = new Map<Node, Set<string>>()
661+
private readonly reassigned = new Set<string>()
659662

660663
constructor(program: Node) {
661664
this.collect(program, program)
@@ -686,9 +689,16 @@ class MessageResolver {
686689
const id = declarator.id as Node
687690
if (id.type === 'Identifier') {
688691
this.declare(scope, id.name as string, (declarator.init as Node | null) ?? undefined)
692+
if (current.kind === 'let') {
693+
this.lets.set(scope, (this.lets.get(scope) ?? new Set()).add(id.name as string))
694+
}
689695
}
690696
}
691697
}
698+
const target = (current.type === 'AssignmentExpression' ? current.left : undefined) as
699+
| Node
700+
| undefined
701+
if (target?.type === 'Identifier') this.reassigned.add(target.name as string)
692702
for (const [key, value] of Object.entries(current)) {
693703
if (key === 'loc' || key === 'start' || key === 'end' || key === 'extra') continue
694704
this.collect(value, inner)
@@ -699,7 +709,8 @@ class MessageResolver {
699709
private resolve(name: string, scopes: Node[]): Node[] {
700710
for (let index = scopes.length - 1; index >= 0; index--) {
701711
const values = this.bindings.get(scopes[index])?.get(name)
702-
if (values) return values
712+
if (!values) continue
713+
return this.lets.get(scopes[index])?.has(name) && this.reassigned.has(name) ? [] : values
703714
}
704715
return []
705716
}
@@ -805,7 +816,7 @@ export function findViolations(file: string, source: string): Violation[] {
805816
const wantsDb = isApi && source.includes('@sim/db')
806817
const wantsRoute = isApi && /(?:^|\/)route\.ts$/.test(file)
807818
const wantsLogTag =
808-
!test && source.includes('@sim/logger') && /\[(?:\$\{|['"`]\s*\+)|requestId/i.test(source)
819+
!test && source.includes('@sim/logger') && /\[(?:\$\{|['"`]\s*\+)|req(?:uest)?Id/i.test(source)
809820
if (!(wantsRelative || wantsTextSize || wantsStyle || wantsDb || wantsRoute || wantsLogTag)) {
810821
return []
811822
}

0 commit comments

Comments
 (0)