Taking Full Advantage of NoSideEffects, Or What's It Like To Be An Obsessional Researcher

private boolean checkForStateChangeHelper(Node n, boolean checkForNewObjects) {
 Node parent = n.getParent();
 // Rather than id which ops may have side effects, id the ones
 // that we know to be safe
 switch (n.getToken()) {

  case EXPORT:
  // Variable declarations are side-effects.
  return true;

  // import() expressions have side effects
  case DYNAMIC_IMPORT:
  return true;
  ...
 }
}

The Use Case

I have been working with Closure Compiler to meet my JavaScript needs for almost 5 years now, and have developed a system where abstract classes are generated from an IDL. The abstract classes are then extended by source code with any number of implementations supplied to the Abstract.__extend method in form of prototypes or other classes. This is known as mixins or traits and vastly improves modularity of the source code. The strategy dates back as far as 1979 (and possibly even earlier): the Flavors paper by Howard I. Cannon from MIT lab gives an example of a problem solved with mixins: he imagines 3 classes to model a GUI -- a Window, a Window with Borders, and a Window with Labels. He then outlines the immediate problem with all hierarchical OOP languages such as Java that only support strict vertical inheritance:

Some windows want to have both a border and a label. Given the paradigm currently outlined, there are two ways of defining such a window: define the class WINDOW-WITH-BORDERAND-LABEL, which has WINDOW-WITH-BORDER as its immediate superclass, and includes methods for labels; define WINDOW-WITH-LABEL-AND-BORDER, which has WINDOW-WITH-LABEL as its immediate superclass, and includes methods for borders. Both of these ways involve copying of code. In the former case, the methods for labels must be copied. In the latter case, the methods for borders must be copied. This copying could be extensive. Since copying in general delocalizes information, this copying works against the goal of modularity.

I think everybody who codes in Java came across this problem and not once. Default implementations in interfaces do not help as they don't allow to copy methods but only static methods (i.e., one can't access the this keyword inside methods implemented in an interface). There are so many examples where a class can be composed of orthogonal features that are impossible to express with inheritance. With mixins, on the other hand, it becomes very easy to combine behaviour.

The recent project I've been working on is a library to perform communication between a back-end and front-end via remote method invocations. The system is modeled in a UART-fashion (universal asynchronous receiver / transmitter) with an interface for a transmitter and a receiver, both of which have access to a uart object responsible for sending/accepting data as part of the IUarter interface. They also have an ILogger interface installed on them. In any way, from an XML definition like that:

<types namespace="com.webcircuits">
 <IAR module="browser">
   <implements init>IUarter</implements>
   <implements init>ILogger</implements>

   <method name="r_inv">
     <arc name="data">
       <string name="mid">
         The method ID.
       </string>
       <prop name="args" type="!Array<?>">
         The arguments to pass to the method, if any.
       </prop>
       The data about the method to invoke and its arguments.
     </arc>
     Receives an invocation command.
   </method>

   The asynchronous receiver.
 </IAR>
</types>

An interface and constructor types is generated:

/**
 * @interface
 */
com.webcircuits.IAR = function() {}
/**
 * @param {!com.webcircuits.IAR.r_inv.Data} data
 */
com.webcircuits.IAR.prototype.r_inv = function(data) {}
/**
 * @constructor
 * @implements {com.webcircuits.IAR}
 */
com.webcircuits.AR=function() {}

And so is an abstract class definition with an __extend methods:

/**
 * @constructor
 * @extends {com.webcircuits.AR}
 */
$com.webcircuits.AbstractAR=function() {}

/**
 * @param {...((!com.webcircuits.IAR|typeof com.webcircuits.AR)|(!com.webcircuits.IUarter|typeof com.webcircuits.Uarter)|(!com.webcircuits.ILogger|typeof com.webcircuits.Logger))} Implementations
 * @return {typeof com.webcircuits.AR}
 */
com.webcircuits.AbstractAR.__extend=function(...Implementations) {}

The abstract class is instantiated in the source code:

import { iu, initAbstract } from '@type.engineering/type-engineer'

/**
 * The asynchronous receiver.
 * @type {typeof com.webcircuits.AbstractAR} ‎
 */
const AbstractAR=initAbstract(
  'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 })

export default AbstractAR

The Type Engineer is a library that facilitates composition of mixins: the initAbstract will produce an abstract class with .__extend method that can be called in source code. Some additional arguments include the name of the interface (IAR), the getter-caster (asIAR) for casting via a field rather than comment-annotation (see code below how asIAR is used), and the definitions of private fields to create (e.g., methodsMap) for which symbols will be maintained internally.

import Uarter from '../Uarter'
import Logger from '../Logger'
import AbstractAR from './gen/AbstractAR'

/** @extends {com.webcircuits.AR} */
export default class AR extends AbstractAR.__extend(
 Uarter,
 Logger,
 AbstractAR.prototype=/** @type {!com.webcircuits.AR} */ ({
  r_inv({mid,args}){
   const{asIAR:{methodsMap:methodsMap}}=this
   const method=methodsMap.get(mid)
   if(!method) {
    console.warn('Received an "inv" command but method %s not found',mid)
    return
   }
   const{[method]:_method}=this

   _method.apply(this,args)
  },
 }),
) {}

The problem is that the overall package exports both AR and AT, so that if was required to import only a receiver or only a transmitter from another library, the calls to initAbstract and .__extend will be considered as potentially state-changing and thus side-effectful, and the compiler would not eliminate the irrelevant branch, making the additional code of the receiver appear in builds that only wanted to transmit data.

// source code entry exports all classes
import AR from '../../../src/class/AR'
export {AR}

import AT from '../../../src/class/AT'
export {AT}

// but target lib imports only one:
import { AR } from '@webcircuits.com/uart'

This is why I decided to add nosideeffects to those functions and see if that will help.

The Painstaking Process

Ideally, just by adding the @nosideeffects tag in the Type Engineer library above the initAbstract definition, as well as the generated abstract classes definitions, I could achieve the ability to import the required classes without pulling the whole library in due to global calls to initAbstract and .__extend. However, things turned out to be much harder than I anticipated, but I got there in the end, so I just wanted to share how I made the compiler do what I wanted to.

Firstly, after the @nosideeffects addition, things didn't work: by importing only a single class (AT), the other (AR) was pulled in as well. I had an idea that it could be because of creating a local variable which might trigger the side-effects detector, so I exported the result of initAbstract directly:

// const AbstractAR=initAbstract(...)
// export default AbstractAR
// ^^ before

export default initAbstract(
  'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 })

This resulted in the compiler crashing:

Exit code 254
java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

null
  Node(FUNCTION ): externs.zip//es3.js:416:31
Object.prototype.constructor = function() {};
  Parent(ASSIGN): externs.zip//es3.js:416:0
Object.prototype.constructor = function() {};

        at com.google.javascript.jscomp.Compiler.throwInternalError(Compiler.java:3243)
        ...
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.NullPointerException
        at
        com.google.javascript.jscomp.TypeCheck.checkConstructor(TypeCheck.java:2558)

OK let's give it some annotations:

/**
 * The asynchronous receiver.
 * @type {typeof com.webcircuits.AbstractAR}
 */
export default initAbstract(
  'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 })

I've included the @type JSDoc comment above the default export to help compiler understand what's going on and hopefully prevent crashing.

package/lib/src/class/AR/gen/AbstractAR.js:23:15: WARNING - [JSC_MISPLACED_ANNOTATION] Misplaced type annotation. Type annotations are not allowed here. Are you missing parentheses?
  23| export default initAbstract(
                     ^^^^^^^^^^^^^
  24|   'IAR', null, {
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
  28|   methodsMap:{_methodsMap:'_methodsMap'},
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  29|  })
      ^^^

The result is a warning about misplaced annotation, i.e., a @type tag cannot be used above export default. Fair enough, let's try to cast instead:

/**
 * The asynchronous receiver.
 */
export default /** @type {typeof com.webcircuits.AbstractAR} */ (initAbstract(
  'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 }))

Crash 💥 again, cast node should have JSDoc says the compiler. Cannot cast during imports it seems.

Exit code 254
java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

CAST node should always have JSDocInfo
        at com.google.javascript.jscomp.Compiler.throwInternalError(Compiler.java:3243)
        at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.NullPointerException: CAST node should always have JSDocInfo
        at com.google.javascript.jscomp.NodeTraversal.pushScope(NodeTraversal.java:1156)
        at com.google.javascript.jscomp.NodeTraversal.traverseWithScope(NodeTraversal.java:588)
        ... 13 more

Let's try with a class:

/**
 * The asynchronous receiver.
 * @extends {com.webcircuits.AbstractAR}
 */
export default class extends initAbstract(
 'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 }) {}

OK that worked fine and the irrelevant code was eliminated. However, this lead to another problem: the initAbstract is designed to return a function whose prototype can be assigned to: `AbstractAR.prototype=/** @type {!com.webcircuits.AR} */ ({})` so that the methods and accessors inside the object literal in the RHS of the .prototype assignment have this keyword set to the correct type. This is not done by casting which only transforms the type into the cast-ed one at the point of expression, what we want is something different, i.e., to notify the compiler of the type of object literal inside the object literal itself. Now the compiler has a tag which is suitable for this scenario:

@lends {objectName}: Indicates that the keys of an object literal should be treated as properties of some other object. This annotation should only appear on object literals.

goog.object.extend(
    Button.prototype,
    /** @lends {Button.prototype} */ ({
      isButton: function() { return true; }
    }));

It is meant to serve precisely this purpose, that is to extend prototypes with mixins, as in the Google's example. Unfortunately, @lends does not lend this a correct type as it ought to, so that a large portion of the code will not be type-checked. This can be validated by a test function which accepts a string as an argument, and passing some field from an instance to it, to see if the compiler will type check:

/**
 * @param {string} s
 */
function test(s) {}

/** @extends {com.webcircuits.AR} */
export default class extends AbstractAR.__extend(
 Uarter,
 Logger,
 /✱✱ @lends {com.webcircuits.AR.prototype} ✱/ ({
  r_inv({mid,args}){
   const{asIAR:{methodsMap:methodsMap}}=this
   test({methodsMap:methodsMap})
   const method=methodsMap.get(mid)
   if(!method) {
    console.warn('Received an "inv" command but method %s not found',mid)
    return
   }
   const{[method]:_method}=this

   _method.apply(this,args)
  },
 }),
) {}

From developer experience point of view, VSCode does not understand the @lends tag so that it's impossible to get autocompletions inside the methods, so it's not a suitable fit anyway. But regarding type checking, the result is a warning:

package/lib/src/class/AR/AR.js:16:8: WARNING - [JSC_TYPE_MISMATCH] actual parameter 1 of test$$module$package$lib$src$class$AR$AR does not match formal parameter
found   : {methodsMap: ?}
required: string
  16|    test({methodsMap:methodsMap})
              ^^^^^^^^^^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 89.3% typed

Which means that the fields are not type-checked. Hence the ONLY way to enrich this inside methods of a mixin object-literal, is via a .prototype= assignment. However, because the abstract class is now changed to an actual class rather than function, as it was before, it's not possible to assign to its prototype as classes' prototypes are not writable, only functions' ones are:

/Volumes/Lab/webcircuits/uart/package/lib/src/class/AR/AR.js:13
 AbstractAR.prototype=({
                     ^

TypeError: Cannot assign to read only property 'prototype' of function 'class T extends initAbstract(
 'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:...<omitted>...{}'
at Object.<anonymous>
(/Volumes/Lab/webcircuits/uart/package/lib/src/class/AR/AR.js:13:22)

This is unfortunate, as the previous version of code structure looked very neat, with an abstract class being extended with implementations in form of object literals that are being assigned to its prototype. Instead, I now would have to create a separate dummy function in source:

/** @constructor @extends {com.webcircuits.AR} */
function _AbstractAR() {}

/** @extends {com.webcircuits.AR} */
export default class AR extends AbstractAR.__extend(
 _AbstractAR.prototype=/** @type {!com.webcircuits.AR} */({
  r_inv({mid,args}){
  },
 }),
) {}

This does not look nice because of boilerplate which means I want AbstractAR to be a function, so let's change the initAbstract method to accept a function which it will modify, rather than return a function.

initAbstract(
 AbstractAR, 'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 })

/**
 * The asynchronous receiver.
 * @constructor @extends {com.webcircuits.AbstractAR}
 */
export default function AbstractAR() {}

This didn't result in code elimination, as initAbstract is still considered to have side effects because it's called in code (despite nosideeffects tag). One crazy idea is to use a class that extends the result of initAbstract, but not export it for anything:

class T extends initAbstract(
 AbstractAR, 'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 }) {}

/**
 * The asynchronous receiver.
 * @constructor @extends {com.webcircuits.AbstractAR}
 */
export default function AbstractAR() {}

Well, that did eliminate the unwanted code. The problem is that now when I import the correct class, i.e., IAR, the call to initAbstract is not made at all. It is kind of expected because I claimed that initAbstract will not have side effects, but I find it interesting though that whereas initAbstract on its own was still considered to be side-effectful before (despite being declared as @nosideeffects), once the class extends it, it becomes pure.

Let's move on to the final solution. Because we want to export the result of initAbstract directly, and because we also want a function whose prototype can be used for assignment to enable this lending, let's just create a static property on the class in form of a function, for dummy prototype assignments:

/**
 * The asynchronous receiver.
 * @extends {com.webcircuits.AbstractAR}
 */
export default class AbstractAR extends initAbstract(
 _AbstractAR, 'IAR', null, {
  asIAR: 1,
 }, false, {
  methodsMap:{_methodsMap:'_methodsMap'},
 }){
}
/** @type {typeof com.webcircuits.AbstractAR} */
AbstractAR.class=function(){}

The assignment to AbstractAR.class.protoype will evaluate to the object literal being assigned, so the __extend method will receive the right mixin, and it doesn't matter that the prototype of the dummy function is modified.

/** @extends {com.webcircuits.AR} */
export default class AR extends AbstractAR.__extend(
 AbstractAR.class.prototype=/** @type {!com.webcircuits.AR} */({
  r_inv({mid,args}){
   const{asIAR:{methodsMap:methodsMap}}=this
   test({methodsMap:methodsMap})
  },
 }),
) {}

Validation:

package/lib/src/class/AR/AR.js:15:8: WARNING - [JSC_TYPE_MISMATCH] actual parameter 1 of test$$module$package$lib$src$class$AR$AR does not match formal parameter
found   : {methodsMap: Map<string,string>}
required: string
  15|    test({methodsMap:methodsMap})
              ^^^^^^^^^^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 90.7% typed

Success. Check pruning? Not success, the AR is preserved in the build. One final try: remove the class name of the source code:

/** @extends {com.webcircuits.AR} */
export default class extends AbstractAR.__implement(
 AbstractAR.class.prototype=/** @type {!com.webcircuits.AR} */({
  r_inv({mid,args}){
  },
 }),
) {}

And it finally works!

The Conclusion

Without the class name, the irrelevant classes are pruned again. I have no idea why it works, and you have no idea how many tries led to it, but that's the point I'm trying to make — when researching of how to make you idea into reality, sometimes you just have to literally bruteforce every possibility there is. This work took about 10+ hours (but the result looks so easy huh), and I almost went into despair 2 times thinking that it's impossible to achieve. Still, when a scientist is obsessed, she does not give up until every option is exhausted. The requirements are that the type-checking should work (both in compiler and IDE), and code should look beautiful.

The compiler is an intricate beast and it's hard to know what's going on inside its systems. There are ways to debug the compiler, such as printing the transformation results after each pass, or downloading the source code and debugging it. But those are time-consuming and it's best to try all possible combinations at the front-end first. Plus nobody really knows how to get Java Bazel debugging working in VSCode.

Some wrong assumptions I made were that an assignment to .prototype is always considered side-effectful but it's not: when tried with simpler code, the compiler eliminated those easily. This was not the case though if the constructors were put in adjacent modules. So there are many factors in play to side-effect analysis such as module scopes, creation of named variables and the place where a .prototype is assigned to (e.g., sometimes assigning during an __extend call wasn't not successful while assigning in the main module body was).

Ideally, I would want to assign mixins directly to AbstractAR.prototype rather than having to do AbstractAR.class.prototype, but that would require export default initAbstract() to work which currently crashes, which requires to create a local variable which makes the state analyser think there's a possible side effect. So in the end, it's a compromise. Still better than having to pull an entire library of classes when only a few are needed.

If you're interested in the code generator, some info is available at typal.dev though it's still VIP, but if you'd like a demo access do drop me a line. The next post will feature a 5-min demo of using Closure Compiler(tm) to build professional NodeJS packages while using flavors/mixins/traits to model a complex real-world scenario. Please feel free to share your thoughts, and follow on Twitter for updates.