From 856661debbe1a5fb1a1a716cea32461c89b0c57c Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 8 Feb 2021 22:29:22 +0000 Subject: [PATCH 1/7] feat(replace): prevent accidental replacement within assignment --- packages/replace/src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/replace/src/index.js b/packages/replace/src/index.js index e662a056c..486996372 100755 --- a/packages/replace/src/index.js +++ b/packages/replace/src/index.js @@ -43,8 +43,8 @@ export default function replace(options = {}) { .sort(longest) .map(escape); 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])}(?!\\s*=[^=])`, 'g') + : new RegExp(`\\b(${keys.join('|')})\\b(?!\\s*=[^=])`, 'g'); return { name: 'replace', From 5eede9aabc5fe2cf3f59869f1e6408462ab6698a Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 8 Feb 2021 23:01:49 +0000 Subject: [PATCH 2/7] test: add test --- packages/replace/src/index.js | 5 ++++- .../test/fixtures/form/assignment/_config.js | 6 ++++++ .../test/fixtures/form/assignment/input.js | 1 + .../test/fixtures/form/assignment/output.js | 1 + packages/replace/test/snapshots/form.js.md | 6 ++++++ packages/replace/test/snapshots/form.js.snap | Bin 439 -> 492 bytes 6 files changed, 18 insertions(+), 1 deletion(-) 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/src/index.js b/packages/replace/src/index.js index 486996372..6f28ea4f6 100755 --- a/packages/replace/src/index.js +++ b/packages/replace/src/index.js @@ -43,7 +43,10 @@ export default function replace(options = {}) { .sort(longest) .map(escape); const pattern = delimiters - ? new RegExp(`${escape(delimiters[0])}(${keys.join('|')})${escape(delimiters[1])}(?!\\s*=[^=])`, 'g') + ? new RegExp( + `${escape(delimiters[0])}(${keys.join('|')})${escape(delimiters[1])}(?!\\s*=[^=])`, + 'g' + ) : new RegExp(`\\b(${keys.join('|')})\\b(?!\\s*=[^=])`, 'g'); return { 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..689496e7f --- /dev/null +++ b/packages/replace/test/fixtures/form/assignment/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: "doesn't replace lvalue in assignment", + options: { + 'process.env.DEBUG': 'replaced' + } +}; 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/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? From a24e2797e9a3f9ef728dd0f16138643d7f7ebc92 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 15 Feb 2021 11:45:12 +0000 Subject: [PATCH 3/7] fix: gate behaviour behind `preventAssignment` option and warn if not configured --- packages/replace/README.md | 7 +++++++ packages/replace/src/index.js | 17 +++++++++++------ .../test/fixtures/form/assignment/_config.js | 3 ++- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/replace/README.md b/packages/replace/README.md index 1e785b210..e1b7e4394 100644 --- a/packages/replace/README.md +++ b/packages/replace/README.md @@ -65,6 +65,13 @@ 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 (e.g. `process.env.DEBUG = false'`). + ### `exclude` Type: `String` | `Array[...String]`
diff --git a/packages/replace/src/index.js b/packages/replace/src/index.js index 6f28ea4f6..6f159230b 100755 --- a/packages/replace/src/index.js +++ b/packages/replace/src/index.js @@ -37,17 +37,22 @@ 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); + if (![true, false].includes(preventAssignment)) { + // eslint-disable-next-line no-console + console.warn( + "replace: 'preventAssignment' currently defaults to false. It is recommended to configure this option explicitly, as it may default to true in a future major version." + ); + } + const lookahead = preventAssignment ? '(?!\\s*=[^=])' : ''; const pattern = delimiters ? new RegExp( - `${escape(delimiters[0])}(${keys.join('|')})${escape(delimiters[1])}(?!\\s*=[^=])`, + `${escape(delimiters[0])}(${keys.join('|')})${escape(delimiters[1])}${lookahead}`, 'g' ) - : new RegExp(`\\b(${keys.join('|')})\\b(?!\\s*=[^=])`, 'g'); + : new RegExp(`\\b(${keys.join('|')})\\b${lookahead}`, 'g'); return { name: 'replace', diff --git a/packages/replace/test/fixtures/form/assignment/_config.js b/packages/replace/test/fixtures/form/assignment/_config.js index 689496e7f..20a53b81e 100755 --- a/packages/replace/test/fixtures/form/assignment/_config.js +++ b/packages/replace/test/fixtures/form/assignment/_config.js @@ -1,6 +1,7 @@ module.exports = { description: "doesn't replace lvalue in assignment", options: { - 'process.env.DEBUG': 'replaced' + 'process.env.DEBUG': 'replaced', + preventAssignment: true } }; From ec2ea7991ba89f24f1c243b0bb73355243ed6b8a Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 15 Feb 2021 11:49:53 +0000 Subject: [PATCH 4/7] fix: typo --- packages/replace/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replace/README.md b/packages/replace/README.md index e1b7e4394..062fe964b 100644 --- a/packages/replace/README.md +++ b/packages/replace/README.md @@ -70,7 +70,7 @@ Specifies the boundaries around which strings will be replaced. By default, deli Type: `Boolean`
Default: `false` -Prevents replacing strings where they are followed by a single equals sign (e.g. `process.env.DEBUG = false'`). +Prevents replacing strings where they are followed by a single equals sign (e.g. `process.env.DEBUG = false`). ### `exclude` From 87946ba5cc3d029c8390e293d6df25699a75161a Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 15 Feb 2021 14:59:02 +0000 Subject: [PATCH 5/7] docs(replace): update readme with example --- packages/replace/README.md | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/replace/README.md b/packages/replace/README.md index 062fe964b..abb42e5f7 100644 --- a/packages/replace/README.md +++ b/packages/replace/README.md @@ -70,7 +70,35 @@ Specifies the boundaries around which strings will be replaced. By default, deli Type: `Boolean`
Default: `false` -Prevents replacing strings where they are followed by a single equals sign (e.g. `process.env.DEBUG = 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` From b6f6a5f83aed82571529b9e592f6fd037e82d8d4 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 15 Feb 2021 14:59:43 +0000 Subject: [PATCH 6/7] refactor(replace): use plugin warning system and update text --- packages/replace/src/index.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/replace/src/index.js b/packages/replace/src/index.js index 6f159230b..28aa0e34b 100755 --- a/packages/replace/src/index.js +++ b/packages/replace/src/index.js @@ -40,12 +40,6 @@ export default function replace(options = {}) { const { delimiters, preventAssignment } = options; const functionValues = mapToFunctions(getReplacements(options)); const keys = Object.keys(functionValues).sort(longest).map(escape); - if (![true, false].includes(preventAssignment)) { - // eslint-disable-next-line no-console - console.warn( - "replace: 'preventAssignment' currently defaults to false. It is recommended to configure this option explicitly, as it may default to true in a future major version." - ); - } const lookahead = preventAssignment ? '(?!\\s*=[^=])' : ''; const pattern = delimiters ? new RegExp( @@ -57,6 +51,15 @@ export default function replace(options = {}) { return { name: 'replace', + buildStart() { + if (!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; From 29adf93934aac1401df18399d42ded41c6d5827f Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 15 Feb 2021 15:19:45 +0000 Subject: [PATCH 7/7] feat(replace): only warn if no value is explicitly provided --- packages/replace/src/index.js | 2 +- packages/replace/test/misc.js | 17 +++++++++ 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 ++++++++++++++++--- 5 files changed, 72 insertions(+), 6 deletions(-) diff --git a/packages/replace/src/index.js b/packages/replace/src/index.js index 28aa0e34b..de9262d59 100755 --- a/packages/replace/src/index.js +++ b/packages/replace/src/index.js @@ -52,7 +52,7 @@ export default function replace(options = {}) { name: 'replace', buildStart() { - if (!preventAssignment) { + 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`." 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/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);