diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index 5614c050ba..62a6e4b6ef 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -63,20 +63,27 @@ const create = context => { const firstArgument = argumentNodes[0] ? sourceCode.getText(argumentNodes[0]) : undefined; const secondArgument = argumentNodes[1] ? sourceCode.getText(argumentNodes[1]) : undefined; - let slice; + let sliceArguments; if (argumentNodes.length === 0) { - slice = []; + sliceArguments = []; } else if (argumentNodes.length === 1) { - slice = [firstArgument]; + sliceArguments = [firstArgument]; } else if (argumentNodes.length === 2) { if (firstArgument === '0') { - slice = [firstArgument, secondArgument]; + sliceArguments = [firstArgument]; + if (isLiteralNumber(secondArgument) || isLengthProperty(argumentNodes[1])) { + sliceArguments.push(secondArgument); + } else if (typeof getNumericValue(argumentNodes[1]) === 'number') { + sliceArguments.push(Math.max(0, getNumericValue(argumentNodes[1]))); + } else { + sliceArguments.push(`Math.max(0, ${secondArgument})`); + } } else if ( isLiteralNumber(argumentNodes[0]) && isLiteralNumber(argumentNodes[1]) ) { - slice = [ + sliceArguments = [ firstArgument, argumentNodes[0].value + argumentNodes[1].value ]; @@ -84,14 +91,14 @@ const create = context => { isLikelyNumeric(argumentNodes[0]) && isLikelyNumeric(argumentNodes[1]) ) { - slice = [firstArgument, firstArgument + ' + ' + secondArgument]; + sliceArguments = [firstArgument, firstArgument + ' + ' + secondArgument]; } } - if (slice) { + if (sliceArguments) { const objectText = getNodeText(objectNode); - problem.fix = fixer => fixer.replaceText(node, `${objectText}.slice(${slice.join(', ')})`); + problem.fix = fixer => fixer.replaceText(node, `${objectText}.slice(${sliceArguments.join(', ')})`); } context.report(problem); @@ -111,27 +118,27 @@ const create = context => { const firstNumber = argumentNodes[0] ? getNumericValue(argumentNodes[0]) : undefined; - let slice; + let sliceArguments; if (argumentNodes.length === 0) { - slice = []; + sliceArguments = []; } else if (argumentNodes.length === 1) { if (firstNumber !== undefined) { - slice = [Math.max(0, firstNumber)]; + sliceArguments = [Math.max(0, firstNumber)]; } else if (isLengthProperty(argumentNodes[0])) { - slice = [firstArgument]; + sliceArguments = [firstArgument]; } else { - slice = [`Math.max(0, ${firstArgument})`]; + sliceArguments = [`Math.max(0, ${firstArgument})`]; } } else if (argumentNodes.length === 2) { - const secondNumber = argumentNodes[1] ? getNumericValue(argumentNodes[1]) : undefined; + const secondNumber = getNumericValue(argumentNodes[1]); if (firstNumber !== undefined && secondNumber !== undefined) { - slice = firstNumber > secondNumber ? + sliceArguments = firstNumber > secondNumber ? [Math.max(0, secondNumber), Math.max(0, firstNumber)] : [Math.max(0, firstNumber), Math.max(0, secondNumber)]; } else if (firstNumber === 0 || secondNumber === 0) { - slice = [0, `Math.max(0, ${firstNumber === 0 ? secondArgument : firstArgument})`]; + sliceArguments = [0, `Math.max(0, ${firstNumber === 0 ? secondArgument : firstArgument})`]; } else { // As values aren't Literal, we can not know whether secondArgument will become smaller than the first or not, causing an issue: // .substring(0, 2) and .substring(2, 0) returns the same result @@ -142,9 +149,9 @@ const create = context => { } } - if (slice) { + if (sliceArguments) { const objectText = getNodeText(objectNode); - problem.fix = fixer => fixer.replaceText(node, `${objectText}.slice(${slice.join(', ')})`); + problem.fix = fixer => fixer.replaceText(node, `${objectText}.slice(${sliceArguments.join(', ')})`); } context.report(problem); diff --git a/test/prefer-string-slice.js b/test/prefer-string-slice.js index 3336f7a8bc..9396f6ebd0 100644 --- a/test/prefer-string-slice.js +++ b/test/prefer-string-slice.js @@ -84,7 +84,7 @@ ruleTester.run('prefer-string-slice', rule, { `, output: outdent` const length = 123; - "foo".slice(0, length) + "foo".slice(0, Math.max(0, length)) `, errors }, @@ -99,6 +99,16 @@ ruleTester.run('prefer-string-slice', rule, { `, errors }, + { + code: '"foo".substr(0, -1)', + output: '"foo".slice(0, 0)', + errors + }, + { + code: '"foo".substr(0, "foo".length)', + output: '"foo".slice(0, "foo".length)', + errors + }, { code: outdent` const length = 123; @@ -138,6 +148,18 @@ ruleTester.run('prefer-string-slice', rule, { code: '"foo".substr(1, 2)', errors }, + // Extra arguments + { + code: 'foo.substr(1, 2, 3)', + output: 'foo.substr(1, 2, 3)', + errors + }, + // #700 + { + code: '"Sample".substr(0, "Sample".lastIndexOf("/"))', + output: '"Sample".slice(0, Math.max(0, "Sample".lastIndexOf("/")))', + errors + }, { code: 'foo.substring()', @@ -210,6 +232,12 @@ ruleTester.run('prefer-string-slice', rule, { { code: '"foo".substring(1, 3)', errors + }, + // Extra arguments + { + code: 'foo.substring(1, 2, 3)', + output: 'foo.substring(1, 2, 3)', + errors } ] });