Issue 6538, 6781 - Implement script compilation for snippets

test/snippets.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /* eslint no-new-func: "off" */

[Manish Jethani, 2018/04/26 13:46:00]

ESLint won't let me use new Function(...) even though it's not the same as eval
[1]

[1]: https://stackoverflow.com/a/4599946

 
 "use strict";
 
 const {createSandbox} = require("./_common");
 
-let SnippetFilter = null;
 let Snippets = null;
-let ElemHide = null;
+let parseScript = null;
+let compileScript = null;
 let Filter = null;
 
 exports.setUp = function(callback)
 {
   let sandboxedRequire = createSandbox();
   (
-    {Filter, SnippetFilter} = sandboxedRequire("../lib/filterClasses"),
-    {ElemHide} = sandboxedRequire("../lib/elemHide"),
-    {Snippets} = sandboxedRequire("../lib/snippets")
+    {Filter} = sandboxedRequire("../lib/filterClasses"),
+    {Snippets, parseScript, compileScript} = sandboxedRequire("../lib/snippets")
   );
 
   callback();
 };
 
 exports.testDomainRestrictions = function(test)
 {
-  function testSelectorMatches(description, filters, domain, expectedMatches)
+  function testScriptMatches(description, filters, domain, expectedMatches)
   {
     for (let filter of filters)
-    {
-      filter = Filter.fromText(filter);
-      if (filter instanceof SnippetFilter)
-        Snippets.add(filter);
-      else
-        ElemHide.add(filter);
-    }
-
-    let matches = Snippets.getScriptsForDomain(domain)
-                  .map(filter => filter.text);
+      Snippets.add(Filter.fromText(filter));
+
+    let matches = Snippets.getScriptsForDomain(domain);
     test.deepEqual(matches.sort(), expectedMatches.sort(), description);
 
     Snippets.clear();
-    ElemHide.clear();
-  }
-
-  testSelectorMatches(
-    "Ignore selectors with exceptions",
-    [
-      "example.com#$#foo",
-      "example.com#$#bar",
-      "example.com#@#foo"
-    ],
-    "example.com",
-    ["example.com#$#bar"]
-  );
-  testSelectorMatches(
+  }
+
+  testScriptMatches(
     "Ignore filters that include parent domain but exclude subdomain",
     [
       "~www.example.com,example.com#$#foo"
     ],
     "www.example.com",
     []
   );
-  testSelectorMatches(
-    "Ignore filters with parent domain if exception matches subdomain",
-    [
-      "www.example.com#@#foo",
-      "example.com#$#foo"
-    ],
-    "www.example.com",
-    []
-  );
-  testSelectorMatches(
+  testScriptMatches(
     "Ignore filters for other subdomain",
     [
       "www.example.com#$#foo",
       "other.example.com#$#foo"
     ],
     "other.example.com",
-    ["other.example.com#$#foo"]
+    ["foo"]
   );
 
   test.done();
 };
 
 exports.testSnippetFiltersContainer = function(test)
 {
   function compareRules(description, domain, expectedMatches)
   {
-    let result = Snippets.getScriptsForDomain(domain)
-                 .map(filter => filter.text);
-    expectedMatches = expectedMatches.map(filter => filter.text);
+    let result = Snippets.getScriptsForDomain(domain);
+    expectedMatches = expectedMatches.map(filter => filter.script);
     test.deepEqual(result.sort(), expectedMatches.sort(), description);
   }
 
   let domainFilter = Filter.fromText("example.com#$#filter1");
   let subdomainFilter = Filter.fromText("www.example.com#$#filter2");
   let otherDomainFilter = Filter.fromText("other.example.com#$#filter3");
 
   Snippets.add(domainFilter);
   Snippets.add(subdomainFilter);
   Snippets.add(otherDomainFilter);
@@ skipping 17 matching lines @@
     []
   );
 
   test.done();
 };
 
 exports.testScriptParsing = function(test)
 {
   function checkParsedScript(description, script, expectedTree)
   {
-    let tree = Snippets.parseScript(script);
+    let tree = parseScript(script);
     test.deepEqual(tree, expectedTree, description);
   }
 
   checkParsedScript("Script with no arguments", "foo", [["foo"]]);
   checkParsedScript("Script with one argument", "foo 1", [["foo", "1"]]);
   checkParsedScript("Script with two arguments", "foo 1 Hello",
                     [["foo", "1", "Hello"]]);
   checkParsedScript("Script with argument containing an escaped space",
                     "foo Hello\\ world",
                     [["foo", "Hello world"]]);
   checkParsedScript("Script with argument containing a quoted space",
                     "foo 'Hello world'",
                     [["foo", "Hello world"]]);
   checkParsedScript("Script with argument containing a quoted escaped quote",
                     "foo 'Hello \\'world\\''",
                     [["foo", "Hello 'world'"]]);
   checkParsedScript("Script with argument containing an escaped semicolon",
                     "foo TL\\;DR",
                     [["foo", "TL;DR"]]);
   checkParsedScript("Script with argument containing a quoted semicolon",
                     "foo 'TL;DR'",
                     [["foo", "TL;DR"]]);
+  checkParsedScript("Script with argument containing single character " +
+                    "escape sequences",
+                    "foo yin\\tyang\\n",
+                    [["foo", "yin\tyang\n"]]);
+  checkParsedScript("Script with argument containing Unicode escape sequences",
+                    "foo \\u0062\\ud83d\\ude42r " +
+                    "'l\\ud83d\\ude02mbd\\ud83d\\ude02'", [
+                      ["foo", "b\ud83d\ude42r", "l\ud83d\ude02mbd\ud83d\ude02"]
+                    ]);
   checkParsedScript("Script with multiple commands", "foo; bar",
                     [["foo"], ["bar"]]);
   checkParsedScript("Script with multiple commands and multiple arguments each",
                     "foo 1 Hello; bar world! #",
                     [["foo", "1", "Hello"], ["bar", "world!", "#"]]);
-  checkParsedScript(
-    "Script with multiple commands and multiple " +
-    "escaped and quoted arguments each",
-    "foo 1 'Hello, \\'Tommy\\'!' ;" +
-    "bar Hi!\\ How\\ are\\ you? http://example.com",
-    [["foo", "1", "Hello, 'Tommy'!"],
-     ["bar", "Hi! How are you?", "http://example.com"]]
-  );
+  checkParsedScript("Script with multiple commands and multiple " +
+                    "escaped and quoted arguments each",
+                    "foo 1 'Hello, \\'Tommy\\'!' ;" +
+                    "bar Hi!\\ How\\ are\\ you? http://example.com", [
+                      ["foo", "1", "Hello, 'Tommy'!"],
+                      ["bar", "Hi! How are you?", "http://example.com"]
+                    ]);
   checkParsedScript("Script with command names containing " +
                     "whitespace (spaces, tabs, newlines, etc.), " +
                     "quotes, and semicolons",
                     "fo\\'\\ \\ \\\t\\\n\\;o 1 2 3; 'b a  r' 1 2",
                     [["fo'  \t\n;o", "1", "2", "3"], ["b a  r", "1", "2"]]);
   checkParsedScript("Script containing Unicode composite characters",
                     "f\ud83d\ude42\ud83d\ude42 b\ud83d\ude02r",
                     [["f\ud83d\ude42\ud83d\ude42", "b\ud83d\ude02r"]]);
   checkParsedScript("Script with no-op commands", "foo; ;;; ;  ; bar 1",
                     [["foo"], ["bar", "1"]]);
 
   test.done();
 };
 
 exports.testScriptCompilation = function(test)
 {
-  let {compileScript, parseScript} = Snippets;
-
   let libraries = [
     `
       let foo = 0;
 
       exports.setFoo = function(value)
       {
         foo = value;
       };
 
       exports.assertFoo = function(expected)
       {
         if (foo != expected)
           throw new Error("Value mismatch");
       };
     `
   ];
 
   let template = `
-      "use strict";
+    "use strict";
+    {
+      const libraries = ${JSON.stringify(libraries)};
+
+      const script = {{{script}}};
+
+      let imports = Object.create(null);
+      for (let library of libraries)
+        new Function("exports", library)(imports);
+
+      for (let [name, ...args] of script)
       {
-        const libraries = ${JSON.stringify(libraries)};
-
-        const script = {{{script}}};
-
-        let imports = Object.create(null);
-        for (let library of libraries)
-          new Function("exports", library)(imports);
-
-        for (let [name, ...args] of script)
+        if (Object.prototype.hasOwnProperty.call(imports, name))
         {
-          if (Object.prototype.hasOwnProperty.call(imports, name))
-          {
-            let value = imports[name];
-            if (typeof value == "function")
-              value(...args);
-          }
+          let value = imports[name];
+          if (typeof value == "function")
+            value(...args);
         }
       }
+    }
   `;
 
   function verifyExecutable(script)

[Manish Jethani, 2018/04/26 13:46:00]

I don't know how valuable this is because "template" is essentially
copy-and-paste from the implementation.

   {
     let actual = compileScript(script, libraries);
     let expected = template.replace("{{{script}}}",
                                     JSON.stringify(parseScript(script)));
 
-    // Trim surrounding spaces because of possible difference in indentation.
-    test.equal(expected.trim(), actual.trim());
+    test.equal(expected, actual);

[Sebastian Noack, 2018/07/16 16:37:35]

Instead of copying the boilerplate over to the tests and verifying that the
output of the generated code matches, I wonder whether we should rather run the
generated code and verify, that it behaves as expected?

[Manish Jethani, 2018/07/17 19:57:15]

Let me try something.

Yeah I agree that the current approach probably isn't the best way to do it.

[Manish Jethani, 2018/07/17 20:10:26]

Wait, this is what the next test does! It tests the _execution_ of the snippet.
setFoo is supposed to set the value of an internal variable, and assertFoo is
supposed to verify the value. We are already doing this.

   }
 
   verifyExecutable("hello 'How are you?'");
 
   // Test script execution.
   new Function(compileScript("setFoo 123; assertFoo 123", libraries))();
 
   // Override setFoo in a second library, without overriding assertFoo. A
   // couple of things to note here: (1) each library has its own variables;
   // (2) script execution is stateless, i.e. the values are not retained
   // between executions. In the example below, assertFoo does not find 456 but
   // it doesn't find 123 either. It's the initial value 0.
   new Function(
     compileScript("setFoo 456; assertFoo 0", [
       ...libraries, "let foo = 1; exports.setFoo = value => { foo = value; };"
     ])
   )();
 
   test.done();
 };