[Gnash-commit] /srv/bzr/gnash/trunk r10018: Test lastIndexOf, fix a bug that would have been obvious with good testing.

[Benjamin Wolsey]

------------------------------------------------------------
revno: 10018
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Mon 2008-10-20 16:27:59 +0200
message:
  Test lastIndexOf, fix a bug that would have been obvious with good testing.
modified:
  libcore/asobj/String_as.cpp
  testsuite/actionscript.all/String.as
    ------------------------------------------------------------
    revno: 10011.1.3
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Mon 2008-10-20 16:13:04 +0200
    message:
      Test lastIndexOf properly and implement it correctly.
      
      Replace ENSURE_FN_ARGS macro with an inline function, give more 
information
      and use the AS function name, not the C function name.
    modified:
      libcore/asobj/String_as.cpp
      testsuite/actionscript.all/String.as

=== modified file 'libcore/asobj/String_as.cpp'
--- a/libcore/asobj/String_as.cpp       2008-10-02 16:39:25 +0000
+++ b/libcore/asobj/String_as.cpp       2008-10-20 14:13:04 +0000
@@ -40,23 +40,36 @@
 #include <locale>
 #include <stdexcept>
 
-#define ENSURE_FN_ARGS(min, max, rv)                                    \
-    if (fn.nargs < min) {                                               \
-        IF_VERBOSE_ASCODING_ERRORS(                                     \
-            log_aserror(_("%s needs one argument"), __FUNCTION__);         \
-            )                                                           \
-         return as_value(rv);                                           \
-    }                                                                   \
-    IF_VERBOSE_ASCODING_ERRORS(                                         \
-        if (fn.nargs > max)                                             \
-            log_aserror(_("%s has more than one argument"), __FUNCTION__); \
-    )
-
-
-
-namespace gnash
+namespace gnash {
+
+/// Check the number of arguments, returning false if there
+/// aren't enough, or true if there are either enough or too many.
+/// Logs an error if the number isn't between min and max.
+inline bool checkArgs(const fn_call& fn, size_t min, size_t max,
+        const std::string& function)
 {
 
+    if (fn.nargs < min) {
+        IF_VERBOSE_ASCODING_ERRORS(
+            std::ostringstream os;
+            fn.dump_args(os);
+                log_aserror(_("%1%(%2%) needs %3% argument(s)"),
+                    function, os.str(), min);
+            )
+         return false;
+    }                          
+    IF_VERBOSE_ASCODING_ERRORS(
+        if (fn.nargs > max)
+        {
+            std::ostringstream os;
+            fn.dump_args(os);
+            log_aserror(_("%1%(%2%) has more than %3% argument(s)"),
+                function, os.str(), max);
+        }
+    );
+    return true;
+}
+
 // Forward declarations
 
 static as_value string_concat(const fn_call& fn);
@@ -236,7 +249,7 @@
 
     std::wstring wstr = utf8::decodeCanonicalString(str, version);
 
-    ENSURE_FN_ARGS(1, 2, as_value());
+    if (!checkArgs(fn, 1, 2, "String.slice()")) return as_value();
 
     size_t start = validIndex(wstr, fn.arg(0).to_int());
 
@@ -389,6 +402,10 @@
     return as_value(array.get());
 }
 
+/// String.lastIndexOf[string[, pos]]
+//
+/// Performs a reverse search for the complete search string, optionally
+/// starting from pos. Returns -1 if not found.
 static as_value
 string_last_index_of(const fn_call& fn)
 {
@@ -397,7 +414,7 @@
     
     const std::string& str = val.to_string();
 
-    ENSURE_FN_ARGS(1, 2, -1);
+    if (!checkArgs(fn, 1, 2, "String.lastIndexOf()")) return as_value(-1);
 
     const std::string& toFind = fn.arg(0).to_string();
 
@@ -411,13 +428,13 @@
         return as_value(-1);
     }
 
-    size_t found = str.find_last_of(toFind, start);
+    size_t found = str.rfind(toFind, start);
 
     if (found == std::string::npos) {
         return as_value(-1);
     }
 
-    return as_value(found - toFind.size() + 1);
+    return as_value(found);
 }
 
 // String.substr(start[, length]).
@@ -437,8 +454,8 @@
 
     std::wstring wstr = utf8::decodeCanonicalString(str, version);
 
-    ENSURE_FN_ARGS(1, 2, str);
-
+    if (!checkArgs(fn, 1, 2, "String.substr()")) return as_value(str);
+    
     int start = validIndex(wstr, fn.arg(0).to_int());
 
     int num = wstr.length();
@@ -477,7 +494,7 @@
 
     const std::wstring& wstr = utf8::decodeCanonicalString(str, version);
 
-    ENSURE_FN_ARGS(1, 2, str);
+    if (!checkArgs(fn, 1, 2, "String.substring()")) return as_value(str);
 
     int start = fn.arg(0).to_int();
     int end = wstr.size();
@@ -522,15 +539,17 @@
 {
     boost::intrusive_ptr<as_object> obj = ensureType<as_object>(fn.this_ptr);
     as_value val(fn.this_ptr);
-    
+ 
+    /// Do not return before this, because the toString method should always
+    /// be called. (TODO: test).   
     const std::string& str = val.to_string();
 
+    if (!checkArgs(fn, 1, 2, "String.indexOf")) return as_value(-1);
+
     int version = obj->getVM().getSWFVersion();
 
     const std::wstring& wstr = utf8::decodeCanonicalString(str, version);
 
-    ENSURE_FN_ARGS(1, 2, -1);
-
     const as_value& tfarg = fn.arg(0); // to find arg
     const std::wstring& toFind = 
utf8::decodeCanonicalString(tfarg.to_string(), version);
 
@@ -655,7 +674,7 @@
 
     const int version = obj->getVM().getSWFVersion();
 
-    ENSURE_FN_ARGS(1, 1, "");
+    if (!checkArgs(fn, 1, 1, "String.charAt()")) return as_value("");
 
     // to_int() makes this safe from overflows.
     const size_t index = static_cast<size_t>(fn.arg(0).to_int());

=== modified file 'testsuite/actionscript.all/String.as'
--- a/testsuite/actionscript.all/String.as      2008-10-20 07:03:54 +0000
+++ b/testsuite/actionscript.all/String.as      2008-10-20 14:13:04 +0000
@@ -108,6 +108,31 @@
 isNaN (a.charAt(21) );
 check_equals ( a.lastIndexOf("lawa"), 8);
 
+// lastIndexOf properly tested.
+r = "abcdefghik7abedc";
+check_equals(r.lastIndexOf("dc"), 14);
+check_equals(r.lastIndexOf("abcde"), 0);
+check_equals(r.lastIndexOf("ab"), 11);
+check_equals(r.lastIndexOf("df"), -1);
+check_equals(r.lastIndexOf("ik"), 8);
+check_equals(r.lastIndexOf("cd"), 2);
+
+check_equals(r.lastIndexOf("edc", 15), 13);
+check_equals(r.lastIndexOf("edc", 14), 13);
+check_equals(r.lastIndexOf("edc", 13), 13);
+check_equals(r.lastIndexOf("edc", 12), -1);
+check_equals(r.lastIndexOf("edc", 11), -1);
+
+check_equals(r.lastIndexOf("ghi", "rt"), -1);
+check_equals(r.lastIndexOf("ghi", 15, 8), 6);
+check_equals(r.lastIndexOf("ghi", 15, 8, 9), 6);
+check_equals(r.lastIndexOf("ghi", -1), -1);
+check_equals(r.lastIndexOf("ghi", 17287638764), 6);
+
+check_equals(r.lastIndexOf(""), 16);
+check_equals(r.lastIndexOf(7), 10);
+
+
 // Applied to object.
 o = new Object;
 o.charCodeAt = String.prototype.charCodeAt;
@@ -1271,7 +1296,7 @@
 
 //----- END OF TESTS
 
-var baseTests = 301;
+var baseTests = 319;
 var asmTests = 23;
 var ge6Tests = 16;