[#114438129]address code analysis errors by pylints

.travis.yml

@@ -6,7 +6,9 @@ python:
 install:
   - pip install azure==2.0.0a1
   - pip install mock==1.3.0
+  - pip install pylint==1.5.4
 script: 
   - export PYTHONPATH=$PATHONPATH:./src
   - python -m azure.cli
-  - python -m unittest discover -s src/azure/cli/tests
+  - pylint src/azure
+  - python -m unittest discover -s src/azure/cli/tests

azure-cli.pyproj

@@ -44,6 +44,7 @@
     <Compile Include="azure\cli\tests\test_connection_verify.py">
       <SubType>Code</SubType>
     </Compile>
+    <Compile Include="azure\cli\tests\test_output.py" />
     <Compile Include="azure\cli\tests\test_profile.py" />
     <Compile Include="azure\cli\_argparse.py" />
     <Compile Include="azure\cli\commands\_command_creation.py">
@@ -52,6 +53,7 @@
     <Compile Include="azure\cli\_debug.py">
       <SubType>Code</SubType>
     </Compile>
+    <Compile Include="azure\cli\_locale.py" />
     <Compile Include="azure\cli\_logging.py" />
     <Compile Include="azure\cli\_output.py" />
     <Compile Include="azure\cli\_profile.py" />

pylintrc

@@ -0,0 +1,13 @@
+[MESSAGES CONTROL]
+# For all codes, run 'pylint --list-msgs' or go to 'http://pylint-messages.wikidot.com/all-codes'
+# C0111 Missing docstring 
+# C0103 Invalid %s name "%s"
+# C0303 Trailing whitespace
+# I0011 Warning locally suppressed using disable-msg
+# W0511 fixme
+disable=C0111,C0103,C0303,I0011,W0511
+[DESIGN]
+# Maximum number of locals for function / method body
+max-locals=25
+# Maximum number of branch for function / method body
+max-branches=20

src/azure/cli/_argparse.py

@@ -1,9 +1,7 @@
 from __future__ import print_function
-import json
-import os
 import sys
 
-from ._locale import get_file as locale_get_file
+from ._locale import L, get_file as locale_get_file
 from ._logging import logger
 
 # Named arguments are prefixed with one of these strings
@@ -20,7 +18,7 @@ class IncorrectUsageError(Exception):
     pass
 
 class Arguments(dict):
-    def __init__(self, source=None):
+    def __init__(self, source=None): #pylint: disable=super-init-not-called
         self.positional = []
         if source:
             self.update(source)
@@ -40,7 +38,7 @@ def __getattr__(self, key):
         except LookupError:
             pass
         logger.debug('Argument %s is required', key)
-        raise IncorrectUsageError(_("Argument {0} is required").format(key))
+        raise IncorrectUsageError(L('Argument {0} is required').format(key))
 
 def _read_arg(string):
     for prefix in ARG_PREFIXES:
@@ -66,11 +64,15 @@ def __init__(self, prog):
         self.noun_map = {
             '$doc': 'azure-cli.txt',
         }
-        self.help_args = { '--help', '-h' }
-        self.complete_args = { '--complete' }
-        self.global_args = { '--verbose', '--debug' }
-
-    def add_command(self, handler, name=None, description=None, args=None):
+        self.help_args = {'--help', '-h'}
+        self.complete_args = {'--complete'}
+        self.global_args = {'--verbose', '--debug'}
+
+    def add_command(self,
+                    handler, 
+                    name=None, 
+                    description=None, 
+                    args=None):
         '''Registers a command that may be parsed by this parser.
 
         `handler` is the function to call with two `Arguments` objects.
@@ -105,7 +107,7 @@ def add_command(self, handler, name=None, description=None, args=None):
         m['$args'] = []
         m['$kwargs'] = kw = {}
         m['$argdoc'] = ad = []
-        for spec, desc in (args or []):
+        for spec, desc in args or []:
             if not any(spec.startswith(p) for p in ARG_PREFIXES):
                 m['$args'].append(spec.strip('<> '))
                 ad.append((spec, desc))
@@ -121,7 +123,11 @@ def add_command(self, handler, name=None, description=None, args=None):
             ad.append(('/'.join(aliases), desc))
 
 
-    def execute(self, args, show_usage=False, show_completions=False, out=sys.stdout):
+    def execute(self,
+                args, 
+                show_usage=False, 
+                show_completions=False, 
+                out=sys.stdout):
         '''Parses `args` and invokes the associated handler.
 
         The handler is passed two `Arguments` objects with all arguments other
@@ -142,10 +148,11 @@ def execute(self, args, show_usage=False, show_completions=False, out=sys.stdout
         if not show_completions:
             show_completions = any(a in self.complete_args for a in args)
 
-        all_global_args = set(a.lstrip('-/') for a in self.help_args | self.complete_args | self.global_args)
+        all_global_args = set(
+            a.lstrip('-/') for a in self.help_args | self.complete_args | self.global_args)
         def not_global(a):
             return a.lstrip('-/') not in all_global_args
-        it = filter(not_global, args).__iter__()
+        it = filter(not_global, args).__iter__() #pylint: disable=bad-builtin
 
         m = self.noun_map
         nouns = []
@@ -161,17 +168,16 @@ def not_global(a):
             n = next(it, '')
 
         try:
-            expected_args = m['$args']
             expected_kwargs = m['$kwargs']
             handler = m['$handler']
         except LookupError:
             logger.debug('Missing data for noun %s', n)
             show_usage = True
 
         if show_completions:
-            return self._display_completions(nouns, m, args, out)
+            return ArgumentParser._display_completions(m, out)
         if show_usage:
-            return self._display_usage(nouns, m, args, out)
+            return self._display_usage(nouns, m, out)
 
         parsed = Arguments()
         others = Arguments()
@@ -189,8 +195,9 @@ def not_global(a):
                 elif target_value[1] is True:
                     # Arg with no value
                     if value is not None:
-                        print(_("argument '{0}' does not take a value").format(key_n), file=out)
-                        return self._display_usage(nouns, m, args, out)
+                        print(L("argument '{0}' does not take a value").format(key_n), 
+                              file=out)
+                        return self._display_usage(nouns, m, out)
                     parsed.add_from_dotted(target_value[0], True)
                 else:
                     # Arg with a value
@@ -208,11 +215,11 @@ def not_global(a):
             return handler(parsed, others)
         except IncorrectUsageError as ex:
             print(str(ex), file=out)
-            return self._display_usage(nouns, m, args, out)
+            return self._display_usage(nouns, m, out)
         finally:
             sys.stdout = old_stdout
 
-    def _display_usage(self, nouns, noun_map, arguments, out=sys.stdout):
+    def _display_usage(self, nouns, noun_map, out=sys.stdout):            
         spec = ' '.join(noun_map.get('$spec') or nouns)
         print('    {} {}'.format(self.prog, spec), file=out)
         print(file=out)
@@ -246,7 +253,8 @@ def _display_usage(self, nouns, noun_map, arguments, out=sys.stdout):
             out.flush()
             logger.debug('Expected documentation at %s', doc_file)
 
-    def _display_completions(self, nouns, noun_map, arguments, out=sys.stdout):
+    @staticmethod
+    def _display_completions(noun_map, out=sys.stdout):
         completions = [k for k in noun_map if not k.startswith('$')]
 
         kwargs = noun_map.get('$kwargs')

src/azure/cli/_debug.py

@@ -5,7 +5,8 @@
 
 def allow_debug_connection(client):  
     if should_disable_connection_verify():  
-        logger.warn("Connection verification disabled by environment variable %s", DISABLE_VERIFY_VARIABLE_NAME)
+        logger.warning("Connection verification disabled by environment variable %s",
+                       DISABLE_VERIFY_VARIABLE_NAME)
         client.config.connection.verify = False
 
 def should_disable_connection_verify():  

src/azure/cli/_locale.py

@@ -1,11 +1,16 @@
 import os.path
+from codecs import open as codecs_open
 
-from codecs import open
+_translations = dict()
+_locale_dir = ''
+
+def L(key):
+    return _translations.get(key) or '<NO_TRANSLATION:{}>'.format(key)  
 
 def install(locale_dir):
     mapping = []
 
-    with open(os.path.join(locale_dir, "messages.txt"), 'r', encoding='utf-8-sig') as f:
+    with codecs_open(os.path.join(locale_dir, "messages.txt"), 'r', encoding='utf-8-sig') as f:
         for i in f:
             if not i or i.startswith('#') or not i.strip():
                 continue
@@ -14,16 +19,12 @@ def install(locale_dir):
             else:
                 mapping[-1] = (mapping[-1][0], i.strip())
 
-    translations = dict(mapping)
-    def _(key):
-        return translations.get(key) or '<NO_TRANSLATION:{}>'.format(key)
-    _.locale_dir = locale_dir
-
-    __builtins__['_'] = _
+    globals()['_translations'] = dict(mapping)
+    globals()['_locale_dir'] = locale_dir
 
 def get_file(name):
     try:
-        src = _.locale_dir
+        src = _locale_dir
     except (NameError, AttributeError):
         raise RuntimeError("localizations not installed")
 

src/azure/cli/_logging.py

@@ -1,7 +1,7 @@
 import logging as _logging
 import sys
 
-__all__ = ['logging', 'configure_logging']
+__all__ = ['logger', 'configure_logging']
 
 logger = _logging.Logger('az', _logging.WARNING)
 

src/azure/cli/_output.py

@@ -8,7 +8,7 @@
     from io import StringIO
 except ImportError:
     # Python 2
-    from StringIO import StringIO
+    from StringIO import StringIO #pylint: disable=import-error
 
 class OutputFormatException(Exception):
     pass
@@ -40,9 +40,9 @@ def format_text(obj):
     except TypeError:
         return ''
 
-class OutputProducer(object):
+class OutputProducer(object): #pylint: disable=too-few-public-methods 
 
-    def __init__(self, formatter=format_json, file=sys.stdout):
+    def __init__(self, formatter=format_json, file=sys.stdout): #pylint: disable=redefined-builtin
         self.formatter = formatter
         self.file = file
 
@@ -100,14 +100,14 @@ def add(self, identifier, value):
 
     def dump(self):
         with StringIO() as io:
-            for id in sorted(self.identifiers):
-                io.write(id.upper())
+            for identifier in sorted(self.identifiers):
+                io.write(identifier.upper())
                 io.write('\t')
-                for col in self.identifiers[id]:
+                for col in self.identifiers[identifier]:
                     if isinstance(col, str):
                         io.write(col)
                     else:
-                        # TODO: Handle complex objects
+                        # TODO: Need to handle complex objects
                         io.write("null")
                     io.write('\t')
                 io.write('\n')

src/azure/cli/_profile.py

@@ -1,6 +1,6 @@
-from msrest.authentication import BasicTokenAuthentication
+import collections
+from msrest.authentication import BasicTokenAuthentication
 from .main import CONFIG
-import collections
 
 class Profile(object):
 
@@ -37,10 +37,9 @@ def set_subscriptions(self, new_subscriptions, access_token):
             for s in subscriptions:
                 s['active'] = False
 
-            if new_active_one:
-                new_active_one['active'] = True
-            else:
-                new_subscriptions[0]['active'] = True
+            if not new_active_one:
+                new_active_one = new_subscriptions[0]
+            new_active_one['active'] = True
         else:
             new_subscriptions[0]['active'] = True
 

src/azure/cli/_session.py

@@ -7,7 +7,7 @@
     import collections
 
 
-from codecs import open
+from codecs import open as codecs_open
 
 class Session(collections.MutableMapping):
     '''A simple dict-like class that is backed by a JSON file.
@@ -28,14 +28,14 @@ def load(self, filename, max_age=0):
                 st = os.stat(self.filename)
                 if st.st_mtime + max_age < time.clock():
                     self.save()
-            with open(self.filename, 'r', encoding='utf-8-sig') as f:
+            with codecs_open(self.filename, 'r', encoding='utf-8-sig') as f:
                 self.data = json.load(f)
         except (OSError, IOError):
             self.save()
 
     def save(self):
         if self.filename:
-            with open(self.filename, 'w', encoding='utf-8-sig') as f:
+            with codecs_open(self.filename, 'w', encoding='utf-8-sig') as f:
                 json.dump(self.data, f)
 
     def save_with_retry(self, retries=5):

src/azure/cli/_util.py

@@ -1,3 +1,2 @@
-
 def normalize_newlines(str_to_normalize):
     return str_to_normalize.replace('\r\n', '\n')

src/azure/cli/commands/__init__.py

@@ -21,21 +21,21 @@ def add_command(handler):
         return handler
     return add_command
 
-def description(description):
+def description(description_text):
     def add_description(handler):
-        _COMMANDS.setdefault(handler, {})['description'] = description
-        logger.debug('Added description "%s" to %s', description, handler)
+        _COMMANDS.setdefault(handler, {})['description'] = description_text
+        logger.debug('Added description "%s" to %s', description_text, handler)
         return handler
     return add_description
 
-def option(spec, description=None):
+def option(spec, description_text=None):
     def add_option(handler):
-        _COMMANDS.setdefault(handler, {}).setdefault('args', []).append((spec, description))
+        _COMMANDS.setdefault(handler, {}).setdefault('args', []).append((spec, description_text))
         logger.debug('Added option "%s" to %s', spec, handler)
         return handler
     return add_option
 
-def add_to_parser(parser, command=None):
+def add_to_parser(parser, command_name=None):
     '''Loads commands into the parser
 
     When `command` is specified, only commands from that module will be loaded.
@@ -45,9 +45,9 @@ def add_to_parser(parser, command=None):
     # Importing the modules is sufficient to invoke the decorators. Then we can
     # get all of the commands from the _COMMANDS variable.
     loaded = False
-    if command:
+    if command_name:
         try:
-            __import__('azure.cli.commands.' + command)
+            __import__('azure.cli.commands.' + command_name)
             loaded = True
         except ImportError:
             # Unknown command - we'll load all below