From 6bee5980155df7b73cfbd9746556517c8d7f0ad7 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 15 Feb 2021 16:32:04 +0000 Subject: [PATCH] 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 --- packages/replace/README.md | 35 ++++++++++++++++ packages/replace/src/index.js | 23 ++++++++--- .../test/fixtures/form/assignment/_config.js | 7 ++++ .../test/fixtures/form/assignment/input.js | 1 + .../test/fixtures/form/assignment/output.js | 1 + packages/replace/test/misc.js | 17 ++++++++ packages/replace/test/snapshots/form.js.md | 6 +++ packages/replace/test/snapshots/form.js.snap | Bin 439 -> 492 bytes packages/replace/test/snapshots/misc.js.md | 21 ++++++++++ packages/replace/test/snapshots/misc.js.snap | Bin 444 -> 716 bytes packages/replace/test/sourcemaps.js | 38 +++++++++++++++--- 11 files changed, 138 insertions(+), 11 deletions(-) create mode 100755 packages/replace/test/fixtures/form/assignment/_config.js create mode 100755 packages/replace/test/fixtures/form/assignment/input.js create mode 100755 packages/replace/test/fixtures/form/assignment/output.js diff --git a/packages/replace/README.md b/packages/replace/README.md index 1e785b210..abb42e5f7 100644 --- 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`
+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]`
diff --git a/packages/replace/src/index.js b/packages/replace/src/index.js index e662a056c..de9262d59 100755 --- 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 new file mode 100755 index 000000000..20a53b81e --- /dev/null +++ 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 new file mode 100755 index 000000000..5fa7f8f80 --- /dev/null +++ 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 new file mode 100755 index 000000000..5fa7f8f80 --- /dev/null +++ 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 index d0a731340..a4aa82aab 100644 --- 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 index 899dcad4a..d35412356 100644 --- 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 index bd26185e9658bbc1e67683ad93dc1a42d42b2817..fdeb4d7d8de0f2ebd74e19e5a944c81335320b11 100644 GIT binary patch literal 492 zcmV8Rql=&{Rp@$JHx*Ld({$yyIF!^ni z6t9+TpvW`@MzH7yAifnT{`Zh!);5MUHcNOM*SIo*MXi7ix%YNWOUlPbXV>c``CMbX zvzZYrIv0pvtxjDpe4uOht)1&G%buC3&IlHL48+U~>|h@-vN8xVDkbOV73b%q>gD98 zYp5rsmLw{ar4|)u=I1FG>KW)6sB2oIsFzBuD9A4=QAkNmODxSPQBcavD=00|%PLkv zRVQ6gl%JehT&$OxSElFU>J;j(V5^{Bl3HA%j;c@x#Q}-Mx|tfsStken!3*8C} ikQF5g`FW|pz%#_(b;kU delta 422 zcmV;X0a^a+1GfWzK~_N^Q*L2!b7*gLAa*he0swkX7^Rb&ZszarFxIpzzs?_v2mk;8 z00003+rz-Xz|L?hQvB~B!>nx#YiySAIIeMJWB>s#klgXCInzI=MI|m|vs7P_w~`So zS_Q=S-mYm$`S|GUdc7o{Ym9d`GlE6$0`bZ@r^GaNO$)exeKe+{_Z7c9X6F9cM{8C{9W&*3B#dx+PCo``^Hzl(;F)1fi7syFP(W#EC z)5M54O;V{91^GoKK-Z@wmgbZwC}rjql$Pja6)T}S%K~J0i9&u}s)DV8Augl2xRBid QRIF(Y0BeihjCKM502RBrp#T5? diff --git a/packages/replace/test/snapshots/misc.js.md b/packages/replace/test/snapshots/misc.js.md index bea3ff428..273bb9cf6 100644 --- 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 index f47b938213a3887c798cb50fb2177207fa0221df..d4325ce9f8ff2aedb31250fba0e7889fe75a8ddb 100644 GIT binary patch literal 716 zcmV;-0yF(VRzVv?Mx29j+ zyj%Tn{Nje2Sl65sV{VGgChp{8-rj@<`XD!g=Ly6<=qW;eLTk1H(7F{01p>PX0D(OK zW38mT!ZSiVLNu_@htPbW=iI^~_blYv*TKO-0{sMB0tJl8?FWZIz6h?TD!lbEaQ^J< z+Z(!=dI&T-pg^{F)5# zHcY)pYrKdRTH#d|32UfTw916xB|2rPZ>Z*;3NX6DE3waTOKlzPDzv_mv1#7wh|iBu zhRJnKC118QV2euAC9X{z)bNF9F>Ie*%Cyy!R^RTj*#&_CfEOre%h6}mjMcuZ5jYU% z7gMSGPDj#<8NliIK)HWRcgLarNl2c=iqEJ>dpuxate5bwa?}$$Tj11}oI-g0=t5*^#jnU=OKV(m%`C?^Oq&X06S>=oEN-zx zPbq0qnR$z`qP!@ccM_2#*0~5f7TrCGXS~6e5!Xm&U*VU1bu!c1<<*&Hp5;2RxUe@n zPEUy_kUMx?;Y;51K}T8nQ9Q>HbBI;M{oj|C{VV;Uea2n*a&Li!~J;QckjJ#)&T$ps7$pV-mcl<f@i}rAU!hkrDSV_!-nvfo~9N&0)ROHO#t&+V@yQ5R1uEYE|%CzFKwmDdR@{~G%d~f&#hUBmGeoJ2V)$2-I$phe6;%;K8{(T&6cG`FwD}{KUi8! m8Vn@NWq#*q)Kyb+_D4sP3LC#RG~;2)wV&@z{yS~D0{{Tv$jLDP diff --git a/packages/replace/test/sourcemaps.js b/packages/replace/test/sourcemaps.js index e9c409c33..b15422388 100644 --- 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);