jsx-no-bind: Report usage of local functions as violations

[p7g]

Fixes #3047

I implemented this in a pretty straightforward way, so there are some trade-offs:

1. Any assignments to a function declaration are not considered. Consequently, a function that is unconditionally re-assigned with something that doesn't bind will still trigger a violation.
2. It uses the same configuration value and error message as the usage of a function expression.

To fix 1 would require some fancy analysis, and is probably not worth the effort, though 2 would be easy to fix and I can do so if needed.

[ljharb]

Seems great.

Arguably this is a breaking change since it will cause more warnings to appear, but equally arguably, it's a bug fix that function declarations were ever ignored (it just never came up because the airbnb styleguide forbids function declarations entirely).

[codecov-commenter]

Codecov Report

Merging #3048 (49b7182) into master (c66c073) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3048   +/-   ##
=======================================
  Coverage   97.40%   97.41%           
=======================================
  Files         111      111           
  Lines        7445     7452    +7     
  Branches     2723     2726    +3     
=======================================
+ Hits         7252     7259    +7     
  Misses        193      193           

Impacted Files	Coverage Δ
lib/rules/jsx-no-bind.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c66c073...49b7182. Read the comment docs.

[p7g]

Cheers!

[andy128k]

This incorrectly marks a valid code as incorrect in BDD-style tests.

describe('<Outer/>', () => {
   const Inner = () => <div/>;

   it('wraps', () => {
      render(
        <Outer inner={Inner} />
      );
   });
});

[JustFly1984]

@andy128k In your case you can extract Inner outside of describe, cos it has no closure

[ljharb]

@andy128k i don't think there's as much value for this rule to apply to tests at all, since it's a rule about runtime performance.

[andy128k]

@JustFly1984 describe block create a scope. It is not a rare case when a single test file contains multiple describes and sometimes they are nested. They may define components with the same name, so scoping is needed.

describe('<Outer/>', () => {
  describe('<div/>, () => {
     const Inner = () => <div/>;
  
     it('wraps', () => {
        render(
          <Outer inner={Inner} />
        );
     });
  });

  describe('<span/>', () => {
     const Inner = () => <span/>;
  
     it('wraps', () => {
        render(
          <Outer inner={Inner} />
        );
     });
  });
});

Extraction just breaks code style.

---

@ljharb This is what I did -- ignored this rule for affected tests. But having a separated configuration of eslint for tests just feels wrong.

[ljharb]

It’s totally normal and expected and it’s weird to have the same exact eslint config for tests and prod code - that said, if there’s a way to handle this case that doesn’t special-case BDD globals, I’m down.

[JustFly1984]

You can rewrite/disable rules in eslintrc for /*.{test|spec}.{j|t}sx?/ files