endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

bundle transformation fails to properly handle trailing comments #1926

Open gibson042 opened 10 months ago

gibson042 commented 10 months ago

Describe the bug

When a statement must be inserted after another statement (as in e.g. export const varName = …;const varName = …;$h‍_once.varName(varName);), the inserted statement follows any trailing comment, even if the comment is logically associated with a later node.

Example

https://github.com/Agoric/dapp-offer-up/blob/774d1826eb54487e3401fe8f07db68b89f790820/contract/src/start-game1-proposal.js#L102

export const startGameContract = async permittedPowers => {
  …
};

/** @type { import("@agoric/vats/src/core/lib-boot").BootstrapManifest } */
const gameManifest = {
  …
};
harden(gameManifest);

export const getManifestForGame1 = ({ restoreRef }, { game1Ref }) => {
  …
};

becomes

const        startGameContract=async(permittedPowers)=>{
…
 };

/** @type { Ximport("@agoric/vats/src/core/lib-boot").BootstrapManifest } */$h‍_once.startGameContract(startGameContract);
const gameManifest={ … };

harden(gameManifest);

const        getManifestForGame1=({restoreRef},{game1Ref})=>{
…
 };$h‍_once.getManifestForGame1(getManifestForGame1);

(with the $h‍_once.startGameContract(startGameContract) placed after the doc comment for the next statement)

Expected behavior

The new statement should be inserted before any trailing comment.

const        startGameContract=async(permittedPowers)=>{
…
 };$h‍_once.startGameContract(startGameContract);

/** @type { Ximport("@agoric/vats/src/core/lib-boot").BootstrapManifest } */
const gameManifest={ … };

harden(gameManifest);

const        getManifestForGame1=({restoreRef},{game1Ref})=>{
…
 };$h‍_once.getManifestForGame1(getManifestForGame1);

Additional context

This is a problem of the sort for which concrete syntax representation such as that of https://github.com/estree/estree/pull/107 would be desirable, but we don't necessary need to go that far here.