From 441d9f7f106a86aac7f0fb15d3edce07d9da4673 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Fri, 24 Nov 2017 18:24:16 +0000
Subject: [PATCH 1/9] Initial tests and code for user friendly regular
 expression syntax errors

---
 .../util/PatternSyntaxExceptionParser.java    | 107 ++++++++++
 .../PatternSyntaxExceptionParserTests.java    | 189 ++++++++++++++++++
 2 files changed, 296 insertions(+)
 create mode 100644 main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
 create mode 100644 main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
new file mode 100644
index 000000000..47c1d65bf
--- /dev/null
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -0,0 +1,107 @@
+/*
+
+Copyright 2010, Google Inc.
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+    * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+    * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,           
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY           
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+package com.google.refine.util;
+
+import java.util.regex.PatternSyntaxException;
+
+public class PatternSyntaxExceptionParser {
+    private final PatternSyntaxException exception;
+
+    public PatternSyntaxExceptionParser(PatternSyntaxException e) {
+        this.exception = e;
+    }
+
+    public String getUserMessage() {
+        StringBuffer sb = new StringBuffer();
+        String desc = exception.getDescription();
+        switch(desc)
+        {
+            case "Unclosed character class":
+            //Need these errors to be more human readable
+            //Possibly include html for formatting
+            //Update tests first with user friendly errors
+                sb.append("The regular expression is missing a closing ']' character.");
+                break;
+            case "Unmatched closing ')'":
+                sb.append("The regular expression is missing a opening '(' character.");
+                break;
+            case "Unclosed group":
+                sb.append("The regular expression is missing a closing ')' character.");
+                break;
+            default:
+                // If no special handling in place fall back on method
+                // as used in java.util.regex.PatternSyntaxException
+                sb.append(desc);
+        }
+        return sb.toString();
+    }
+        /* Error messages from java.util.regex.Pattern
+        "Unclosed character class"
+        "Unmatched closing ')'"
+        "Unexpected internal error"
+        "Dangling meta character '" + ((char)ch) + "'"
+        "\\k is not followed by '<' for named capturing group"
+        "(named capturing group <"+ name+"> does not exist"
+        "Illegal/unsupported escape sequence"
+        "Bad class syntax"
+        "Unclosed character class"
+        "Illegal character range"
+        "Unexpected character '"+((char)ch)+"'"
+        "Unclosed character family"
+        "Empty character family"
+        "Unknown Unicode property {name=<" + name + ">, "+ "value=<" + value + ">}"
+        "Unknown character script name {" + name + "}"
+        "Unknown character block name {" + name + "}"
+        "Unknown character property name {" + name + "}"
+        "named capturing group has 0 length name"
+        "named capturing group is missing trailing '>'"
+        "Named capturing group <" + name + "> is already defined"
+        "Look-behind group does not have " + "an obvious maximum length"
+        "Unknown look-behind group"
+        "Unknown group type"
+        "Unknown inline modifier"
+        "Internal logic error"
+        "Unclosed counted closure"
+        "Illegal repetition range"
+        "Illegal repetition"
+        "Illegal control escape sequence"
+        "Illegal octal escape sequence"
+        "Hexadecimal codepoint is too big"
+        "Unclosed hexadecimal escape sequence"
+        "Illegal hexadecimal escape sequence"
+        "Illegal Unicode escape sequence"
+        */
+
+
+}
diff --git a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
new file mode 100644
index 000000000..9b06f7a5d
--- /dev/null
+++ b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
@@ -0,0 +1,189 @@
+/*
+
+Copyright 2010, Google Inc.
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+    * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+    * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,           
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY           
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+package com.google.refine.tests.util;
+
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+import com.google.refine.tests.RefineTest;
+import com.google.refine.util.PatternSyntaxExceptionParser;
+
+public class PatternSyntaxExceptionParserTests extends RefineTest {
+    
+    @Override
+    @BeforeTest
+    public void init() {
+        logger = LoggerFactory.getLogger(this.getClass());
+    }
+    
+/* 
+Potential errors and error messages from PatternSyntaxException
+groupopen:"Unmatched opening parenthesis."
+    Unclosed group near index 1 ( ^
+groupclose:"Unmatched closing parenthesis."
+    Unmatched closing ')' )
+setopen:"Unmatched opening square bracket."
+    Unclosed character class near index 0 [ ^
+quanttarg:"Invalid target for quantifier."
+    Dangling meta character '+' near index 0 +{4} ^
+    Dangling meta character '*' near index 0 * ^
+    Dangling meta character '?' near index 0 ? ^
+esccharopen:"Dangling backslash."
+    Unexpected internal error near index 1 \ ^
+quantrev:"Quantifier minimum is greater than maximum."
+    Illegal repetition range near index 5 a{3,2} ^
+rangerev:"Range values reversed. Start char is greater than end char."
+    Illegal character range near index 3 [9-0] ^
+esccharbad:"Invalid escape sequence."
+    Illegal control escape sequence: \c
+    Illegal/unsupported escape sequence: \g \i \j \l \m \o \q \y
+    \k is not followed by '<' for named capturing group: \k
+    Unknown character property name {}: \p
+    Illegal Unicode escape sequence: {backslash}u
+    Illegal hexadecimal escape sequence: \x
+    Illegal octal escape sequence: \0
+invalidnamegroup:
+    named capturing group is missing trailing '>' near index 5 (?<as?>a) ^
+*/
+
+    @Test
+    public void unmatchedOpeningParenthesisTest(){
+        String s = "(abc";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression is missing a closing ')' character.");
+        }
+    }
+
+    @Test
+    public void unmatchedClosingParenthesisTest(){
+        String s = "abc)";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression is missing a opening '(' character.");
+        }
+    }
+
+    @Test
+    public void unmatchedOpeningSquareBracketTest(){
+        String s = "[abc";
+        try {
+            Pattern pattern = Pattern.compile(s);
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression is missing a closing ']' character.");
+        }
+    }
+
+    @Test
+    public void quantifierTargetValidityTest(){
+        String s = "abc+*";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression has a '*','+' or '?' in the wrong place");
+        }
+    }
+
+    @Test
+    public void danglingBackslashTest(){
+        String s = "abc\\";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression has a backslash '\\' at the end.");
+        }
+    }
+
+    @Test
+    public void quantifierMagnitudeTest(){
+        String s = "a{4,3}";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression has a quantifier statement where the minimum is larger than the maximum (e.g. {4,3}).");
+        }
+    }
+
+    @Test
+    public void rangeOrderTest(){
+        String s = "abc[9-0]";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression has a range statement with the characters in the incorrect order (e.g. [9-0])");
+        }
+    }
+
+/* This needs to be different to the others - see all the variations on invalid escape sequences
+    @Test
+    public void escapeSequenceValidityTest(){
+        String s = "";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "Invalid escape sequence.");
+        }
+    }
+*/
+}

From d64dee71dda96f16502fea33ebfe9ded9e0ac2de Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Tue, 28 Nov 2017 16:20:24 +0000
Subject: [PATCH 2/9] Extended tests and code for user friendly regular
 expression syntax errors

---
 .../util/PatternSyntaxExceptionParser.java    | 104 +++++++++++-------
 .../PatternSyntaxExceptionParserTests.java    |  98 +++++++----------
 2 files changed, 101 insertions(+), 101 deletions(-)

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
index 47c1d65bf..fddc08d92 100644
--- a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -36,6 +36,44 @@ package com.google.refine.util;
 import java.util.regex.PatternSyntaxException;
 
 public class PatternSyntaxExceptionParser {
+    /*  Class to translate PatternSyntaxExceptions into more user friendly error messages
+        Currently translates the following error messages from java.util.regex.Pattern
+            "Unclosed character class"
+            "Unmatched closing ')'"
+            "Unexpected internal error"
+            "Dangling meta character '" + ((char)ch) + "'"
+            "Unclosed counted closure"
+            "Illegal repetition"
+            "Illegal repetition range"
+            "Illegal character range"
+        
+        The following messages are not currently translated and are output as per PatternSyntaxException
+            "\\k is not followed by '<' for named capturing group"
+            "(named capturing group <"+ name+"> does not exist"
+            "Illegal/unsupported escape sequence"
+            "Bad class syntax"
+            "Unexpected character '"+((char)ch)+"'"
+            "Unclosed character family"
+            "Empty character family"
+            "Unknown Unicode property {name=<" + name + ">, "+ "value=<" + value + ">}"
+            "Unknown character script name {" + name + "}"
+            "Unknown character block name {" + name + "}"
+            "Unknown character property name {" + name + "}"
+            "named capturing group has 0 length name"
+            "named capturing group is missing trailing '>'"
+            "Named capturing group <" + name + "> is already defined"
+            "Look-behind group does not have " + "an obvious maximum length"
+            "Unknown look-behind group"
+            "Unknown group type"
+            "Unknown inline modifier"
+            "Internal logic error"
+            "Illegal control escape sequence"
+            "Illegal octal escape sequence"
+            "Hexadecimal codepoint is too big"
+            "Unclosed hexadecimal escape sequence"
+            "Illegal hexadecimal escape sequence"
+            "Illegal Unicode escape sequence"
+        */
     private final PatternSyntaxException exception;
 
     public PatternSyntaxExceptionParser(PatternSyntaxException e) {
@@ -51,7 +89,7 @@ public class PatternSyntaxExceptionParser {
             //Need these errors to be more human readable
             //Possibly include html for formatting
             //Update tests first with user friendly errors
-                sb.append("The regular expression is missing a closing ']' character.");
+                sb.append("The regular expression is missing a closing ']' character, or has an empty pair of square brackets '[]'.");
                 break;
             case "Unmatched closing ')'":
                 sb.append("The regular expression is missing a opening '(' character.");
@@ -59,49 +97,31 @@ public class PatternSyntaxExceptionParser {
             case "Unclosed group":
                 sb.append("The regular expression is missing a closing ')' character.");
                 break;
+            case "Dangling meta character '*'":
+            case "Dangling meta character '+'":
+            case "Dangling meta character '?'":
+                sb.append("The regular expression has a '*','+' or '?' in the wrong place.");
+                break;
+            case "Unexpected internal error":
+                sb.append("The regular expression has a backslash '\\' at the end.");
+                break;
+            case "Unclosed counted closure":
+                sb.append("The regular expression is missing a closing '}' character, or has an incorrect quantifier statement in curly brackets '{}'.");
+                break;
+            case "Illegal repetition":
+                sb.append("The regular expression has an incomplete or incorrect quantifier statement in curly brackets '{}'.");
+                break;
+            case "Illegal repetition range":
+                sb.append("The regular expression has a quantifier statement where the minimum is larger than the maximum (e.g. {4,3}).");
+                break;
+            case "Illegal character range":
+                sb.append("The regular expression has a range statement which is incomplete or has the characters in the incorrect order (e.g. [9-0])");
+                break;
             default:
-                // If no special handling in place fall back on method
-                // as used in java.util.regex.PatternSyntaxException
-                sb.append(desc);
+                // If no special handling in place fall back on error msg
+                // created by java.util.regex.PatternSyntaxException
+                sb.append(exception.getMessage());
         }
         return sb.toString();
     }
-        /* Error messages from java.util.regex.Pattern
-        "Unclosed character class"
-        "Unmatched closing ')'"
-        "Unexpected internal error"
-        "Dangling meta character '" + ((char)ch) + "'"
-        "\\k is not followed by '<' for named capturing group"
-        "(named capturing group <"+ name+"> does not exist"
-        "Illegal/unsupported escape sequence"
-        "Bad class syntax"
-        "Unclosed character class"
-        "Illegal character range"
-        "Unexpected character '"+((char)ch)+"'"
-        "Unclosed character family"
-        "Empty character family"
-        "Unknown Unicode property {name=<" + name + ">, "+ "value=<" + value + ">}"
-        "Unknown character script name {" + name + "}"
-        "Unknown character block name {" + name + "}"
-        "Unknown character property name {" + name + "}"
-        "named capturing group has 0 length name"
-        "named capturing group is missing trailing '>'"
-        "Named capturing group <" + name + "> is already defined"
-        "Look-behind group does not have " + "an obvious maximum length"
-        "Unknown look-behind group"
-        "Unknown group type"
-        "Unknown inline modifier"
-        "Internal logic error"
-        "Unclosed counted closure"
-        "Illegal repetition range"
-        "Illegal repetition"
-        "Illegal control escape sequence"
-        "Illegal octal escape sequence"
-        "Hexadecimal codepoint is too big"
-        "Unclosed hexadecimal escape sequence"
-        "Illegal hexadecimal escape sequence"
-        "Illegal Unicode escape sequence"
-        */
-
-
 }
diff --git a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
index 9b06f7a5d..b3cff97ea 100644
--- a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
+++ b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
@@ -51,36 +51,6 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void init() {
         logger = LoggerFactory.getLogger(this.getClass());
     }
-    
-/* 
-Potential errors and error messages from PatternSyntaxException
-groupopen:"Unmatched opening parenthesis."
-    Unclosed group near index 1 ( ^
-groupclose:"Unmatched closing parenthesis."
-    Unmatched closing ')' )
-setopen:"Unmatched opening square bracket."
-    Unclosed character class near index 0 [ ^
-quanttarg:"Invalid target for quantifier."
-    Dangling meta character '+' near index 0 +{4} ^
-    Dangling meta character '*' near index 0 * ^
-    Dangling meta character '?' near index 0 ? ^
-esccharopen:"Dangling backslash."
-    Unexpected internal error near index 1 \ ^
-quantrev:"Quantifier minimum is greater than maximum."
-    Illegal repetition range near index 5 a{3,2} ^
-rangerev:"Range values reversed. Start char is greater than end char."
-    Illegal character range near index 3 [9-0] ^
-esccharbad:"Invalid escape sequence."
-    Illegal control escape sequence: \c
-    Illegal/unsupported escape sequence: \g \i \j \l \m \o \q \y
-    \k is not followed by '<' for named capturing group: \k
-    Unknown character property name {}: \p
-    Illegal Unicode escape sequence: {backslash}u
-    Illegal hexadecimal escape sequence: \x
-    Illegal octal escape sequence: \0
-invalidnamegroup:
-    named capturing group is missing trailing '>' near index 5 (?<as?>a) ^
-*/
 
     @Test
     public void unmatchedOpeningParenthesisTest(){
@@ -116,20 +86,7 @@ invalidnamegroup:
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
             Assert.assertEquals(e.getUserMessage(),
-                                "The regular expression is missing a closing ']' character.");
-        }
-    }
-
-    @Test
-    public void quantifierTargetValidityTest(){
-        String s = "abc+*";
-        try {
-            Pattern pattern = Pattern.compile(s);
-            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
-        } catch (PatternSyntaxException err) {
-            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
-            Assert.assertEquals(e.getUserMessage(),
-                                "The regular expression has a '*','+' or '?' in the wrong place");
+                                "The regular expression is missing a closing ']' character, or has an empty pair of square brackets '[]'.");
         }
     }
 
@@ -146,6 +103,43 @@ invalidnamegroup:
         }
     }
 
+    @Test
+    public void unmatchedOpeningCurlyBracketTest(){
+        String s = "abc{3";
+        try {
+            Pattern pattern = Pattern.compile(s);
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression is missing a closing '}' character, or has an incorrect quantifier statement in curly brackets '{}'.");
+        }
+    }
+
+    @Test
+    public void illegalQuantifierStatement(){
+        String s = "abc{";
+        try {
+            Pattern pattern = Pattern.compile(s);
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression has an incomplete or incorrect quantifier statement in curly brackets '{}'.");
+        }
+    }
+
+    @Test
+    public void quantifierTargetValidityTest(){
+        String s = "abc+*";
+        try {
+            Pattern pattern = Pattern.compile(s);
+            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
+        } catch (PatternSyntaxException err) {
+            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
+            Assert.assertEquals(e.getUserMessage(),
+                                "The regular expression has a '*','+' or '?' in the wrong place.");
+        }
+    }
+
     @Test
     public void quantifierMagnitudeTest(){
         String s = "a{4,3}";
@@ -168,22 +162,8 @@ invalidnamegroup:
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
             Assert.assertEquals(e.getUserMessage(),
-                                "The regular expression has a range statement with the characters in the incorrect order (e.g. [9-0])");
+                                "The regular expression has a range statement which is incomplete or has the characters in the incorrect order (e.g. [9-0])");
         }
     }
 
-/* This needs to be different to the others - see all the variations on invalid escape sequences
-    @Test
-    public void escapeSequenceValidityTest(){
-        String s = "";
-        try {
-            Pattern pattern = Pattern.compile(s);
-            Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
-        } catch (PatternSyntaxException err) {
-            PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
-            Assert.assertEquals(e.getUserMessage(),
-                                "Invalid escape sequence.");
-        }
-    }
-*/
 }

From 21741ad09eb0486f4737f28c71a5cecefbf87f27 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Tue, 28 Nov 2017 16:22:17 +0000
Subject: [PATCH 3/9] Pass user friendly error message to UI on invalid regular
 expression

---
 .../com/google/refine/browsing/facets/TextSearchFacet.java    | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/main/src/com/google/refine/browsing/facets/TextSearchFacet.java b/main/src/com/google/refine/browsing/facets/TextSearchFacet.java
index 03c6c1cf9..e8270d4fc 100644
--- a/main/src/com/google/refine/browsing/facets/TextSearchFacet.java
+++ b/main/src/com/google/refine/browsing/facets/TextSearchFacet.java
@@ -50,6 +50,7 @@ import com.google.refine.expr.Evaluable;
 import com.google.refine.grel.ast.VariableExpr;
 import com.google.refine.model.Column;
 import com.google.refine.model.Project;
+import com.google.refine.util.PatternSyntaxExceptionParser;
 
 public class TextSearchFacet implements Facet {
     /*
@@ -106,7 +107,8 @@ public class TextSearchFacet implements Facet {
                             _query, 
                             _caseSensitive ? 0 : Pattern.CASE_INSENSITIVE);
                 } catch (java.util.regex.PatternSyntaxException e) {
-                    throw new JSONException(e);
+                    PatternSyntaxExceptionParser err = new PatternSyntaxExceptionParser(e);
+                    throw new JSONException(err.getUserMessage());
                 }
             } else if (!_caseSensitive) {
                 _query = _query.toLowerCase();

From 1d1f3e67cd5346cd714e29e3ed8c4d9951bd1edb Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Wed, 29 Nov 2017 00:58:51 +0000
Subject: [PATCH 4/9] Added break statement for default case

---
 .../src/com/google/refine/util/PatternSyntaxExceptionParser.java | 1 +
 1 file changed, 1 insertion(+)

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
index fddc08d92..c23faf231 100644
--- a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -121,6 +121,7 @@ public class PatternSyntaxExceptionParser {
                 // If no special handling in place fall back on error msg
                 // created by java.util.regex.PatternSyntaxException
                 sb.append(exception.getMessage());
+                break;
         }
         return sb.toString();
     }

From 0b433fc5eced4e8f2315a05ec0a85c215517be07 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Wed, 29 Nov 2017 00:59:49 +0000
Subject: [PATCH 5/9] Updated copyright statement

---
 .../com/google/refine/util/PatternSyntaxExceptionParser.java  | 4 ++--
 .../refine/tests/util/PatternSyntaxExceptionParserTests.java  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
index c23faf231..29f323b76 100644
--- a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -1,6 +1,6 @@
 /*
 
-Copyright 2010, Google Inc.
+Copyright 2017, Owen Stephens.
 All rights reserved.
 
 Redistribution and use in source and binary forms, with or without
@@ -13,7 +13,7 @@ notice, this list of conditions and the following disclaimer.
 copyright notice, this list of conditions and the following disclaimer
 in the documentation and/or other materials provided with the
 distribution.
-    * Neither the name of Google Inc. nor the names of its
+    * Neither the name of the copyright holder nor the names of its
 contributors may be used to endorse or promote products derived from
 this software without specific prior written permission.
 
diff --git a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
index b3cff97ea..72bdf94b9 100644
--- a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
+++ b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
@@ -1,6 +1,6 @@
 /*
 
-Copyright 2010, Google Inc.
+Copyright 2017, Owen Stephens.
 All rights reserved.
 
 Redistribution and use in source and binary forms, with or without
@@ -13,7 +13,7 @@ notice, this list of conditions and the following disclaimer.
 copyright notice, this list of conditions and the following disclaimer
 in the documentation and/or other materials provided with the
 distribution.
-    * Neither the name of Google Inc. nor the names of its
+    * Neither the name of the copyright holder nor the names of its
 contributors may be used to endorse or promote products derived from
 this software without specific prior written permission.
 

From 316da5e917ce65e2592568561ef4bc9851034266 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Wed, 29 Nov 2017 01:00:19 +0000
Subject: [PATCH 6/9] Removed comment no longer needed

---
 .../com/google/refine/util/PatternSyntaxExceptionParser.java   | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
index 29f323b76..7a95a897d 100644
--- a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -86,9 +86,6 @@ public class PatternSyntaxExceptionParser {
         switch(desc)
         {
             case "Unclosed character class":
-            //Need these errors to be more human readable
-            //Possibly include html for formatting
-            //Update tests first with user friendly errors
                 sb.append("The regular expression is missing a closing ']' character, or has an empty pair of square brackets '[]'.");
                 break;
             case "Unmatched closing ')'":

From 93431daecf99508cb2adc40ebdb9c0e74ed87876 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Wed, 29 Nov 2017 09:47:03 +0000
Subject: [PATCH 7/9] Remove unused variable

---
 .../PatternSyntaxExceptionParserTests.java     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
index 72bdf94b9..f42dcabc0 100644
--- a/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
+++ b/main/tests/server/src/com/google/refine/tests/util/PatternSyntaxExceptionParserTests.java
@@ -56,7 +56,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void unmatchedOpeningParenthesisTest(){
         String s = "(abc";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
             Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
@@ -69,7 +69,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void unmatchedClosingParenthesisTest(){
         String s = "abc)";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
             Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
@@ -82,7 +82,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void unmatchedOpeningSquareBracketTest(){
         String s = "[abc";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
             Assert.assertEquals(e.getUserMessage(),
@@ -94,7 +94,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void danglingBackslashTest(){
         String s = "abc\\";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
             Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
@@ -107,7 +107,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void unmatchedOpeningCurlyBracketTest(){
         String s = "abc{3";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
             Assert.assertEquals(e.getUserMessage(),
@@ -119,7 +119,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void illegalQuantifierStatement(){
         String s = "abc{";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
             Assert.assertEquals(e.getUserMessage(),
@@ -131,7 +131,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void quantifierTargetValidityTest(){
         String s = "abc+*";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
             Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
@@ -144,7 +144,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void quantifierMagnitudeTest(){
         String s = "a{4,3}";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
             Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);
@@ -157,7 +157,7 @@ public class PatternSyntaxExceptionParserTests extends RefineTest {
     public void rangeOrderTest(){
         String s = "abc[9-0]";
         try {
-            Pattern pattern = Pattern.compile(s);
+            Pattern.compile(s);
             Assert.assertTrue(false,"Test pattern successfully compiled when it should fail");
         } catch (PatternSyntaxException err) {
             PatternSyntaxExceptionParser e = new PatternSyntaxExceptionParser(err);

From 0c97ad1b697442a215811baecbcc697a03b62ba0 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Wed, 29 Nov 2017 09:47:31 +0000
Subject: [PATCH 8/9] Use string instead of string buffer

---
 .../util/PatternSyntaxExceptionParser.java    | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
index 7a95a897d..ab0724f50 100644
--- a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -81,45 +81,46 @@ public class PatternSyntaxExceptionParser {
     }
 
     public String getUserMessage() {
+        String msg = "";
         StringBuffer sb = new StringBuffer();
         String desc = exception.getDescription();
         switch(desc)
         {
             case "Unclosed character class":
-                sb.append("The regular expression is missing a closing ']' character, or has an empty pair of square brackets '[]'.");
+                msg = "The regular expression is missing a closing ']' character, or has an empty pair of square brackets '[]'.";
                 break;
             case "Unmatched closing ')'":
-                sb.append("The regular expression is missing a opening '(' character.");
+                msg = "The regular expression is missing a opening '(' character.";
                 break;
             case "Unclosed group":
-                sb.append("The regular expression is missing a closing ')' character.");
+                msg = "The regular expression is missing a closing ')' character.";
                 break;
             case "Dangling meta character '*'":
             case "Dangling meta character '+'":
             case "Dangling meta character '?'":
-                sb.append("The regular expression has a '*','+' or '?' in the wrong place.");
+                msg = "The regular expression has a '*','+' or '?' in the wrong place.";
                 break;
             case "Unexpected internal error":
-                sb.append("The regular expression has a backslash '\\' at the end.");
+                msg = "The regular expression has a backslash '\\' at the end.";
                 break;
             case "Unclosed counted closure":
-                sb.append("The regular expression is missing a closing '}' character, or has an incorrect quantifier statement in curly brackets '{}'.");
+                msg = "The regular expression is missing a closing '}' character, or has an incorrect quantifier statement in curly brackets '{}'.";
                 break;
             case "Illegal repetition":
-                sb.append("The regular expression has an incomplete or incorrect quantifier statement in curly brackets '{}'.");
+                msg = "The regular expression has an incomplete or incorrect quantifier statement in curly brackets '{}'.";
                 break;
             case "Illegal repetition range":
-                sb.append("The regular expression has a quantifier statement where the minimum is larger than the maximum (e.g. {4,3}).");
+                msg = "The regular expression has a quantifier statement where the minimum is larger than the maximum (e.g. {4,3}).";
                 break;
             case "Illegal character range":
-                sb.append("The regular expression has a range statement which is incomplete or has the characters in the incorrect order (e.g. [9-0])");
+                msg = "The regular expression has a range statement which is incomplete or has the characters in the incorrect order (e.g. [9-0])";
                 break;
             default:
                 // If no special handling in place fall back on error msg
                 // created by java.util.regex.PatternSyntaxException
-                sb.append(exception.getMessage());
+                msg = exception.getMessage();
                 break;
         }
-        return sb.toString();
+        return msg;
     }
 }

From 02f864121d6e5bcc2724b8241700d8da2c9c3383 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@ostephens.com>
Date: Wed, 29 Nov 2017 11:01:43 +0000
Subject: [PATCH 9/9] Removed unused variable

---
 .../src/com/google/refine/util/PatternSyntaxExceptionParser.java | 1 -
 1 file changed, 1 deletion(-)

diff --git a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
index ab0724f50..050e0032b 100644
--- a/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
+++ b/main/src/com/google/refine/util/PatternSyntaxExceptionParser.java
@@ -82,7 +82,6 @@ public class PatternSyntaxExceptionParser {
 
     public String getUserMessage() {
         String msg = "";
-        StringBuffer sb = new StringBuffer();
         String desc = exception.getDescription();
         switch(desc)
         {