Ruling Report

Code no longer flagged (21 issues)

S6767

ant-design/components/input/ClearableLabeledInput.tsx:32

    30 |   disabled?: boolean;
    31 |   direction?: DirectionType;
>   32 |   focused?: boolean;
    33 |   readOnly?: boolean;
    34 |   bordered: boolean;

ant-design/components/input/ClearableLabeledInput.tsx:40

    38 | /** This props only for input. */
    39 | export interface ClearableInputProps extends BasicProps {
>   40 |   size?: SizeType;
    41 |   suffix?: React.ReactNode;
    42 |   prefix?: React.ReactNode;

ant-design/components/input/ClearableLabeledInput.tsx:42

    40 |   size?: SizeType;
    41 |   suffix?: React.ReactNode;
>   42 |   prefix?: React.ReactNode;
    43 |   addonBefore?: React.ReactNode;
    44 |   addonAfter?: React.ReactNode;

ant-design/components/input/ClearableLabeledInput.tsx:43

    41 |   suffix?: React.ReactNode;
    42 |   prefix?: React.ReactNode;
>   43 |   addonBefore?: React.ReactNode;
    44 |   addonAfter?: React.ReactNode;
    45 |   triggerFocus?: () => void;

ant-design/components/input/ClearableLabeledInput.tsx:44

    42 |   prefix?: React.ReactNode;
    43 |   addonBefore?: React.ReactNode;
>   44 |   addonAfter?: React.ReactNode;
    45 |   triggerFocus?: () => void;
    46 |   status?: InputStatus;

ant-design/components/input/ClearableLabeledInput.tsx:45

    43 |   addonBefore?: React.ReactNode;
    44 |   addonAfter?: React.ReactNode;
>   45 |   triggerFocus?: () => void;
    46 |   status?: InputStatus;
    47 | }

desktop/app/src/ui/app-menu/app-menu.tsx:41

    39 |    * menu will receive focus.
    40 |    */
>   41 |   readonly openedWithAccessKey: boolean
    42 | 
    43 |   /**

desktop/app/src/ui/branches/branch-list.tsx:30

    28 |    * See IBranchesState.defaultBranch
    29 |    */
>   30 |   readonly defaultBranch: Branch | null
    31 | 
    32 |   /**

desktop/app/src/ui/branches/branch-list.tsx:35

    33 |    * The currently checked out branch or null if HEAD is detached
    34 |    */
>   35 |   readonly currentBranch: Branch | null
    36 | 
    37 |   /**

desktop/app/src/ui/branches/branch-list.tsx:45

    43 |    * See IBranchesState.recentBranches
    44 |    */
>   45 |   readonly recentBranches: ReadonlyArray<Branch>
    46 | 
    47 |   /**

...and 11 more

New issues flagged (6 issues)

S6767

eigen/src/app/Components/Bidding/Components/Timer.tsx:28

    26 | 
    27 | interface Props {
>   28 |   liveStartsAt?: string
    29 |   isPreview?: boolean
    30 |   isClosed?: boolean

eigen/src/app/Components/Bidding/Components/Timer.tsx:29

    27 | interface Props {
    28 |   liveStartsAt?: string
>   29 |   isPreview?: boolean
    30 |   isClosed?: boolean
    31 |   startsAt?: string

eigen/src/app/Components/Bidding/Components/Timer.tsx:30

    28 |   liveStartsAt?: string
    29 |   isPreview?: boolean
>   30 |   isClosed?: boolean
    31 |   startsAt?: string
    32 |   biddingEndAt?: string | null

eigen/src/app/Components/Bidding/Components/Timer.tsx:31

    29 |   isPreview?: boolean
    30 |   isClosed?: boolean
>   31 |   startsAt?: string
    32 |   biddingEndAt?: string | null
    33 |   lotEndAt?: string | null

eigen/src/app/Components/Bidding/Components/Timer.tsx:32

    30 |   isClosed?: boolean
    31 |   startsAt?: string
>   32 |   biddingEndAt?: string | null
    33 |   lotEndAt?: string | null
    34 | }

eigen/src/app/Components/Bidding/Components/Timer.tsx:33

    31 |   startsAt?: string
    32 |   biddingEndAt?: string | null
>   33 |   lotEndAt?: string | null
    34 | }
    35 | function formatDate(date: string) {

Summary

What changed

This PR fixes false positives in S6767 (no-unused-prop-types) for React components wrapped and directly exported via HOCs. The rule now recognizes when specific props are injected by known HOC wrappers and suppresses only those injected props, leaving genuine unused-prop findings intact.

Key additions:

• Whitelist of stable HOC wrappers: withStyles (injects classes), withTheme (theme), withTranslation (t, i18n, tReady), withLocalization (getString)
• Scope-limited suppression: Only applies to directly exported forms like export default withStyles(Comp) or export const X = withTheme(Comp)
• Safety exclusions: Non-injecting wrappers (memo, observer) and data-dependent HOCs (connect) are intentionally unsupported
• Enhanced React helpers: Added syntax-based fallback for class components and annotation-based detection for React.FC<Props> arrow functions when type info is unavailable
• Test updates: Ruling expectations updated to reflect ~20–40 fewer false positives per codebase (ant-design, desktop, eigen, etc.)

Why: The original decorator was too conservative, suppressing all reported props for any HOC-exported component. This PR narrows it to only known, safe cases where the injected props are stable and documented up front.

What reviewers should know

Where to start

1. Main logic: packages/analysis/src/jsts/rules/S6767/decorator.ts — This is where the HOC detection and suppression happen. The key function is isPropInjectedByDirectlyExportedHoc() and its helpers buildHocExportCache(), getDirectHocExportMetadataFromStatement(), and getHocApplicationMetadata(). Start by understanding how these collect and recognize HOC chains.

2. React helpers: packages/analysis/src/jsts/rules/helpers/react.ts — Two important changes here:

   • findOwningComponentNode() (renamed from findComponentNode()) — unchanged logic, just renamed
   • getComponentIdentifierFromNode() — new helper that extracts the name from a component node
   • matchesFunctionProps() — now uses a two-phase strategy: first tries annotation-based typing for React.FC<Props> vars, then falls back to signature-based
   • getReactImportBindings() — new caching helper that tracks React import aliases to recognize React.Component, FC, etc.

3. Test expectations: The JSON files in its/ruling/src/test/expected/*/typescript-S6767.json show which lines are now correctly not reported. Deleted entries mean "this was a false positive, now fixed".

Non-obvious decisions & gotchas

• Whitelist, not blacklist: Only withStyles, withTheme, withTranslation, and withLocalization suppress. Everything else is unsupported. This means connect(mapState)(Comp) will not suppress, even though it injects props—intentional, because the injected shape is unknown at static analysis time.
• Direct export only: The HOC detection only works for top-level exports. If you wrap a component inside another function or store it in a variable first without exporting, no suppression.
• Three-layer defense: Suppression happens in order. If opaque usage is detected (spread props), both HOC and forwardRef suppression are skipped. Understand this ordering if debugging edge cases.
• Two-phase function typing: The new two-phase approach for matchesFunctionProps() is a workaround for unresolved React imports (missing node_modules). If phase 1 (annotation) doesn't match, phase 2 (signature) is tried. Both must use areMutuallyAssignable() for bidirectional assignability, not just one-way.
• Non-injecting HOCs blacklist (memo, observer): These are explicitly excluded because they don't add props. Without this, export default memo(MyComponent) would incorrectly suppress.

Testing strategy

• Look at ruling test diffs to verify the suppressed lines are real false positives (grep the original code to confirm those props aren't used).
• Unit tests in S6767/unit.test.ts should cover:
  • Detection of each known HOC in various export forms (default, named, CommonJS)
  • Non-injecting wrapper exclusion
  • Curried HOC detection (withStyles(styles)(Comp))
  • Stacking (memo(withTheme(Comp)))
  • Unresolved/unknown HOCs returning empty props

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

The matchesClassProps fallback looks too broad. When props is unavailable, it currently accepts the first type argument of any extends X<FooProps> clause, so a non-React class could match before the real component, for example class StoreModel extends Store<FooProps> {}.

Could you keep this fix simple and make the fallback syntax-based here: before reading typeArguments[0], check that the extended base is exactly React.Component or bare Component, and ignore every other extends target? That should be enough to avoid matching unrelated generic classes without introducing heavier symbol/declaration-file resolution logic.

The new TypeScript HOC coverage is all positive. Since this PR intentionally supports only direct export forms, could you add a negative boundary test for an unsupported split-export shape such as const Wrapped = hoc(Component); export { Wrapped }; or const Wrapped = hoc(Component); export default Wrapped;? That would pin the intended scope and prevent accidental expansion later.

The Jira review report attached to JS-1391 looks out of sync with the current branch. It still describes Phase 2 / two-statement handling and RSPEC updates, while the current implementation and tests cover only direct export forms. Could you refresh or replace that report so the review metadata matches the actual PR state?

The Jira attachment is stale — here is the refreshed review report reflecting the current branch state. The key differences from the previous report:

What changed:

• Phase 2 (two-statement form) was intentionally removed. The current implementation handles only three direct export forms: export default HOC(Comp), export const X = HOC(Comp), module.exports = HOC(Comp). Two explicit TP boundary tests document this scope decision.
• No RSPEC changes are included in this branch (the approach document requested them — this is flagged as a gap).
• decorator.ts grows from 183 → 402 lines (+219 lines, 119.7%), higher than the 85% previously reported because the baseline is the current master (which already had the forwardRef/opaque-props/bracket-notation layers).
• helpers/react.ts adds: matchesClassProps fallback for extends React.Component<Props> heritage clauses (extends-only, base-class-filtered), and a two-phase matchesFunctionProps strategy for React.FC<Props> arrow functions.

JS-1391 — S6767 — SUSPICIOUS (updated)

Verdict: SUSPICIOUS
Note: This supersedes the previous Jira attachment. Reflects current branch after Phase 2 removal and scope refinement.

Claimed FP

Three direct export forms only: export default HOC(Comp), export const X = HOC(Comp), module.exports = HOC(Comp). Two-statement forms (const X = HOC(Comp); export { X } and const X = HOC(Comp); export default X) are intentionally out of scope, pinned by TP boundary tests.

Assessment summary

Verdict Rationale

The FP is genuine and the three direct export forms are correctly handled. Ruling improvements confirm real-world FP reduction (eigen, ant-design, console, courselit, desktop).

Open concerns:

1. Criterion 17 (code size): 119.7% growth of decorator.ts. SUSPICIOUS.
2. Criterion 6 (RSPEC): No RSPEC update despite approach document requirement.
3. Criterion 13 (under-generalization): Two-statement form intentionally out of scope.

Recommended action

Accept with a required RSPEC follow-up: the Known Issues/Limitations section should document HOC export suppression and clarify that split-export forms are not suppressed.

One more Peach case seems worth supporting in the same fixed-injected-props bucket: from .

This follows the same pattern as the wrappers already handled here: the HOC has a well-known injected prop, so we can suppress the FP when that prop is only used through the wrapper.

In Peach I found these examples:

• :
  • the wrapped component extends
  • it destructures and uses
• :

So I think we should extend the fixed map with something like:

• ->

That would cover another established HOC with a documented injected prop, without broadening the suppression to wrappers whose injected prop names are user-defined.

One more Peach case seems worth supporting in the same fixed-injected-props bucket: withLocalization from @fluent/react.

This follows the same pattern as the wrappers already handled here: the HOC has a well-known injected prop, so we can suppress the FP when that prop is only used through the wrapper.

In Peach I found these examples:

• fxa-auth-server/packages/fxa-payments-server/src/components/fields/index.tsx: export const Input = withLocalization(UnwrappedInput);
  • the wrapped component extends WithLocalizationProps
  • it destructures and uses getString
• fxa-auth-server/packages/fxa-payments-server/src/components/PaymentForm/index.tsx: export default withLocalization(WrappedPaymentForm);

So I think we should extend the fixed map with something like:

• withLocalization -> getString

That would cover another established HOC with a documented injected prop, without broadening the suppression to wrappers whose injected prop names are user-defined.

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
88.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube