From 0ca4dfaf632b243728af604fbf0451cfced15ecf Mon Sep 17 00:00:00 2001 From: Tilman Potthof Date: Tue, 13 Oct 2015 00:44:51 +0200 Subject: [PATCH 1/3] Add failing test for typeof undefined check (#232) --- test/definedundefined.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/definedundefined.js b/test/definedundefined.js index 4a6710fd..1c58cd65 100644 --- a/test/definedundefined.js +++ b/test/definedundefined.js @@ -23,6 +23,8 @@ eslintTester.run('definedundefined', rule, { {code: 'undefined === variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: 'undefined !== variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: 'variable !== undefined', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'typeof variable == "undefined"', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'typeof variable !== "undefined"', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: '!angular.isUndefined(variable)', errors: [{message: 'Instead of !angular.isUndefined, you can use the out-of-box angular.isDefined method'}]}, {code: '!angular.isDefined(variable)', errors: [{message: 'Instead of !angular.isDefined, you can use the out-of-box angular.isUndefined method'}]} ] From 7e70866494ca8f2a22e422d34f24f4b8528c3b33 Mon Sep 17 00:00:00 2001 From: Tilman Potthof Date: Sun, 6 Dec 2015 22:30:05 +0100 Subject: [PATCH 2/3] cover typeof checks in definedundefined rule (fixes #232) --- rules/definedundefined.js | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/rules/definedundefined.js b/rules/definedundefined.js index e747a9ed..4f9265e4 100644 --- a/rules/definedundefined.js +++ b/rules/definedundefined.js @@ -9,7 +9,17 @@ */ 'use strict'; +var utils = require('./utils/utils'); + + module.exports = function(context) { + function isCompareOperator(operator) { + return operator === '===' || operator === '!==' || operator === '==' || operator === '!='; + } + function reportError(node) { + context.report(node, 'You should not use directly the "undefined" keyword. Prefer ' + + 'angular.isUndefined or angular.isDefined', {}); + } /** * Rule that check if we use angular.is(Un)defined() instead of the undefined keyword */ @@ -27,15 +37,15 @@ module.exports = function(context) { } }, BinaryExpression: function(node) { - if (node.operator === '===' || node.operator === '!==') { - if (node.left.type === 'Identifier' && node.left.name === 'undefined') { - context.report(node, 'You should not use directly the "undefined" keyword. Prefer ' + - 'angular.isUndefined or angular.isDefined', {}); - } - - if (node.right.type === 'Identifier' && node.right.name === 'undefined') { - context.report(node, 'You should not use directly the "undefined" keyword. Prefer ' + - 'angular.isUndefined or angular.isDefined', {}); + if (isCompareOperator(node.operator)) { + if (utils.isTypeOfStatement(node.left) || utils.isToStringStatement(node.left)) { + reportError(node); + } else if (utils.isTypeOfStatement(node.right) || utils.isToStringStatement(node.right)) { + reportError(node); + } else if (node.left.type === 'Identifier' && node.left.name === 'undefined') { + reportError(node); + } else if (node.right.type === 'Identifier' && node.right.name === 'undefined') { + reportError(node); } } } From 8e3640553dd6a20da38a646d65958db765f7aeb3 Mon Sep 17 00:00:00 2001 From: Tilman Potthof Date: Sun, 6 Dec 2015 23:35:03 +0100 Subject: [PATCH 3/3] fixed typeof check and add possible positives (#232) --- rules/definedundefined.js | 4 ++-- test/definedundefined.js | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/rules/definedundefined.js b/rules/definedundefined.js index 4f9265e4..8005cf27 100644 --- a/rules/definedundefined.js +++ b/rules/definedundefined.js @@ -38,9 +38,9 @@ module.exports = function(context) { }, BinaryExpression: function(node) { if (isCompareOperator(node.operator)) { - if (utils.isTypeOfStatement(node.left) || utils.isToStringStatement(node.left)) { + if (utils.isTypeOfStatement(node.left) && node.right.value === 'undefined') { reportError(node); - } else if (utils.isTypeOfStatement(node.right) || utils.isToStringStatement(node.right)) { + } else if (utils.isTypeOfStatement(node.right) && node.left.value === 'undefined') { reportError(node); } else if (node.left.type === 'Identifier' && node.left.name === 'undefined') { reportError(node); diff --git a/test/definedundefined.js b/test/definedundefined.js index 1c58cd65..f3e1cc4a 100644 --- a/test/definedundefined.js +++ b/test/definedundefined.js @@ -16,15 +16,26 @@ var eslintTester = new RuleTester(); eslintTester.run('definedundefined', rule, { valid: [ 'angular.isUndefined(toto)', - 'angular.isDefined(toto)' + 'angular.isDefined(toto)', + // possible false positives + 'variable === otherValue', + 'variable === null', + 'variable > undefined', + 'angular.isString(null)' ].concat(commonFalsePositives), invalid: [ {code: 'variable === undefined', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: 'undefined === variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: 'undefined !== variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: 'variable !== undefined', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, - {code: 'typeof variable == "undefined"', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'variable == undefined', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'undefined == variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'undefined != variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'variable != undefined', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: 'typeof variable === "undefined"', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: 'typeof variable !== "undefined"', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: '"undefined" == typeof variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, + {code: '"undefined" != typeof variable', errors: [{message: 'You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined'}]}, {code: '!angular.isUndefined(variable)', errors: [{message: 'Instead of !angular.isUndefined, you can use the out-of-box angular.isDefined method'}]}, {code: '!angular.isDefined(variable)', errors: [{message: 'Instead of !angular.isDefined, you can use the out-of-box angular.isUndefined method'}]} ]