GOptionContext: Warn about invalid arg/flag combinations

This was proposed by Kjell Ahlstedt in bug 642825.
Also adding a few tests for this new behaviour.

glib/goption.c

@@ -2112,13 +2112,26 @@ g_option_group_add_entries (GOptionGroup       *group,
     {
       gchar c = group->entries[i].short_name;
 
-      if (c)
+      if (c == '-' || (c != 0 && !g_ascii_isprint (c)))
         {
-          if (c == '-' || !g_ascii_isprint (c))
+          g_warning (G_STRLOC ": ignoring invalid short option '%c' (%d)", c, c);
-            {
+          group->entries[i].short_name = 0;
-              g_warning (G_STRLOC": ignoring invalid short option '%c' (%d)", c, c);
+        }
-              group->entries[i].short_name = 0;
+
-            }
+      if (group->entries[i].arg != G_OPTION_ARG_NONE &&
+          (group->entries[i].flags & G_OPTION_FLAG_REVERSE) != 0)
+        {
+          g_warning (G_STRLOC ": ignoring reverse flag on option of type %d", group->entries[i].arg);
+
+          group->entries[i].flags &= ~G_OPTION_FLAG_REVERSE;
+        }
+
+      if (group->entries[i].arg != G_OPTION_ARG_CALLBACK &&
+          (group->entries[i].flags & (G_OPTION_FLAG_NO_ARG|G_OPTION_FLAG_OPTIONAL_ARG|G_OPTION_FLAG_FILENAME)) != 0)
+        {
+          g_warning (G_STRLOC ": ignoring no-arg, optional-arg or filename flags (%d) on option of type %d", group->entries[i].flags, group->entries[i].arg);
+
+          group->entries[i].flags &= ~(G_OPTION_FLAG_NO_ARG|G_OPTION_FLAG_OPTIONAL_ARG|G_OPTION_FLAG_FILENAME);
         }
     }
 

glib/tests/option-context.c

@@ -970,6 +970,66 @@ callback_test_optional_8 (void)
   g_option_context_free (context);
 }
 
+void
+callback_test_optional_9 (void)
+{
+  GOptionContext *context;
+  gboolean retval;
+  GError *error = NULL;
+  gchar **argv;
+  int argc;
+  gchar *string = NULL;
+  GOptionEntry entries [] =
+    { { "test", 't', G_OPTION_FLAG_OPTIONAL_ARG, G_OPTION_ARG_STRING,
+                &string, NULL, NULL },
+      { NULL } };
+
+  context = g_option_context_new (NULL);
+  g_option_context_add_main_entries (context, entries, NULL);
+
+  /* Now try parsing */
+  argv = split_string ("program -t", &argc);
+
+  retval = g_option_context_parse (context, &argc, &argv, &error);
+  g_assert_error (error, G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE);
+  g_assert (!retval);
+  g_assert (string == NULL);
+
+  g_error_free (error);
+  g_strfreev (argv);
+  g_option_context_free (context);
+}
+
+void
+callback_test_optional_10 (void)
+{
+  GOptionContext *context;
+  gboolean retval;
+  GError *error = NULL;
+  gchar **argv;
+  int argc;
+  gchar *string = NULL;
+  GOptionEntry entries [] =
+    { { "test", 't', G_OPTION_FLAG_OPTIONAL_ARG, G_OPTION_ARG_STRING,
+                &string, NULL, NULL },
+      { NULL } };
+
+  context = g_option_context_new (NULL);
+  g_option_context_add_main_entries (context, entries, NULL);
+
+  /* Now try parsing */
+  argv = split_string ("program --test", &argc);
+
+  retval = g_option_context_parse (context, &argc, &argv, &error);
+  g_assert_error (error, G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE);
+  g_assert (!retval);
+  g_assert (string == NULL);
+
+  g_error_free (error);
+  g_strfreev (argv);
+  g_option_context_free (context);
+}
+
 static GPtrArray *callback_remaining_args;
 static gboolean
 callback_remaining_test1_callback (const gchar *option_name, const gchar *value,
@@ -1854,6 +1914,67 @@ test_error_hook (void)
   g_option_context_free (context);
 }
 
+static void
+flag_reverse_string (void)
+{
+  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+    {
+      GOptionContext *context;
+      gchar *arg = NULL;
+      GOptionEntry entries [] =
+        { { "test", 't', G_OPTION_FLAG_REVERSE, G_OPTION_ARG_STRING, &arg, NULL, NULL },
+          { NULL } };
+      gchar **argv;
+      gint argc;
+      gboolean retval;
+      GError *error = NULL;
+
+      context = g_option_context_new (NULL);
+      g_option_context_add_main_entries (context, entries, NULL);
+
+      argv = split_string ("program --test bla", &argc);
+
+      retval = g_option_context_parse (context, &argc, &argv, &error);
+      g_assert (retval == FALSE);
+      g_clear_error (&error);
+      g_strfreev (argv);
+      g_option_context_free (context);
+      exit (0);
+    }
+  g_test_trap_assert_failed ();
+  g_test_trap_assert_stderr ("*ignoring reverse flag*");
+}
+
+static void
+flag_optional_int (void)
+{
+  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+    {
+      GOptionContext *context;
+      gint arg = 0;
+      GOptionEntry entries [] =
+        { { "test", 't', G_OPTION_FLAG_OPTIONAL_ARG, G_OPTION_ARG_INT, &arg, NULL, NULL },
+          { NULL } };
+      gchar **argv;
+      gint argc;
+      gboolean retval;
+      GError *error = NULL;
+
+      context = g_option_context_new (NULL);
+      g_option_context_add_main_entries (context, entries, NULL);
+
+      argv = split_string ("program --test 5", &argc);
+
+      retval = g_option_context_parse (context, &argc, &argv, &error);
+      g_assert (retval == FALSE);
+      g_clear_error (&error);
+      g_strfreev (argv);
+      g_option_context_free (context);
+      exit (0);
+    }
+  g_test_trap_assert_failed ();
+  g_test_trap_assert_stderr ("*ignoring no-arg, optional-arg or filename flags*");
+}
 int
 main (int   argc,
       char *argv[])
@@ -1871,7 +1992,7 @@ main (int   argc,
   g_test_add_func ("/option/restoration/int", error_test1);
   g_test_add_func ("/option/restoration/string", error_test2);
   g_test_add_func ("/option/restoration/boolean", error_test3);
-  
+
   /* Test that special argument parsing works */
   g_test_add_func ("/option/arg/repetition/int", arg_test1);
   g_test_add_func ("/option/arg/repetition/string", arg_test2);
@@ -1899,15 +2020,14 @@ main (int   argc,
 
   /* Test callback with G_OPTION_REMAINING */
   g_test_add_func ("/option/arg/remaining/callback", callback_remaining_test1);
-  
+
   /* Test callbacks which return FALSE */
   g_test_add_func ("/option/arg/remaining/callback-false", callback_returns_false);
-  
+
   /* Test ignoring options */
   g_test_add_func ("/option/arg/ignore/long", ignore_test1);
   g_test_add_func ("/option/arg/ignore/short", ignore_test2);
   g_test_add_func ("/option/arg/ignore/arg", ignore_test3);
-
   g_test_add_func ("/option/context/add", add_test1);
 
   /* Test parsing empty args */
@@ -1926,6 +2046,10 @@ main (int   argc,
   g_test_add_func ("/option/arg/remaining/separator", rest_test4);
   g_test_add_func ("/option/arg/remaining/array", rest_test5);
 
+  /* Test some invalid flag combinations */
+  g_test_add_func ("/option/arg/reverse-string", flag_reverse_string);
+  g_test_add_func ("/option/arg/optional-int", flag_optional_int);
+
   /* regression tests for individual bugs */
   g_test_add_func ("/option/bug/unknown-short", unknown_short_test);
   g_test_add_func ("/option/bug/lonely-dash", lonely_dash_test);