Site-kit-wp: Don't require imports or externally-defined APIs to conform to our acronym naming rules

[tofumatt]

Bug Description

Currently we don't do everything we can to detect the original of a function name/variable/etc. when checking if it conforms to our ESLint rules for naming. This causes false positives with acronyms eg (ESLint will error if importing a function named useInstanceId even though it is exported that way from @wordpress/compose). See: https://github.com/google/site-kit-wp/pull/2313#discussion_r517727137 for more info.

Essentially, we should inspect the _origin_ of a variable/function name, and if it's from an external source (eg imported from _outside_ the file), we should not lint it for naming. Detecting that the source of the name can be traced to an import is sufficient to cover external APIs, because our own code should error when an invalid name is defined or exported.

Steps to reproduce

1. import { useInstanceId } from '@wordpress/compose;'
2. npm run lint:js
3. Observe ESLint error

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

• Imports and their descendants should not trigger ESLint warnings about capitalisation.

Implementation Brief

• For a rule violation of camelcase-acronyms, use a recursive check up any variable's node structure to determine if it belongs to an external module (eg another file). We do these kinds of checks for isFunction already: https://github.com/google/site-kit-wp/blob/77aa482d628c689177d266b056c86de4fb57ca4b/packages/eslint-plugin/utils.js#L61, so check that the origin of the variable is inside the file.
• Use [ 'ImportDefaultSpecifier', 'ImportSpecifier', 'ExportDefaultDeclaration', 'ExportDeclaration', 'ImportDeclaration' ] as types that trigger import detection. Ignore nodes with these types (or with these types as parents), but also store the names of imports to ignore in future checks as well.
• Use #2431 as a basis for the approach.

Test Coverage

• Add ESLint test cases for this if straightforward, but if not then at least verify it can be reproduced then ensure that code doesn't trigger an ESLint violation after the fix.

Visual Regression Changes

• None.

QA Brief

• Follow "Steps to reproduce" above and confirm that no ESLint errors are thrown.

Changelog entry

• N/A

[felixarntz]

@tofumatt

For a rule violation of camelcase-acronyms, use a recursive check up any variable's node structure to determine if it belongs to an external module (eg another file).

How would be be able to determine that? Is it possible to inspect _where_ a variable was imported from? We need to make sure this doesn't avoid the rule when something comes from another file, but only when it comes from another _external_ module that is not part of Site Kit.

[tofumatt]

We can check using the above (IB slightly clarified): essentially we check for a node's type and its name. If a node has a type (or a parent with the type): [ 'ImportDefaultSpecifier', 'ImportSpecifier', 'ExportDefaultDeclaration', 'ExportDeclaration', 'ImportDeclaration' ], then it's an import from another module. We can then ignore that name for the rest of the file, knowing it's an import.

We _can_ see where the module was imported from, but in this case it isn't _really_ necessary: the actual module inside Site Kit that _exports_ the acronym-rule-defying rule will be flagged as violating the rule, which should trigger the developer to change the export name (and thus all the imports as well).

Checking to see if the import is external would be an added cost to every lint that I don't think is necessary for that reason.

[felixarntz]

IB ✅

[aaemnnosttv]

@caseymorrisus – would you please add a QA Brief? Once added go ahead an unassign yourself 👍

[eugene-manuilov]

QA :heavy_check_mark: Everything works as expected. I have been able to import a function from an external package that violates our naming conventions and eslint hasn't thrown an error for it.