Issue 6538, 6781 - Implement script parsing for snippets

lib/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/>.
  */
 
 "use strict";
 
 /**
  * @fileOverview Snippets implementation.
  */
 
 const {Filter} = require("./filterClasses");
+
+const singleCharacterEscapes = new Map([
+  ["n", "\n"], ["r", "\r"], ["t", "\t"]
+]);
 
 let filters = new Set();
 
 /**
  * Container for snippet filters
  * @class
  */
 let Snippets = {
   /**
    * Removes all known filters
@@ skipping 39 matching lines @@
   }
 };
 
 exports.Snippets = Snippets;
 
 /**
  * Parses a script and returns a list of all its commands and their arguments
  * @param {string} script
  * @return {Array.<string[]>}
  */
 function parseScript(script)

[kzar, 2018/07/10 13:55:20]

I found this terminology confusing, "script" is the body of the snippet filter
which in turns calls a "snippet" script. How about `parseSnippetCalls`?

[Manish Jethani, 2018/07/11 19:04:41]

The argument to this function is the SnippetFilter.script property, I think we
agrees on that terminology. In that case parseScript sounds like an appropriate
name to me. What do you think?

[kzar, 2018/07/12 10:30:36]

Fair enough.

 {
-  const singleCharacterEscapes = new Map([

[kzar, 2018/07/10 13:55:20]

Might be better to declare this once at the top of the file, instead of every
time the function is called?

[Manish Jethani, 2018/07/11 19:04:41]

Done.

-    ["n", "\n"], ["r", "\r"], ["t", "\t"]
-  ]);
-
   let tree = [];
 
   let escape = false;
-  let literal = false;
+  let withinQuotes = false;
 
   let unicodeEscape = null;
 
   let call = [];
   let argument = "";
 
-  for (let character of [...script.trim(), ";"])

[kzar, 2018/07/10 13:55:20]

Why not just do `script = script.trim() + ";";` above and then `for (let
character of script)`?

[Manish Jethani, 2018/07/11 19:04:41]

Done.

+  for (let character of script.trim() + ";")
   {
     if (unicodeEscape != null)
     {
       unicodeEscape += character;
 
       if (unicodeEscape.length == 4)

[kzar, 2018/07/10 13:55:20]

What if unicodeEscape never reaches this length? I guess if the script ends with
"\u123" those last characters are lost?

[Manish Jethani, 2018/07/11 19:04:41]

Yes, they would be lost.

If it's not at the end of the script, then it'll eat up the 4 characters
following \u and then discard them because they don't parse as a number.

[kzar, 2018/07/12 10:30:36]

Acknowledged.

       {
         let codePoint = parseInt(unicodeEscape, 16);
         if (!isNaN(codePoint))
           argument += String.fromCodePoint(codePoint);
 
         unicodeEscape = null;
       }
     }
     else if (escape)
     {
       escape = false;
 
       if (character == "u")
         unicodeEscape = "";
       else
         argument += singleCharacterEscapes.get(character) || character;
     }
     else if (character == "\\")
     {
       escape = true;
     }
     else if (character == "'")
     {
-      literal = !literal;
+      withinQuotes = !withinQuotes;
     }
-    else if (literal || character != ";" && !/\s/u.test(character))

[kzar, 2018/07/10 13:55:20]

So the literal flag has no effect on escaped characters? For example, for the
literal string "\n" I'd have to use "\\n" or "'\\n'".

[Manish Jethani, 2018/07/11 19:04:41]

No, mainly because we need a way to escape the single quote within single
quotes. i.e. '\''. The single quotes in a way only group the characters within
as a single argument.

Literally what it's doing here is that if literal is true then it ignores
semicolons and whitespace and takes the character literally.

[kzar, 2018/07/12 10:30:36]

Acknowledged.

+    else if (withinQuotes || character != ";" && !/\s/u.test(character))
     {
       argument += character;
     }
     else
     {
       if (argument)
       {
         call.push(argument);
         argument = "";
       }
 
       if (character == ";" && call.length > 0)
       {
         tree.push(call);
         call = [];
       }
     }
   }
 
   return tree;
 }
 
 exports.parseScript = parseScript;