Skip to content

Commit

Permalink
Merge pull request #206 from plone/fix-code-analysis
Browse files Browse the repository at this point in the history
Fix code analysis
  • Loading branch information
gforcada committed Aug 22, 2017
2 parents 9b7ec0e + ec56af4 commit 8731f5e
Show file tree
Hide file tree
Showing 18 changed files with 139 additions and 119 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ Change history
- Remove bootstrap-buildout.py as it is no longer used.
[gforcada]

- Fix code analysis errors.
[gforcada]

2.2 (2016-02-20)
----------------

Expand Down
28 changes: 17 additions & 11 deletions plone/recipe/codeanalysis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ def __init__(self, buildout, name, options):
self.egg = zc.recipe.egg.Scripts(
buildout,
self.options['recipe'],
options
options,
)

options['location'] = os.path.join(
buildout['buildout']['parts-directory'], name)
buildout['buildout']['parts-directory'], name,
)

# Set required default options
self.options.setdefault('directory', '.')
Expand Down Expand Up @@ -84,7 +85,7 @@ def __init__(self, buildout, name, options):
self.options.setdefault('zptlint-bin', '')
# Figure out default output file
plone_jenkins = os.path.join(
self.buildout['buildout']['parts-directory'], 'code-analysis'
self.buildout['buildout']['parts-directory'], 'code-analysis',
)
if not os.path.exists(plone_jenkins):
os.makedirs(plone_jenkins)
Expand All @@ -93,8 +94,8 @@ def __init__(self, buildout, name, options):
self.files = [
plone_jenkins,
os.path.join(
self.buildout['buildout']['bin-directory'], self.name
)
self.buildout['buildout']['bin-directory'], self.name,
),
]

def install(self):
Expand Down Expand Up @@ -143,7 +144,8 @@ def install_scripts(self):

def add_script(cmd, **kwargs):
zc.buildout.easy_install.scripts(
[cmd], eggs, python, directory, **kwargs)
[cmd], eggs, python, directory, **kwargs
)

# flake8
add_script('flake8')
Expand All @@ -152,7 +154,7 @@ def add_script(cmd, **kwargs):
# bin/code-analysis
add_script(
(self.name, self.__module__, 'code_analysis'),
arguments=arguments
arguments=arguments,
)
# isort
if 'flake8-isort' in self.extensions:
Expand All @@ -165,16 +167,20 @@ def add_script(cmd, **kwargs):
if not instance.enabled and 'console_script' in klass.__module__:
continue

cmd = ('{0}-{1}'.format(self.name, instance.name),
klass.__module__, 'console_script')
cmd = (
'{0}-{1}'.format(self.name, instance.name),
klass.__module__, 'console_script',
)

add_script(cmd, arguments=arguments)

def install_pre_commit_hook(self):
git_directory = self.buildout['buildout']['directory'] + '/.git'
if not os.path.exists(git_directory):
print('Unable to create git pre-commit hook, '
'this does not seem to be a git repository.')
print(
'Unable to create git pre-commit hook, '
'this does not seem to be a git repository.'
)
return

git_hooks_directory = git_directory + '/hooks'
Expand Down
19 changes: 11 additions & 8 deletions plone/recipe/codeanalysis/analyser.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def find_files(self, regex='.*', paths=None, exclude=None):
process_files = subprocess.Popen(
cmd,
stderr=subprocess.STDOUT,
stdout=subprocess.PIPE
stdout=subprocess.PIPE,
)
exclude, err = process_files.communicate()
if isinstance(exclude, bytes):
Expand All @@ -176,7 +176,8 @@ def open_output_file(self):
return TemporaryFile('w+')

return open(
os.path.join(self.options['location'], self.output_filename), 'w+')
os.path.join(self.options['location'], self.output_filename), 'w+',
)

def process_output(self, output):
"""Replace all occurrences of substring ``self.output_regex``
Expand All @@ -187,7 +188,7 @@ def process_output(self, output):
"""
error = self.output_regex
output = map(
lambda x: error.sub(self.output_replace, x), output.splitlines()
lambda x: error.sub(self.output_replace, x), output.splitlines(),
)
return '\n'.join(output).strip()

Expand All @@ -198,12 +199,12 @@ def parse_output(self, output_file, return_code):
if self.use_jenkins:
self.log(
'failure',
'Output file written to {0}.'.format(output_file.name)
'Output file written to {0}.'.format(output_file.name),
)
else:
self.log(
'failure',
self.process_output(output_file.read())
self.process_output(output_file.read()),
)
return False
else:
Expand All @@ -229,9 +230,11 @@ def run(self):
if not len(command):
self.log('ok')
return True
process = subprocess.Popen(command,
stderr=subprocess.STDOUT,
stdout=output_file)
process = subprocess.Popen(
command,
stderr=subprocess.STDOUT,
stdout=output_file,
)
process.wait()
output_file.flush()
output_file.seek(0)
Expand Down
16 changes: 10 additions & 6 deletions plone/recipe/codeanalysis/check_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ class CheckManifest(Analyser):
def cmd(self):
return [
os.path.join(self.options['bin-directory'], 'check-manifest'),
'-v'
'-v',
]

@property
def packages(self):
paths = set()
items = CheckManifest.split_lines(
self.options['check-manifest-directory'])
self.options['check-manifest-directory'],
)
for item in items:
paths.add(os.path.realpath(item))
if not paths:
Expand All @@ -40,14 +41,17 @@ def run(self):
cmd = self.cmd
cmd.append(package)
try:
process = subprocess.Popen(cmd,
stderr=subprocess.STDOUT,
stdout=output_file)
process = subprocess.Popen(
cmd,
stderr=subprocess.STDOUT,
stdout=output_file,
)
process.wait()
output_file.flush()
output_file.seek(0)
status = status and self.parse_output(
output_file, process.returncode)
output_file, process.returncode,
)
except OSError:
self.log('skip')
finally:
Expand Down
6 changes: 3 additions & 3 deletions plone/recipe/codeanalysis/clean_lines.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class CleanLines(Analyser):
re.compile(r' $'): 'trailing spaces',
re.compile(r'\t'): 'tabs',
},
}
},
]
ignore_patterns = (
re.compile(r'#\snoqa'),
Expand Down Expand Up @@ -52,7 +52,7 @@ def check(self, file_path, fail={}, succeed={}, **kwargs):
if match:
message = message.format(match.group())
errors.append(self.message.format(
file_path, 1 + linenumber, message
file_path, 1 + linenumber, message,
))

# Fail if succeed was not found
Expand All @@ -61,7 +61,7 @@ def check(self, file_path, fail={}, succeed={}, **kwargs):
continue

errors.append(self.message.format(
file_path, 1 + linenumber, message
file_path, 1 + linenumber, message,
))

return errors
Expand Down
4 changes: 2 additions & 2 deletions plone/recipe/codeanalysis/flake8.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def get_flake8_options(self):
# by flake8 itself or any of its plugins
no_options = (
'flake8-extensions',
'flake8-filesystem'
'flake8-filesystem',
)
# get the options
options = [
Expand All @@ -32,7 +32,7 @@ def get_flake8_options(self):
options = [
'{0}={1}'.format(
o.replace('flake8', '-'),
self.options.get(o)
self.options.get(o),
)
for o in options
]
Expand Down
3 changes: 2 additions & 1 deletion plone/recipe/codeanalysis/jshint.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ def cmd(self):
@property
def suppress_warnings(self):
return JSHint.normalize_boolean(
self.get_prefixed_option('suppress-warnings'))
self.get_prefixed_option('suppress-warnings'),
)

def parse_output(self, output_file, return_code):
"""Search for error markers as JSHint always return an exit code of 2
Expand Down
10 changes: 5 additions & 5 deletions plone/recipe/codeanalysis/tests/test_check_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,23 @@ def setUp(self): # noqa

def test_check_manifest_cmd(self):
executable = '{0:s}/check-manifest'.format(
self.options['bin-directory']
self.options['bin-directory'],
)
with OutputCapture():
self.assertEqual(
CheckManifest(self.options).cmd,
[executable, '-v', ]
[executable, '-v', ],
)

def test_check_manifest_packages(self):
with OutputCapture():
self.assertEqual(
CheckManifest(self.options).packages, set([self.test_dir])
CheckManifest(self.options).packages, set([self.test_dir]),
)

def test_check_manifest_should_return_true_on_this_package(self):
self.options['check-manifest-directory'] = os.path.realpath(
os.path.join(os.path.dirname(__file__), '../../../..')
os.path.join(os.path.dirname(__file__), '../../../..'),
)
with OutputCapture():
self.assertTrue(CheckManifest(self.options).run())
Expand All @@ -49,7 +49,7 @@ def test_check_manifest_should_return_true_if_no_check_manifest_installed(self):

def test_check_manifest_should_raise_systemexit_0_in_console_script(self):
self.options['check-manifest-directory'] = os.path.realpath(
os.path.join(os.path.dirname(__file__), '../../../..')
os.path.join(os.path.dirname(__file__), '../../../..'),
)
with OutputCapture():
with self.assertRaisesRegexp(SystemExit, '0'):
Expand Down
4 changes: 2 additions & 2 deletions plone/recipe/codeanalysis/tests/test_clean_lines.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_analysis_should_return_true_if_invalid_file_is_excluded(self):
filename = 'invalid.py'
self.given_a_file_in_test_dir(filename, INVALID_CODE)
self.options['clean-lines-exclude'] = '{0:s}/{1:s}'.format(
self.test_dir, filename
self.test_dir, filename,
)
with OutputCapture():
self.assertTrue(CleanLines(self.options).run())
Expand All @@ -59,7 +59,7 @@ def test_analysis_should_return_true_if_file_with_tabs_is_excluded(self):
filename = 'invalid.xml'
self.given_a_file_in_test_dir(filename, INVALID_TABS)
self.options['clean-lines-exclude'] = '{0:s}/{1:s}'.format(
self.test_dir, filename
self.test_dir, filename,
)
with OutputCapture():
self.assertTrue(CleanLines(self.options).run())
Expand Down
2 changes: 1 addition & 1 deletion plone/recipe/codeanalysis/tests/test_csslint.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

INCORRECT_CSS = """\
a:link {color: blue}
{}
<>
h3 {color: red}
bodyy {color: purple}
"""
Expand Down
8 changes: 4 additions & 4 deletions plone/recipe/codeanalysis/tests/test_flake8.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def test_analysis_should_return_true_if_invalid_file_is_excluded(self):
filename = 'invalid.py'
self.given_a_file_in_test_dir(filename, INVALID_CODE)
self.options['flake8-exclude'] = '{0:s}/{1:s}'.format(
self.test_dir, filename
self.test_dir, filename,
)
with OutputCapture():
self.assertTrue(Flake8(self.options).run())
Expand All @@ -102,7 +102,7 @@ def test_get_flake8_options(self):
self.assertEqual(
len(options),
# --one=something --two=else --jobs=1 + default options
2 + 1 + len(self.flake8_default_options)
2 + 1 + len(self.flake8_default_options),
)

def test_get_flake8_options_ignored(self):
Expand All @@ -115,7 +115,7 @@ def test_get_flake8_options_ignored(self):
self.assertEqual(
len(options),
# --one=something --jobs=1 + default options
1 + 1 + len(self.flake8_default_options)
1 + 1 + len(self.flake8_default_options),
)

def test_get_flake8_options_on_deactivated_multiprocessing(self):
Expand All @@ -125,5 +125,5 @@ def test_get_flake8_options_on_deactivated_multiprocessing(self):
options = Flake8(self.options).get_flake8_options()
self.assertEqual(
len(options),
len(self.flake8_default_options) # just default options
len(self.flake8_default_options), # just default options
)
2 changes: 1 addition & 1 deletion plone/recipe/codeanalysis/tests/test_i18ndude.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_analysis_should_return_true_if_file_invalid_is_excluded(self):
filename = 'invalid.pt'
self.given_a_file_in_test_dir(filename, INVALID_CODE)
self.options['find-untranslated-exclude'] = '{0:s}/{1:s}'.format(
self.test_dir, filename
self.test_dir, filename,
)
with OutputCapture():
self.assertTrue(I18NDude(self.options).run())
Expand Down
2 changes: 1 addition & 1 deletion plone/recipe/codeanalysis/tests/test_jscs.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_analysis_should_return_true_when_invalid_file_is_excluded(self):
filename = 'incorrect.js'
self.given_a_file_in_test_dir(filename, INCORRECT_FILE)
self.options['jscs-exclude'] = '{0:s}/{1:s}'.format(
self.test_dir, filename
self.test_dir, filename,
)
with OutputCapture():
self.assertTrue(JSCS(self.options).run())
Expand Down
4 changes: 2 additions & 2 deletions plone/recipe/codeanalysis/tests/test_jshint.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def test_analysis_file_should_exist_when_jenkins_is_true(self):
def test_jshint_parse_output_should_return_true_empty_xml_output(self):
file_path = self.given_a_file_in_test_dir(
'jshint.xml',
XML_EMPTY_OUTPUT
XML_EMPTY_OUTPUT,
)
self.options['jenkins'] = 'True'
linter = JSHint(self.options)
Expand Down Expand Up @@ -145,7 +145,7 @@ def test_jshint_parse_output_should_return_true_empty_normal_output(self):
def test_jshint_parse_output_should_return_false_if_warnings_not_suppressed(self): # noqa
file_path = self.given_a_file_in_test_dir(
'jshint.xml',
EXPECTED_WARNINGS_OUTPUT
EXPECTED_WARNINGS_OUTPUT,
)
self.options['jenkins'] = 'False'
self.options['jshint-suppress-warnings'] = 'False'
Expand Down
Loading

1 comment on commit 8731f5e

@jenkins-plone-org
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gforcada Jenkins CI reporting about code analysis
See the full report here: http://jenkins.plone.org/job/package-plone.recipe.codeanalysis/38/violations

plone/recipe/codeanalysis/__init__.py:147:57: C815 missing trailing comma in Python 3.5+
plone/recipe/codeanalysis/__init__.py:182:61: C813 missing trailing comma in Python 3

Follow these instructions to reproduce it locally.

Please sign in to comment.