In Test/System.Globalization:
authorSebastien Pouliot <sebastien@ximian.com>
Thu, 29 Oct 2009 14:11:53 +0000 (14:11 -0000)
committerSebastien Pouliot <sebastien@ximian.com>
Thu, 29 Oct 2009 14:11:53 +0000 (14:11 -0000)
2009-10-29  Sebastien Pouliot  <sebastien@ximian.com>

* CompareInfoTest.cs: Add test cases to validate parameters

In System.Globalization:
2009-10-29  Sebastien Pouliot  <sebastien@ximian.com>

* CompareInfo.cs: Add/fix CompareOptions validations. Reduce code
duplication in Compare methods (wrt CompareOptions). Fix
calculation bug in Compare(string,int.string,int,CompareOptions).

svn path=/trunk/mcs/; revision=145023

mcs/class/corlib/System.Globalization/ChangeLog
mcs/class/corlib/System.Globalization/CompareInfo.cs
mcs/class/corlib/Test/System.Globalization/ChangeLog
mcs/class/corlib/Test/System.Globalization/CompareInfoTest.cs

index 6efc232e25f2cbd1a3b2c117cbe5922796bdc324..a6fb0d7bc857d2ac743e8d045456d8ab1aff9e1a 100644 (file)
@@ -1,3 +1,9 @@
+2009-10-29  Sebastien Pouliot  <sebastien@ximian.com>
+
+       * CompareInfo.cs: Add/fix CompareOptions validations. Reduce code
+       duplication in Compare methods (wrt CompareOptions). Fix 
+       calculation bug in Compare(string,int.string,int,CompareOptions).
+
 2009-10-27  Sebastien Pouliot  <sebastien@ximian.com>
 
        * TextInfo.cs: Avoid allocating zero-length strings in ToLower 
index 98a325d9459257d047d8e3394ec9136768463ad5..f72ce7390f680ec729685b8b75f4609b970682f5 100644 (file)
@@ -103,6 +103,12 @@ namespace System.Globalization
                        get { return true; }
                }
 #endif
+               const CompareOptions ValidCompareOptions_NoStringSort =
+                       CompareOptions.None | CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace |
+                       CompareOptions.IgnoreSymbols | CompareOptions.IgnoreKanaType | CompareOptions.IgnoreWidth |
+                       CompareOptions.OrdinalIgnoreCase | CompareOptions.Ordinal;
+
+               const CompareOptions ValidCompareOptions = ValidCompareOptions_NoStringSort | CompareOptions.StringSort;
 
                // Keep in synch with MonoCompareInfo in the runtime. 
                private int culture;
@@ -190,26 +196,15 @@ namespace System.Globalization
 #endif
                public virtual int Compare (string string1, string string2)
                {
-                       if (string1 == null) {
-                               if (string2 == null)
-                                       return 0;
-                               return -1;
-                       }
-                       if (string2 == null)
-                               return 1;
-                               
-                       /* Short cut... */
-                       if(string1.Length == 0 && string2.Length == 0)
-                               return(0);
-
-                       return(internal_compare_switch (string1, 0, string1.Length,
-                                                string2, 0, string2.Length,
-                                                CompareOptions.None));
+                       return Compare (string1, string2, CompareOptions.None);
                }
 
                public virtual int Compare (string string1, string string2,
                                            CompareOptions options)
                {
+                       if ((options & ValidCompareOptions) != options)
+                               throw new ArgumentException ("options");
+
                        if (string1 == null) {
                                if (string2 == null)
                                        return 0;
@@ -230,46 +225,16 @@ namespace System.Globalization
                public virtual int Compare (string string1, int offset1,
                                            string string2, int offset2)
                {
-                       if (string1 == null) {
-                               if (string2 == null)
-                                       return 0;
-                               return -1;
-                       }
-                       if (string2 == null)
-                               return 1;
-
-                       /* Not in the spec, but ms does these short
-                        * cuts before checking the offsets (breaking
-                        * the offset >= string length specified check
-                        * in the process...)
-                        */
-                       if ((string1.Length == 0 || offset1 == string1.Length) &&
-                               (string2.Length == 0 || offset2 == string2.Length))
-                               return(0);
-
-                       if(offset1 < 0 || offset2 < 0) {
-                               throw new ArgumentOutOfRangeException ("Offsets must not be less than zero");
-                       }
-                       
-                       if(offset1 > string1.Length) {
-                               throw new ArgumentOutOfRangeException ("Offset1 is greater than or equal to the length of string1");
-                       }
-                       
-                       if(offset2 > string2.Length) {
-                               throw new ArgumentOutOfRangeException ("Offset2 is greater than or equal to the length of string2");
-                       }
-                       
-                       return(internal_compare_switch (string1, offset1,
-                                                string1.Length-offset1,
-                                                string2, offset2,
-                                                string2.Length-offset2,
-                                                CompareOptions.None));
+                       return Compare (string1, offset1, string2, offset2, CompareOptions.None);
                }
 
                public virtual int Compare (string string1, int offset1,
                                            string string2, int offset2,
                                            CompareOptions options)
                {
+                       if ((options & ValidCompareOptions) != options)
+                               throw new ArgumentException ("options");
+
                        if (string1 == null) {
                                if (string2 == null)
                                        return 0;
@@ -302,7 +267,7 @@ namespace System.Globalization
                        return(internal_compare_switch (string1, offset1,
                                                 string1.Length-offset1,
                                                 string2, offset2,
-                                                string2.Length-offset1,
+                                                string2.Length-offset2,
                                                 options));
                }
 
@@ -310,51 +275,7 @@ namespace System.Globalization
                                            int length1, string string2,
                                            int offset2, int length2)
                {
-                       if (string1 == null) {
-                               if (string2 == null)
-                                       return 0;
-                               return -1;
-                       }
-                       if (string2 == null)
-                               return 1;
-
-                       /* Not in the spec, but ms does these short
-                        * cuts before checking the offsets (breaking
-                        * the offset >= string length specified check
-                        * in the process...)
-                        */
-                       if((string1.Length == 0 ||
-                               offset1 == string1.Length ||
-                               length1 == 0) &&
-                               (string2.Length == 0 ||
-                               offset2 == string2.Length ||
-                               length2 == 0))
-                               return(0);
-
-                       if(offset1 < 0 || length1 < 0 ||
-                          offset2 < 0 || length2 < 0) {
-                               throw new ArgumentOutOfRangeException ("Offsets and lengths must not be less than zero");
-                       }
-                       
-                       if(offset1 > string1.Length) {
-                               throw new ArgumentOutOfRangeException ("Offset1 is greater than or equal to the length of string1");
-                       }
-                       
-                       if(offset2 > string2.Length) {
-                               throw new ArgumentOutOfRangeException ("Offset2 is greater than or equal to the length of string2");
-                       }
-                       
-                       if(length1 > string1.Length-offset1) {
-                               throw new ArgumentOutOfRangeException ("Length1 is greater than the number of characters from offset1 to the end of string1");
-                       }
-                       
-                       if(length2 > string2.Length-offset2) {
-                               throw new ArgumentOutOfRangeException ("Length2 is greater than the number of characters from offset2 to the end of string2");
-                       }
-                       
-                       return(internal_compare_switch (string1, offset1, length1,
-                                                string2, offset2, length2,
-                                                CompareOptions.None));
+                       return Compare (string1, offset1, length1, string2, offset2, length2, CompareOptions.None);
                }
 
                public virtual int Compare (string string1, int offset1,
@@ -362,6 +283,9 @@ namespace System.Globalization
                                            int offset2, int length2,
                                            CompareOptions options)
                {
+                       if ((options & ValidCompareOptions) != options)
+                               throw new ArgumentException ("options");
+
                        if (string1 == null) {
                                if (string2 == null)
                                        return 0;
@@ -620,9 +544,8 @@ namespace System.Globalization
                        if(count<0 || (source.Length - startIndex) < count) {
                                throw new ArgumentOutOfRangeException ("count");
                        }
-                       if((options & CompareOptions.StringSort)!=0) {
-                               throw new ArgumentException ("StringSort is not a valid CompareOption for this method");
-                       }
+                       if ((options & ValidCompareOptions_NoStringSort) != options)
+                               throw new ArgumentException ("options");
                        
                        if(count==0) {
                                return(-1);
@@ -693,6 +616,8 @@ namespace System.Globalization
                        if(count<0 || (source.Length - startIndex) < count) {
                                throw new ArgumentOutOfRangeException ("count");
                        }
+                       if ((options & ValidCompareOptions_NoStringSort) != options)
+                               throw new ArgumentException ("options");
                        if(value.Length==0) {
                                return(0);
                        }
@@ -719,6 +644,9 @@ namespace System.Globalization
                                throw new ArgumentNullException("prefix");
                        }
 
+                       if ((options & ValidCompareOptions_NoStringSort) != options)
+                               throw new ArgumentException ("options");
+
                        if (UseManagedCollation)
                                return collator.IsPrefix (source, prefix, options);
 
@@ -746,6 +674,9 @@ namespace System.Globalization
                                throw new ArgumentNullException("suffix");
                        }
 
+                       if ((options & ValidCompareOptions_NoStringSort) != options)
+                               throw new ArgumentException ("options");
+
                        if (UseManagedCollation)
                                return collator.IsSuffix (source, suffix, options);
 
@@ -846,9 +777,8 @@ namespace System.Globalization
                        if(count < 0 || (startIndex - count) < -1) {
                                throw new ArgumentOutOfRangeException("count");
                        }
-                       if((options & CompareOptions.StringSort)!=0) {
-                               throw new ArgumentException ("StringSort is not a valid CompareOption for this method");
-                       }
+                       if ((options & ValidCompareOptions_NoStringSort) != options)
+                               throw new ArgumentException ("options");
                        
                        if(count==0) {
                                return(-1);
@@ -886,6 +816,8 @@ namespace System.Globalization
                        if(count < 0 || (startIndex - count) < -1) {
                                throw new ArgumentOutOfRangeException("count");
                        }
+                       if ((options & ValidCompareOptions_NoStringSort) != options)
+                               throw new ArgumentException ("options");
                        if(count == 0) {
                                return(-1);
                        }
index c9f8eabfe108e1fa92d33f2df4c79ec5dfce6a72..0e0243006c60bc72636bfd622ae16f8cee363c98 100644 (file)
@@ -1,3 +1,7 @@
+2009-10-29  Sebastien Pouliot  <sebastien@ximian.com>
+
+       * CompareInfoTest.cs: Add test cases to validate parameters
+
 2009-06-25  Zoltan Varga  <vargaz@gmail.com>
 
        * *.cs: Convert all tests to new-style nunit classes/methods.   
index 421db927cbefc428b04020eff8abcb29e72ac759..67d3a2fe7f787f531306c49a025641cbed402890 100644 (file)
@@ -1213,6 +1213,160 @@ public class CompareInfoTest
                Assert.IsTrue ("aaaa".StartsWith ("A", StringComparison.OrdinalIgnoreCase));
        }
 #endif
+
+       [Test]
+       [ExpectedException (typeof (ArgumentNullException))]
+       public void IsPrefix_SourceNull ()
+       {
+               invariant.IsPrefix (null, "b");
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentNullException))]
+       public void IsPrefix_PrefixNull ()
+       {
+               invariant.IsPrefix ("a", null, CompareOptions.None);
+       }
+
+       [Test]
+       public void IsPrefix_PrefixEmpty ()
+       {
+               Assert.IsTrue (invariant.IsPrefix ("a", String.Empty));
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IsPrefix_CompareOptions_Invalid ()
+       {
+               invariant.IsPrefix ("a", "b", (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IsPrefix_CompareOptions_StringSort ()
+       {
+               invariant.IsPrefix ("a", "b", CompareOptions.StringSort);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentNullException))]
+       public void IsSuffix_SourceNull ()
+       {
+               invariant.IsSuffix (null, "b");
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentNullException))]
+       public void IsSuffix_SuffixNull ()
+       {
+               invariant.IsSuffix ("a", null, CompareOptions.None);
+       }
+
+       [Test]
+       public void IsSuffix_PrefixEmpty ()
+       {
+               Assert.IsTrue (invariant.IsSuffix ("a", String.Empty));
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IsSuffix_CompareOptions_Invalid ()
+       {
+               invariant.IsSuffix ("a", "b", (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IsSuffix_CompareOptions_StringSort ()
+       {
+               invariant.IsSuffix ("a", "b", CompareOptions.StringSort);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void Compare_String_String_CompareOptions_Invalid ()
+       {
+               // validation of CompareOptions is made before null checks
+               invariant.Compare (null, null, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       public void Compare_String_String_CompareOptions_StringSort ()
+       {
+               // StringSort is valid for Compare only
+               Assert.AreEqual (-1, invariant.Compare ("a", "b", CompareOptions.StringSort));
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void Compare_String_Int_String_Int_CompareOptions_Invalid ()
+       {
+               invariant.Compare (null, 0, null, 0, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void Compare_String_Int_Int_String_Int_Int_CompareOptions_Invalid ()
+       {
+               invariant.Compare (null, 0, 0, null, 0, 0, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IndexOf_String_Char_Int_Int_CompareOptions_Invalid ()
+       {
+               invariant.IndexOf ("a", 'a', 0, 1, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IndexOf_String_Char_Int_Int_CompareOptions_StringSort ()
+       {
+               invariant.IndexOf ("a", 'a', 0, 1, CompareOptions.StringSort);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IndexOf_String_String_Int_Int_CompareOptions_Invalid ()
+       {
+               invariant.IndexOf ("a", "a", 0, 1, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void IndexOf_String_String_Int_Int_CompareOptions_StringSort ()
+       {
+               invariant.IndexOf ("a", "a", 0, 1, CompareOptions.StringSort);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void LastIndexOf_String_Char_Int_Int_CompareOptions_Invalid ()
+       {
+               invariant.LastIndexOf ("a", 'a', 0, 1, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void LastIndexOf_String_Char_Int_Int_CompareOptions_StringSort ()
+       {
+               invariant.LastIndexOf ("a", 'a', 0, 1, CompareOptions.StringSort);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void LastIndexOf_String_String_Int_Int_CompareOptions_Invalid ()
+       {
+               invariant.LastIndexOf ("a", "a", 0, 1, (CompareOptions) Int32.MinValue);
+       }
+
+       [Test]
+       [ExpectedException (typeof (ArgumentException))]
+       public void LastIndexOf_String_String_Int_Int_CompareOptions_StringSort ()
+       {
+               invariant.LastIndexOf ("a", "a", 0, 1, CompareOptions.StringSort);
+       }
 }
 
 }
+