self._get_args -> OptionParser._get_args

lib/vsc/utils/generaloption.py

@@ -222,6 +222,32 @@ def store_or(option, opt_str, value, parser, *args, **kwargs):
             else:
                 self.log.raiseException("_set_attrs: unknown store_or %s" % self.store_or, exception=ValueError)
 
+    def process(self, opt, value, values, parser):
+        """Handle option-as-value issues before actually processing option."""
+
+        if hasattr(parser, 'orig_rargs') and (value in parser._long_opt or value in parser._short_opt):
+            orig_rargs = getattr(parser, 'orig_rargs')
+
+            # Check if this is a commandline issue: values that are options
+            # that are passed via --longopt=value or -sVALUE are not a problem.
+            # When processing the enviroment and/or configfile, we always set
+            # --longopt=value, so no issues there.
+
+            # index of last parsed arg in orig_rargs via remainder of parser.rargs
+            last_parsed_idx = len(orig_rargs) - len(parser.rargs) - 1
+            if orig_rargs[last_parsed_idx] == value:
+                # the value that was passed (implying the option always requires a value)
+                # is an option, and it's very ambigous.
+                if self._long_opts:
+                    force_fmt = "%s=" % self._long_opts[0]
+                else:
+                    force_fmt = "%s" % self._short_opts[0]
+
+                errmsg = "Value '%s' is also a valid option, use '%s%s' instead." % (value, force_fmt, value)
+                raise OptionValueError(errmsg)
+
+        return Option.process(self, opt, value, values, parser)
+
     def take_action(self, action, dest, opt, value, values, parser):
         """Extended take_action"""
         orig_action = action  # keep copy
@@ -470,6 +496,15 @@ def __init__(self, *args, **kwargs):
 
         self.environment_arguments = None
         self.commandline_arguments = None
+        self.orig_rargs = None  # copy of the original arguments
+
+    def parse_args(self, args=None, values=None):
+        """Keep a copy of the original arguments for callback / option processing"""
+        # seems like _get_args returns a copy, but better safe then sorry here
+        self.orig_rargs = OptionParser._get_args(self, args)[:]
+
+        # continue as usual
+        return OptionParser.parse_args(self, args=args, values=values)
 
     def set_description_docstring(self):
         """Try to find the main docstring and add it if description is not None"""

setup.py

@@ -52,7 +52,7 @@ def remove_bdist_rpm_source_file():
 
 PACKAGE = {
     'name': 'vsc-base',
-    'version': '2.2.5',
+    'version': '2.2.6',
     'author': [sdw, jt, ag, kh],
     'maintainer': [sdw, jt, ag, kh],
     'packages': ['vsc', 'vsc.utils', 'vsc.install'],

test/generaloption.py

@@ -55,7 +55,7 @@ def base_options(self):
             "base":("Long and short base option", None, "store_true", False, 'b'),
             "longbase":("Long-only base option", None, "store_true", True),
             "justatest":("Another long based option", None, "store_true", True),
-            "store":("Store option", None, "store", None),
+            "store":("Store option", None, "store", None, 's'),
             "store-with-dash":("Store option with dash in name", None, "store", None),
             "aregexopt":("A regex option", None, "regex", None),
             }
@@ -765,6 +765,59 @@ def raise_error(msg, *args):
                               go_args=['--level-level'], envvar_prefix='GENERALOPTIONTEST', error_env_options=True,
                               error_env_option_method=raise_error)
 
+    def test_nosuchoption(self):
+        """Test catching of non-existing options."""
+        self.mock_stderr(True)  # reset
+        self.assertErrorRegex(SystemExit, '.*', TestOption1, go_args=['--nosuchoptiondefinedfoobar'])
+        stderr = self.get_stderr()
+        self.assertTrue("no such option: --nosuchoptiondefinedfoobar" in stderr)
+
+    def test_option_as_value(self):
+        """Test how valid options being used as values are handled."""
+        # -s requires an argument
+        self.mock_stderr(True)
+        self.assertErrorRegex(SystemExit, '.*', TestOption1, go_args=['-b', '-s'])
+        stderr = self.get_stderr()
+        self.assertTrue("-s option requires an argument" in stderr)
+
+        # valid ways of specifying '-b' as a value to --store/-s
+        topt = TestOption1(go_args=['--store=-b'])
+        self.assertEqual(topt.options.store, '-b')
+        self.assertFalse(topt.options.base)  # remain False (default value)
+
+        topt = TestOption1(go_args=['-s-b'])  # equivalent to --store=-b
+        self.assertEqual(topt.options.store, '-b')
+        self.assertFalse(topt.options.base)  # remain False (default value)
+
+        # -s -b is not a valid list of flags, since it's ambiguous: is '-b' a value for '-s', or an option?
+        self.mock_stderr(True)  # reset
+        self.assertErrorRegex(SystemExit, '.*', TestOption1, go_args=['-s', '-b'])
+        stderr = self.get_stderr()
+        self.assertTrue("Value '-b' is also a valid option" in stderr)
+
+        # same for --store -b
+        self.mock_stderr(True)  # reset
+        self.assertErrorRegex(SystemExit, '.*', TestOption1, go_args=['--store', '-b'])
+        stderr = self.get_stderr()
+        self.assertTrue("Value '-b' is also a valid option" in stderr)
+
+        # same for --store --base
+        self.mock_stderr(True)  # reset
+        self.assertErrorRegex(SystemExit, '.*', TestOption1, go_args=['--store', '--base'])
+        stderr = self.get_stderr()
+        self.assertTrue("Value '--base' is also a valid option" in stderr)
+
+        # including a non-existing option will result in it being treated as a value
+        topt = TestOption1(go_args=['--store', '-f'])
+        self.assertEqual(topt.options.store, '-f')
+        topt = TestOption1(go_args=['--store', '--foo'])
+        self.assertEqual(topt.options.store, '--foo')
+
+        # non-existing options are still reported if they're not consumed as a value
+        self.mock_stderr(True)  # reset
+        self.assertErrorRegex(SystemExit, '.*', TestOption1, go_args=['--store', '--foo', '--nosuchoptiondefinedfoobar'])
+        stderr = self.get_stderr()
+        self.assertTrue("no such option: --nosuchoptiondefinedfoobar" in stderr)
 
 def suite():
     """ returns all the testcases in this module """