Fix preprocessor: nesting, error & tests

Features / bug fixes in the preprocessor:

- Add word boundary after regex for preprocessor token matching.
  Previously, when you mistakenly used "#ifdef" instead of "#if", the
  line would be parsed as a preprocessor directive (because "#ifdef"
  starts with "#if"), but without condition (because "def" does not
  start with a space). Consequently, the condition would always be false
  and anything between "#ifdef" and "#endif" would not be included.

- Add validation and error reporting everywhere, to aid debugging.

- Support nested comments (by accounting for the whole stack of
  conditions, instead of only the current one).

- Add #elif preprocessor command. Could be used as follows:
  //#if !FEATURE_ENABLED
  //#error FEATURE_ENABLED must be set
  //#endif

- Add #error preprocessor command.

- Add end-of-line word boundary after "-->" in the comment trimmer.
  Otherwise the pattern would also match "-->" in the middle of a line,
  and incorrectly convert something like "while(i-->0)" to "while(i0)".

Code health:

- Add unit tests for the preprocessor (run external/builder/test.js).

- Fix broken link to MDN (resolved to DXR).

- Refactor to use STATE_* names instead of magic numbers (the original
  meaning of the numbers is preserved, with one exception).

- State 3 has been split in two states, to distinguish between being in
  an #if and #else. This is needed to ensure that #else cannot be
  started without an #if.

external/builder/builder.js

@@ -10,16 +10,32 @@ var fs = require('fs'),
     vm = require('vm');
 
 /**
- * A simple preprocessor that is based on the firefox preprocessor
- * see (https://developer.mozilla.org/en/Build/Text_Preprocessor).  The main
- * difference is that this supports a subset of the commands and it supports
- * preproccesor commands in html style comments.
- * Currently Supported commands:
+ * A simple preprocessor that is based on the Firefox preprocessor
+ * (https://dxr.mozilla.org/mozilla-central/source/build/docs/preprocessor.rst).
+ * The main difference is that this supports a subset of the commands and it
+ * supports preprocessor commands in HTML-style comments.
+ *
+ * Currently supported commands:
  * - if
+ * - elif
  * - else
  * - endif
  * - include
  * - expand
+ * - error
+ *
+ * Every #if must be closed with an #endif. Nested conditions are supported.
+ *
+ * Within an #if or #else block, one level of comment tokens is stripped. This
+ * allows us to write code that can run even without preprocessing. For example:
+ *
+ * //#if SOME_RARE_CONDITION
+ * // // Decrement by one
+ * // --i;
+ * //#else
+ * // // Increment by one.
+ * ++i;
+ * //#endif
  */
 function preprocess(inFilename, outFilename, defines) {
   // TODO make this really read line by line.
@@ -37,10 +53,28 @@ function preprocess(inFilename, outFilename, defines) {
     function(line) {
       out += line + '\n';
     });
+  function evaluateCondition(code) {
+    if (!code || !code.trim()) {
+      throw new Error('No JavaScript expression given at ' + loc());
+    }
+    try {
+      return vm.runInNewContext(code, defines);
+    } catch (e) {
+      throw new Error('Could not evaluate "' + code + '" at ' + loc() + '\n' +
+          e.name + ': ' + e.message);
+    }
+  }
   function include(file) {
     var realPath = fs.realpathSync(inFilename);
     var dir = path.dirname(realPath);
-    preprocess(path.join(dir, file), writeLine, defines);
+    try {
+      preprocess(path.join(dir, file), writeLine, defines);
+    } catch (e) {
+      if (e.code === 'ENOENT') {
+        throw new Error('Failed to include "' + file + '" at ' + loc());
+      }
+      throw e; // Some other error
+    }
   }
   function expand(line) {
     line = line.replace(/__[\w]+__/g, function(variable) {
@@ -53,52 +87,92 @@ function preprocess(inFilename, outFilename, defines) {
     writeLine(line);
   }
 
-  var s, state = 0, stack = [];
+  // not inside if or else (process lines)
+  var STATE_NONE = 0;
+  // inside if, condition false (ignore until #else or #endif)
+  var STATE_IF_FALSE = 1;
+  // inside else, #if was false, so #else is true (process lines until #endif)
+  var STATE_ELSE_TRUE = 2;
+  // inside if, condition true (process lines until #else or #endif)
+  var STATE_IF_TRUE = 3;
+  // inside else, #if was true, so #else is false (ignore lines until #endif)
+  var STATE_ELSE_FALSE = 4;
+
+  var line;
+  var state = STATE_NONE;
+  var stack = [];
   var control =
-    /^(?:\/\/|<!--)\s*#(if|else|endif|expand|include)(?:\s+(.*?)(?:-->)?$)?/;
+  /* jshint -W101 */
+    /^(?:\/\/|<!--)\s*#(if|elif|else|endif|expand|include|error)\b(?:\s+(.*?)(?:-->)?$)?/;
+  /* jshint +W101 */
   var lineNumber = 0;
-  while ((s = readLine()) !== null) {
+  var loc = function() {
+    return fs.realpathSync(inFilename) + ':' + lineNumber;
+  };
+  while ((line = readLine()) !== null) {
     ++lineNumber;
-    var m = control.exec(s);
+    var m = control.exec(line);
     if (m) {
       switch (m[1]) {
         case 'if':
           stack.push(state);
-          try {
-            state = vm.runInNewContext(m[2], defines) ? 3 : 1;
-          } catch (e) {
-            console.error('Could not evalute line \'' + m[2] + '\' at ' +
-                          fs.realpathSync(inFilename) + ':' + lineNumber);
-            throw e;
+          state = evaluateCondition(m[2]) ? STATE_IF_TRUE : STATE_IF_FALSE;
+          break;
+        case 'elif':
+          if (state === STATE_IF_TRUE) {
+            state = STATE_ELSE_FALSE;
+          } else if (state === STATE_IF_FALSE) {
+            state = evaluateCondition(m[2]) ? STATE_IF_TRUE : STATE_IF_FALSE;
+          } else if (state === STATE_ELSE_TRUE || state === STATE_ELSE_FALSE) {
+            throw new Error('Found #elif after #else at ' + loc());
+          } else if (state !== STATE_IF_FALSE) {
+            throw new Error('Found #elif without matching #if at ' + loc());
           }
           break;
         case 'else':
-          state = state === 1 ? 3 : 2;
+          if (state === STATE_IF_TRUE) {
+            state = STATE_ELSE_FALSE;
+          } else if (state === STATE_IF_FALSE) {
+            state = STATE_ELSE_TRUE;
+          } else {
+            throw new Error('Found #else without matching #if at ' + loc());
+          }
           break;
         case 'endif':
+          if (state === STATE_NONE) {
+            throw new Error('Found #endif without #if at ' + loc());
+          }
           state = stack.pop();
           break;
         case 'expand':
-          if (state === 0 || state === 3) {
+          if (state !== STATE_IF_FALSE && state !== STATE_ELSE_FALSE) {
             expand(m[2]);
           }
           break;
         case 'include':
-          if (state === 0 || state === 3) {
+          if (state !== STATE_IF_FALSE && state !== STATE_ELSE_FALSE) {
             include(m[2]);
           }
           break;
+        case 'error':
+          if (state !== STATE_IF_FALSE && state !== STATE_ELSE_FALSE) {
+            throw new Error('Found #error ' + m[2] + ' at ' + loc());
+          }
+          break;
       }
     } else {
-      if (state === 0) {
-        writeLine(s);
-      } else if (state === 3) {
-        writeLine(s.replace(/^\/\/|^<!--|-->/g, '  '));
+      if (state === STATE_NONE) {
+        writeLine(line);
+      } else if ((state === STATE_IF_TRUE || state === STATE_ELSE_TRUE) &&
+          stack.indexOf(STATE_IF_FALSE) === -1 &&
+          stack.indexOf(STATE_ELSE_FALSE) === -1) {
+        writeLine(line.replace(/^\/\/|^<!--|-->$/g, '  '));
       }
     }
   }
-  if (state !== 0 || stack.length !== 0) {
-    throw new Error('Missing endif in preprocessor.');
+  if (state !== STATE_NONE || stack.length !== 0) {
+    throw new Error('Missing #endif in preprocessor for ' +
+        fs.realpathSync(inFilename));
   }
   if (typeof outFilename !== 'function') {
     fs.writeFileSync(outFilename, out);

external/builder/fixtures/confusing-comment-expected.js

@@ -0,0 +1,4 @@
+'use strict';
+var i = 0;
+while(i-->0) {
+}

external/builder/fixtures/confusing-comment.js

@@ -0,0 +1,6 @@
+'use strict';
+//#if TRUE
+var i = 0;
+while(i-->0) {
+}
+//#endif

external/builder/fixtures/elif-expected.js

@@ -0,0 +1 @@
+//Error: Found #elif without matching #if at __filename:2

external/builder/fixtures/elif.js

@@ -0,0 +1,4 @@
+'use strict';
+//#elif TRUE
+var a;
+//#endif

external/builder/fixtures/else-expected.js

@@ -0,0 +1 @@
+//Error: Found #else without matching #if at __filename:2

external/builder/fixtures/else.js

@@ -0,0 +1,3 @@
+'use strict';
+//#else
+//#endif

external/builder/fixtures/error-expected.js

@@ -0,0 +1 @@
+//Error: Found #error "Some Error" at __filename:3

external/builder/fixtures/error-false-expected.js

@@ -0,0 +1,2 @@
+'use strict';
+var a;

external/builder/fixtures/error-false.js

@@ -0,0 +1,5 @@
+'use strict';
+//#if FALSE
+//#error "Some Error"
+//#endif
+var a;

external/builder/fixtures/error.js

@@ -0,0 +1,5 @@
+'use strict';
+//#if TRUE
+//#error "Some Error"
+//#endif
+var b;

external/builder/fixtures/expand-expected.html

@@ -0,0 +1 @@
+prefixtruesuffix

external/builder/fixtures/expand.html

@@ -0,0 +1 @@
+<!--#expand prefix__TRUE__suffix-->

external/builder/fixtures/if-empty-expected.js

@@ -0,0 +1 @@
+//Error: No JavaScript expression given at __filename:2

external/builder/fixtures/if-empty.js

@@ -0,0 +1,6 @@
+'use strict';
+//#if
+var a;
+//#else
+var b;
+//#endif

external/builder/fixtures/if-false-elif-false-else-expected.js

@@ -0,0 +1,2 @@
+'use strict';
+var c;

external/builder/fixtures/if-false-elif-false-else.js

@@ -0,0 +1,8 @@
+'use strict';
+//#if FALSE
+var a;
+//#elif FALSE
+var b;
+//#else
+var c;
+//#endif

external/builder/fixtures/if-false-elif-true-else-expected.js

@@ -0,0 +1,2 @@
+'use strict';
+var b;

external/builder/fixtures/if-false-elif-true-else.js

@@ -0,0 +1,8 @@
+'use strict';
+//#if FALSE
+var a;
+//#elif TRUE
+var b;
+//#else
+var c;
+//#endif

external/builder/fixtures/if-false-else-expected.js

@@ -0,0 +1,2 @@
+'use strict';
+var b;

external/builder/fixtures/if-false-else.js

@@ -0,0 +1,6 @@
+'use strict';
+//#if FALSE
+var a;
+//#else
+var b;
+//#endif

external/builder/fixtures/if-nested-expected.js

@@ -0,0 +1,6 @@
+'use strict';
+var a;
+
+var b;
+
+var d;

external/builder/fixtures/if-nested.js

@@ -0,0 +1,19 @@
+'use strict';
+//#if TRUE
+var a;
+
+//#if TRUE
+var b;
+//#else
+var c;
+//#endif
+
+var d;
+//#else
+var e;
+//#if TRUE
+var f;
+//#endif
+
+var g;
+//#endif

external/builder/fixtures/if-true-else-expected.js

@@ -0,0 +1,2 @@
+'use strict';
+var a;

external/builder/fixtures/if-true-else.js

@@ -0,0 +1,6 @@
+'use strict';
+//#if TRUE
+var a;
+//#else
+var b;
+//#endif

external/builder/fixtures/if-unclosed-expected.js

@@ -0,0 +1 @@
+//Error: Missing #endif in preprocessor for __filename

external/builder/fixtures/if-unclosed.js

@@ -0,0 +1,3 @@
+'use strict';
+//#if TRUE
+var a;

external/builder/fixtures/include-expected.html

@@ -0,0 +1,5 @@
+<script>
+'use strict';
+var a;
+
+</script>

external/builder/fixtures/include-non-existent-expected.html

@@ -0,0 +1 @@
+//Error: Failed to include "some file that does not exist" at __filename:2

external/builder/fixtures/include-non-existent.html

@@ -0,0 +1,2 @@
+<!-- Non-existent file -->
+<!--#include some file that does not exist-->

external/builder/fixtures/include.html

@@ -0,0 +1,5 @@
+<script>
+<!--#if TRUE-->
+<!--#include if-true-else.js-->
+<!--#endif-->
+</script>

external/builder/fixtures/js-comment-expected.js

@@ -0,0 +1,4 @@
+'use strict';
+  //var a;
+  var b;
+var c;

external/builder/fixtures/js-comment.js

@@ -0,0 +1,6 @@
+'use strict';
+//#if TRUE
+////var a;
+//var b;
+var c;
+//#endif

external/builder/fixtures/undefined-define-expected.js

@@ -0,0 +1,5 @@
+//Error: Could not evaluate "notdefined" at __filename:2
+//ReferenceError: evalmachine.<anonymous>:1
+//notdefined
+//^
+//notdefined is not defined

external/builder/fixtures/undefined-define.js

@@ -0,0 +1,6 @@
+'use strict';
+//#if notdefined
+var a;
+//#else
+var b;
+//#endif

external/builder/fixtures/unsupported-ifdef-expected.js

@@ -0,0 +1 @@
+//Error: Found #endif without #if at __filename:4

external/builder/fixtures/unsupported-ifdef.js

@@ -0,0 +1,5 @@
+'use strict';
+//#ifdef TRUE
+//ifdef should not be recognized
+//#endif
+var a;