From 5d5cd06a791445204efae2f4f2f9b3fe8adbd9e7 Mon Sep 17 00:00:00 2001 From: "buf0-bot[bot]" <252831055+buf0-bot[bot]@users.noreply.github.com> Date: Sun, 31 May 2026 01:41:23 -0700 Subject: [PATCH] fix: pr-bug-scan validated finding from #2437 (#2445) * fix: address pr-bug-scan validated finding from #2437 Fixed jsxElementReports: spread className now overrides earlier explicit className (later wins); expressionHasStyledScrollbarLiteral skips conditional/short-circuit branches so a conditional scrollbar * test: cover styled scrollbar checker regressions --------- Co-authored-by: orca-bug-scan-bot Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- .../scripts/check-styled-scrollbars.test.mjs | 25 +++++++++++++++++++ .../styled-scrollbar-jsx-check.mjs | 21 +++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/config/scripts/check-styled-scrollbars.test.mjs b/config/scripts/check-styled-scrollbars.test.mjs index 6483fd540ea..5ab2f46b1f9 100644 --- a/config/scripts/check-styled-scrollbars.test.mjs +++ b/config/scripts/check-styled-scrollbars.test.mjs @@ -129,6 +129,22 @@ describe('check-styled-scrollbars', () => { expect(reports).toHaveLength(0) }) + it('reports inline vertical overflow when the scrollbar class is conditional or short-circuited', () => { + for (const classNameExpression of [ + "enabled && 'scrollbar-sleek'", + "enabled ? 'scrollbar-sleek' : undefined", + "enabled || 'scrollbar-sleek'", + "enabled ?? 'scrollbar-sleek'" + ]) { + const reports = reportUnstyledScrollbars( + 'Example.tsx', + `export function Example({ enabled }) { return
}` + ) + + expect(reports, classNameExpression).toHaveLength(1) + } + }) + it('reports logical inline style spreads without an Orca scrollbar class', () => { const reports = reportUnstyledScrollbars( 'Example.tsx', @@ -156,6 +172,15 @@ describe('check-styled-scrollbars', () => { expect(reports).toHaveLength(0) }) + it('uses later spread className props over earlier explicit className props', () => { + const reports = reportUnstyledScrollbars( + 'Example.tsx', + "export function Example() { return
}" + ) + + expect(reports).toHaveLength(1) + }) + it('supports variant helper className config', () => { const reports = reportUnstyledScrollbars( 'Example.tsx', diff --git a/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs b/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs index 0ca8a25aabc..4dab6f3b687 100644 --- a/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs +++ b/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs @@ -138,6 +138,20 @@ function expressionHasStyledScrollbarLiteral(node) { hasStyledScrollbar = true return } + // Why: a scrollbar literal that only renders on some branches must not be + // treated as covering an unconditional inline overflow. Skip conditional + // and short-circuit expressions when proving unconditional coverage. + if (ts.isConditionalExpression(current)) { + return + } + if ( + ts.isBinaryExpression(current) && + (current.operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken || + current.operatorToken.kind === ts.SyntaxKind.BarBarToken || + current.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken) + ) { + return + } ts.forEachChild(current, visit) } @@ -246,7 +260,12 @@ function jsxElementReports(node, filePath, sourceFile) { for (const attribute of node.attributes.properties) { if (ts.isJsxSpreadAttribute(attribute)) { - classExpression ??= spreadPropExpressions(attribute.expression, 'className').at(-1) + // Why: at runtime React applies attributes in source order, so a later + // spread that supplies className overrides an earlier explicit className. + const spreadClassExpression = spreadPropExpressions(attribute.expression, 'className').at(-1) + if (spreadClassExpression) { + classExpression = spreadClassExpression + } styleExpressions.push(...spreadPropExpressions(attribute.expression, 'style')) } else if (jsxAttributeName(attribute) === 'className') { classExpression = jsxAttributeExpression(attribute)