Fix: id-blacklist false positives on renamed imports

[mdjermanovic]

Prerequisites checklist

• I have read the contributing guidelines.
• The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix

Tell us about your environment

• ESLint Version: 7.0.0-alpha.0
• Node Version: v12.14.0
• npm Version: v6.13.4

What parser (default, Babel-ESLint, etc.) are you using?

default

Please show your full configuration:

Configuration

module.exports = {
    parserOptions: {
        ecmaVersion: 2015,
        sourceType: "module"
    },
};

What did you do? Please include the actual source code causing the issue.

/* eslint id-blacklist: ["error", "foo"] */

import { foo as bar } from "mod";
export { foo as bar } from "mod";

import { foo } from "mod";

Online Demo Link - the actual demo is v6.8.0, but it's the same behavior regarding this issue.

What did you expect to happen?

Not to report foo in import { foo as bar } and export { foo as bar }.

This is a stylistic rule and shouldn't restrict imports of existing external names (and thus basically their use, that's the responsibility of the no-restricted-imports rule).

On the other hand, the rule should report import { foo } and enforce renaming, but it shouldn't report the same identifier twice.

What actually happened? Please include the actual, raw output from ESLint.

  3:10  error  Identifier 'foo' is blacklisted  id-blacklist
  4:10  error  Identifier 'foo' is blacklisted  id-blacklist
  6:10  error  Identifier 'foo' is blacklisted  id-blacklist
  6:10  error  Identifier 'foo' is blacklisted  id-blacklist

These are two different bugs:

• The first two errors are false positives.
• One of the other two errors is a duplicate.

What changes did you make? (Give an overview)

• Fixed the rule to not report renamed imports/re-exports.
• Fixed the rule to avoid reporting same nodes multiple times.
• Refactoring
  • if (node.parent.type === "Property") branch was same as the next branch, so it's removed.
  • effectiveParent was different than node.parent just for member expressions, so it's moved to that branch to simplify the rest of the code.

Is there anything you'd like reviewers to focus on?

• Refactoring, did I miss something.
• Local names in exports (not re-exports) will be still reported:

/* eslint id-blacklist: ["error", "foo"] */

var foo; // error

export { foo as bar }; // also error, although it's renamed here

That seemed consistent with how this rule works: it reports all references, not just declarations.

[kaicataldo]

LGTM, thanks!