diff --git a/packages/cli/tests/business-imports.mjs b/packages/cli/tests/business-imports.mjs index e16494171..6e9ab274b 100644 --- a/packages/cli/tests/business-imports.mjs +++ b/packages/cli/tests/business-imports.mjs @@ -8,11 +8,18 @@ // it cannot read. Comments are blanked before matching, so an import in a comment is not one, and an // import form inside a string or a regular expression is not one either. A dynamic import written // with a space, `import ('./x.mjs')`, counts. These throw: a dynamic import whose argument is not a -// plain string, a file with an unterminated string, comment, template or regular expression, a -// target that is missing or not a regular file, and a target whose real path leaves the repository. +// plain string, a dynamic import inside a template substitution (${...}), a file with an unterminated +// string, comment, template or regular expression, a target that is missing or not a regular file, +// and a target whose real path leaves the repository. A substitution with no dynamic import is +// blanked with the rest of the template, as is `${import.meta.url}`; the text of a template and a +// substitution that holds a template of its own are read to their ends. // The scanner knows line and block comments, quoted strings, template literals, regular expression -// literals (by the usual guess from the previous token) and a hashbang. A template literal with a -// backtick nested inside its ${} is outside that, and so is a regular expression it guesses wrong. +// literals (by the usual guess from the previous token) and a hashbang. The guess is wrong for a +// regular expression after `)` or a keyword, as in `return /x/.test(y)`. After `++` or `--` a slash +// is division when the character before the pair ends an operand (`n++ / 2`) and starts a regular +// expression when it does not (`x = ++/re/.lastIndex`). That misreads one rare form as division: a +// prefix `++` or `--` applied to a regular expression's member right after `)`, `]`, a word or a +// line break, as in `if (c) ++/re/.lastIndex`. import {readdirSync,readFileSync,realpathSync,statSync} from 'node:fs'; import {dirname,join,relative,resolve,sep} from 'node:path'; @@ -20,16 +27,34 @@ const IMPORT=/(?:import|export)\s[^'"`;]*?\bfrom\s*['"](\.{1,2}\/[^'"]+)['"]|\bi const DYNAMIC=/(?~^'; +const inert=(literals,index)=>literals.some(([from,to])=>index>=from&&indexreaddirSync(dir,{withFileTypes:true}).flatMap((e)=>e.isDirectory()?sources(join(dir,e.name)):e.name.endsWith('.mjs')?[join(dir,e.name)]:[]); // The text with every comment blanked and every template literal emptied, and the ranges of the // quoted strings and regular expressions left in it, as [from,to) offsets into the returned code. -// Throws on a string, comment, template or regular expression that never ends. +// Throws on a string, comment, template or regular expression that never ends, and on a dynamic +// import inside a template substitution. function scan(text){ - let code='',i=0,previous=''; - const literals=[]; + let code='',i=0,previous='',before=''; + const literals=[],open=[]; const fail=(what)=>{throw new Error(`unterminated ${what}`);}; - const keep=(end,last)=>{literals.push([code.length,code.length+end-i]);code+=text.slice(i,end);i=end;previous=last;}; + const mark=(token)=>{before=previous;previous=token;}; + const keep=(end,last)=>{literals.push([code.length,code.length+end-i]);code+=text.slice(i,end);i=end;mark(last);}; + // Template text from i to the closing backtick, or to the ${ that opens a substitution, which is then read as code. + const template=()=>{ + for(;i{ + for(const m of code.slice(start).matchAll(DYNAMIC))if(!inert(literals,start+m.index))throw new Error('unsupported import inside a template substitution'); + code=code.slice(0,start); + while(literals.length&&literals[literals.length-1][0]>=start)literals.pop(); + }; if(text.startsWith('#!')){const end=text.indexOf('\n');i=end<0?text.length:end;} while(i=text.length)fail('string'); keep(j+1,c);continue; } - if(c==='`'){ - let j=i+1; - for(;j=text.length)fail('template'); - code+='``';i=j+1;previous='`';continue; - } + if(c==='`'){code+='``';i++;template();continue;} if(c==='/'&&(previous===''||REGEX_AFTER.includes(previous))){ let j=i+1,inClass=false; for(;j=text.length)fail('regular expression'); keep(j+1,'/');continue; } + if(open.length&&(c==='{'||c==='}')){ + const top=open[open.length-1]; + if(c==='{')top.braces++; + else if(top.braces===0){close(open.pop());i++;template();continue;} + else top.braces--; + } code+=c;i++; - if(!/\s/.test(c))previous=c; + if(!/\s/.test(c)){ + const postfix=(c==='+'||c==='-')&&previous===c&&text[i-2]===c&&before!==''&&!REGEX_AFTER.includes(before); + mark(postfix?c+c:c); + } } + if(open.length)fail('template'); return {code,literals}; } @@ -68,7 +98,7 @@ function specifiers(file,text){ let scanned; try{scanned=scan(text);} catch(error){throw new Error(`${file} cannot be read for imports: ${error.message}`);} - const {code,literals}=scanned,live=(m)=>!literals.some(([from,to])=>m.index>=from&&m.index!inert(literals,m.index); for(const m of code.matchAll(DYNAMIC)){ if(live(m)&&!DYNAMIC_LITERAL.test(code.slice(m.index,m.index+512)))throw new Error(`${file} has a dynamic import that is not a plain string literal`); } diff --git a/packages/cli/tests/business-imports.test.mjs b/packages/cli/tests/business-imports.test.mjs index 977cee31a..66f952081 100644 --- a/packages/cli/tests/business-imports.test.mjs +++ b/packages/cli/tests/business-imports.test.mjs @@ -152,3 +152,79 @@ test('#779 a symlink that stays inside the repository, and a repository reached symlinkSync(repo,via); assert.deepEqual(businessImportedFiles(via),['packages/other/src/link.mjs']); }); + +const SUBSTITUTION=/packages\/business\/src\/a\.mjs cannot be read for imports: unsupported import inside a template substitution/; + +test('#779 R2 a dynamic import inside a template substitution throws, whether the target exists, is missing or is outside', ()=>{ + const exists=withBusiness("const v=`${await import('../../other/src/real.mjs')}`;\n",{[`${EXT}real.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(exists),SUBSTITUTION); + const missing=withBusiness("const v=`${await import('../../other/src/real.mjs')}`;\n"); + assert.throws(()=>businessImportedFiles(missing),SUBSTITUTION); + const outside=tree({[`${BIZ}a.mjs`]:"const v=`${await import('../../../../outside.mjs')}`;\n",'outside.mjs':EMPTY}); + assert.throws(()=>businessImportedFiles(outside),SUBSTITUTION); +}); + +test('#779 R2 a dynamic import inside a substitution throws however the substitution is written', ()=>{ + for(const source of [ + "const v=`a ${b} ${await import ('../../other/src/real.mjs')} c`;", + "const v=`${await import(name)}`;", + "const v=`${/* c */ import('../../other/src/real.mjs') /* d */}`;", + "const v=`x ${`y ${await import('../../other/src/real.mjs')}`}`;", + "const v=`${ {a:1}.a + '}' + (await import('../../other/src/real.mjs')) }`;", + "const v=`${\n await import(\n '../../other/src/real.mjs'\n )\n}`;", + ]){ + const repo=withBusiness(`${source}\n`,{[`${EXT}real.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(repo),SUBSTITUTION,source); + } + const external=withBusiness("import '../../other/src/a.mjs';\n",{[`${EXT}a.mjs`]:"const v=`${await import('./b.mjs')}`;\n",[`${EXT}b.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(external),/packages\/other\/src\/a\.mjs cannot be read for imports: unsupported import inside a template substitution/); +}); + +test('#779 R2 a substitution without a dynamic import is allowed, and the import around it is found', ()=>{ + const repo=withBusiness([ + "const a=`plain ${b} text ${c.d({e:1})} end`;", + "const u=`${import.meta.url}/${x.import}/${loader.import('y')}`;", + "const s=`${'}'}${\"import('../../other/src/missing1.mjs')\"}${/import('..\\/x')/.source}`;", + "const t=`text import('../../other/src/missing2.mjs') ${a} import '../../other/src/missing3.mjs'`;", + "const e=`\\${await import('../../other/src/missing4.mjs')}`;", + "const n=`outer ${`inner ${a}`} tail`;", + "import '../../other/src/real.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']); +}); + +test('#779 R2 a substitution or the template text after it that never ends throws', ()=>{ + for(const text of ['const a=`${b','const a=`${b} open','const a=`${`${b}']){ + const repo=withBusiness(text); + assert.throws(()=>businessImportedFiles(repo),/packages\/business\/src\/a\.mjs cannot be read for imports: unterminated template/,text); + } +}); + +test('#779 R2 a slash after a postfix ++ or -- is division, not a regular expression', ()=>{ + for(const op of ['++','--']){ + const repo=withBusiness([ + 'let n=2;', + `const half=n${op} / 2;`, + `const spaced=n ${op}/2;`, + `const member=arr[0]${op} / 2;`, + `const grouped=(n)${op} / 2 / 3;`, + "import '../../other/src/real.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs'],op); + } +}); + +test('#779 R2 a regular expression after an operator, a bracket or a prefix ++ or -- is still a regular expression', ()=>{ + // Each body holds a quote, which would start an unterminated string if the slash were read as division. + const repo=withBusiness([ + "const a = /it's/;", + "if (/it's/.test(s)) {}", + "const b = f(1, /it's \\/ x/);", + "const c = a + /it's/.source;", + "const d = a++ + /it's/.source;", + "const e = ++/it's/.lastIndex;", + "const f = 1 + --/it's/.lastIndex;", + "import '../../other/src/real.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']); +});