refactor: use includes where appropriate (#4412)

dnalborczyk · lukastaegert · web-flow · commit 70d0025c36ed · 2022-02-21T16:15:51.000+01:00
* refactor: use includes where appropriate

* refactor: replace .substr with .substring

* Extract some regular expressions

Co-authored-by: Lukas Taegert-Atkinson &lt;lukastaegert@users.noreply.github.com&gt;
Co-authored-by: Lukas Taegert-Atkinson &lt;lukas.taegert-atkinson@tngtech.com&gt;
diff --git a/browser/path.ts b/browser/path.ts
@@ -1,20 +1,23 @@
-export const absolutePath = /^(?:\/|(?:[A-Za-z]:)?[\\|/])/;
-export const relativePath = /^\.?\.\//;
+const ABSOLUTE_PATH_REGEX = /^(?:\/|(?:[A-Za-z]:)?[\\|/])/;
+const RELATIVE_PATH_REGEX = /^\.?\.\//;
+const ALL_BACKSLASHES_REGEX = /\\/g;
+const ANY_SLASH_REGEX = /[/\\]/;
+const EXTNAME_REGEX = /\.[^.]+$/;
 
 export function isAbsolute(path: string): boolean {
-	return absolutePath.test(path);
+	return ABSOLUTE_PATH_REGEX.test(path);
 }
 
 export function isRelative(path: string): boolean {
-	return relativePath.test(path);
+	return RELATIVE_PATH_REGEX.test(path);
 }
 
 export function normalize(path: string): string {
-	return path.replace(/\\/g, '/');
+	return path.replace(ALL_BACKSLASHES_REGEX, '/');
 }
 
 export function basename(path: string): string {
-	return path.split(/[/\\]/).pop() || '';
+	return path.split(ANY_SLASH_REGEX).pop() || '';
 }
 
 export function dirname(path: string): string {
@@ -28,14 +31,13 @@ export function dirname(path: string): string {
 }
 
 export function extname(path: string): string {
-	const match = /\.[^.]+$/.exec(basename(path)!);
-	if (!match) return '';
-	return match[0];
+	const match = EXTNAME_REGEX.exec(basename(path)!);
+	return match ? match[0] : '';
 }
 
 export function relative(from: string, to: string): string {
-	const fromParts = from.split(/[/\\]/).filter(Boolean);
-	const toParts = to.split(/[/\\]/).filter(Boolean);
+	const fromParts = from.split(ANY_SLASH_REGEX).filter(Boolean);
+	const toParts = to.split(ANY_SLASH_REGEX).filter(Boolean);
 
 	if (fromParts[0] === '.') fromParts.shift();
 	if (toParts[0] === '.') toParts.shift();
@@ -62,13 +64,13 @@ export function resolve(...paths: string[]): string {
 	if (!firstPathSegment) {
 		return '/';
 	}
-	let resolvedParts = firstPathSegment.split(/[/\\]/);
+	let resolvedParts = firstPathSegment.split(ANY_SLASH_REGEX);
 
 	for (const path of paths) {
 		if (isAbsolute(path)) {
-			resolvedParts = path.split(/[/\\]/);
+			resolvedParts = path.split(ANY_SLASH_REGEX);
 		} else {
-			const parts = path.split(/[/\\]/);
+			const parts = path.split(ANY_SLASH_REGEX);
 
 			while (parts[0] === '.' || parts[0] === '..') {
 				const part = parts.shift();
diff --git a/cli/run/index.ts b/cli/run/index.ts
@@ -26,13 +26,13 @@ export default async function runRollup(command: Record<string, any>): Promise<v
 	}
 
 	if (inputSource && inputSource.length > 0) {
-		if (inputSource.some((input: string) => input.indexOf('=') !== -1)) {
+		if (inputSource.some((input: string) => input.includes('='))) {
 			command.input = {};
 			inputSource.forEach((input: string) => {
 				const equalsIndex = input.indexOf('=');
-				const value = input.substr(equalsIndex + 1);
-				let key = input.substr(0, equalsIndex);
-				if (!key) key = getAliasName(input);
+				const value = input.substring(equalsIndex + 1);
+				const key = input.substring(0, equalsIndex) || getAliasName(input);
+
 				command.input[key] = value;
 			});
 		} else {
diff --git a/docs/05-plugin-development.md b/docs/05-plugin-development.md
@@ -788,7 +788,7 @@ export default function addProxyPlugin() {
       if (resolution && !resolution.external) {
         // we pass on the entire resolution information
         const moduleInfo = await this.load(resolution);
-        if (moduleInfo.code.indexOf('/* use proxy */') >= 0) {
+        if (moduleInfo.code.includes('/* use proxy */')) {
           return `${resolution.id}?proxy`;
         }
       }
diff --git a/src/Chunk.ts b/src/Chunk.ts
@@ -605,7 +605,7 @@ export default class Chunk {
 				const source = module.render(renderOptions).trim();
 				renderedLength = source.length();
 				if (renderedLength) {
-					if (options.compact && source.lastLine().indexOf('//') !== -1) source.append('\n');
+					if (options.compact && source.lastLine().includes('//')) source.append('\n');
 					this.renderedModuleSources.set(module, source);
 					magicString.addSource(source);
 					this.usedModules.push(module);
diff --git a/src/finalisers/iife.ts b/src/finalisers/iife.ts
@@ -38,7 +38,7 @@ export default function iife(
 	}: NormalizedOutputOptions
 ): Bundle {
 	const { _, cnst, getNonArrowFunctionIntro, getPropertyAccess, n } = snippets;
-	const isNamespaced = name && name.indexOf('.') !== -1;
+	const isNamespaced = name && name.includes('.');
 	const useVariableAssignment = !extend && !isNamespaced;
 
 	if (name && useVariableAssignment && !isLegal(name)) {
diff --git a/src/utils/getExportMode.ts b/src/utils/getExportMode.ts
@@ -33,7 +33,7 @@ export default function getExportMode(
 			}
 			exportMode = 'default';
 		} else {
-			if (format !== 'es' && format !== 'system' && exportKeys.indexOf('default') !== -1) {
+			if (format !== 'es' && format !== 'system' && exportKeys.includes('default')) {
 				warn(errMixedExport(facadeModuleId, name));
 			}
 			exportMode = 'named';
diff --git a/src/utils/options/mergeOptions.ts b/src/utils/options/mergeOptions.ts
@@ -87,7 +87,7 @@ function getCommandOptions(rawCommandOptions: GenericConfigObject): CommandConfi
 				? rawCommandOptions.globals.split(',').reduce((globals, globalDefinition) => {
 						const [id, variableName] = globalDefinition.split(':');
 						globals[id] = variableName;
-						if (external.indexOf(id) === -1) {
+						if (!external.includes(id)) {
 							external.push(id);
 						}
 						return globals;
@@ -156,7 +156,7 @@ const getExternal = (
 	const configExternal = config.external as ExternalOption | undefined;
 	return typeof configExternal === 'function'
 		? (source: string, importer: string | undefined, isResolved: boolean) =>
-				configExternal(source, importer, isResolved) || overrides.external.indexOf(source) !== -1
+				configExternal(source, importer, isResolved) || overrides.external.includes(source)
 		: ensureArray(configExternal).concat(overrides.external);
 };
 
diff --git a/src/utils/path.ts b/src/utils/path.ts
@@ -1,17 +1,18 @@
-const absolutePath = /^(?:\/|(?:[A-Za-z]:)?[\\|/])/;
-const relativePath = /^\.?\.\//;
+const ABSOLUTE_PATH_REGEX = /^(?:\/|(?:[A-Za-z]:)?[\\|/])/;
+const RELATIVE_PATH_REGEX = /^\.?\.\//;
 
 export function isAbsolute(path: string): boolean {
-	return absolutePath.test(path);
+	return ABSOLUTE_PATH_REGEX.test(path);
 }
 
 export function isRelative(path: string): boolean {
-	return relativePath.test(path);
+	return RELATIVE_PATH_REGEX.test(path);
 }
 
+const BACKSLASH_REGEX = /\\/g;
+
 export function normalize(path: string): string {
-	if (path.indexOf('\\') == -1) return path;
-	return path.replace(/\\/g, '/');
+	return path.replace(BACKSLASH_REGEX, '/');
 }
 
 export { basename, dirname, extname, relative, resolve } from 'path';
diff --git a/src/watch/watch.ts b/src/watch/watch.ts
@@ -163,7 +163,7 @@ export class Task {
 		this.invalidated = true;
 		if (details.isTransformDependency) {
 			for (const module of this.cache.modules) {
-				if (module.transformDependencies.indexOf(id) === -1) continue;
+				if (!module.transformDependencies.includes(id)) continue;
 				// effective invalidation
 				module.originalCode = null as never;
 			}
diff --git a/test/browser/index.js b/test/browser/index.js
@@ -17,7 +17,7 @@ runTestSuiteWithSamples('browser', resolve(__dirname, 'samples'), (dir, config)
 				bundle = await rollup({
 					input: 'main',
 					onwarn: warning => {
-						if (!(config.expectedWarnings && config.expectedWarnings.indexOf(warning.code) >= 0)) {
+						if (!(config.expectedWarnings && config.expectedWarnings.includes(warning.code))) {
 							throw new Error(
 								`Unexpected warnings (${warning.code}): ${warning.message}\n` +
 									'If you expect warnings, list their codes in config.expectedWarnings'
diff --git a/test/chunking-form/index.js b/test/chunking-form/index.js
@@ -1,26 +1,25 @@
-const path = require('path');
-const rollup = require('../../dist/rollup');
+const { basename, resolve } = require('path');
+const { chdir } = require('process');
+const { rollup } = require('../../dist/rollup');
 const { runTestSuiteWithSamples, assertDirectoriesAreEqual } = require('../utils.js');
 
 const FORMATS = ['es', 'cjs', 'amd', 'system'];
 
-runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (dir, config) => {
+runTestSuiteWithSamples('chunking form', resolve(__dirname, 'samples'), (dir, config) => {
 	(config.skip ? describe.skip : config.solo ? describe.only : describe)(
-		path.basename(dir) + ': ' + config.description,
+		basename(dir) + ': ' + config.description,
 		() => {
 			let bundle;
 
 			for (const format of FORMATS) {
 				it('generates ' + format, async () => {
-					process.chdir(dir);
+					chdir(dir);
 					bundle =
 						bundle ||
-						(await rollup.rollup({
+						(await rollup({
 							input: [dir + '/main.js'],
 							onwarn: warning => {
-								if (
-									!(config.expectedWarnings && config.expectedWarnings.indexOf(warning.code) >= 0)
-								) {
+								if (!(config.expectedWarnings && config.expectedWarnings.includes(warning.code))) {
 									throw new Error(
 										`Unexpected warnings (${warning.code}): ${warning.message}\n` +
 											'If you expect warnings, list their codes in config.expectedWarnings'
diff --git a/test/cli/samples/external-modules-auto-global/_config.js b/test/cli/samples/external-modules-auto-global/_config.js
@@ -6,6 +6,6 @@ module.exports = {
 		'rollup main.js --format iife --globals mathematics:Math,promises:Promise --external promises',
 	execute: true,
 	stderr(stderr) {
-		assert.strictEqual(stderr.indexOf('(!)'), -1);
+		assert.ok(!stderr.includes('(!)'));
 	}
 };
diff --git a/test/cli/samples/external-modules/_config.js b/test/cli/samples/external-modules/_config.js
@@ -5,6 +5,6 @@ module.exports = {
 	command: 'rollup main.js --format cjs --external=path,util',
 	execute: true,
 	stderr(stderr) {
-		assert.strictEqual(stderr.indexOf('(!)'), -1);
+		assert.ok(!stderr.includes('(!)'));
 	}
 };
diff --git a/test/form/index.js b/test/form/index.js
@@ -1,16 +1,16 @@
 const assert = require('assert');
 const { existsSync, readFileSync } = require('fs');
-const path = require('path');
-const rollup = require('../../dist/rollup');
+const { basename, resolve } = require('path');
+const { rollup } = require('../../dist/rollup');
 const { normaliseOutput, runTestSuiteWithSamples } = require('../utils.js');
 
 const FORMATS = ['amd', 'cjs', 'system', 'es', 'iife', 'umd'];
 
-runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config) => {
+runTestSuiteWithSamples('form', resolve(__dirname, 'samples'), (dir, config) => {
 	const isSingleFormatTest = existsSync(dir + '/_expected.js');
 	const itOrDescribe = isSingleFormatTest ? it : describe;
 	(config.skip ? itOrDescribe.skip : config.solo ? itOrDescribe.only : itOrDescribe)(
-		path.basename(dir) + ': ' + config.description,
+		basename(dir) + ': ' + config.description,
 		() => {
 			let bundle;
 			const runRollupTest = async (inputFile, bundleFile, defaultFormat) => {
@@ -20,12 +20,10 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
 					process.chdir(dir);
 					bundle =
 						bundle ||
-						(await rollup.rollup({
+						(await rollup({
 							input: dir + '/main.js',
 							onwarn: warning => {
-								if (
-									!(config.expectedWarnings && config.expectedWarnings.indexOf(warning.code) >= 0)
-								) {
+								if (!(config.expectedWarnings && config.expectedWarnings.includes(warning.code))) {
 									warnings.push(warning);
 								}
 							},
diff --git a/test/function/samples/function-directive-not-first/_config.js b/test/function/samples/function-directive-not-first/_config.js
@@ -3,6 +3,6 @@ const assert = require('assert');
 module.exports = {
 	description: 'should delete use asm from function body if it is not the first expression',
 	code(code) {
-		assert.equal(code.indexOf('use asm'), -1);
+		assert.ok(!code.includes('use asm'));
 	}
 };
diff --git a/test/function/samples/member-expression-assignment-in-function/_config.js b/test/function/samples/member-expression-assignment-in-function/_config.js
@@ -3,8 +3,8 @@ const assert = require('assert');
 module.exports = {
 	description: 'detect side effect in member expression assignment when not top level',
 	code(code) {
-		assert.equal(code.indexOf('function set(key, value) { foo[key] = value; }') >= 0, true, code);
-		assert.equal(code.indexOf('set("bar", 2);') >= 0, true, code);
-		assert.equal(code.indexOf('set("qux", 3);') >= 0, true, code);
+		assert.ok(code.includes('function set(key, value) { foo[key] = value; }'), code);
+		assert.ok(code.includes('set("bar", 2);'), code);
+		assert.ok(code.includes('set("qux", 3);'), code);
 	}
 };
diff --git a/test/function/samples/preload-cyclic-module/_config.js b/test/function/samples/preload-cyclic-module/_config.js
@@ -18,7 +18,7 @@ module.exports = {
 					const resolution = await this.resolve(source, importer, { skipSelf: true, ...options });
 					if (resolution && !resolution.external) {
 						const moduleInfo = await this.load(resolution);
-						if (moduleInfo.code.indexOf('/* use proxy */') >= 0) {
+						if (moduleInfo.code.includes('/* use proxy */')) {
 							return `${resolution.id}?proxy`;
 						}
 					}
diff --git a/test/function/samples/relative-external-include-once-two-external/_config.js b/test/function/samples/relative-external-include-once-two-external/_config.js
@@ -1,18 +1,18 @@
 const assert = require('assert');
-const path = require('path');
+const { join } = require('path');
 
 module.exports = {
 	description: 'includes a relative external module only once (two external deps)',
 	options: {
-		external: [path.join(__dirname, './foo.js'), path.join(__dirname, './first/foo.js')]
+		external: [join(__dirname, './foo.js'), join(__dirname, './first/foo.js')]
 	},
 	context: {
 		require(required) {
-			assert(['./foo.js', './first/foo.js'].indexOf(required) !== -1, 'required wrong module');
+			assert.ok(['./foo.js', './first/foo.js'].includes(required), 'required wrong module');
 			return required === './foo.js' ? 'a' : 'b';
 		}
 	},
 	exports(exports) {
-		assert(exports === 'ab' || exports === 'ba', 'two different modules should be required');
+		assert.ok(exports === 'ab' || exports === 'ba', 'two different modules should be required');
 	}
 };
diff --git a/test/function/samples/transparent-dynamic-inlining/_config.js b/test/function/samples/transparent-dynamic-inlining/_config.js
@@ -3,8 +3,8 @@ const assert = require('assert');
 module.exports = {
 	description: 'Dynamic import inlining when resolution id is a module in the bundle',
 	code(code) {
-		assert.equal(code.indexOf('import('), -1);
-		assert.notEqual(code.indexOf('Promise.resolve('), -1);
+		assert.ok(!code.includes('import('));
+		assert.ok(code.includes('Promise.resolve('));
 	},
 	exports(exports) {
 		assert.deepEqual(exports, { y: 42 });
diff --git a/test/misc/iife.js b/test/misc/iife.js
@@ -1,5 +1,5 @@
 const assert = require('assert');
-const rollup = require('../../dist/rollup');
+const { rollup } = require('../../dist/rollup');
 const { loader } = require('../utils.js');
 const { compareError } = require('../utils.js');
 
@@ -17,7 +17,7 @@ function runIifeTest(code, outputOptions) {
 	const bundleName = outputOptions.name.split('.')[0];
 	const globals = { external: 'external', __exports: {} };
 	runTestCode(
-		bundleName && bundleName.indexOf('@') === -1
+		bundleName && !bundleName.includes('@')
 			? `${code}if (typeof ${bundleName} !== 'undefined') __exports.${bundleName} = ${bundleName};`
 			: code,
 		globals
@@ -35,12 +35,11 @@ function getIifeExports(global, outputOptions) {
 }
 
 function getIifeCode(inputCode, outputOptions) {
-	return rollup
-		.rollup({
-			input: 'input',
-			external: ['external'],
-			plugins: [loader({ input: inputCode })]
-		})
+	return rollup({
+		input: 'input',
+		external: ['external'],
+		plugins: [loader({ input: inputCode })]
+	})
 		.then(bundle =>
 			bundle.generate({ format: 'iife', globals: { external: 'external' }, ...outputOptions })
 		)
diff --git a/test/misc/umd.js b/test/misc/umd.js
@@ -1,5 +1,5 @@
 const assert = require('assert');
-const rollup = require('../../dist/rollup');
+const { rollup } = require('../../dist/rollup');
 const { loader } = require('../utils.js');
 
 function runTestCode(code, thisValue, globals) {
@@ -60,7 +60,7 @@ function runAmdTest(code, outputOptions) {
 				}
 			})
 		) || {};
-	return defineArgs[0].indexOf('exports') === -1 ? result : module.exports;
+	return defineArgs[0].includes('exports') ? module.exports : result;
 }
 
 function runIifeTestWithThis(code, outputOptions) {
@@ -119,7 +119,7 @@ function getIifeExports(global, outputOptions) {
 }
 
 async function getUmdCode(inputCode, outputOptions) {
-	const bundle = await rollup.rollup({
+	const bundle = await rollup({
 		input: 'input',
 		external: ['external'],
 		plugins: [loader({ input: inputCode })]
diff --git a/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-external-sourcemaps-combined/_config.js
@@ -3,6 +3,6 @@ const assert = require('assert');
 module.exports = {
 	description: 'removes sourcemap comments',
 	async test(code) {
-		assert.strictEqual(code.indexOf('sourceMappingURL'), -1);
+		assert.ok(!code.includes('sourceMappingURL'));
 	}
 };
diff --git a/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js b/test/sourcemaps/samples/existing-inline-sourcemaps-combined/_config.js
diff --git a/test/utils.js b/test/utils.js

Original file line number	Diff line number	Diff line change
`@@ -788,7 +788,7 @@ export default function addProxyPlugin() {`
`788`	`788`	`if (resolution && !resolution.external) {`
`789`	`789`	`// we pass on the entire resolution information`
`790`	`790`	`const moduleInfo = await this.load(resolution);`
`791`		`- if (moduleInfo.code.indexOf('/* use proxy */') >= 0) {`
	`791`	`+ if (moduleInfo.code.includes('/* use proxy */')) {`
`792`	`792`	return `${resolution.id}?proxy`;
`793`	`793`	`}`
`794`	`794`	`}`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ export default function getExportMode(`
`33`	`33`	`}`
`34`	`34`	`exportMode = 'default';`
`35`	`35`	`} else {`
`36`		`- if (format !== 'es' && format !== 'system' && exportKeys.indexOf('default') !== -1) {`
	`36`	`+ if (format !== 'es' && format !== 'system' && exportKeys.includes('default')) {`
`37`	`37`	`warn(errMixedExport(facadeModuleId, name));`
`38`	`38`	`}`
`39`	`39`	`exportMode = 'named';`
Original file line number	Diff line number	Diff line change
`@@ -163,7 +163,7 @@ export class Task {`
`163`	`163`	`this.invalidated = true;`
`164`	`164`	`if (details.isTransformDependency) {`
`165`	`165`	`for (const module of this.cache.modules) {`
`166`		`- if (module.transformDependencies.indexOf(id) === -1) continue;`
	`166`	`+ if (!module.transformDependencies.includes(id)) continue;`
`167`	`167`	`// effective invalidation`
`168`	`168`	`module.originalCode = null as never;`
`169`	`169`	`}`