Merge lp:~canonical-platform-qa/uci-config/decreased-complexity into lp:~canonical-ci-engineering/uci-config/trunk

[Federico Gimenez]

mccabe complexity reduction

There are methods reported to be too complex by the McCabe script. This is attempt to reduce the reported complexity, while keeping the api interface and the test suite passing.

[lp:~canonical-platform-qa/uci-config/decreased-complexity]

67. By Federico Gimenez on 2015-03-02

some tests fixed

68. By Federico Gimenez on 2015-03-02

only 4 tests red

69. By Federico Gimenez on 2015-03-03

3 to go

Unmerged revisions

69. By Federico Gimenez on 2015-03-03

3 to go

68. By Federico Gimenez on 2015-03-02

only 4 tests red

67. By Federico Gimenez on 2015-03-02

some tests fixed

66. By Federico Gimenez on 2015-03-02

reported complexity reduced to 10

=== modified file 'setup.py'
--- setup.py	2014-04-01 07:41:42 +0000
+++ setup.py	2015-03-03 15:56:20 +0000
@@ -3,16 +3,16 @@
 # This file is part of Ubuntu Continuous Integration configuration framework.
 #
 # Copyright 2013, 2014 Canonical Ltd.
-#  
+#
 # This program is free software: you can redistribute it and/or modify it under
 # the terms of the GNU General Public License version 3, as published by the
 # Free Software Foundation.
-#  
+#
 # This program is distributed in the hope that it will be useful, but WITHOUT
 # ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
 # SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 # General Public License for more details.
-#  
+#
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 

=== modified file 'uciconfig/options.py'
--- uciconfig/options.py	2014-02-15 16:49:46 +0000
+++ uciconfig/options.py	2015-03-03 15:56:20 +0000
@@ -77,19 +77,12 @@
            safely unquote them. It is provided so daughter classes can handle
            the quoting themselves.
         """
-        if override_from_env is None:
-            override_from_env = []
-        if default_from_env is None:
-            default_from_env = []
         self.name = name
         self._help = help_string
-        self.override_from_env = override_from_env
+        self.override_from_env = override_from_env or []
         # Convert the default value to a unicode string so all values are
         # strings internally before conversion (via from_unicode) is attempted.
-        if default is None:
-            self.default = None
-        elif default is MANDATORY:
-            # Special case, no further check needed
+        if default is None or default is MANDATORY or callable(default):
             self.default = default
         elif isinstance(default, list):
             # Only the empty list is supported
@@ -100,13 +93,11 @@
         elif isinstance(default, (unicode, bool, int, float)):
             # Rely on python to convert strings, booleans, floats and integers
             self.default = '{}'.format(default)
-        elif callable(default):
-            self.default = default
         else:
             # other python objects are not expected
             raise AssertionError(
                 '{!r} is not supported as a default value'.format(default))
-        self.default_from_env = default_from_env
+        self.default_from_env = default_from_env or []
         self.from_unicode = from_unicode
         self.unquote = unquote
         if invalid and invalid not in ('warning', 'error'):

=== modified file 'uciconfig/parsers.py'
--- uciconfig/parsers.py	2015-01-29 10:17:25 +0000
+++ uciconfig/parsers.py	2015-03-03 15:56:20 +0000
@@ -174,109 +174,75 @@
         pre = None
         while remaining:
             pre_comment = self.comment_re.match(remaining)
-            if pre_comment:
-                # Buffer the comment lines until a section or an option collect
-                # it
-                if pre is None:
-                    pre = pre_comment.group(1)
-                else:
-                    pre = pre + pre_comment.group(1)
-                remaining = remaining[pre_comment.end():]
-                self.line += 1
-                continue
-
             section = self.section_re.match(remaining)
-            if section:
-                # A new section
-                name = section.group(1)
-                if name == '':
-                    raise errors.SectionEmptyName(self.path, self.line)
-                remaining = remaining[section.end():]
-                post_comment = self.comment_re.match(remaining)
-                if post_comment:
-                    post = post_comment.group(1)
-                    remaining = remaining[post_comment.end():]
-                else:
-                    post = None
-                tokens.append(SectionDefinition(name, pre, post))
-                pre = None
-                if post is not None:
-                    self.line += 1
-                post = None
-                continue
-
             key_matches = self.option_re.match(remaining)
-            if key_matches:
-                if not tokens:
-                    # First option definition without a previous section, it's
-                    # the None section.
-                    tokens.append(SectionDefinition(None))
-                key = key_matches.group(1)
-                remaining = remaining[key_matches.end():]
-                # the option value is still in 'remaining' which can be empty
-                # or start with a newline if no value is defined for the
-                # option.
-                value, post, remaining = self.parse_value(remaining)
-                tokens.append(OptionDefinition(key, value, pre, post))
-                pre = None
-                continue
-
-            if remaining.startswith('\n'):
+            if pre_comment:
+                remaining, pre = self._parse_precomment(
+                    pre_comment, remaining, pre)
+            elif section:
+                tokens, remaining, pre = self._parse_section(
+                    section, tokens, remaining, pre)
+            elif key_matches:
+                tokens, remaining, pre = self._parse_key(
+                    key_matches, tokens, remaining, pre)
+            elif remaining.startswith('\n'):
                 # Keep track of the current line
                 self.line += 1
                 remaining = remaining[1:]
-                continue
-
-            if remaining.startswith(' '):
+            elif remaining.startswith(' '):
                 # Consume spaces to get rid of empty lines
                 remaining = remaining[1:]
-                continue
-
-            raise errors.InvalidSyntax(self.path, self.line)
+            else:
+                raise errors.InvalidSyntax(self.path, self.line)
         return tokens
 
-    def parse_value(self, text):
-        """Parse an option value.
-
-        :param text: The unicode string to parse containing the value.
-
-        :return: A (value, post, remaining) tuple where 'value' is the string
-            representing the value, 'post' the associated comment (can be None)
-            and 'remaining' the rest of text after parsing.
-        """
-        value = ''
-        post = ''
-        cur = 0
-        post_last = len(text)
-        # ignore leading spaces
-        while cur < post_last and text[cur] == ' ':
-            cur += 1
-        # build value until we encounter end of line or a comment
-        while cur < post_last and text[cur] not in ('#', '\n'):
-            value += text[cur]
-            cur += 1
-        # remove trailing spaces
-        spaces = ''
-        while value.endswith(' '):
-            spaces += value[-1]
-            value = value[:-1]
-        # comment (if present) ends at end of line
-        if cur < post_last and text[cur] == '#':
-            post = spaces
-            while cur < post_last and text[cur] != '\n':
-                post += text[cur]
-                cur += 1
-        if post == '':
+    def _parse_precomment(self, pre_comment, remaining, pre):
+        # Buffer the comment lines until a section or an option collect
+        # it
+        if pre is None:
+            pre = pre_comment.group(1)
+        else:
+            pre = pre + pre_comment.group(1)
+        remaining = remaining[pre_comment.end():]
+        self.line += 1
+        return (remaining, pre)
+
+    def _parse_section(self, section, tokens, remaining, pre):
+        name = section.group(1)
+        if name == '':
+            raise errors.SectionEmptyName(self.path, self.line)
+        remaining = remaining[section.end():]
+        post_comment = self.comment_re.match(remaining)
+        if post_comment:
+            post = post_comment.group(1)
+            remaining = remaining[post_comment.end():]
+        else:
             post = None
-        # Consume end of line if present
-        if cur < post_last and text[cur] == '\n':
-            if post is not None:
-                # If there is a post comment, it needs to include the '\n',
-                # yet, the current line shouldn't be updated yet, so we leave
-                # it in remaining.
-                post += '\n'
-        remaining = text[cur:]
-        return value, post, remaining
+        tokens.append(SectionDefinition(name, pre, post))
+        pre = None
+        if post is not None:
+            self.line += 1
+
+        return (tokens, remaining, pre)
+
+    def _parse_key(self, key_matches, tokens, remaining, pre):
+        if not tokens:
+            # First option definition without a previous section, it's
+            # the None section.
+            tokens.append(SectionDefinition(None))
+        key = key_matches.group(1)
+        remaining = remaining[key_matches.end():]
+        result = re.match(
+            r'\ *([\\,\{\}\w /\-\._]*)?([ \t\r\f\v]*#.*\n)?(.*)',
+            remaining,
+            re.DOTALL)
+        value, post, remaining = (
+            result.group(1).strip(), result.group(2), result.group(3))
+        tokens.append(OptionDefinition(key, value, pre, post))
+        if post is not None:
+            self.line += 1
+        pre = None
+        return (tokens, remaining, pre)
 
     def make_sections(self, tokens):
         """Yields the sections built from the received tokens.

=== modified file 'uciconfig/stacks.py'
--- uciconfig/stacks.py	2014-03-26 23:22:19 +0000
+++ uciconfig/stacks.py	2015-03-03 15:56:20 +0000
@@ -225,36 +225,37 @@
             # Not registered
             opt = None
 
-        def expand_and_convert(val):
-            # This may need to be called in different contexts if the value is
-            # None or ends up being None during expansion or conversion.
-            if val is not None:
-                if expand:
-                    val = self._expand_options_in_string(val)
-                if opt is None:
-                    val = found_store.unquote(val)
-                elif convert:
-                    val = opt.convert_from_unicode(found_store, val)
-            return val
-
         # First of all, check if the environment can override the configuration
         # value
         if opt is not None and opt.override_from_env:
-            value = opt.get_override()
-            value = expand_and_convert(value)
+            value = self.expand_and_convert(
+                opt.get_override(), expand, opt, found_store, convert)
         if value is None:
             for store, section in self.iter_sections():
                 value = section.get(name)
                 if value is not None:
                     found_store = store
                     break
-            value = expand_and_convert(value)
+            value = self.expand_and_convert(
+                value, expand, opt, found_store, convert)
             if opt is not None and value is None:
                 # If the option is registered, it may provide a default value
-                value = opt.get_default()
-                value = expand_and_convert(value)
+                value = self.expand_and_convert(
+                    opt.get_default(), expand, opt, found_store, convert)
         return value
 
+    def expand_and_convert(self, val, expand, opt, found_store, convert):
+        # This may need to be called in different contexts if the value is
+        # None or ends up being None during expansion or conversion.
+        if val is not None:
+            if expand:
+                val = self._expand_options_in_string(val)
+            if opt is None:
+                val = found_store.unquote(val)
+            elif convert:
+                val = opt.convert_from_unicode(found_store, val)
+        return val
+
     def expand_options(self, string, env=None):
         """Expand option references in the string in the configuration context.