#12776,#11839: call argparse type function only once.

Before, the type function was called twice in the case where the default
was specified and the argument was given as well.  This was especially
problematic for the FileType type, as a default file would always be
opened, even if a file argument was specified on the command line.

Patch by Arnaud Fontaine, with additional test by Mike Meyer.

diff --git a/Lib/argparse.py b/Lib/argparse.py
index 80df97b4f0b2e5db0c691a9a280e3c4290bf47e6..a73845594f594eef00458e9b224463aba46b44f8 100644 (file)
--- a/Lib/argparse.py
+++ b/Lib/argparse.py
@@ -1705,10 +1705,7 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer):
             if action.dest is not SUPPRESS:
                 if not hasattr(namespace, action.dest):
                     if action.default is not SUPPRESS:
-                        default = action.default
-                        if isinstance(action.default, basestring):
-                            default = self._get_value(action, default)
-                        setattr(namespace, action.dest, default)
+                        setattr(namespace, action.dest, action.default)
 
         # add any parser defaults that aren't present
         for dest in self._defaults:
@@ -1936,12 +1933,22 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer):
         if positionals:
             self.error(_('too few arguments'))
 
-        # make sure all required actions were present
+        # make sure all required actions were present, and convert defaults.
         for action in self._actions:
-            if action.required:
-                if action not in seen_actions:
+            if action not in seen_actions:
+                if action.required:
                     name = _get_action_name(action)
                     self.error(_('argument %s is required') % name)
+                else:
+                    # Convert action default now instead of doing it before
+                    # parsing arguments to avoid calling convert functions
+                    # twice (which may fail) if the argument was given, but
+                    # only if it was defined already in the namespace
+                    if (action.default is not None and
+                            hasattr(namespace, action.dest) and
+                            action.default is getattr(namespace, action.dest)):
+                        setattr(namespace, action.dest,
+                                self._get_value(action, action.default))
 
         # make sure all required groups had one option present
         for group in self._mutually_exclusive_groups:

diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py
index 80dbc225be89c50fff4772dae41b1cdfa48e4c04..77f5aca21e06ec3910df2df21e8148d8831eda29 100644 (file)
--- a/Lib/test/test_argparse.py
+++ b/Lib/test/test_argparse.py
@@ -1467,6 +1467,22 @@ class TestFileTypeR(TempDirMixin, ParserTestCase):
         ('readonly', NS(x=None, spam=RFile('readonly'))),
     ]
 
+class TestFileTypeDefaults(TempDirMixin, ParserTestCase):
+    """Test that a file is not created unless the default is needed"""
+    def setUp(self):
+        super(TestFileTypeDefaults, self).setUp()
+        file = open(os.path.join(self.temp_dir, 'good'), 'w')
+        file.write('good')
+        file.close()
+
+    argument_signatures = [
+        Sig('-c', type=argparse.FileType('r'), default='no-file.txt'),
+    ]
+    # should provoke no such file error
+    failures = ['']
+    # should not provoke error because default file is created
+    successes = [('-c good', NS(c=RFile('good')))]
+
 
 class TestFileTypeRB(TempDirMixin, ParserTestCase):
     """Test the FileType option/argument type for reading files"""
@@ -4432,6 +4448,38 @@ class TestArgumentTypeError(TestCase):
         else:
             self.fail()
 
+# ================================================
+# Check that the type function is called only once
+# ================================================
+
+class TestTypeFunctionCallOnlyOnce(TestCase):
+
+    def test_type_function_call_only_once(self):
+        def spam(string_to_convert):
+            self.assertEqual(string_to_convert, 'spam!')
+            return 'foo_converted'
+
+        parser = argparse.ArgumentParser()
+        parser.add_argument('--foo', type=spam, default='bar')
+        args = parser.parse_args('--foo spam!'.split())
+        self.assertEqual(NS(foo='foo_converted'), args)
+
+# ================================================================
+# Check that the type function is called with a non-string default
+# ================================================================
+
+class TestTypeFunctionCallWithNonStringDefault(TestCase):
+
+    def test_type_function_call_with_non_string_default(self):
+        def spam(int_to_convert):
+            self.assertEqual(int_to_convert, 0)
+            return 'foo_converted'
+
+        parser = argparse.ArgumentParser()
+        parser.add_argument('--foo', type=spam, default=0)
+        args = parser.parse_args([])
+        self.assertEqual(NS(foo='foo_converted'), args)
+
 # ======================
 # parse_known_args tests
 # ======================

diff --git a/Misc/ACKS b/Misc/ACKS
index d8ddd3253589d67d676e58104d46bf5fb8cef674..2a93d66d9d776ae1b7c1d8fa3d0111fa6fc68be6 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -271,6 +271,7 @@ Nils Fischbeck
 Frederik Fix
 Matt Fleming
 Hernán Martínez Foffani
+Arnaud Fontaine
 Michael Foord
 Amaury Forgeot d'Arc
 Doug Fort

diff --git a/Misc/NEWS b/Misc/NEWS
index f13499fbf066d80fbd1461f00b546a2e028e3621..bdfd0b797ad8351e671a484f782b9f19a853d510 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -98,6 +98,12 @@ Core and Builtins
 Library
 -------
 
+- Issue #12776,#11839: call argparse type function (specified by add_argument)
+  only once. Before, the type function was called twice in the case where the
+  default was specified and the argument was given as well.  This was
+  especially problematic for the FileType type, as a default file would always
+  be opened, even if a file argument was specified on the command line.
+
 - Issue #13370: Ensure that ctypes works on Mac OS X when Python is
   compiled using the clang compiler