Compiling twice in ADVANCED needed for a registry?

[co...@colinalworth.com]

(Contents of this, with a diagram for readability, syntax highlighted sourcecode, and source that can be edited or compiled directly can be found at https://github.com/Vertispan/closure-compiler-registry-dead-code.)

This project is a demonstration of an idea that closure-compiler doesn't seem to be able to follow all the time. A simple interface is declared, with a registry to declare and look up instances. Libraries can depend on the interface and registry itself, but usually won't specify implementations. In turn, an application will specify the library, and any implementations of the underlying interface.

Each different application might depend on different implementations, and use runtime logic, or defines to pick a specific implementation at build time.

The part that doesn't work consistently is using defines for this. For example, suppose that this is our registry:

var map = {};
function register(key, creator) {
    map[key] = creator;
}
function lookup(key) {
    return map[key]();
}


// Specify a default, and allow it to be overridden at build time
/** @define {string} */
const exampleDefault = goog.define('exampleDefault', 'python');
function lookupDefault() {
    return lookup(exampleDefault);
}

and a simple interface to implement

/**
 *
 * @interface
 */
function ProgrammingLanguage() {
   
}
/**
 * @return {string}
 */
ProgrammingLanguage.prototype.getName = function() {};

Then, we have two implementations, which register themselves (rather than require that our application write code for each implementation):

/**
 *
 * @constructor
 * @implements ProgrammingLanguage
 */
function PythonLanguage() {
   
}
PythonLanguage.prototype.getName = function() {
    return "Python";
};
register("python", () => new PythonLanguage());

/**
 *
 * @constructor
 * @implements ProgrammingLanguage
 */
function JavaLanguage() {
   
}
JavaLanguage.prototype.getName = function() {
    return "Java";
};
register("java", () => new JavaLanguage());

Now our library can use this any way it wants, and expect to have an instance provided.

function sayHello() {
    var defaultLanguage = lookupDefault();
    return "Hello, " + defaultLanguage.getName() + "!";
}

Our application need not use the original factory or implementations, as long as it correctly includes their sources as inputs to closure-compiler, but can just call on the library:

console.log(sayHello());

Invoking this in closure-compiler from the command line looks like this:

export CLOSURE_LIBRARY=~/workspace/closure-library/
java -jar closure-compiler-shaded.jar --compilation_level ADVANCED \
  --js src/languages/registry.js \
  --js src/languages/language.js \
  --js src/java/java.js \
  --js src/python/python.js \
  --js src/library/library.js \
  --js src/app.js \
  --js $CLOSURE_LIBRARY/closure/goog/base.js

Our naive expectation is that such an application should be optimized out to practically nothing:

console.log("Hello, Python!");

Instead, we get this (here using the web service, so that we can easily pretty print the output, and make it reproducible on other computers):

var a = {};
function b() {
}
b.prototype.g = function() {
  return "Python";
};
a.python = function() {
  return new b();
};
function c() {
}
c.prototype.g = function() {
  return "Java";
};
a.java = function() {
  return new c();
};
console.log("Hello, " + a.python().g() + "!");

But, if we take that closure output, and run it again, we do get the correct result, albeit with two warnings for "omitting" our required @constructor jsdocs:

console.log("Hello, Python!");

It seems as though the compiler can tell that an string key could be replaced by a property, but then once it has done so, it cannot remove unused properties. However, if the input already has that property with no string key, it is safe to remove the unreachable code. We still want to key this off of --define, so this might limit our options somewhat.

Removing the @define, and just using a constant string has no effect - this is probably what the compiler is doing already, so this makes sense. We then tried to go a step further and replace our string literals with either properties, or use goog.reflect.objectProperty(...) to indicate that the given key would correspond to a known property on that object:

goog.require('goog.reflect');
//...

// Specify a default, and allow it to be overridden at build time
function lookupDefault() {
    return map[goog.reflect.objectProperty('python', map)]();
    //return lookup('python')
}

//...

map.python = () => new PythonLanguage();
//register("python", () => new PythonLanguage());

//...

map.java = () => new JavaLanguage();
//register("java", () => new JavaLanguage());

This gets us slightly closer, but only in that the properties python() and java() are rewritten right away, so a subsequent recompile should still solve this, but still can't seem to be done within a single compile:

var a = {};
function b() {
}
b.prototype.g = function() {
    return "Python";
};
a.h = function() {
    return new b();
};
function c() {
}
c.prototype.g = function() {
    return "Java";
};
a.i = function() {
    return new c();
};
console.log("Hello, " + a.h().g() + "!");

Even if this did work, we would need the extra step of passing a string reference (from define, or from another method) to objectProperty() - from the documentation it is not clear that we could do that.

Is there a better way to achieve this idea?

[rish...@google.com]

Thanks for the detailed reproduction. I was able to reproduce this with the internal compiler as well where the compiler fails to optimize all the way in one step. 

[rish...@google.com]

Pasting below the output after each pass that I got:

// parseInputs yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var map = {};
function register(key, creator) {
  map[key] = creator;
}
function lookup(key) {
  return map[key]();
}

const exampleDefault = goog.define("exampleDefault", "python");
function lookupDefault() {
  return lookup(exampleDefault);
}

function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};
register("python", () => new PythonLanguage());

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};
register("java", () => new JavaLanguage());

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************
INFO: From Doing JavaScript library-level checking of //experimental/users/rishipal/javascript/repro-registry-dead-code:lib:

// parseInputs yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var map = {};
function register(key, creator) {
  map[key] = creator;
}
function lookup(key) {
  return map[key]();
}

/** @define {string} */
const exampleDefault = goog.define("exampleDefault", "python");
function lookupDefault() {
  return lookup(exampleDefault);
}

/** @interface */
function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};
/** @constructor */

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};
register("python", () => new PythonLanguage());

/** @constructor */

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};
register("java", () => new JavaLanguage());

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************
INFO: From Compiling JavaScript for optimizations //experimental/users/rishipal/javascript/repro-registry-dead-code:bin:

// removeCastNodes yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var map = {};
function register(key, creator) {
  map[key] = creator;
}
function lookup(key) {
  return map[key]();
}

const exampleDefault = goog.define("exampleDefault", "python");
function lookupDefault() {
  return lookup(exampleDefault);
}

function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};
register("python", () => new PythonLanguage());

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};
register("java", () => new JavaLanguage());

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************

// processDefines_OPTIMIZE yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var map = {};
function register(key, creator) {
  map[key] = creator;
}
function lookup(key) {
  return map[key]();
}

const exampleDefault = "python";
function lookupDefault() {
  return lookup(exampleDefault);
}
function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};
register("python", () => new PythonLanguage());

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};
register("java", () => new JavaLanguage());

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************

// Es6RewriteArrowFunction yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var map = {};
function register(key, creator) {
  map[key] = creator;
}
function lookup(key) {
  return map[key]();
}

const exampleDefault = "python";
function lookupDefault() {
  return lookup(exampleDefault);
}
function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************

// Es6RewriteBlockScopedDeclaration yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var map = {};
function register(key, creator) {
  map[key] = creator;
}
function lookup(key) {
  return map[key]();
}

var exampleDefault = "python";
function lookupDefault() {
  return lookup(exampleDefault);
}
function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************

// normalize yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function register(key$jscomp$45, creator) {
  map[key$jscomp$45] = creator;
}
function lookup(key$jscomp$46) {
  return map[key$jscomp$46]();
}
var exampleDefault = "python";
function lookupDefault() {
  return lookup(exampleDefault);
}
function ProgrammingLanguage() {
}
ProgrammingLanguage.prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});

function sayHello() {
  var defaultLanguage = lookupDefault();
  return "Hello, " + defaultLanguage.getName() + "!";
}

console.log(sayHello());

// ************************************

// earlyInlineVariables yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function register(key$jscomp$45, creator) {
  map[key$jscomp$45] = creator;
}
function lookup(key$jscomp$46) {
  return map[key$jscomp$46]();
}
function lookupDefault() {
  return lookup("python");
}
(function ProgrammingLanguage() {
}).prototype.getName = function() {
};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});
console.log(function sayHello() {
  return "Hello, " + lookupDefault().getName() + "!";
}());

// ************************************

// earlyPeepholeOptimizations yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function register(key$jscomp$45, creator) {
  map[key$jscomp$45] = creator;
}
function lookup(key$jscomp$46) {
  return map[key$jscomp$46]();
}
function lookupDefault() {
  return lookup("python");

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});
console.log(function sayHello() {
  return "Hello, " + lookupDefault().getName() + "!";
}());

// ************************************

// removeUnusedCode yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function register(key$jscomp$45, creator) {
  map[key$jscomp$45] = creator;
}
function lookup(key$jscomp$46) {
  return map[key$jscomp$46]();
}
function lookupDefault() {
  return lookup("python");

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});
console.log(function() {
  return "Hello, " + lookupDefault().getName() + "!";
}());

// ************************************

// optimizeCalls yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function register(key$jscomp$45, creator) {
  map[key$jscomp$45] = creator;
}
function lookup() {
  var key$jscomp$46 = "python";
  return map[key$jscomp$46]();
}
function lookupDefault() {
  return lookup();

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

register("python", function() {
  return new PythonLanguage();
});

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

register("java", function() {
  return new JavaLanguage();
});
console.log(function() {
  return "Hello, " + lookupDefault().getName() + "!";
}());

// ************************************

// inlineFunctions yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function lookup() {
  var key$jscomp$46 = "python";
  return map[key$jscomp$46]();
}
function lookupDefault() {
  return lookup();

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

{
  var creator$jscomp$inline_1 = function() {
    return new PythonLanguage();
  };
  map["python"] = creator$jscomp$inline_1;

}
function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

{
  var creator$jscomp$inline_4 = function() {
    return new JavaLanguage();
  };
  map["java"] = creator$jscomp$inline_4;
}
console.log("Hello, " + lookupDefault().getName() + "!");

// ************************************

// inlineVariables yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function lookup() {
  return map["python"]();

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

{
  map["python"] = function() {
    return new PythonLanguage();
  };

}
function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

{
  map["java"] = function() {
    return new JavaLanguage();
  };
}
console.log("Hello, " + function lookupDefault() {
  return lookup();
}().getName() + "!");

// ************************************

// removeUnusedCode yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function lookup() {
  return map["python"]();

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

{
  map["python"] = function() {
    return new PythonLanguage();
  };

}
function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

{
  map["java"] = function() {
    return new JavaLanguage();
  };
}
console.log("Hello, " + function() {
  return lookup();
}().getName() + "!");

// ************************************

// peepholeOptimizations yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function lookup() {
  return map["python"]();

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

map["python"] = function() {
  return new PythonLanguage();
};

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + function() {
  return lookup();
}().getName() + "!");

// ************************************

// inlineFunctions yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function lookup() {
  return map["python"]();

}
function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

map["python"] = function() {
  return new PythonLanguage();
};

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + lookup().getName() + "!");

// ************************************

// inlineVariables yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

map["python"] = function() {
  return new PythonLanguage();
};

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + function lookup() {
  return map["python"]();
}().getName() + "!");

// ************************************

// removeUnusedCode yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

map["python"] = function() {
  return new PythonLanguage();
};

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + function() {
  return map["python"]();
}().getName() + "!");

// ************************************

// inlineFunctions yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

map["python"] = function() {
  return new PythonLanguage();
};

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + map["python"]().getName() + "!");

// ************************************
INFO: From Compiling JavaScript for finish default //experimental/users/rishipal/javascript/repro-registry-dead-code:bin:

// replaceProtectedMessages yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};

function PythonLanguage() {
}
PythonLanguage.prototype.getName = function() {
  return "Python";
};

map["python"] = function() {
  return new PythonLanguage();
};

function JavaLanguage() {
}
JavaLanguage.prototype.getName = function() {
  return "Java";
};

map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + map["python"]().getName() + "!");

// ************************************

// renameProperties yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function PythonLanguage() {
}
PythonLanguage.prototype.g = function() {
  return "Python";
};
map["python"] = function() {
  return new PythonLanguage();
};
function JavaLanguage() {
}
JavaLanguage.prototype.g = function() {
  return "Java";
};
map["java"] = function() {
  return new JavaLanguage();
};
console.log("Hello, " + map["python"]().g() + "!");

// ************************************

// convertToDottedProperties yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js
var map = {};
function PythonLanguage() {
}
PythonLanguage.prototype.g = function() {
  return "Python";
};
map.python = function() {
  return new PythonLanguage();
};
function JavaLanguage() {
}
JavaLanguage.prototype.g = function() {
  return "Java";
};
map.java = function() {
  return new JavaLanguage();
};
console.log("Hello, " + map.python().g() + "!");

// ************************************

// renameVars yields:
// ************************************
// experimental/users/rishipal/javascript/repro-registry-dead-code/some.js

var a = {};
function b() {
}
b.prototype.g = function() {
  return "Python";
};
a.python = function() {
  return new b();
};
function c() {
}
c.prototype.g = function() {
  return "Java";
};
a.java = function() {
  return new c();
};
console.log("Hello, " + a.python().g() + "!");

[co...@colinalworth.com]

Thanks for investigating - would it be appropriate for me to file a bug, or is this expected/reasonable?

If this is expected, are there steps I can take to get to the expected result here? It looks as though something along the lines of convertToDottedProperties would need to be run earlier (or at least have another general pass at optimizing out unused properties). It appears that convertToDottedProperties is considered to be a "finalization" pass, and a fairly late one at that. There appears to be another case similar to this, where localization is applied early in finalization, then another optimization loop takes place.

This seems to suggest to me that I'm either going to need to implement a custom PassConfig, or find a different way to achieve this feature that I'm after?

P.S. Thanks for the hint about using --print_source_after_each_pass, that looks handy. I'll try it when I get the chance with the objectProperty variant of the test, and see if there are any clear paths to getting an optimization loop after that renaming takes place.

[lha...@google.com]

Unfortunately this object registry pattern is just not going to optimize well with Closure Compiler.

The compiler was intentionally designed such that properties using dotted syntax are more optimizable than string literal/computed properties (https://developers.google.com/closure/compiler/docs/api-tutorial3#propnames). I don't see a way to support this pattern in a general way without potentially breaking existing code that depends on that lack of optimization.

If you are interested in writing your own PassConfig - maybe an early convertToDottedProperties pass could work if you specifically target only properties this registry object. In general we can't run ConvertToDottedProperties early, or we'd risk accidentally renaming/otherwise misoptimizing computed properties that people expect Closure Compiler not to touch.

 

[co...@colinalworth.com]

(Yikes, can't believe it was May that I started this...) I finally found time to pursue this, and implemented a simple, specific compiler pass that rewrites certain keys to properties, by inlining the relevant parts of ConvertToDottedProperties, but only if the key has a specific suffix. Roughly:

    @Override
    public void visit(NodeTraversal t, Node n, Node parent) {
        if (Objects.requireNonNull(n.getToken()) == Token.OPTCHAIN_GETELEM || n.getToken() == Token.GETELEM) {
            Node left = n.getFirstChild();
            Node right = left.getNext();
            System.err.println(right);
            if (right.isStringLit() && right.getString().endsWith("$service$loader$key") && isValidPropertyName(FeatureSet.ES3, right.getString())) {
                left.detach();
                right.detach();

                Node newGetProp =
                        n.isGetElem()
                                ? IR.getprop(left, right.getString())
                                : (n.isOptionalChainStart()
                                ? IR.startOptChainGetprop(left, right.getString())
                                : IR.continueOptChainGetprop(left, right.getString()));
                n.replaceWith(newGetProp);
                compiler.reportChangeToEnclosingScope(newGetProp);
            }
        }
    }

Given the JS samples above (with minor changes to the registry.js to add the string constant to the keys), the earliest this can run is after the register() and lookup() functions are inlined and their string constants concatenated - and indeed, this pass then does run and rewrite those keys. Unfortunately, the used function still isn't inlined, and the unused one still isn't removed. I strongly suspect that this is a question of the type of the "map" node, that it is still functionally a "@dict" rather than a struct/record, so the compiler isn't sure that it is safe to rewrite in this way. Does that seem correct, that various references to the "map" Node at least need to have a proper type/color set on them to further enable this? Is there another way to ask the compiler to re-examine nodes and extract the apparent type of an object?

I tried using the ReferenceCollector to try creating a RecordType for the "map" Node's JSType, but while RecordCollector is public, a required parameter to its public constructor is not, the ScopeCollector interface. Additionally, NodeUtil is used above for isValidPropertyName, and while NodeUtil is public, many of its members (including isValidPropertyName) are package-private. Is it expected that those are not accessible, or should I expect that my own code should need to be written in the "com.google.javascript.jscomp" package? I do note that the https://github.com/bazelbuild/rules_closure/ repository has many of its customizations in that package rather than "where they belong" in "io.bazel.rules.closure.*". Would it be helpful to open up access rules for some/all of these in a PR?

At this point, I'm leaning towards giving up on the specialized pass to rewrite foo['bar'] to f.bar, and instead inline assignments to their invocations, both to simplify the process of writing the pass and avoid future sensitivity to when passes must run. This does necessarily raise the bar for any code using this convention, to ensure they don't accidentally close over something they shouldn't, etc.