Merge lp:~adiroiban/pocket-lint/pep-in-memory into lp:pocket-lint

Proposed by Adi Roiban
Status: Needs review
Proposed branch: lp:~adiroiban/pocket-lint/pep-in-memory
Merge into: lp:pocket-lint
Diff against target: 235 lines (+39/-43)
3 files modified
pocketlint/formatcheck.py (+16/-3)
pocketlint/tests/test_python.py (+19/-37)
pocketlint/tests/test_text.py (+4/-3)
To merge this branch: bzr merge lp:~adiroiban/pocket-lint/pep-in-memory
Reviewer Review Type Date Requested Status
Curtis Hovey Pending
Review via email: mp+215549@code.launchpad.net

Description of the change

Description
----------

To allow in memory check of pep8 code , pocket-lint should pass already parsed lines to pep8 instead of only passing the filepath so that a file is only read once.

Changes
-------

I have updated check_pep8

I have updated tests to use the new PocketLintOptions object and stop using the mocked Bunch object.

I have updated the max_line_lenght update in PocketLintOption to keep in sync the pep8 option.

I have updated style so that pocket-lint code will pass pocket-lint checks.

Maybe we should update test.py to also run pocket-lint on the pocket-lint source code? What do you think? I can do that in separate branch. This can also be usefull for travis-ci integration.

Please let me know what do you think.

To post a comment you must log in.

Unmerged revisions

453. By Adi Roiban <email address hidden>

Initial code to fix pep8 in memory check.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'pocketlint/formatcheck.py'
--- pocketlint/formatcheck.py 2014-03-09 21:04:28 +0000
+++ pocketlint/formatcheck.py 2014-04-12 16:02:11 +0000
@@ -265,8 +265,8 @@
265 """Default options used by pocketlint"""265 """Default options used by pocketlint"""
266266
267 def __init__(self, command_options=None):267 def __init__(self, command_options=None):
268 self.max_line_length = 0268 self._max_line_length = 0
269269 self.regex_line = []
270 self.jslint = {270 self.jslint = {
271 'enabled': True,271 'enabled': True,
272 }272 }
@@ -290,6 +290,15 @@
290 if command_options:290 if command_options:
291 self._updateFromCommandLineOptions(command_options)291 self._updateFromCommandLineOptions(command_options)
292292
293 @property
294 def max_line_length(self):
295 return self._max_line_length
296
297 @max_line_length.setter
298 def max_line_length(self, value):
299 self._max_line_length = value
300 self.pep8['max_line_length'] = value - 1
301
293 def _updateFromCommandLineOptions(self, options):302 def _updateFromCommandLineOptions(self, options):
294 """303 """
295 Update with options received from command line.304 Update with options received from command line.
@@ -734,7 +743,11 @@
734 pep8_report = PEP8Report(options, self.message)743 pep8_report = PEP8Report(options, self.message)
735 try:744 try:
736 pep8_checker = pep8.Checker(745 pep8_checker = pep8.Checker(
737 self.file_path, options=options, report=pep8_report)746 filename=self.file_path,
747 lines=self.text.splitlines(True),
748 options=options,
749 report=pep8_report,
750 )
738 pep8_checker.check_all()751 pep8_checker.check_all()
739 except TokenError as er:752 except TokenError as er:
740 message, location = er.args753 message, location = er.args
741754
=== modified file 'pocketlint/tests/test_python.py'
--- pocketlint/tests/test_python.py 2014-03-09 21:00:26 +0000
+++ pocketlint/tests/test_python.py 2014-04-12 16:02:11 +0000
@@ -5,17 +5,16 @@
5 absolute_import,5 absolute_import,
6 print_function,6 print_function,
7 unicode_literals,7 unicode_literals,
8)8 )
99
10import unittest10import unittest
11from tempfile import NamedTemporaryFile11from tempfile import NamedTemporaryFile
1212
13from pocketlint.formatcheck import (13from pocketlint.formatcheck import (
14 get_option_parser,
15 # Imported via pocketlint to avoid duplication of conditional import.14 # Imported via pocketlint to avoid duplication of conditional import.
16 pep257,15 pep257,
17 PythonChecker,16 PocketLintOptions, PythonChecker,
18)17 )
19from pocketlint.tests import CheckerTestCase18from pocketlint.tests import CheckerTestCase
20from pocketlint.tests.test_text import TestAnyTextMixin19from pocketlint.tests.test_text import TestAnyTextMixin
2120
@@ -171,7 +170,7 @@
171 source = (170 source = (
172 '# -*- coding: utf-8 -*-\n'171 '# -*- coding: utf-8 -*-\n'
173 'variable = u"r\xe9sum\xe9"'172 'variable = u"r\xe9sum\xe9"'
174 )173 )
175 checker = PythonChecker('bogus', source, self.reporter)174 checker = PythonChecker('bogus', source, self.reporter)
176 # This should set the correct encoding.175 # This should set the correct encoding.
177 checker.check_text()176 checker.check_text()
@@ -183,33 +182,23 @@
183class TestPEP8(CheckerTestCase):182class TestPEP8(CheckerTestCase):
184 """Verify PEP8 integration."""183 """Verify PEP8 integration."""
185184
186 def setUp(self):
187 super(TestPEP8, self).setUp()
188 self.file = NamedTemporaryFile(prefix='pocketlint_')
189
190 def tearDown(self):
191 self.file.close()
192
193 def test_code_without_issues(self):185 def test_code_without_issues(self):
194 self.write_to_file(self.file, good_python)
195 checker = PythonChecker(186 checker = PythonChecker(
196 self.file.name, good_python, self.reporter)187 'file/path', good_python, self.reporter)
197 checker.check_pep8()188 checker.check_pep8()
198 self.assertEqual([], self.reporter.messages)189 self.assertEqual([], self.reporter.messages)
199190
200 def test_bad_syntax(self):191 def test_bad_syntax(self):
201 self.write_to_file(self.file, bad_syntax2_python)
202 checker = PythonChecker(192 checker = PythonChecker(
203 self.file.name, ugly_style_python, self.reporter)193 'file/path', ugly_style_python, self.reporter)
204 checker.check_pep8()194 checker.check_pep8()
205 self.assertEqual(195 self.assertEqual(
206 [(4, 'E901 TokenError: EOF in multi-line statement')],196 [(4, 'E222 multiple spaces after operator')],
207 self.reporter.messages)197 self.reporter.messages)
208198
209 def test_code_with_IndentationError(self):199 def test_code_with_IndentationError(self):
210 self.write_to_file(self.file, bad_indentation_python)
211 checker = PythonChecker(200 checker = PythonChecker(
212 self.file.name, bad_indentation_python, self.reporter)201 'file/path', bad_indentation_python, self.reporter)
213 checker.check_pep8()202 checker.check_pep8()
214 expected = [(203 expected = [(
215 4,204 4,
@@ -219,9 +208,8 @@
219 checker.check_pep8()208 checker.check_pep8()
220209
221 def test_code_closing_bracket(self):210 def test_code_closing_bracket(self):
222 self.write_to_file(self.file, hanging_style_python)
223 checker = PythonChecker(211 checker = PythonChecker(
224 self.file.name, hanging_style_python, self.reporter)212 'file/path', hanging_style_python, self.reporter)
225 checker.options.pep8['hang_closing'] = True213 checker.options.pep8['hang_closing'] = True
226 checker.check_pep8()214 checker.check_pep8()
227 self.assertEqual([], self.reporter.messages)215 self.assertEqual([], self.reporter.messages)
@@ -233,32 +221,28 @@
233 self.reporter.messages)221 self.reporter.messages)
234222
235 def test_code_with_issues(self):223 def test_code_with_issues(self):
236 self.write_to_file(self.file, ugly_style_python)
237 checker = PythonChecker(224 checker = PythonChecker(
238 self.file.name, ugly_style_python, self.reporter)225 'file/path', ugly_style_python, self.reporter)
239 checker.check_pep8()226 checker.check_pep8()
240 self.assertEqual(227 self.assertEqual(
241 [(4, 'E222 multiple spaces after operator')],228 [(4, 'E222 multiple spaces after operator')],
242 self.reporter.messages)229 self.reporter.messages)
243230
244 def test_code_with_comments(self):231 def test_code_with_comments(self):
245 self.write_to_file(self.file, ugly_style_lines_python)
246 checker = PythonChecker(232 checker = PythonChecker(
247 self.file.name, ugly_style_lines_python, self.reporter)233 'file/path', ugly_style_lines_python, self.reporter)
248 checker.check_pep8()234 checker.check_pep8()
249 self.assertEqual([], self.reporter.messages)235 self.assertEqual([], self.reporter.messages)
250236
251 def test_long_length_good(self):237 def test_long_length_good(self):
252 long_line = '1234 56189' * 7 + '12345678' + '\n'238 long_line = '1234 56189' * 7 + '12345678' + '\n'
253 self.write_to_file(self.file, long_line)239 checker = PythonChecker('file/path', long_line, self.reporter)
254 checker = PythonChecker(self.file.name, long_line, self.reporter)
255 checker.check_pep8()240 checker.check_pep8()
256 self.assertEqual([], self.reporter.messages)241 self.assertEqual([], self.reporter.messages)
257242
258 def test_long_length_bad(self):243 def test_long_length_bad(self):
259 long_line = '1234 56189' * 8 + '\n'244 long_line = '1234 56189' * 8 + '\n'
260 self.write_to_file(self.file, long_line)245 checker = PythonChecker('file/path', long_line, self.reporter)
261 checker = PythonChecker(self.file.name, long_line, self.reporter)
262 checker.check_pep8()246 checker.check_pep8()
263 self.assertEqual(247 self.assertEqual(
264 [(1, 'E501 line too long (80 > 79 characters)')],248 [(1, 'E501 line too long (80 > 79 characters)')],
@@ -266,11 +250,10 @@
266250
267 def test_long_length_options(self):251 def test_long_length_options(self):
268 long_line = '1234 56189' * 7 + '\n'252 long_line = '1234 56189' * 7 + '\n'
269 parser = get_option_parser()253 options = PocketLintOptions()
270 (options, sources) = parser.parse_args(['-m', '60'])254 options.max_line_length = 60
271 self.write_to_file(self.file, long_line)
272 checker = PythonChecker(255 checker = PythonChecker(
273 self.file.name, long_line, self.reporter, options)256 'file/path', long_line, self.reporter, options)
274 checker.check_pep8()257 checker.check_pep8()
275 self.assertEqual(258 self.assertEqual(
276 [(1, 'E501 line too long (70 > 59 characters)')],259 [(1, 'E501 line too long (70 > 59 characters)')],
@@ -322,8 +305,7 @@
322 def test_pep8_options(self):305 def test_pep8_options(self):
323 """It can set PEP8 options."""306 """It can set PEP8 options."""
324 long_line = '1234 56189' * 7 + '\n'307 long_line = '1234 56189' * 7 + '\n'
325 self.write_to_file(self.file, long_line)308 checker = PythonChecker('file/path', long_line, self.reporter)
326 checker = PythonChecker(self.file.name, long_line, self.reporter)
327 checker.options.pep8['ignore'] = ['E501']309 checker.options.pep8['ignore'] = ['E501']
328 checker.options.pep8['max_line_length'] = 60310 checker.options.pep8['max_line_length'] = 60
329 checker.check_pep8()311 checker.check_pep8()
@@ -346,7 +328,7 @@
346 [(1, 'All modules should have docstrings.'),328 [(1, 'All modules should have docstrings.'),
347 (3, 'First line should end with a period.'), ],329 (3, 'First line should end with a period.'), ],
348 self.reporter.messages,330 self.reporter.messages,
349 )331 )
350332
351 def test_code_with_ignore(self):333 def test_code_with_ignore(self):
352 """A list with error message which should be ignored can be334 """A list with error message which should be ignored can be
@@ -364,7 +346,7 @@
364 self.assertEqual(346 self.assertEqual(
365 [(1, 'All modules should have docstrings.'), ],347 [(1, 'All modules should have docstrings.'), ],
366 self.reporter.messages,348 self.reporter.messages,
367 )349 )
368350
369351
370class TestText(CheckerTestCase, TestAnyTextMixin):352class TestText(CheckerTestCase, TestAnyTextMixin):
371353
=== modified file 'pocketlint/tests/test_text.py'
--- pocketlint/tests/test_text.py 2014-03-09 21:00:26 +0000
+++ pocketlint/tests/test_text.py 2014-04-12 16:02:11 +0000
@@ -5,12 +5,12 @@
5 absolute_import,5 absolute_import,
6 print_function,6 print_function,
7 unicode_literals,7 unicode_literals,
8)8 )
99
10from pocketlint.formatcheck import (10from pocketlint.formatcheck import (
11 AnyTextChecker,11 AnyTextChecker,
12 get_option_parser,12 get_option_parser,
13)13 )
14from pocketlint.tests import Bunch, CheckerTestCase14from pocketlint.tests import Bunch, CheckerTestCase
1515
1616
@@ -71,7 +71,8 @@
71 regex_line=[71 regex_line=[
72 ('.*marker.*', 'Explanation.'),72 ('.*marker.*', 'Explanation.'),
73 ('.*sign.*', 'Message.'),73 ('.*sign.*', 'Message.'),
74 ])74 ],
75 )
7576
76 self.create_and_check(77 self.create_and_check(
77 'bogus',78 'bogus',

Subscribers

People subscribed via source and target branches

to all changes: