← Back to team overview

zorba-coders team mailing list archive

[Merge] lp:~paul-lucas/zorba/pjl-misc into lp:zorba

 

Paul J. Lucas has proposed merging lp:~paul-lucas/zorba/pjl-misc into lp:zorba.

Commit message:
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.

Requested reviews:
  Paul J. Lucas (paul-lucas)

For more details, see:
https://code.launchpad.net/~paul-lucas/zorba/pjl-misc/+merge/201882

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.
-- 
https://code.launchpad.net/~paul-lucas/zorba/pjl-misc/+merge/201882
Your team Zorba Coders is subscribed to branch lp:zorba.
=== modified file 'NOTICE.txt'
--- NOTICE.txt	2014-01-10 02:36:58 +0000
+++ NOTICE.txt	2014-01-16 05:10:48 +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 05:10:48 +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,7 +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
+ * <code>std::ostream& operator&lt;&lt;(std::ostream&,T const&)</code> defined
+ * for
  * it.
  * For example:
  * \code

=== 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 05:10:48 +0000
@@ -1001,6 +1001,8 @@
 
 extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode INVALID_OPTION;
 
+extern ZORBA_DLL_PUBLIC ZorbaCSVErrorCode INVALID_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 05:10:48 +0000
@@ -134,9 +134,13 @@
  :  </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_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 +298,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 05:10:48 +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_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 05:10:48 +0000
@@ -1473,6 +1473,9 @@
 ZorbaCSVErrorCode INVALID_OPTION( "INVALID_OPTION" );
 
 
+ZorbaCSVErrorCode INVALID_VALUE( "INVALID_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 05:10:48 +0000
@@ -120,6 +120,7 @@
   { "INVALID_OPTION", "${\"1\": }invalid value for \"$2\" option${; 3}" },
   { "INVALID_OPTION", "${\"1\": }invalid value for \"$2\" option${; 3}" },
   { "INVALID_SPECIFICATION", "'$1': invalid % conversion specification" },
+  { "INVALID_VALUE", "\"$1\": invalid type for value of key \"$2\"" },
   { "INVALID_VALUE", "\"$1\": invalid value for conversion specification(s) %$2" },
   { "JNDY0003", "\"$1\": pair with the same name already exists in object" },
   { "JNDY0021", "$1" },
@@ -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 05:10:48 +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 05:10:48 +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_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 );
 }
 


Follow ups