Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Commit

Permalink
Merge 2d98cc7 into 97dd2c4
Browse files Browse the repository at this point in the history
  • Loading branch information
@qfox
qfox committed Mar 17, 2016
2 parents 97dd2c4 + 2d98cc7 commit bcda0f6
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 155 deletions.
176 changes: 33 additions & 143 deletions lib/rules/require-var-decl-first.js
View file
Expand Up @@ -108,111 +108,44 @@

var assert = require('assert');

function getVariableScope(node) {
while (node.type !== 'Program' &&
node.type !== 'FunctionDeclaration' &&
node.type !== 'FunctionExpression') {
node = node.parentElement;
}

return node;
}

function getOffsetForBlockStatement(enclosingScope, varDecl, commentTokens, file) {
var offset = 0;
var parentElement = varDecl.parentElement;
if (enclosingScope.type !== 'Program' && parentElement.type === 'BlockStatement') {
offset += 1;
offset += getCommentOffsetBetweenNodes(parentElement, varDecl, commentTokens, file);
}
return offset;
}

function getUseStrictDeclFirst(enclosingScope) {
var firstNode;
if (enclosingScope.type === 'Program') {
firstNode = enclosingScope.body[0];
} else {
firstNode = enclosingScope.body.body[0];
}

if (firstNode.type === 'ExpressionStatement' &&
firstNode.hasOwnProperty('expression') === true &&
firstNode.expression.hasOwnProperty('value') === true &&
firstNode.expression.value === 'use strict') {
return firstNode;
}

return null;
}

function isFirstVarDeclInScope(enclosingScope, varDecl, whitespaceOffsetBeforeVarDecl, commentTokens, file) {
var adjustedVarDeclStart = varDecl.range[0];
var adjustedScopeStart = enclosingScope.range[0];

if (enclosingScope.type !== 'Program') {
// For function declaration and function expression scope use the top block statement as start
// This removes the requirement to offset the function declaration or expression related tokens
adjustedScopeStart = enclosingScope.body.range[0];
// If enclosing scope node type is Program the range start ignores all comments and whitespace before the
// variable declaration
adjustedVarDeclStart -= whitespaceOffsetBeforeVarDecl;
}

adjustedVarDeclStart -= getOffsetForBlockStatement(enclosingScope, varDecl, commentTokens, file);

if (adjustedVarDeclStart === adjustedScopeStart) {
return true;
}

return false;
function isScopeElement(elem) {
return elem.type === 'Program' ||
elem.type === 'BlockStatement' && (
elem.parentElement.type === 'FunctionExpression' ||
elem.parentElement.type === 'FunctionDeclaration' ||
elem.parentElement.type === 'ArrowFunctionExpression'
);
}

function getCommentOffsetBetweenNodes(previousNode, currentNode, commentTokens, file) {
var count;
var comment;
var commentLength = 0;

for (count = 0; count < commentTokens.length; count++) {
comment = commentTokens[count];
if (comment.range[0] >= currentNode.range[1]) {
// Stop processing comments that are occurred after current node
break;
}

if (previousNode.range[1] < currentNode.range[0] &&
comment.range[0] > previousNode.range[0] &&
comment.range[1] < previousNode.range[1]) {
// Stop processing comments that are within multiple declarators in a single variable declaration
// of the previous node and the previousNode is not the parent of currentNode
break;
}
/**
* Checks for allowed elements before variable declaration
* @param {Object} elem
* @returns {Boolean}
*/
function isRelevantElement(elem) {
// Allow comments and whitespaces.
var itIs = elem.isComment || elem.isWhitespace;

if (comment.range[0] > currentNode.range[0] &&
comment.range[1] < currentNode.range[1]) {
// Stop processing comments that are within multiple declarators in a single variable declaration
break;
}
// Variable declaration itself.
itIs = itIs || elem.type === 'VariableDeclaration';

if (previousNode.range[0] >= comment.range[1]) {
// Skip comments that occurred before the previous node
continue;
}
// For '{' in `function() { var a; }`.
itIs = itIs || elem.type === 'Punctuator';

commentLength += comment.range[1] - comment.range[0] + file.getWhitespaceBefore(comment).length;
}
// Skip 'use strict' statements at the beginning.
itIs = itIs || elem.type === 'ExpressionStatement' && elem.expression.value === 'use strict';

return commentLength;
return itIs;
}

function isPreviousNodeAVarDecl(previousNode, varDecl, whitespaceOffsetBeforeVarDecl, commentTokens, file) {
var offsetForComments;
if (varDecl.range[0] === previousNode.range[1]) {
return true;
function isVarDeclFirst(varDecl) {
var elem = varDecl;
// All elements should be relevant.
while (elem && (elem.isComment || elem.isWhitespace || isRelevantElement(elem))) {
elem = elem.previousSibling;
}

offsetForComments = getCommentOffsetBetweenNodes(previousNode, varDecl, commentTokens, file);
if (varDecl.range[0] - whitespaceOffsetBeforeVarDecl - offsetForComments === previousNode.range[1]) {
if (elem === null) {
return true;
}

Expand All @@ -234,62 +167,19 @@ module.exports.prototype = {
},

check: function(file, errors) {
var scopesFoundInFile = {};
var commentTokens = [];

file.iterateTokensByType(['CommentLine', 'CommentBlock'], function(commentToken) {
commentTokens.push(commentToken);
});

file.iterateNodesByType(['VariableDeclaration'], function(varDecl) {
// Ignore let and const for now #1783
if (varDecl.kind !== 'var') {
return;
}

var enclosingScope;
var scopeContents;
var previousNode;
var useStrictDirective;
var isVarDeclFirst = false;

var whitespaceOffsetBeforeVarDecl = file.getWhitespaceBefore(file.getFirstNodeToken(varDecl)).length;

enclosingScope = getVariableScope(varDecl.parentElement);
if (!scopesFoundInFile.hasOwnProperty(enclosingScope.range[0])) {
scopesFoundInFile[enclosingScope.range[0]] = { hasNonVarDecl: false, varDecl: [] };
// placing the handling 'use strict' declared as the first statement of scope here to improve
// performance to run once per scope discovered in file
useStrictDirective = getUseStrictDeclFirst(enclosingScope);
if (useStrictDirective !== null) {
// Special case to make varDecl stack contain the use strict as first node
// this reduces the complexity of the isFirstVarDecInScope and reuses
// isPreviousNodeAVarDecl to handle this special scenario
scopesFoundInFile[enclosingScope.range[0]].varDecl.push(useStrictDirective);
}
}

scopeContents = scopesFoundInFile[enclosingScope.range[0]];

if (scopeContents.varDecl.length === 0) {
isVarDeclFirst = isFirstVarDeclInScope(
enclosingScope, varDecl, whitespaceOffsetBeforeVarDecl, commentTokens, file
);
} else {
previousNode = scopeContents.varDecl[scopeContents.varDecl.length - 1];
if (!scopeContents.hasNonVarDecl) {
isVarDeclFirst = isPreviousNodeAVarDecl(
previousNode, varDecl, whitespaceOffsetBeforeVarDecl, commentTokens, file
);
}
// Checking scope to not allow vars inside block statements.
if (isScopeElement(varDecl.parentElement) && isVarDeclFirst(varDecl)) {
return;
}

scopeContents.varDecl.push(varDecl);
if (!isVarDeclFirst) {
scopeContents.hasNonVarDecl = true;
errors.add('Variable declarations must be the first statements of a function scope.',
varDecl);
}
errors.add('Variable declarations must be the first statements of a function scope.',
varDecl);
});
}
};
42 changes: 30 additions & 12 deletions test/specs/rules/require-var-decl-first.js
View file
@@ -1,7 +1,7 @@
var Checker = require('../../../lib/checker');
var expect = require('chai').expect;

describe.skip('rules/require-var-decl-first', function() {
describe('rules/require-var-decl-first', function() {
describe('boolean', function() {
var checker;
beforeEach(function() {
Expand Down Expand Up @@ -32,7 +32,8 @@ describe.skip('rules/require-var-decl-first', function() {
expect(resultCount).to.equal(expectedErrorCount);
for (count = 0; count < resultCount; count++) {
expect(errors[count].message)
.to.equal('Variable declarations must be the first statements of a function scope.');
.to.equal('requireVarDeclFirst: Variable declarations ' +
'must be the first statements of a function scope.');
}
};

Expand Down Expand Up @@ -102,17 +103,17 @@ describe.skip('rules/require-var-decl-first', function() {
});

it('should not return errors for multiple var declaration at top of program scope', function() {
testDeclStatements(checker, 'var a;var b;', 0);
testDeclStatements(checker, 'var a, c;var b;', 0);
});

it('should not return errors for multiple var declaration sandwiching a single comment ' +
'at top of program scope', function() {
testDeclStatements(checker, 'var a;/*block comment*/var b;', 0);
testDeclStatements(checker, 'var a, c;/*block comment*/var b;', 0);
});

it('should not return errors for multiple var declaration with a single comment before the ' +
'first var decl at top of program scope', function() {
testDeclStatements(checker, 'var a;/*block comment*/var b;/*block comment 2*/var c;', 0);
testDeclStatements(checker, 'var a, d;/*block comment*/var b;/*block comment 2*/var c;', 0);
});

it('should not return errors for a single var declaration with comments sandwiched ' +
Expand Down Expand Up @@ -180,7 +181,7 @@ describe.skip('rules/require-var-decl-first', function() {
});

it('should not return errors for multiple var declaration at top of program scope', function() {
testDeclStatements(checker, 'var a; var b;', 0);
testDeclStatements(checker, 'var a, A; var b;', 0);
});

it('should not return errors for a single var declaration with comments sandwiched ' +
Expand All @@ -200,10 +201,6 @@ describe.skip('rules/require-var-decl-first', function() {
testDeclStatements(checker, '"use strict";\nvar a;', 0);
});

it('should not return errors for single const var declaration at top of program scope', function() {
testDeclStatements(checker, '\nconst a = 1;', 0);
});

it('should not return errors for single var declaration after single comment at top of program scope',
function() {
testDeclStatements(checker, '/*block comment*/\nvar a;', 0);
Expand All @@ -225,7 +222,7 @@ describe.skip('rules/require-var-decl-first', function() {
});

it('should not return errors for multiple var declaration at top of program scope', function() {
testDeclStatements(checker, 'var a;\nvar b;', 0);
testDeclStatements(checker, 'var a, A;\nvar b;', 0);
});

it('should not return errors for a single var declaration with comments sandwiched ' +
Expand All @@ -236,7 +233,28 @@ describe.skip('rules/require-var-decl-first', function() {

it('should not return errors for multiple var declaration with a comment inside the scope of' +
' first variable declaration', function() {
testDeclStatements(checker, 'var a = function() { var b;\n/*block comment*/\n};\nvar c;', 0);
testDeclStatements(checker, 'var a = function() { var b;\n/*block comment*/\n}, A;\nvar c;', 0);
});
});

describe('statements inside blocks', function() {
it('should report for single var declaration inside block', function() {
testDeclStatements(checker, '{var a;}', 1);
});

it('should report for single var declaration at top of program scope after prologue',
function() {
testDeclStatements(checker, '{"use strict";\nvar a;}', 1);
});

it('should report for single var declaration after single comment inside a block',
function() {
testDeclStatements(checker, '{/*block comment*/\nvar a;}', 1);
});

it('should report for single var decl inside a block after assignment',
function() {
testDeclStatements(checker, 'x=1;{var a;}', 1);
});
});
});
Expand Down

0 comments on commit bcda0f6

Please sign in to comment.