Merge lp:~adiroiban/pocket-lint/pocket-lint-764627 into lp:pocket-lint

[Curtis Hovey]

Subclass PyFlakesChecker to supress irrelevant errors.

[Curtis Hovey (sinzui)]

I was initial -1 on handling this issue in pocket-lint, but I see your implementation provides a lot of opportunities for future development. Your PocketLintPyFlakesChecker subclass could accept a Reporter instance either in __init__ or check to remove the integration hack that is visible in the diff. I am not +1 about this change.

I want to merge your branch, but before I do, can you help me understand the test change? I expect to see an explicit test that will raise WindowsError or cause it to be raised. The change I see looks like it it updating an existing test, but it is not clear which test is checking for this. I also do not understand how the added try/except tests that the subclass suppresses the error

review: Needs Information (code)

[lp:~adiroiban/pocket-lint/pocket-lint-764627]

377. By Adi Roiban on 2011-04-18

Fix pep8 and pyflakes overlaping.

[Adi Roiban (adiroiban)]

The updated 'good_python' text is raising the WindowsError in the default pyflakes.

If run the tests on Linux using the updated test with the current formatchecker.py code you will see that the tests are failing.

Do you want me to test that WindowsError is raised when using the default pyflakes checker and the it is ignored when using the modified pyflackes checker?

There was an error in the initial code this is why PocketLintPyFlakesChecker was also added to the pep8 check.
If you look at the check_pep8 you will see that pep8.Checker.report_error is monkey patched, but then pyflakes Checker.report_error is restored.

I have updated the code to reflect this error.

I have imported the pyflackes Checker as PyFlakesChecker since using the generic Checker name can be confusing and it could lead to errors like the one discovered now.

I am not sure what I understood your request.

We can also talk via FreeNode IRC . my nickname is adiroiban

Cheers,

[Curtis Hovey (sinzui)]

Firstly, good_python is intended to be good code that does not cause any errors. Hacking it to not do that implies it is not good python. The good_python should not be used to test a rare case.

I expect to see a new test that only exercises the lines of code that changed. I think you want to create a separate example to test called bad_windowserrror_python and add a new test to TestPyflakes that verifies everything is okay.

review: Needs Fixing (code)

[lp:~adiroiban/pocket-lint/pocket-lint-764627]

378. By Adi Roiban on 2011-04-19

Add a separate test for WindowsError.

379. By Adi Roiban on 2011-04-19

Fix the test.

[Adi Roiban (adiroiban)]

Hi,

Thanks for the review and Sorry for the delay. I added a different test for pyflakes.

Hope it is ok now . Feel feel add any comments :)

Adi

[Curtis Hovey (sinzui)]

review: Approve (code)

=== modified file 'pocketlint/formatcheck.py'
--- pocketlint/formatcheck.py	2011-04-18 02:04:09 +0000
+++ pocketlint/formatcheck.py	2011-04-19 16:34:52 +0000
@@ -32,7 +32,7 @@
 
 import contrib.pep8 as pep8
 from contrib.cssccc import CSSCodingConventionChecker
-from contrib.pyflakes.checker import Checker
+from contrib.pyflakes.checker import Checker as PyFlakesChecker
 try:
     import cssutils
     HAS_CSSUTILS = True
@@ -54,6 +54,19 @@
     ]
 
 
+class PocketLintPyFlakesChecker(PyFlakesChecker):
+    '''PocketLint checker for pyflakes.
+
+    This is here to work around some of the pyflakes problems.
+    '''
+
+    def NAME(self, node):
+        '''Locate name. Ignore WindowsErrors.'''
+        if node.name == 'WindowsError':
+            return
+        return super(PocketLintPyFlakesChecker, self).NAME(node)
+
+
 class Reporter:
     """Common rules for checkers."""
     CONSOLE = object()
@@ -345,7 +358,7 @@
                     error_lineno = 0
             else:
                 error_message, location = str(error).rsplit(':')
-                error_lineno = int(location.split(',')[0].split()[1])- offset
+                error_lineno = int(location.split(',')[0].split()[1]) - offset
             self.message(error_lineno, error_message, icon='error')
         self.check_text()
 
@@ -459,7 +472,7 @@
             message = '%s: %s' % (explanation, line.strip())
             self.message(line_no, message, icon='error')
         else:
-            warnings = Checker(tree)
+            warnings = PocketLintPyFlakesChecker(tree)
             for warning in warnings.messages:
                 dummy, line_no, message = str(warning).split(':')
                 self.message(int(line_no), message.strip(), icon='error')
@@ -481,7 +494,7 @@
                 message, location = er.args
                 self.message(location[0], message, icon='error')
         finally:
-            Checker.report_error = original_report_error
+            pep8.Checker.report_error = original_report_error
 
     def check_text(self):
         """Call each line_method for each line in text."""

=== modified file 'pocketlint/tests/test_python.py'
--- pocketlint/tests/test_python.py	2011-04-17 16:32:41 +0000
+++ pocketlint/tests/test_python.py	2011-04-19 16:34:52 +0000
@@ -15,6 +15,16 @@
         print "Good night."
 """
 
+good_python_on_windows = """\
+class example:
+
+    def __init__(self, value):
+        try:
+            open("some.file")
+        except WindowsError:
+            pass
+"""
+
 bad_syntax_python = """\
 class Test():
     def __init__(self, default='', non_default):
@@ -77,6 +87,14 @@
         self.assertEqual([], self.reporter.messages)
         self.assertEqual(0, self.reporter.call_count)
 
+    def test_windows_code_without_issues(self):
+        self.reporter.call_count = 0
+        checker = PythonChecker(
+            'bogus', good_python_on_windows, self.reporter)
+        checker.check_flakes()
+        self.assertEqual([], self.reporter.messages)
+        self.assertEqual(0, self.reporter.call_count)
+
     def test_code_with_SyntaxError(self):
         self.reporter.call_count = 0
         checker = PythonChecker(