Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update: no-inner-declarations false negative in non-block (fixes #12222) #13062

Merged
merged 10 commits into from Mar 31, 2020
Merged

Update: no-inner-declarations false negative in non-block (fixes #12222) #13062

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
83f83c5
Fix: consistencies for blockstmt no-inner-fn (fix #12222)
anikethsaha Mar 18, 2020
1c813f0
Chore: added some more tests
anikethsaha Mar 19, 2020
4c9adf8
Chore: allowing named and default exports and tests
anikethsaha Mar 19, 2020
0727032
Chore: simply the check conditions
anikethsaha Mar 22, 2020
d7a8d2b
Chore: simplify checks and ancestor loop
anikethsaha Mar 23, 2020
45c92ee
Chore: using getUpperFunction to get body
anikethsaha Mar 23, 2020
bb9fe95
chore: added tests and minor changes
anikethsaha Mar 25, 2020
ac476af
Merge remote-tracking branch 'upstream/master' into fixes-12222
anikethsaha Mar 25, 2020
7c77296
Docs: eg for both non-blockstatement
anikethsaha Mar 26, 2020
cc0e5b3
Chore: right docs example and test refactore
anikethsaha Mar 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 31 additions & 38 deletions lib/rules/no-inner-declarations.js
View file
Open in desktop
Expand Up @@ -5,10 +5,19 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const validParent = new Set(["Program", "ExportNamedDeclaration", "ExportDefaultDeclaration"]);
const validBlockStatementParent = new Set(["FunctionDeclaration", "FunctionExpression", "ArrowFunctionExpression"]);

module.exports = {
meta: {
type: "problem",
Expand All @@ -34,52 +43,36 @@ module.exports = {
create(context) {

/**
* Find the nearest Program or Function ancestor node.
* @returns {Object} Ancestor's type and distance from node.
* Ensure that a given node is at a program or function body's root.
* @param {ASTNode} node Declaration node to check.
* @returns {void}
*/
function nearestBody() {
function check(node) {
const ancestors = context.getAncestors();
let ancestor = ancestors.pop(),
generation = 1;

while (ancestor && ["Program", "FunctionDeclaration",
"FunctionExpression", "ArrowFunctionExpression"
].indexOf(ancestor.type) < 0) {
generation += 1;
ancestor = ancestors.pop();
const ancestor = ancestors.pop();
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved

if (
ancestor.type === "BlockStatement" && validBlockStatementParent.has(ancestor.parent.type)
) {
return;
}

return {
if ((validParent.has(ancestor.type))) {
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved
return;
}

// Type of containing ancestor
type: ancestor.type,
const body = astUtils.getUpperFunction(ancestor);
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved

// Separation between ancestor and node
distance: generation
};
context.report({
node,
messageId: "moveDeclToRoot",
data: {
type: (node.type === "FunctionDeclaration" ? "function" : "variable"),
body: (body === null ? "program" : "function body")
}
});
}

/**
* Ensure that a given node is at a program or function body's root.
* @param {ASTNode} node Declaration node to check.
* @returns {void}
*/
function check(node) {
const body = nearestBody(),
valid = ((body.type === "Program" && body.distance === 1) ||
body.distance === 2);

if (!valid) {
context.report({
node,
messageId: "moveDeclToRoot",
data: {
type: (node.type === "FunctionDeclaration" ? "function" : "variable"),
body: (body.type === "Program" ? "program" : "function body")
}
});
}
}

return {

Expand Down
279 changes: 214 additions & 65 deletions tests/lib/rules/no-inner-declarations.js
View file
Open in desktop
Expand Up @@ -45,74 +45,223 @@ ruleTester.run("no-inner-declarations", rule, {
code: "var x = {doSomething() {var foo;}}",
options: ["both"],
parserOptions: { ecmaVersion: 6 }
},
{
code: "export var foo;",
options: ["both"],
parserOptions: { sourceType: "module", ecmaVersion: 6 }
},
{
code: "export function bar() {}",
options: ["both"],
parserOptions: { sourceType: "module", ecmaVersion: 6 }
},
{
code: "export default function baz() {}",
options: ["both"],
parserOptions: { sourceType: "module", ecmaVersion: 6 }
},
{
code: "exports.foo = () => {}",
options: ["both"],
parserOptions: { ecmaVersion: 6 }
},
{
code: "exports.foo = function(){}",
options: ["both"],
parserOptions: { ecmaVersion: 6 }
},
{
code: "module.exports = function foo(){}",
options: ["both"],
parserOptions: { ecmaVersion: 6 }
}
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved

],

// Examples of code that should trigger the rule
invalid: [{
code: "if (test) { function doSomething() { } }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "program"
},
type: "FunctionDeclaration"
}]
}, {
code: "function doSomething() { do { function somethingElse() { } } while (test); }",
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "function body"
},
type: "FunctionDeclaration"
}]
}, {
code: "(function() { if (test) { function doSomething() { } } }());",
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "function body"
},
type: "FunctionDeclaration"
}]
}, {
code: "while (test) { var foo; }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "program"
},
type: "VariableDeclaration"
}]
}, {
code: "function doSomething() { if (test) { var foo = 42; } }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
}, {
code: "(function() { if (test) { var foo; } }());",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
}]
invalid: [
{
code: "if (test) { function doSomething() { } }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "program"
},
type: "FunctionDeclaration"
}]
}, {
code: "if (foo) var a; ",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "program"
},
type: "VariableDeclaration"
}]
}, {
code: "if (foo) /* some comments */ var a; ",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "program"
},
type: "VariableDeclaration"
}]
}, {
code: "if (foo){ function f(){ if(bar){ var a; } } }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "program"
},
type: "FunctionDeclaration"
}, {
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
}, {
code: "if (foo) function f(){ if(bar) var a; } ",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "program"
},
type: "FunctionDeclaration"
}, {
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
}, {
code: "if (foo) { var fn = function(){} } ",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "program"
},
type: "VariableDeclaration"
}]
},
{
code: "if (foo) function f(){} ",
options: ["both"],
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "program"
},
type: "FunctionDeclaration"
}]
},
{
code: "function bar() { if (foo) function f(){}; }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "function body"
},
type: "FunctionDeclaration"
}]
},
{
code: "function bar() { if (foo) var a; }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
},
{
code: "if (foo){ var a; }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "program"
},
type: "VariableDeclaration"
}]
}, {
code: "function doSomething() { do { function somethingElse() { } } while (test); }",
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "function body"
},
type: "FunctionDeclaration"
}]
}, {
code: "(function() { if (test) { function doSomething() { } } }());",
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "function",
body: "function body"
},
type: "FunctionDeclaration"
}]
}, {
code: "while (test) { var foo; }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "program"
},
type: "VariableDeclaration"
}]
}, {
code: "function doSomething() { if (test) { var foo = 42; } }",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
}, {
code: "(function() { if (test) { var foo; } }());",
options: ["both"],
errors: [{
messageId: "moveDeclToRoot",
data: {
type: "variable",
body: "function body"
},
type: "VariableDeclaration"
}]
}
]
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved
});