Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

Don't report nested arrow functions in jest/no-if #331

Merged
merged 6 commits into from
Jul 5, 2019
Merged

Conversation

cartogram
Copy link
Contributor

@cartogram cartogram commented Jul 3, 2019

Fixes #311

This PR changes jest/no-if so that it reports on if statements that are nested in arrow functions (all of which would be nested in a test CallExpression). This fix was done in 2 steps:

  1. Push ArrowFunctions on to our in-memory stack
  2. Store those items as a boolean, true if it is part of the test structure (the second argument to it/test/etc...)

The rest of the logic remained the same. We check to see if the last item pushed was true/false, and report if it is.

In other words, "if we are an if statement and we are just inside an arrow function and that arrow function that we are just inside is part of an it/test/etc... function, then we report it".

I implemented this by adding a new jest utility for checking if an ArrowFunction is part of an it/test/etc... function.

cc @BPScott

@BPScott
Copy link
Member

BPScott commented Jul 3, 2019

Can we identify regular functions in addition to arrow function?

The following should all be valid:

{
      code: `it('foo', () => {
        const foo = function(bar) {
          return foo ? bar : null;
        };
      });`,
    },
    {
      code: `it('foo', () => {
        const foo = function(bar) {
          if (bar) {
            return 1;
          } else {
            return 2;
          }
        };
      });`,
    },
    {
      code: `it('foo', () => {
        function foo(bar) {
          return foo ? bar : null;
        };
      });`,
    },
    {
      code: `it('foo', () => {
        function foo(bar) {
          if (bar) {
            return 1;
          } else {
            return 2;
          }
        };
      });`,
    },

@cartogram
Copy link
Contributor Author

@BPScott makes sense! may tackle normal functions in a follow-up though.

@cartogram
Copy link
Contributor Author

@BPScott Okay, regular functions are in 3d31d91

Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@cartogram cartogram merged commit 00ff31d into master Jul 5, 2019
@cartogram cartogram deleted the fix-jest-no-if branch July 5, 2019 18:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive in shopify/jest/no-if
2 participants