Skip to content

Fix: Correctly handle rules that are missing meta or have meta / create defined in variables #225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

bmish
Copy link
Member

@bmish bmish commented Oct 25, 2021

Previously, many of our rules using our getRuleInfo util (especially the require-meta-* rules) had inconsistent behavior when encountering a rule that was missing meta or using variables for meta / create).

Example structures:

module.exports = { create: function (context) { /* ... */ } }; // no `meta`
const meta = { /* ... */ };
const create = function (context) { /* ... */ }
module.exports = { meta, create }; // variables

These are two very closed-related / overlapping issues so I fixed them together and added a variety of tests.

This addresses some crashes / false negatives / false positives.

Fixes #181.

@bmish bmish force-pushed the rules-without-meta branch from b240cc1 to 0534276 Compare October 25, 2021 01:54
@bmish bmish marked this pull request as draft October 25, 2021 02:16
@bmish bmish force-pushed the rules-without-meta branch 4 times, most recently from a377956 to 1d02934 Compare October 25, 2021 15:34
@bmish bmish force-pushed the rules-without-meta branch from 1d02934 to 293a350 Compare October 25, 2021 15:41
* @param {ScopeManager} scopeManager The scope manager to use for resolving references
* @returns {boolean} `true` if the node is a reference to a function expression
*/
function isNormalFunctionExpressionReference (node, scopeManager) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm very happy to be able to remove this overly-complicated function. I have been able to dramatically simplify the code using my new findVariableValue function.

@bmish bmish marked this pull request as ready for review October 25, 2021 15:47
@bmish
Copy link
Member Author

bmish commented Oct 26, 2021

Side note: ready for a patch release after this is merged.

Copy link
Contributor

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit rules to ensure correct behavior when no meta object present
2 participants