Merge lp:~zorba-coders/zorba/csv-fixes into lp:zorba

[Paul J. Lucas]

1. Now reporting error location for objects that are not convertible to string.
2. Now checking to ensure field-names is an array.
3. Better error messages.
4. Fixed some comments.

1. Now reporting error location for objects that are not convertible to string.
2. Now checking to ensure field-names is an array.
3. Better error messages.
4. Fixed some comments.

[Paul J. Lucas (paul-lucas)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/csv-fixes/+merge/202019

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

[Matthias Brantner (matthias-brantner)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue succeeded - proposal merged!

=== modified file 'NOTICE.txt'
--- NOTICE.txt	2014-01-10 02:36:58 +0000
+++ NOTICE.txt	2014-01-16 23:23:15 +0000
@@ -482,7 +482,8 @@
 Copyright: 2000 D. J. Bernstein
 Website: http://cr.yp.to/ftpparse.html
 
-      Commercial use is fine, if you let me know what programs you're using this in.
+      Commercial use is fine, if you let me know what programs you're
+      using this in.
     
 
 External libraries used by this project:

=== modified file 'include/zorba/internal/ztd.h'
--- include/zorba/internal/ztd.h	2013-12-14 00:45:54 +0000
+++ include/zorba/internal/ztd.h	2014-01-16 23:23:15 +0000
@@ -92,8 +92,9 @@
   typedef char yes[2];
 
   /**
-   * This dummy class is used to make the matching of the dummy operator<<()
-   * \e worse than the global \c operator<<(), if any.
+   * This dummy class is used to make the matching of the dummy
+   * \c operator&lt;&lt;() \e worse than the global \c operator&lt;&lt;(),
+   * if any.
    */
   struct any_t {
     template<typename T> any_t( T const& );
@@ -101,23 +102,24 @@
 
   /**
    * This dummy operator is matched only when there is \e no global
-   * operator<<() otherwise declared for type \c T.
+   * \c operator&lt;&lt;() otherwise declared for type \c T.
    *
    * @return Returns a \c no that selects defined(no).
    */
   no operator<<( std::ostream const&, any_t const& );
 
   /**
-   * This function is matched only when there \e is a global \c operator<<()
-   * declared for type \c T because \c operator<<()'s return type is
-   * \c std::ostream&.
+   * This function is matched only when there \e is a global
+   * \c operator&lt;&lt;() declared for type \c T because
+   * \c operator&lt;&lt;()'s return type is \c std::ostream&.
    *
    * @return Returns a yes& whose \c sizeof() equals \c sizeof(yes).
    */
   yes& defined( std::ostream& );
 
   /**
-   * This function is matched only when the dummy \c operator<<() is matched.
+   * This function is matched only when the dummy \c operator&lt;&lt;() is
+   * matched.
    *
    * @return Returns a no whose \c sizeof() equals \c sizeof(no).
    */
@@ -125,8 +127,10 @@
 
   /**
    * The implementation class that can be used to determine whether a given
-   * type \c T has a global <code>std::ostream& operator<<(std::ostream&,T
-   * const&)</code> defined for it.  However, do not use this class directly.
+   * type \c T has a global
+   * <code>std::ostream& operator&lt;&lt;(std::ostream&,T const&)</code>
+   * defined for it.
+   * However, do not use this class directly.
    *
    * @tparam T The type to check.
    */
@@ -136,8 +140,8 @@
     static T const &t;
   public:
     /**
-     * This is \c true only when the type \c T has a global \c operator<<()
-     * declared for it.
+     * This is \c true only when the type \c T has a global
+     * \c operator&lt;&lt;() declared for it.
      * \hideinitializer
      */
     static bool const value = sizeof( defined( s << t ) ) == sizeof( yes );
@@ -147,8 +151,8 @@
 /**
  * \internal
  * A class that can be used to determine whether a given type \c T has a global
- * <code>std::ostream& operator<<(std::ostream&,T const&)</code> defined for
- * it.
+ * <code>std::ostream& operator&lt;&lt;(std::ostream&,T const&)</code> defined
+ * for it.
  * For example:
  * \code
  * template<typename T> inline

=== modified file 'include/zorba/pregenerated/diagnostic_list.h'
--- include/zorba/pregenerated/diagnostic_list.h	2013-12-18 06:15:17 +0000
+++ include/zorba/pregenerated/diagnostic_list.h	2014-01-16 23:23:15 +0000
@@ -1001,6 +1001,8 @@
 
 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode INVALID_OPTION;
 
+extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode INVALID_CSV_VALUE;
+
 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode MISSING_VALUE;
 
 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode EXTRA_VALUE;

=== modified file 'modules/json/json-csv.jq'
--- modules/json/json-csv.jq	2013-09-26 23:15:11 +0000
+++ modules/json/json-csv.jq	2014-01-16 23:23:15 +0000
@@ -134,9 +134,14 @@
  :  </dl>
  : @return a sequence of zero or more JSON objects where each key is a field
  : name and each value is a parsed value.
- : @error csv:INVALID_OPTION if the <code>quote-char</code>,
- : <code>quote-escape</code>, or <code>separator</code> option is given
- : and it's not a single ASCII character.
+ : @error csv:INVALID_OPTION if a value of <code>field-names</code> is not an
+ : array or one of the array values is not a string;
+ : if the <code>quote-char</code>, <code>quote-escape</code>, or
+ : <code>separator</code> option is given and it's not a single ASCII
+ : character.
+ : @error csv:INVALID_CSV_VALUE if a value of some key is not castable to
+ : string.
+ : <code>field-names</code> option is not a string.
  : @error csv:MISSING_VALUE if a missing value is detected and the
  : <code>missing-value</code> option is "<code>error</code>".
  : @error csv:EXTRA_VALUE if an extra value is detected and the
@@ -294,6 +299,11 @@
  :  </dl>
  : @return a sequence of strings where each string corresponds to a JSON
  : object, aka, "record."
+ : @error csv:INVALID_OPTION if a value of <code>field-names</code> is not an
+ : array or one of the array values is not a string;
+ : if the <code>quote-char</code>, <code>quote-escape</code>, or
+ : <code>separator</code> option is given and it's not a single ASCII
+ : character.
  :)
 declare function csv:serialize( $obj as object()*, $options as object() )
   as string* external;

=== modified file 'src/diagnostics/diagnostic_en.xml'
--- src/diagnostics/diagnostic_en.xml	2013-12-18 06:15:17 +0000
+++ src/diagnostics/diagnostic_en.xml	2014-01-16 23:23:15 +0000
@@ -3777,6 +3777,9 @@
 
     <diagnostic code="INVALID_OPTION">
       <value>${"1": }invalid value for "$2" option${; 3}</value>
+      <entry key="MustBeArray">
+        <value>must be JSON array</value>
+      </entry>
       <entry key="MustBeASCIIChar">
         <value>must be single ASCII character</value>
       </entry>
@@ -3789,6 +3792,13 @@
       <entry key="MustBeString">
         <value>must be string</value>
       </entry>
+      <entry key="ArrayElementsMustBeString">
+        <value>array elements must be string</value>
+      </entry>
+    </diagnostic>
+
+    <diagnostic code="INVALID_CSV_VALUE">
+      <value>"$1": invalid type for value of key "$2"</value>
     </diagnostic>
 
     <diagnostic code="MISSING_VALUE">
@@ -3807,7 +3817,7 @@
 
   </namespace>
 
-  <!--////////// dateTime Module Errors //////////////////////////////////////////-->
+  <!--////////// dateTime Module Errors ////////////////////////////////////-->
 
   <namespace prefix="dt">
     <diagnostic code="INVALID_SPECIFICATION">
@@ -3843,7 +3853,7 @@
     </diagnostic>
   </namespace>
   
-      <!-- /////////// URI procesing errors ///////////////////////////////// -->
+  <!-- /////////// URI procesing errors /////////////////////////////////// -->
 
   <namespace prefix="zuri">
   
@@ -3874,7 +3884,6 @@
     
   </namespace>
 
-
   <!--////////// Subvalues /////////////////////////////////////////////////-->
 
   <subvalues>

=== modified file 'src/diagnostics/pregenerated/diagnostic_list.cpp'
--- src/diagnostics/pregenerated/diagnostic_list.cpp	2013-12-18 06:15:17 +0000
+++ src/diagnostics/pregenerated/diagnostic_list.cpp	2014-01-16 23:23:15 +0000
@@ -1473,6 +1473,9 @@
 ZorbaCSVErrorCode INVALID_OPTION( "INVALID_OPTION" );
 
 
+ZorbaCSVErrorCode INVALID_CSV_VALUE( "INVALID_CSV_VALUE" );
+
+
 ZorbaCSVErrorCode MISSING_VALUE( "MISSING_VALUE" );
 
 

=== modified file 'src/diagnostics/pregenerated/dict_en.cpp'
--- src/diagnostics/pregenerated/dict_en.cpp	2013-12-18 06:15:17 +0000
+++ src/diagnostics/pregenerated/dict_en.cpp	2014-01-16 23:23:15 +0000
@@ -116,6 +116,7 @@
   { "INCOMPLETE_DATE_OR_TIME", "'$1': incomplete date, time, or dateTime format" },
   { "INSUFFICIENT_BUFFER", "\"$1\": insufficient value to parse for \"$2\"" },
   { "INVALID_ABSOLUTE_PATH", "path component of absolute URI must begin with /" },
+  { "INVALID_CSV_VALUE", "\"$1\": invalid type for value of key \"$2\"" },
   { "INVALID_LOCALE", "\"$1\": invalid locale" },
   { "INVALID_OPTION", "${\"1\": }invalid value for \"$2\" option${; 3}" },
   { "INVALID_OPTION", "${\"1\": }invalid value for \"$2\" option${; 3}" },
@@ -697,7 +698,9 @@
   { "~ILLEGAL_KEY_FieldDescriptor", "field descriptor" },
   { "~ILLEGAL_KEY_Type_34o", "$3 type${ \"4\"}" },
   { "~ILLEGAL_KEY_UnionNoBaseType", "union type: unions may not have base types" },
+  { "~INVALID_OPTION_ArrayElementsMustBeString", "array elements must be string" },
   { "~INVALID_OPTION_MustBeASCIIChar", "must be single ASCII character" },
+  { "~INVALID_OPTION_MustBeArray", "must be JSON array" },
   { "~INVALID_OPTION_MustBeBoolean", "must be boolean" },
   { "~INVALID_OPTION_MustBeString", "must be string" },
   { "~INVALID_OPTION_MustBeTrueFalse", "must be sub-object with \"true\" and \"false\" keys" },

=== modified file 'src/diagnostics/pregenerated/dict_zed_keys.h'
--- src/diagnostics/pregenerated/dict_zed_keys.h	2013-12-18 06:15:17 +0000
+++ src/diagnostics/pregenerated/dict_zed_keys.h	2014-01-16 23:23:15 +0000
@@ -236,10 +236,12 @@
 #define ZED_ZWST0009_JSONIQ_EMPTY_SEQUENCE "~ZWST0009_JSONIQ_EMPTY_SEQUENCE"
 #define ZED_ZWST0009_OBJECT_KEY_NOT_QUOTED "~ZWST0009_OBJECT_KEY_NOT_QUOTED"
 #define ZED_ZWST0009_AXIS_STEP "~ZWST0009_AXIS_STEP"
+#define ZED_INVALID_OPTION_MustBeArray "~INVALID_OPTION_MustBeArray"
 #define ZED_INVALID_OPTION_MustBeASCIIChar "~INVALID_OPTION_MustBeASCIIChar"
 #define ZED_INVALID_OPTION_MustBeTrueFalse "~INVALID_OPTION_MustBeTrueFalse"
 #define ZED_INVALID_OPTION_MustBeBoolean "~INVALID_OPTION_MustBeBoolean"
 #define ZED_INVALID_OPTION_MustBeString "~INVALID_OPTION_MustBeString"
+#define ZED_INVALID_OPTION_ArrayElementsMustBeString "~INVALID_OPTION_ArrayElementsMustBeString"
 #define ZED_MISSING_VALUE_Default "~MISSING_VALUE_Default"
 #define ZED_MISSING_VALUE_EmptyHeader "~MISSING_VALUE_EmptyHeader"
 #define ZED_AllMatchesHasExcludes "~AllMatchesHasExcludes"

=== modified file 'src/runtime/csv/csv_impl.cpp'
--- src/runtime/csv/csv_impl.cpp	2013-12-18 06:15:17 +0000
+++ src/runtime/csv/csv_impl.cpp	2014-01-16 23:23:15 +0000
@@ -54,6 +54,39 @@
 #define IS_JSON_NULL(ITEM) \
   ( (ITEM)->isAtomic() && (ITEM)->getTypeCode() == store::JS_NULL )
 
+static bool is_stringable( store::Item_t const &item ) {
+  switch ( item->getKind() ) {
+    case store::Item::ATOMIC:
+    case store::Item::NODE:
+      return true;
+    default:
+      return false;
+  }
+}
+
+inline zstring get_string_value( store::Item_t const &item ) {
+  return is_stringable( item ) ? item->getStringValue() : "";
+}
+
+static bool get_array_opt( store::Item_t const &object,
+                           char const *opt_name, store::Item_t *result,
+                           QueryLoc const &loc ) {
+  if ( get_json_option( object, opt_name, result ) ) {
+    if ( (*result)->getKind() != store::Item::ARRAY )
+      throw XQUERY_EXCEPTION(
+        csv::INVALID_OPTION,
+        ERROR_PARAMS(
+          get_string_value( *result ),
+          opt_name,
+          ZED( INVALID_OPTION_MustBeArray )
+        ),
+        ERROR_LOC( loc )
+      );
+    return true;
+  }
+  return false;
+}
+
 static bool get_bool_opt( store::Item_t const &object,
                           char const *opt_name, bool *result,
                           QueryLoc const &loc ) {
@@ -63,7 +96,7 @@
       throw XQUERY_EXCEPTION(
         csv::INVALID_OPTION,
         ERROR_PARAMS(
-          opt_item->getStringValue(),
+          get_string_value( opt_item ),
           opt_name,
           ZED( INVALID_OPTION_MustBeBoolean )
         ),
@@ -80,15 +113,23 @@
                           QueryLoc const &loc ) {
   store::Item_t opt_item;
   if ( get_json_option( object, opt_name, &opt_item ) ) {
+    if ( !IS_ATOMIC_TYPE( opt_item, XS_STRING ) )
+      throw XQUERY_EXCEPTION(
+        csv::INVALID_OPTION,
+        ERROR_PARAMS(
+          get_string_value( opt_item ),
+          opt_name,
+          ZED( INVALID_OPTION_MustBeASCIIChar )
+        ),
+        ERROR_LOC( loc )
+      );
     zstring const value( opt_item->getStringValue() );
-    if ( !IS_ATOMIC_TYPE( opt_item, XS_STRING ) ||
-         value.size() != 1 || !ascii::is_ascii( value[0] ) ) {
+    if ( value.size() != 1 || !ascii::is_ascii( value[0] ) )
       throw XQUERY_EXCEPTION(
         csv::INVALID_OPTION,
         ERROR_PARAMS( value, opt_name, ZED( INVALID_OPTION_MustBeASCIIChar ) ),
         ERROR_LOC( loc )
       );
-    }
     *result = value[0];
     return true;
   }
@@ -104,7 +145,7 @@
       throw XQUERY_EXCEPTION(
         csv::INVALID_OPTION,
         ERROR_PARAMS(
-          opt_item->getStringValue(),
+          get_string_value( opt_item ),
           opt_name,
           ZED( INVALID_OPTION_MustBeString )
         ),
@@ -125,6 +166,30 @@
     json::map_type( ptoken->get_type() ) : json::none;
 }
 
+static void set_keys( store::Item_t const &item, vector<store::Item_t> *keys,
+                      QueryLoc const &loc ) {
+  store::Item_t opt_item;
+  if ( get_array_opt( item, "field-names", &opt_item, loc ) ) {
+    store::Iterator_t i( opt_item->getArrayValues() );
+    i->open();
+    store::Item_t name_item;
+    while ( i->next( name_item ) ) {
+      if ( !IS_ATOMIC_TYPE( name_item, XS_STRING ) )
+        throw XQUERY_EXCEPTION(
+          csv::INVALID_OPTION,
+          ERROR_PARAMS(
+            get_string_value( name_item ),
+            "field-names",
+            ZED( INVALID_OPTION_ArrayElementsMustBeString )
+          ),
+          ERROR_LOC( loc )
+        );
+      keys->push_back( name_item );
+    } // while
+    i->close();
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 void CsvParseIterator::set_input( store::Item_t const &item,
@@ -147,14 +212,7 @@
 
   get_bool_opt( item, "cast-unquoted-values", &state->cast_unquoted_, loc );
   get_string_opt( item, "extra-name", &state->extra_name_, loc );
-  if ( get_json_option( item, "field-names", &opt_item ) ) {
-    store::Iterator_t i( opt_item->getArrayValues() );
-    i->open();
-    store::Item_t name_item;
-    while ( i->next( name_item ) )
-      state->keys_.push_back( name_item );
-    i->close();
-  }
+  set_keys( item, &state->keys_, loc );
   if ( get_string_opt( item, "missing-value", &value, loc ) ) {
     if ( value == "error" )
       state->missing_ = missing::error;
@@ -488,14 +546,7 @@
 
   // $options as object()
   consumeNext( item, theChildren[1], plan_state );
-  if ( get_json_option( item, "field-names", &opt_item ) ) {
-    store::Iterator_t i( opt_item->getArrayValues() );
-    i->open();
-    store::Item_t name_item;
-    while ( i->next( name_item ) )
-      state->keys_.push_back( name_item );
-    i->close();
-  }
+  set_keys( item, &state->keys_, loc );
   if ( !get_char_opt( item, "quote-char", &state->quote_, loc ) )
     state->quote_ = '"';
   if ( get_char_opt( item, "quote-escape", &char_opt, loc ) ) {
@@ -515,7 +566,7 @@
       throw XQUERY_EXCEPTION(
         csv::INVALID_OPTION,
         ERROR_PARAMS(
-          "", "serialize-boolea-as",
+          "", "serialize-boolean-as",
           ZED( INVALID_OPTION_MustBeTrueFalse )
         ),
         ERROR_LOC( loc )
@@ -582,7 +633,7 @@
           line += state->boolean_string_[ value_item->getBooleanValue() ];
         else if ( IS_JSON_NULL( value_item ) )
           line += state->null_string_;
-        else {
+        else if ( is_stringable( value_item ) ) {
           value_item->getStringValue2( value );
           bool const quote =
             value.find_first_of( state->must_quote_ ) != zstring::npos;
@@ -592,14 +643,18 @@
           line += value;
           if ( quote )
             line += state->quote_;
-        }
+        } else
+          throw XQUERY_EXCEPTION(
+            csv::INVALID_CSV_VALUE,
+            ERROR_PARAMS( value_item->getKind(), (*key)->getStringValue() ),
+            ERROR_LOC( loc )
+          );
       }
     } // for
     line += "\r\n";
     GENV_ITEMFACTORY->createString( result, line );
     STACK_PUSH( true, state );
   } // while
-
   STACK_END( state );
 }