feat(replace): prevent accidental replacement within assignment (#798)

* feat(replace): prevent accidental replacement within assignment * test: add test * fix: gate behaviour behind `preventAssignment` option and warn if not configured * fix: typo * docs(replace): update readme with example * refactor(replace): use plugin warning system and update text * feat(replace): only warn if no value is explicitly provided
rollup · Feb 15, 2021 · 6bee598 · 6bee598
1 parent 990ad2f
commit 6bee598
Show file tree

Hide file tree

Showing 11 changed files with 138 additions and 11 deletions.
diff --git a/packages/replace/README.md b/packages/replace/README.md
@@ -65,6 +65,41 @@ Default: `['\b', '\b']`
 
 Specifies the boundaries around which strings will be replaced. By default, delimiters are [word boundaries](https://www.regular-expressions.info/wordboundaries.html). See [Word Boundaries](#word-boundaries) below for more information.
 
+### `preventAssignment`
+
+Type: `Boolean`<br>
+Default: `false`
+
+Prevents replacing strings where they are followed by a single equals sign. For example, where the plugin is called as follows:
+
+```js
+replace({
+  values: {
+    'process.env.DEBUG': 'false',
+  },
+});
+```
+
+Observe the following code:
+
+```js
+// Input
+process.env.DEBUG = false;
+if (process.env.DEBUG == true) {
+  //
+}
+// Without `preventAssignment`
+false = false; // this throws an error because false cannot be assigned to
+if (false == true) {
+  //
+}
+// With `preventAssignment`
+process.env.DEBUG = false;
+if (false == true) {
+  //
+}
+```
+
 ### `exclude`
 
 Type: `String` | `Array[...String]`<br>

diff --git a/packages/replace/src/index.js b/packages/replace/src/index.js
@@ -37,18 +37,29 @@ function mapToFunctions(object) {
 
 export default function replace(options = {}) {
   const filter = createFilter(options.include, options.exclude);
-  const { delimiters } = options;
+  const { delimiters, preventAssignment } = options;
   const functionValues = mapToFunctions(getReplacements(options));
-  const keys = Object.keys(functionValues)
-    .sort(longest)
-    .map(escape);
+  const keys = Object.keys(functionValues).sort(longest).map(escape);
+  const lookahead = preventAssignment ? '(?!\\s*=[^=])' : '';
   const pattern = delimiters
-    ? new RegExp(`${escape(delimiters[0])}(${keys.join('|')})${escape(delimiters[1])}`, 'g')
-    : new RegExp(`\\b(${keys.join('|')})\\b`, 'g');
+    ? new RegExp(
+        `${escape(delimiters[0])}(${keys.join('|')})${escape(delimiters[1])}${lookahead}`,
+        'g'
+      )
+    : new RegExp(`\\b(${keys.join('|')})\\b${lookahead}`, 'g');
 
   return {
     name: 'replace',
 
+    buildStart() {
+      if (![true, false].includes(preventAssignment)) {
+        this.warn({
+          message:
+            "@rollup/plugin-replace: 'preventAssignment' currently defaults to false. It is recommended to set this option to `true`, as the next major version will default this option to `true`."
+        });
+      }
+    },
+
     renderChunk(code, chunk) {
       const id = chunk.fileName;
       if (!keys.length) return null;

diff --git a/packages/replace/test/fixtures/form/assignment/_config.js b/packages/replace/test/fixtures/form/assignment/_config.js
@@ -0,0 +1,7 @@
+module.exports = {
+  description: "doesn't replace lvalue in assignment",
+  options: {
+    'process.env.DEBUG': 'replaced',
+    preventAssignment: true
+  }
+};
diff --git a/packages/replace/test/fixtures/form/assignment/input.js b/packages/replace/test/fixtures/form/assignment/input.js
@@ -0,0 +1 @@
+process.env.DEBUG = 'test';
diff --git a/packages/replace/test/fixtures/form/assignment/output.js b/packages/replace/test/fixtures/form/assignment/output.js
@@ -0,0 +1 @@
+process.env.DEBUG = 'test';
diff --git a/packages/replace/test/misc.js b/packages/replace/test/misc.js
@@ -1,5 +1,7 @@
 /* eslint-disable consistent-return */
 
+const { join } = require('path');
+
 const test = require('ava');
 const { rollup } = require('rollup');
 
@@ -64,3 +66,18 @@ test('can be configured with output plugins', async (t) => {
   t.is(code.trim(), 'log("environment", "production");');
   t.truthy(map);
 });
+
+process.chdir(join(__dirname, 'fixtures', 'form', 'assignment'));
+
+test.serial('no explicit setting of preventAssignment', async (t) => {
+  const possibleValues = [undefined, true, false];
+  for await (const value of possibleValues) {
+    const warnings = [];
+    await rollup({
+      input: 'input.js',
+      onwarn: (warning) => warnings.push(warning),
+      plugins: [replace({ preventAssignment: value })]
+    });
+    t.snapshot(warnings);
+  }
+});
diff --git a/packages/replace/test/snapshots/form.js.md b/packages/replace/test/snapshots/form.js.md
@@ -58,3 +58,9 @@ Generated by [AVA](https://ava.li).
     `const one = 1; // eslint-disable-line␊
     ␊
     console.log(one);`
+
+## assignment: doesn't replace lvalue in assignment
+
+> Snapshot 1
+
+    'process.env.DEBUG = \'test\';'
diff --git a/packages/replace/test/snapshots/form.js.snap b/packages/replace/test/snapshots/form.js.snap
diff --git a/packages/replace/test/snapshots/misc.js.md b/packages/replace/test/snapshots/misc.js.md
@@ -45,3 +45,24 @@ Generated by [AVA](https://ava.li).
       name: null,
       source: 'main.js',
     }
+
+## no explicit setting of preventAssignment
+
+> Snapshot 1
+
+    [
+      {
+        code: 'PLUGIN_WARNING',
+        message: '@rollup/plugin-replace: \'preventAssignment\' currently defaults to false. It is recommended to set this option to `true`, as the next major version will default this option to `true`.',
+        plugin: 'replace',
+        toString: Function {},
+      },
+    ]
+
+> Snapshot 2
+
+    []
+
+> Snapshot 3
+
+    []
diff --git a/packages/replace/test/snapshots/misc.js.snap b/packages/replace/test/snapshots/misc.js.snap
diff --git a/packages/replace/test/sourcemaps.js b/packages/replace/test/sourcemaps.js
@@ -44,7 +44,7 @@ test('generates sourcemaps by default', async (t) => {
     onwarn(warning) {
       throw new Error(warning.message);
     },
-    plugins: [replace({ values: { ANSWER: '42' } }), testPlugin]
+    plugins: [replace({ values: { ANSWER: '42' }, preventAssignment: true }), testPlugin]
   });
 
   const { code, map } = getOutputFromGenerated(
@@ -60,7 +60,14 @@ test('generates sourcemaps if enabled in plugin', async (t) => {
     onwarn(warning) {
       throw new Error(warning.message);
     },
-    plugins: [replace({ values: { ANSWER: '42' }, sourcemap: true }), testPlugin]
+    plugins: [
+      replace({
+        values: { ANSWER: '42' },
+        sourcemap: true,
+        preventAssignment: true
+      }),
+      testPlugin
+    ]
   });
 
   const { code, map } = getOutputFromGenerated(
@@ -76,7 +83,14 @@ test('generates sourcemaps if enabled in plugin (camelcase)', async (t) => {
     onwarn(warning) {
       throw new Error(warning.message);
     },
-    plugins: [replace({ values: { ANSWER: '42' }, sourceMap: true }), testPlugin]
+    plugins: [
+      replace({
+        values: { ANSWER: '42' },
+        sourceMap: true,
+        preventAssignment: true
+      }),
+      testPlugin
+    ]
   });
 
   const { code, map } = getOutputFromGenerated(
@@ -98,7 +112,14 @@ test('does not generate sourcemaps if disabled in plugin', async (t) => {
       );
       warned = true;
     },
-    plugins: [replace({ values: { ANSWER: '42' }, sourcemap: false }), testPlugin]
+    plugins: [
+      replace({
+        values: { ANSWER: '42' },
+        sourcemap: false,
+        preventAssignment: true
+      }),
+      testPlugin
+    ]
   });
 
   t.truthy(!warned);
@@ -118,7 +139,14 @@ test('does not generate sourcemaps if disabled in plugin (camelcase)', async (t)
       );
       warned = true;
     },
-    plugins: [replace({ values: { ANSWER: '42' }, sourceMap: false }), testPlugin]
+    plugins: [
+      replace({
+        values: { ANSWER: '42' },
+        sourceMap: false,
+        preventAssignment: true
+      }),
+      testPlugin
+    ]
   });
 
   t.truthy(!warned);