Skip to content

Commit

Permalink
fix(no-trailing-text-in-html-strings): extend rule coverage
Browse files Browse the repository at this point in the history
now handles i18n function calls, and values constructed over multiple
lines
  • Loading branch information
david-at-frog committed Sep 10, 2019
1 parent c492530 commit 5ae4068
Show file tree
Hide file tree
Showing 20 changed files with 669 additions and 199 deletions.
109 changes: 57 additions & 52 deletions lib/rules/no-trailing-text-in-html-strings.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
const { getStaticValue } = require('eslint-utils')

const {
resolveIdentifier,
resolve,
REGEXP_HTML_STRING,
detectJQueryCollections
detectJQueryCollections,
detectDangerousI18n
} = require('../util')

module.exports = {
Expand Down Expand Up @@ -50,62 +49,68 @@ module.exports = {
fixFrom: '1.0.0'
},
messages: {
'no-trailing-text-in-html-strings': 'Trailing text after last tag in HTML strings is ignored'
'no-trailing-text-in-html-strings': 'Trailing text after last tag in HTML strings is ignored',
'no-trailing-text-in-html-strings-via-i18n': 'Trailing text after last tag in HTML I18n strings is ignored'
},
type: 'problem'
},
create: detectJQueryCollections((
context,
{ foundAPositiveMatch }
) => ({
'CallExpression:exit': node => {
if (
!foundAPositiveMatch(node)
) {
return
}

const args = node.arguments.map(arg =>
arg.type === 'Identifier'
? resolveIdentifier(arg, context)
: arg
).filter(arg => arg)
create: context => {
const DEFAULT_OPTIONS = { i18nIdentifier: '_', knownHtmlKeys: [] }
const options = {
...DEFAULT_OPTIONS,
...(context.options.filter(item => typeof item === 'object')[0] || {})
}
return detectDangerousI18n(
options
)(
detectJQueryCollections((
context,
{ foundAPositiveMatch },
{ couldBeDangerousHtmlString }
) => {
return {
'CallExpression:exit': node => {
if (
!foundAPositiveMatch(node)
) {
return
}

args.forEach(arg => {
let value = null
const badI18nString = `__dangerous_i18n_${Date.now()}_<html />string__`

if (arg.type === 'Literal') {
value = arg.value
} else {
try {
({ value } = getStaticValue(arg, context.getScope()))
} catch (e) {
// ignore
}
}
node.arguments
.map(arg => resolve((node, context, UNRESOLVED) => {
switch (node.type) {
case 'CallExpression':
if (couldBeDangerousHtmlString(node)) {
return badI18nString
}
}
return UNRESOLVED
})(arg, context))
.forEach(value => {
if (!value || typeof value !== 'string') return

if (!value || typeof value !== 'string') return
const match = REGEXP_HTML_STRING.exec(value)

const match = REGEXP_HTML_STRING.exec(value)
if (!match || !match[3]) return

if (!match || !match[3]) return
const messageId = (
value.includes(badI18nString) ?
'no-trailing-text-in-html-strings-via-i18n':
'no-trailing-text-in-html-strings'
)

const startColumn = arg.loc.start.column + match[2].length + 1

context.report({
loc: {
start: {
line: arg.loc.start.line,
column: startColumn
},
end: {
line: arg.loc.end.line,
column: startColumn + match[3].length
}
},
messageId: 'no-trailing-text-in-html-strings'
})
context.report({
node: node.arguments[0],
messageId
})
})
}
}
})
}
}))
)(
context
)
}
}
141 changes: 2 additions & 139 deletions lib/util.js
Original file line number Diff line number Diff line change
@@ -1,144 +1,7 @@
// @todo add explicit unit tests for these functions
const JQUERY_GLOBALS = ['$', 'jQuery'];

const isJQuery = node =>
node.type === 'Identifier' && JQUERY_GLOBALS.includes(node.name)
module.exports.isJQuery = isJQuery

const identifierSmellsLikeJQuery = ({ name }) => (
['element'].includes(name) ||
/^\$.+/.test(name)
)

const identifierSmellsLikeNotJQuery = ({ name }) => name === 'model'

const mergeVisitors = (...visitors) => {
const allDeclaredKeys = visitors
.map(visitor => Object.keys(visitor))
.reduce((acc, next) => acc.concat(next), [])
.filter((value, index, array) => array.indexOf(value) === index)

return allDeclaredKeys
.map(key =>
({
[key]: node => visitors.map(visitor => visitor[key])
.filter(value => !!value)
.map(callback => callback(node))
.reverse()[0]
})
)
.reduce((acc, next) => ({ ...acc, ...next }), {})
}

const detectJQueryCollections = (() => {
const symbol = Symbol('mightBeJQueryCollection');
const EMPTY = 0 ; // 0b00000
const POSITIVE = 1<<0; // 0b00001
const NEGATIVE = 1<<1; // 0b00010
const unionBitfields = (left, right) => (left || EMPTY) | (right || EMPTY);

// Positive matches on:
// $()
// $foo
// element
// this.element
//
// Negative matches on:
// this.element.model()
// $foo.model()
//
// Ignores:
// $.whatever

return create => (context, ...moreArgs) =>
mergeVisitors(
{
Identifier: node => {
if (identifierSmellsLikeJQuery(node)) {
node[symbol] = POSITIVE;
} else if (identifierSmellsLikeNotJQuery(node)) {
node[symbol] = NEGATIVE;
}
},
'MemberExpression:exit': node => {
if (node.property[symbol] === NEGATIVE) {
node[symbol] = NEGATIVE
} else {
node[symbol] = unionBitfields(node.object[symbol], node.property[symbol]);
}
},
'CallExpression:exit': node => {
node[symbol] = node.callee[symbol]

if (isJQuery(node.callee)) {
node[symbol] = POSITIVE;
}
},
'LogicalExpression:exit': node => {
node[symbol] = unionBitfields(node.left[symbol], node.right[symbol]);
},
'ConditionalExpression:exit': node => {
node[symbol] = unionBitfields(node.consequent[symbol], node.alternate[symbol]);
}
},
create(
context,
{
foundOnlyPositiveMatches: node => (node[symbol] & (POSITIVE | NEGATIVE)) === POSITIVE,
foundOnlyNegativeMatches: node => (node[symbol] & (POSITIVE | NEGATIVE)) === NEGATIVE,
foundAPositiveMatch: node => !!(node[symbol] & POSITIVE),
foundANegativeMatch: node => !!(node[symbol] & NEGATIVE)
},
...moreArgs
)
)
})()
module.exports.detectJQueryCollections = detectJQueryCollections;

const isFunction = (node, context) => {
if (node.type !== 'Identifier') {
return /FunctionExpression/.test(node.type)
}

return isFunction(
resolveIdentifier(node, context),
context
)
}

const resolveIdentifier = (node, context) => {
if (!node || node.type !== 'Identifier') return null

/* Check if the passed Identifier is a variable which is declared
* within a visble scope */
const { scopeManager } = context.getSourceCode()

let parent = node, scope = null

while (parent && !scope) {
scope = scopeManager.acquire(parent)
parent = parent.parent
}

if (!scope) return false

const variable = scope.variables.find(({ name }) => name === node.name)

if (!variable) return false

// Find last write reference to variable
const ref = [...variable.references].reverse().find(ref => ref.isWrite())

if (ref && ref.writeExpr && ref.writeExpr.type === 'Identifier') {
return resolveIdentifier(ref.writeExpr, context)
}

return ref && ref.writeExpr
}
module.exports = require('./utils/index');

// Regex taken from jquery-migrate source
module.exports.REGEXP_HTML_STRING = /^([^<]*)(<[\w\W]+>)([^>]*)$/
module.exports.isHTMLString = str => module.exports.REGEXP_HTML_STRING.test(str)

module.exports.isFunction = isFunction
module.exports.resolveIdentifier = resolveIdentifier
module.exports.isHTMLString = str => module.exports.REGEXP_HTML_STRING.test(str)
67 changes: 67 additions & 0 deletions lib/utils/detectDangerousI18n.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
const mergeVisitors = require('./mergeVisitors')

const detectDangerousI18n = ({ i18nIdentifier, knownHtmlKeys }) => {
const symbol = Symbol('mightBeDangerousI18n');
const EMPTY = 0 ; // 0b00000
const HTMLI18N = 1<<0; // 0b00001 -- bad i18n string
const I18NCALL = 1<<1; // 0b00010 -- called i18n function
const HTMLOPEN = 1<<2; // 0b00100 -- "<open-tag>..."
const HTMLTERM = 1<<3; // 0b01000 -- "...</close-tag>"
const unionBitfields = (left, right) => (left || EMPTY) | (right || EMPTY);
const couldBeDangerousHtmlString = ({ [symbol]: value }) => (
((value & HTMLI18N) !== EMPTY) &&
((value & I18NCALL) !== EMPTY) &&
(
((value & HTMLOPEN) === EMPTY) ||
((value & HTMLTERM) === EMPTY)
)
);

return create => (context, ...moreArgs) =>
mergeVisitors(
{
Literal: node => {
node[symbol] = EMPTY
if (knownHtmlKeys.includes(node.value)) {
node[symbol] = HTMLI18N
} else {
if (/<\/[a-zA-Z][a-zA-Z-]*>$/.test(node.value)) {
node[symbol] = unionBitfields(node[symbol], HTMLTERM)
}
if (/^<[a-zA-Z]/.test(node.value)) {
node[symbol] = unionBitfields(node[symbol], HTMLOPEN)
}
}
},
CallExpression: node => {
switch (node.callee.type) {
case 'Identifier':
if (node.callee.name === i18nIdentifier) {
node[symbol] = I18NCALL
}
break
case 'MemberExpression':
if (node.callee.property.type === "Identifier" && node.callee.property.name === i18nIdentifier) {
node[symbol] = I18NCALL
}
break
}
},
"CallExpression:exit": node => {
if ((node[symbol] & I18NCALL) !== EMPTY) {
// look at arguments, see if they are bad strings!
if (node.arguments.some(argNode => argNode[symbol] & HTMLI18N)) {
node[symbol] |= HTMLI18N
}
}
}
},
create(
context,
{ couldBeDangerousHtmlString },
...moreArgs
)
)
}

module.exports = detectDangerousI18n
Loading

0 comments on commit 5ae4068

Please sign in to comment.