fixed escaping of sheet names. Thanks to gallonfizik. This closes #134

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1847418 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/formula/SheetNameFormatter.java

@@ -59,6 +59,7 @@ public final class SheetNameFormatter {
      * @param rawSheetName - sheet name
      * @deprecated use <code>appendFormat(StringBuilder out, String rawSheetName)</code> instead
 	 */
+	@Deprecated
 	public static void appendFormat(StringBuffer out, String rawSheetName) {
 		boolean needsQuotes = needsDelimiting(rawSheetName);
 		if(needsQuotes) {
@@ -73,6 +74,7 @@ public final class SheetNameFormatter {
     /**
      * @deprecated use <code>appendFormat(StringBuilder out, String workbookName, String rawSheetName)</code> instead
      */
+    @Deprecated
 	public static void appendFormat(StringBuffer out, String workbookName, String rawSheetName) {
 		boolean needsQuotes = needsDelimiting(workbookName) || needsDelimiting(rawSheetName);
 		if(needsQuotes) {
@@ -123,7 +125,7 @@ public final class SheetNameFormatter {
 		}
 	}
 
-    private static void appendAndEscape(Appendable sb, String rawSheetName) {
+    static void appendAndEscape(Appendable sb, String rawSheetName) {
         int len = rawSheetName.length();
         for(int i=0; i<len; i++) {
             char ch = rawSheetName.charAt(i);
@@ -139,7 +141,12 @@ public final class SheetNameFormatter {
         }
     }
 
-	private static boolean needsDelimiting(String rawSheetName) {
+	/**
+	 * Tell if the given raw sheet name needs screening/delimiting.
+	 * @param rawSheetName the sheet name.
+	 * @return true if the given raw sheet name needs screening/delimiting, false otherwise.
+	 */
+	static boolean needsDelimiting(String rawSheetName) {
 		int len = rawSheetName.length();
 		if(len < 1) {
 			throw new RuntimeException("Zero length string is an invalid sheet name");

src/java/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatter.java

@@ -0,0 +1,73 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.ss.formula;
+
+public class SheetRangeAndWorkbookIndexFormatter {
+    private SheetRangeAndWorkbookIndexFormatter() {
+    }
+
+    public static String format(StringBuilder sb, int workbookIndex, String firstSheetName, String lastSheetName) {
+        if (anySheetNameNeedsEscaping(firstSheetName, lastSheetName)) {
+            return formatWithDelimiting(sb, workbookIndex, firstSheetName, lastSheetName);
+        } else {
+            return formatWithoutDelimiting(sb, workbookIndex, firstSheetName, lastSheetName);
+        }
+    }
+
+    private static String formatWithDelimiting(StringBuilder sb, int workbookIndex, String firstSheetName, String lastSheetName) {
+        sb.append('\'');
+        if (workbookIndex >= 0) {
+            sb.append('[');
+            sb.append(workbookIndex);
+            sb.append(']');
+        }
+
+        SheetNameFormatter.appendAndEscape(sb, firstSheetName);
+
+        if (lastSheetName != null) {
+            sb.append(':');
+            SheetNameFormatter.appendAndEscape(sb, lastSheetName);
+        }
+
+        sb.append('\'');
+        return sb.toString();
+    }
+
+    private static String formatWithoutDelimiting(StringBuilder sb, int workbookIndex, String firstSheetName, String lastSheetName) {
+        if (workbookIndex >= 0) {
+            sb.append('[');
+            sb.append(workbookIndex);
+            sb.append(']');
+        }
+
+        sb.append(firstSheetName);
+
+        if (lastSheetName != null) {
+            sb.append(':');
+            sb.append(lastSheetName);
+        }
+
+        return sb.toString();
+    }
+
+    private static boolean anySheetNameNeedsEscaping(String firstSheetName, String lastSheetName) {
+        boolean anySheetNameNeedsDelimiting = firstSheetName != null && SheetNameFormatter.needsDelimiting(firstSheetName);
+        anySheetNameNeedsDelimiting |= lastSheetName != null && SheetNameFormatter.needsDelimiting(lastSheetName);
+        return anySheetNameNeedsDelimiting;
+    }
+}

src/java/org/apache/poi/ss/formula/ptg/Area3DPxg.java

@@ -20,6 +20,7 @@ package org.apache.poi.ss.formula.ptg;
 import org.apache.poi.ss.SpreadsheetVersion;
 import org.apache.poi.ss.formula.SheetIdentifier;
 import org.apache.poi.ss.formula.SheetNameFormatter;
+import org.apache.poi.ss.formula.SheetRangeAndWorkbookIndexFormatter;
 import org.apache.poi.ss.formula.SheetRangeIdentifier;
 import org.apache.poi.ss.util.AreaReference;
 import org.apache.poi.util.LittleEndianOutput;
@@ -102,16 +103,8 @@ public final class Area3DPxg extends AreaPtgBase implements Pxg3D {
     
     public String toFormulaString() {
         StringBuilder sb = new StringBuilder(64);
-        if (externalWorkbookNumber >= 0) {
+
-            sb.append('[');
+        SheetRangeAndWorkbookIndexFormatter.format(sb, externalWorkbookNumber, firstSheetName, lastSheetName);
-            sb.append(externalWorkbookNumber);
-            sb.append(']');
-        }
-        SheetNameFormatter.appendFormat(sb, firstSheetName);
-        if (lastSheetName != null) {
-            sb.append(':');
-            SheetNameFormatter.appendFormat(sb, lastSheetName);
-        }
         sb.append('!');
         sb.append(formatReferenceAsString());
         return sb.toString();

src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java

@@ -18,7 +18,7 @@
 package org.apache.poi.ss.formula.ptg;
 
 import org.apache.poi.ss.formula.SheetIdentifier;
-import org.apache.poi.ss.formula.SheetNameFormatter;
+import org.apache.poi.ss.formula.SheetRangeAndWorkbookIndexFormatter;
 import org.apache.poi.ss.formula.SheetRangeIdentifier;
 import org.apache.poi.ss.util.CellReference;
 import org.apache.poi.util.LittleEndianOutput;
@@ -101,18 +101,8 @@ public final class Ref3DPxg extends RefPtgBase implements Pxg3D {
 
     public String toFormulaString() {
         StringBuilder sb = new StringBuilder(64);
-        if (externalWorkbookNumber >= 0) {
+
-            sb.append('[');
+        SheetRangeAndWorkbookIndexFormatter.format(sb, externalWorkbookNumber, firstSheetName, lastSheetName);
-            sb.append(externalWorkbookNumber);
-            sb.append(']');
-        }
-        if (firstSheetName != null) {
-            SheetNameFormatter.appendFormat(sb, firstSheetName);
-        }
-        if (lastSheetName != null) {
-            sb.append(':');
-            SheetNameFormatter.appendFormat(sb, lastSheetName);
-        }
         sb.append('!');
         sb.append(formatReferenceAsString());
         return sb.toString();

src/testcases/org/apache/poi/ss/formula/SheetRangeAndWorkbookIndexFormatterTest.java

@@ -0,0 +1,66 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.ss.formula;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class SheetRangeAndWorkbookIndexFormatterTest {
+    @Test
+    public void noDelimiting_ifASingleSheetNameDoesntNeedDelimiting() {
+        StringBuilder sb = new StringBuilder();
+        String result = SheetRangeAndWorkbookIndexFormatter.format(sb, 0, "noDelimiting", null);
+        assertEquals("[0]noDelimiting", result);
+    }
+
+    @Test
+    public void everythingIsScreened_ifASingleSheetNameNeedsDelimiting() {
+        StringBuilder sb = new StringBuilder();
+        String result = SheetRangeAndWorkbookIndexFormatter.format(sb, 0, "1delimiting", null);
+        assertEquals("'[0]1delimiting'", result);
+    }
+
+    @Test
+    public void noDelimiting_ifBothSheetNamesDontNeedDelimiting() {
+        StringBuilder sb = new StringBuilder();
+        String result = SheetRangeAndWorkbookIndexFormatter.format(sb, 0, "noDelimiting1", "noDelimiting2");
+        assertEquals("[0]noDelimiting1:noDelimiting2", result);
+    }
+
+    @Test
+    public void everythingIsScreened_ifFirstSheetNamesNeedsDelimiting() {
+        StringBuilder sb = new StringBuilder();
+        String result = SheetRangeAndWorkbookIndexFormatter.format(sb, 0, "1delimiting", "noDelimiting");
+        assertEquals("'[0]1delimiting:noDelimiting'", result);
+    }
+
+    @Test
+    public void everythingIsScreened_ifLastSheetNamesNeedsDelimiting() {
+        StringBuilder sb = new StringBuilder();
+        String result = SheetRangeAndWorkbookIndexFormatter.format(sb, 0, "noDelimiting", "1delimiting");
+        assertEquals("'[0]noDelimiting:1delimiting'", result);
+    }
+
+    @Test
+    public void everythingIsScreened_ifBothSheetNamesNeedDelimiting() {
+        StringBuilder sb = new StringBuilder();
+        String result = SheetRangeAndWorkbookIndexFormatter.format(sb, 0, "1delimiting", "2delimiting");
+        assertEquals("'[0]1delimiting:2delimiting'", result);
+    }
+}