JS-1802

Ruling Report

Code no longer flagged (20 issues)

S3801

ant-design/components/checkbox/Checkbox.tsx:77

    75 |   }, []);
    76 | 
>   77 |   React.useEffect(() => {
    78 |     if (skipGroup) {
    79 |       return;

eigen/src/app/Components/ArtsyWebView.tsx:298

   296 | 
   297 | function useUrlCookies(url: string, accessToken: string | null, isLoggedIn: boolean) {
>  298 |   useEffect(() => {
   299 |     if (accessToken && isLoggedIn) {
   300 |       const attempt = new CookieRequestAttempt(url, accessToken)

eigen/src/app/Scenes/Artwork/Components/CommercialButtons/InquiryModal.tsx:177

   175 |   }
   176 | 
>  177 |   useEffect(() => {
   178 |     if (mutationSuccessful) {
   179 |       resetAndExit()

eigen/src/app/Scenes/Onboarding/OnboardingWelcome.tsx:54

    52 |   }, [])
    53 | 
>   54 |   useEffect(() => {
    55 |     if (Platform.OS === "ios") {
    56 |       return

eigen/src/app/Scenes/Onboarding/OnboardingWelcome.tsx:67

    65 |   }, [navigation])
    66 | 
>   67 |   useEffect(() => {
    68 |     if (Platform.OS === "ios") {
    69 |       return

eigen/src/app/Scenes/Search/components/SearchInput.tsx:38

    36 |   )
    37 | 
>   38 |   useEffect(() => {
    39 |     if (searchProviderValues.inputRef?.current && isAndroid) {
    40 |       const unsubscribe = navigation?.addListener("focus", () => {

eigen/src/app/Websockets/auctions/AuctionSocketContext.tsx:50

    48 |   const { cable, channelsHolder } = useCable()
    49 | 
>   50 |   useEffect(() => {
    51 |     if (!enabled) {
    52 |       return

eigen/src/app/utils/AboveTheFoldQueryRenderer.tsx:90

    88 |   const [renderBelowTheFold, setRenderBelowTheFold] = useState(false)
    89 | 
>   90 |   useEffect(() => {
    91 |     if (props.belowTheFoldTimeout === undefined) {
    92 |       return

eigen/src/palette/elements/Input/Input.tsx:127

   125 |     const fontFamily = theme.fonts.sans.regular
   126 | 
>  127 |     useEffect(() => {
   128 |       if (!addClearListener) {
   129 |         return

fireact/src/pages/auth/user/Invite/index.js:23

    21 |     const { setBreadcrumb } = useContext(BreadcrumbContext);
    22 | 
>   23 |     useEffect(() => {
    24 |         setBreadcrumb([
    25 |             {

...and 10 more

gitar auto-apply:on         

gitar display:verbose         

_{Was this helpful? React with 👍 / 👎 | Gitar}

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

Rule Profile

function getValue(condition) {
  if (condition) {
    return 42;
  }
  return;
}

False Positive Pattern

The false positives happen in React effect callbacks that return a cleanup function on one path and return nothing on another path. S3801 normally treats this as inconsistent API behavior, but React gives effect callback returns a special meaning: returning a function registers cleanup, while returning nothing means there is no cleanup for that effect run.

useEffect(() => {
  if (!enabled) {
    return;
  }

  const subscription = subscribe();
  return () => subscription.unsubscribe();
}, [enabled]);

False Negative Risk

The risk is bounded because the fix does not suppress all functions with mixed returns, nor all callbacks. It suppresses only the first argument passed to useEffect, useLayoutEffect, or useInsertionEffect when those hooks are imported from react, including named aliases and React.useEffect-style calls. Normal functions, callbacks passed to other APIs, and untracked custom wrappers are still checked by S3801.

Fix Summary

The implementation tracks React imports, recognizes React effect hook calls, and skips S3801 reporting for the effect callback itself. This matches the React contract where cleanup returns are optional and should not be interpreted as inconsistent function return behavior.

• Tracks named React effect hook imports, aliased imports, default imports, and namespace imports.
• Recognizes direct hook calls and React.useEffect-style member calls.
• Adds regression tests for useEffect, useLayoutEffect, useInsertionEffect, React.useEffect, and aliased useEffect imports.
• Updates ruling snapshots for the 20 React effect false positives no longer reported.

For TypeScript, maybe this FP could be more general, I think it applies to any function that returns a union of (either undefined or void) and another type.

See the signature for the useEffect callback argument: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L1665

Here the return type is void | Destructor, so an empty return; or return undefined; should be considered valid even if another return statement returns something else (granted we have type information)