Merge lp:~adiroiban/pocket-lint/pep-options into lp:pocket-lint

[Adi Roiban]

Description
-----------

PEP8 has now at least one controversion option... but users might want to ignore some pep8 messages.

Changes
-------

I have created a dedicated (more complex) object to hold all pocket-lint options.
It can extend the command line options.

The current code was done to minimize changes and provide backward compatibility.

I have done a refactored the UniversalChecker to use the init from BaseClass.

I have also updated the NEWS file. I find it easier to check latest changes in a single file, rather than look into VC history.

Test
----

I have run the tests on py2.7 ... looking forward for travis-ci integration to land on master and then we can see a neutral results on both 2.7 and 3.3

Please review the changes.

Thanks!

[Curtis Hovey (sinzui)]

Thank you.

I reconciled some changes/tests with the previous branches I just merged. I am going to extend PocketLintOptions
in a moment to support PEP8's options.hang_closing so that I can stop contradicting python's rule that continuations
are indented.

review: Approve (code)

=== modified file 'NEWS'
--- NEWS	2010-07-02 19:16:39 +0000
+++ NEWS	2014-02-05 13:36:41 +0000
@@ -1,3 +1,10 @@
+pocket-lint-0.5.0: No Release Date
+==================================
+
+ * Add support for setting pep8 options using the new PocketLintOptions.
+ * By default disable closure-linter.
+
+
 pocket-lint-0.3: The first release
 ==================================
 

=== modified file 'pocketlint/formatcheck.py'
--- pocketlint/formatcheck.py	2013-11-06 15:21:20 +0000
+++ pocketlint/formatcheck.py	2014-02-05 13:36:41 +0000
@@ -254,6 +254,41 @@
         return Language.get_language(file_path) is not None
 
 
+class PocketLintOptions(object):
+    """Default options used by pocketlint"""
+
+    def __init__(self, command_options=None):
+        self.max_line_length = 0
+
+        self.jslint = {
+            'enabled': True,
+            }
+
+        self.closure_linter = {
+            # Disabled by default, since jslint is the default linter.
+            'enabled': False,
+            # List of Google Closure Errors to ignore.
+            # Ex 110 is line to long which is already provided by pocket-lint.
+            'ignore': [110],
+            }
+
+        # See pep8.StyleGuide for available options.
+        self.pep8 = {
+            'max_line_length': pep8.MAX_LINE_LENGTH,
+            }
+
+        if command_options:
+            self._updateFromCommandLineOptions(command_options)
+
+    def _updateFromCommandLineOptions(self, options):
+        """
+        Update with options received from command line.
+        """
+        # Update maximum line length.
+        self.max_line_length = options.max_line_length
+        self.pep8['max_line_length'] = options.max_line_length - 1
+
+
 class BaseChecker(object):
     """Common rules for checkers.
 
@@ -269,7 +304,13 @@
         if self.REENCODE:
             self.text = u(text)
         self.set_reporter(reporter=reporter)
-        self.options = options
+
+        if options is None:
+            self.options = PocketLintOptions()
+        elif not isinstance(options, PocketLintOptions):
+            self.options = PocketLintOptions(command_options=options)
+        else:
+            self.options = options
 
     def set_reporter(self, reporter=None):
         """Set the reporter for messages."""
@@ -295,7 +336,7 @@
     @property
     def check_length_filter(self):
         '''Default filter used by default for checking line length.'''
-        if self.options:
+        if self.options.max_line_length:
             return self.options.max_line_length
         else:
             return DEFAULT_MAX_LENGTH
@@ -306,13 +347,13 @@
 
     def __init__(self, file_path, text,
                  language=None, reporter=None, options=None):
-        self.file_path = file_path
-        self.base_dir = os.path.dirname(file_path)
-        self.file_name = os.path.basename(file_path)
-        self.text = text
-        self.set_reporter(reporter=reporter)
+        super(UniversalChecker, self).__init__(
+            file_path=file_path,
+            text=text,
+            reporter=reporter,
+            options=options,
+            )
         self.language = language
-        self.options = options
         self.file_lines_view = None
 
     def check(self):
@@ -649,8 +690,7 @@
 
     def check_pep8(self):
         """Check style."""
-        style_options = pep8.StyleGuide(
-            max_line_length=self.check_length_filter)
+        style_options = pep8.StyleGuide(**self.options.pep8)
         options = style_options.options
         pep8_report = PEP8Report(options, self.message)
         try:
@@ -687,7 +727,7 @@
     @property
     def check_length_filter(self):
         # The pep8 lib counts from 0.
-        if self.options:
+        if self.options.max_line_length:
             return self.options.max_line_length - 1
         else:
             return pep8.MAX_LINE_LENGTH
@@ -711,20 +751,16 @@
     FULLJSLINT = os.path.join(HERE, 'contrib/fulljslint.js')
     JSREPORTER = os.path.join(HERE, 'jsreporter.js')
 
-    # List of Google Closure Errors to ignore.
-    # Ex 110 is line to long which is already provided by pocket-lint.
-    GOOGLE_CLOSURE_IGNORE = [110]
-
     def check(self):
         """Check the syntax of the JavaScript code."""
         self.check_jslint()
-        self.check_google_closure()
+        self.check_closure_linter()
         self.check_text()
         self.check_windows_endlines()
 
     def check_jslint(self):
         """Check file using jslint."""
-        if JS is None or self.text == '':
+        if JS is None or self.text == '' or not self.options.jslint['enabled']:
             return
         args = [JS, self.JSREPORTER, self.FULLJSLINT, self.file_path]
         jslint = subprocess.Popen(
@@ -738,9 +774,9 @@
                 line_no -= 1
                 self.message(line_no, message, icon='error')
 
-    def check_google_closure(self):
+    def check_closure_linter(self):
         """Check file using Google Closure Linter."""
-        if not closure_linter:
+        if not self.options.closure_linter['enabled']:
             return
 
         from closure_linter import runner
@@ -749,28 +785,13 @@
         error_handler = erroraccumulator.ErrorAccumulator()
         runner.Run(self.file_path, error_handler)
         for error in error_handler.GetErrors():
-            if error.code in self.google_closure_ignore:
+            if error.code in self.options.closure_linter['ignore']:
                 continue
             # Use a similar format as default Google Closure Linter formatter.
             # Line 12, E:0010: Missing semicolon at end of line
             message = 'E:%04d: %s' % (error.code, error.message)
             self.message(error.token.line_number, message, icon='error')
 
-    @property
-    def google_closure_ignore(self):
-        """
-        Return the list of ignored errors for Google Closure Linter.
-
-        Return either the default list or the list specified as options.
-        """
-        if not self.options:
-            return self.GOOGLE_CLOSURE_IGNORE
-        ignore_list = getattr(self.options, 'google_closure_ignore', None)
-        if ignore_list is None:
-            return self.GOOGLE_CLOSURE_IGNORE
-        else:
-            return ignore_list
-
     def check_debugger(self, line_no, line):
         """Check the length of the line."""
         debugger_call = 'debugger;'
@@ -1063,7 +1084,7 @@
     @property
     def check_length_filter(self):
         # Go land standards don't have a max length; it suggests common sense.
-        if self.options:
+        if self.options.max_line_length:
             return self.options.max_line_length - 1
         else:
             return 160

=== modified file 'pocketlint/tests/test_javascript.py'
--- pocketlint/tests/test_javascript.py	2013-10-09 12:22:58 +0000
+++ pocketlint/tests/test_javascript.py	2014-02-05 13:36:41 +0000
@@ -14,7 +14,7 @@
     JavascriptChecker,
     JS
 )
-from pocketlint.tests import Bunch, CheckerTestCase
+from pocketlint.tests import CheckerTestCase
 from pocketlint.tests.test_text import TestAnyTextMixin
 
 try:
@@ -61,14 +61,21 @@
             [(1, "Expected ';' and instead saw '(end)'.")],
             self.reporter.messages)
 
-    def test_google_closure(self):
+    closure_linter_trigger = (
+        'var test = 1+ 1;\n'
+        )
+
+    def test_closure_linter(self):
         if closure_linter is None:
             raise unittest.SkipTest('Google Closure Linter not available.')
-        google_closure_trigger = (
-            'var test = 1+ 1;\n'
-            )
-        self.write_to_file(self.file, google_closure_trigger)
-        checker = JavascriptChecker(self.file.name, invalid_js, self.reporter)
+
+        self.write_to_file(self.file, self.closure_linter_trigger)
+        checker = JavascriptChecker(
+            self.file.name, invalid_js, self.reporter)
+        checker.options.closure_linter.update({
+                'enabled': True,
+                'ignore': [],
+                })
 
         checker.check()
 
@@ -76,17 +83,31 @@
             [(1, u'E:0002: Missing space before "+"')],
             self.reporter.messages)
 
-    def test_google_closure_ignore(self):
+    def test_closure_linter_ignore(self):
         if closure_linter is None:
             raise unittest.SkipTest('Google Closure Linter not available.')
-        google_closure_trigger = (
-            'var test = 1+ 1;\n'
-            )
-        self.write_to_file(self.file, google_closure_trigger)
-        options = Bunch(
-            google_closure_ignore=[2], max_line_length=80)
-        checker = JavascriptChecker(
-            self.file.name, invalid_js, self.reporter, options)
+
+        self.write_to_file(self.file, self.closure_linter_trigger)
+        checker = JavascriptChecker(
+            self.file.name, invalid_js, self.reporter)
+        checker.options.closure_linter.update({
+                'enabled': True,
+                'ignore': [2],
+                })
+
+        checker.check()
+
+        self.assertEqual([], self.reporter.messages)
+
+    def test_closure_linter_disabled(self):
+        """No errors are reported when closure linter is disabled."""
+        self.write_to_file(self.file, self.closure_linter_trigger)
+        checker = JavascriptChecker(
+            self.file.name, invalid_js, self.reporter)
+        checker.options.closure_linter.update({
+                'enabled': False,
+                'ignore': [],
+                })
 
         checker.check()
 

=== modified file 'pocketlint/tests/test_python.py'
--- pocketlint/tests/test_python.py	2013-02-16 18:03:51 +0000
+++ pocketlint/tests/test_python.py	2014-02-05 13:36:41 +0000
@@ -232,6 +232,18 @@
             [(1, 'E501 line too long (70 > 59 characters)')],
             self.reporter.messages)
 
+    def test_pep8_options(self):
+        """It can set PEP8 options."""
+        long_line = '1234 56189' * 7 + '\n'
+        self.write_to_file(self.file, long_line)
+        checker = PythonChecker(self.file.name, long_line, self.reporter)
+        checker.options.pep8['ignore'] = ['E501']
+        checker.options.pep8['max_line_length'] = 60
+
+        checker.check_pep8()
+
+        self.assertEqual([], self.reporter.messages)
+
 
 class TestText(CheckerTestCase, TestAnyTextMixin):
     """Verify text integration."""