Add and test proper quoting facility in parenthesized arch arguments

Author: manning

src/edu/stanford/nlp/tagger/maxent/Extractor.java

@@ -8,7 +8,7 @@
 
 import java.io.Serializable;
 
-// import edu.stanford.nlp.util.logging.Redwood;
+import edu.stanford.nlp.util.StringUtils;
 
 
 /**
@@ -105,7 +105,7 @@ protected void setGlobalHolder(MaxentTagger tagger) {}
    *  @param tag The possible tag that the feature will be generated for
    *  @return Whether the feature extractor is applicable (true) or not (false)
    */
-  @SuppressWarnings({"MethodMayBeStatic", "UnusedDeclaration"})
+  @SuppressWarnings({"UnusedDeclaration"})
   public boolean precondition(String tag) {
     return true;
   }
@@ -238,22 +238,30 @@ public String toString() {
 
 
   /** This is used for argument parsing in arch variable.
-   *  It can extract a comma separated argument.
-   *  Assumes the input format is "name(arg,arg,arg)".
+   *  It can extract from a comma separated values argument list.
+   *  Values can be quoted with double quotes (with a second double quote as double quote escape char)
+   *  like in a regular CSV file. It assumes the input format is "name(arg,arg,arg)".
    *
    *  @param str arch variable component input
-   *  @param num Number of argument
+   *  @param num Number of argument. Numbers are 1-indexed (i.e., start from 1 not 0)
    *  @return The parenthesized String, or null if none.
    */
   static String getParenthesizedArg(String str, int num) {
-    String[] args = str.split("\\s*[,()]\\s*");
-    if (args.length <= num) {
-      return null;
+    int left = str.indexOf('(');
+    int right = str.lastIndexOf(')');
+    if (left < 0 || right <= left) {
+      throw new IllegalArgumentException("getParenthesizedArg: Bad format String: " + str);
     }
+    String argStr = str.substring(left + 1, right);
+    String[] args = StringUtils.splitOnCharWithQuoting(argStr, ',', '"', '"');
     // log.info("getParenthesizedArg split " + str + " into " + args.length + " pieces; returning number " + num);
     // for (int i = 0; i < args.length; i++) {
     //   log.info("  " + args[i]);
     // }
+    num--;
+    if (args.length <= num || num < 0) {
+      return null;
+    }
     return args[num];
   }
 
@@ -266,11 +274,12 @@ static String getParenthesizedArg(String str, int num) {
    *  @param num Number of argument
    *  @return The int value of the arg or 0 if missing or empty
    */
+  @SuppressWarnings("ConstantConditions")
   static int getParenthesizedNum(String str, int num) {
-    String[] args = str.split("\\s*[,()]\\s*");
+    String arg = getParenthesizedArg(str, num);
     int ans = 0;
     try {
-      ans = Integer.parseInt(args[num]);
+      ans = Integer.parseInt(arg);
     } catch (NumberFormatException | ArrayIndexOutOfBoundsException nfe) {
       // just leave ans as 0
     }

test/src/edu/stanford/nlp/tagger/maxent/ExtractorTest.java

@@ -1,4 +1,23 @@
 package edu.stanford.nlp.tagger.maxent;
 
+import org.junit.Assert;
+import org.junit.Test;
+
 public class ExtractorTest {
+
+  @Test
+  public void testGetParenthesizedArg() {
+    Assert.assertEquals("yellow", Extractor.getParenthesizedArg("foo(bar,gak,yellow)", 3));
+    Assert.assertEquals("ye(,)ow", Extractor.getParenthesizedArg("foo(bar,gak,\"ye(,)ow\")", 3));
+    Assert.assertEquals("ye()ow", Extractor.getParenthesizedArg("foo(bar,gak,ye()ow)", 3));
+    Assert.assertEquals("'\"", Extractor.getParenthesizedArg("foo(\"'\"\"\")", 1));
+    Assert.assertEquals("'\"", Extractor.getParenthesizedArg("foo(\"bar\",\"'\"\"\",gak)", 2));
+    Assert.assertNull(Extractor.getParenthesizedArg("foo(bar,gak,yellow)", 0));
+    Assert.assertNull(Extractor.getParenthesizedArg("foo(bar,gak,yellow)", 4));
+    Assert.assertEquals(-15, Extractor.getParenthesizedNum("foo(bar,-15,yellow)", 2));
+    Assert.assertEquals(0, Extractor.getParenthesizedNum("foo(bar,gak,yellow)", 0));
+    Assert.assertEquals(0, Extractor.getParenthesizedNum("foo(bar,gak,yellow)", 3));
+    Assert.assertEquals(0, Extractor.getParenthesizedNum("foo(bar,gak,yellow)", 4));
+  }
+
 }