Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix license check #28

Merged
merged 7 commits into from
Aug 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions inspektor/cli/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,7 @@ def __init__(self):
deferred_help=True,
)

def initialize_app(self, argv):
self.LOG.debug('initialize_app')

def prepare_to_run_command(self, cmd):
self.LOG.debug('prepare_to_run_command %s', cmd.__class__.__name__)

def clean_up(self, cmd, result, err):
self.LOG.debug('clean_up %s', cmd.__class__.__name__)
Copy link
Contributor

Choose a reason for hiding this comment

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

yep, at this point they are useless...

if err:
self.LOG.debug('got an error: %s', err)

Expand Down
14 changes: 11 additions & 3 deletions inspektor/commands/checkall.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

class CheckAllCommand(Command):
"""
check indentation, style, licensing headers and lint
check indentation, style, lint and (optionally) license headers
"""
log = logging.getLogger(__name__)

Expand Down Expand Up @@ -56,6 +56,8 @@ def get_parser(self, prog_name):
help='Quoted string containing paths or '
'patterns to be excluded from '
'checking, comma separated')
parser.add_argument('--no-license-check', action='store_true', default=False,
help='Do not perform license check')
parser.add_argument('--license', type=str,
help=('License type. Supported license types: %s. '
'Default: %s' %
Expand All @@ -79,14 +81,20 @@ def take_action(self, parsed_args):
reindenter = Reindenter(parsed_args, logger=self.log)
style_checker = StyleChecker(parsed_args, logger=self.log)
linter = Linter(parsed_args, logger=self.log)
license_checker = LicenseChecker(parsed_args, logger=self.log)
if not parsed_args.no_license_check:
self.log.info('License check: (%s)', parsed_args.license)
license_checker = LicenseChecker(parsed_args, logger=self.log)
else:
self.log.info('License check: disabled')
license_checker = None

status = True
for path in checked_paths:
status &= linter.check(path=path)
status &= reindenter.check(path=path)
status &= style_checker.check(path=path)
status &= license_checker.check(path=path)
if license_checker is not None:
status &= license_checker.check(path=path)

if status:
self.log.info('Global check PASS')
Expand Down
9 changes: 6 additions & 3 deletions inspektor/indent.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ def check_file(self, path):
didn't find problems, path is not a python module or
script.
"""
checker = PathChecker(path=path, args=self.args, label='Indent')
checker = PathChecker(path=path, args=self.args, label='Indent',
logger=self.log)
if not checker.check_attributes('text', 'python', 'not_empty'):
return True

Expand All @@ -219,15 +220,17 @@ def check_file(self, path):
f.close()
try:
if r.run():
self.log.error('Indentation check fail : %s', path)
self.failed_paths.append(path)
fix_status = ''
if self.args.fix:
f = open(path, "w")
r.write(f)
f.close()
self.log.info('FIX OK')
fix_status = 'FIX OK'
checker.log_status(status='FAIL', extra=fix_status)
return False
else:
checker.log_status(status='PASS')
return True
except IndentationError:
self.log.error("Indentation check fail : %s", path)
Expand Down
37 changes: 19 additions & 18 deletions inspektor/license.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,13 @@
class LicenseChecker(object):

def __init__(self, args, logger=logging.getLogger('')):
license_type = args.license
self.license_type = args.license
cpyright = args.copyright
author = args.author
self.args = args
self.failed_paths = []
self.log = logger

self.license_contents = license_mapping[license_type]
self.license_contents = license_mapping[self.license_type]
self.base_license_contents = self.license_contents

if cpyright:
Expand All @@ -73,7 +72,8 @@ def check_dir(self, path):
return not self.failed_paths

def check_file(self, path):
checker = PathChecker(path=path, args=self.args, label='License')
checker = PathChecker(path=path, args=self.args, label='License',
logger=self.log)
# Don't put license info in empty __init__.py files.
if not checker.check_attributes('text', 'python', 'not_empty'):
return True
Expand All @@ -82,28 +82,29 @@ def check_file(self, path):
if checker.path.script('python'):
first_line = checker.path.first_line

new_content = None
with open(path, 'r') as inspected_file:
content = inspected_file.readlines()
if first_line is not None:
content = content[1:]
content = "".join(content)
if self.base_license_contents not in content:
if not self.args.fix:
return False
new_content = ""
if first_line is not None:
new_content += first_line
new_content += '\n'
new_content += self.license_contents + '\n' + content

if new_content is not None:
with open(path, 'w') as inspected_file:
inspected_file.write(new_content)
fix_status = ''
if self.args.fix:
new_content = ""
if first_line is not None:
new_content += first_line
new_content += '\n'
new_content += self.license_contents + '\n' + content
with open(path, 'w') as inspected_file:
inspected_file.write(new_content)
fix_status = 'FIX OK'

self.failed_paths.append(path)
checker.log_status(status='FAIL', extra=fix_status)
return False

return True
else:
checker.log_status(status='PASS')
return True

def check(self, path):
if os.path.isfile(path):
Expand Down
20 changes: 16 additions & 4 deletions inspektor/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
from pylint.lint import Run

from .path import PathChecker
from .utils import process

_PYLINT_HELP_TEXT = process.run('pylint --help', verbose=False).stdout


class Linter(object):
Expand Down Expand Up @@ -49,6 +52,10 @@ def __init__(self, args, logger=logging.getLogger('')):
def set_verbose(self):
self.verbose = True

@staticmethod
def _pylint_has_option(option):
return option in _PYLINT_HELP_TEXT

def get_opts(self):
"""
If VERBOSE is set, show pylint reports. If not, only an issue summary.
Expand All @@ -63,8 +70,9 @@ def get_opts(self):
pylint_args.append('--disable=%s' % self.ignored_errors)
if self.enabled_errors:
pylint_args.append('--enable=%s' % self.enabled_errors)
pylint_args.append('--reports=no')
if sys.version_info[:2] > (2, 6):
if self._pylint_has_option('--reports='):
pylint_args.append('--reports=no')
if self._pylint_has_option('--score='):
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, hopefully they won't change that format 😄

pylint_args.append('--score=no')

return pylint_args
Expand All @@ -89,19 +97,23 @@ def check_file(self, path):
:return: False, if pylint found syntax problems, True, if pylint didn't
find problems, or path is not a python module or script.
"""
checker = PathChecker(path=path, args=self.args, label='Lint')
checker = PathChecker(path=path, args=self.args, label='Lint',
logger=self.log)
if not checker.check_attributes('text', 'python', 'not_empty'):
return True
try:
runner = Run(self.get_opts() + [path], exit=False)
if runner.linter.msg_status != 0:
self.log.error('Pylint check fail: %s', path)
self.failed_paths.append(path)
checker.log_status(status='FAIL')
else:
checker.log_status(status='PASS')
return runner.linter.msg_status == 0
except Exception as details:
self.log.error('Pylint check fail: %s (pylint exception: %s)',
path, details)
self.failed_paths.append(path)
checker.log_status(status='FAIL')
return False

def check(self, path):
Expand Down
10 changes: 8 additions & 2 deletions inspektor/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ def check_attributes(self, *args):
checks_passed = True
for arg in args:
checks_passed &= getattr(self.path, arg)
if checks_passed:
self.log.debug('%s: %s', self.label, self.path)
return checks_passed

def log_status(self, status, extra=''):
if extra:
extra = '(%s)' % extra
if status == 'PASS':
self.log.debug('%s: %s %s %s', self.label, self.path, status, extra)
elif status == 'FAIL':
self.log.error('%s: %s %s %s', self.label, self.path, status, extra)
13 changes: 10 additions & 3 deletions inspektor/style.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ def check_file(self, path):
:return: False, if pylint found syntax problems, True, if pylint didn't
find problems, or path is not a python module or script.
"""
checker = PathChecker(path=path, args=self.args, label='Style')
checker = PathChecker(path=path, args=self.args, label='Style',
logger=self.log)
if not checker.check_attributes('text', 'python', 'not_empty'):
return True

Expand All @@ -82,20 +83,26 @@ def check_file(self, path):
status = 1

if status != 0:
self.log.error('Style check fail: %s', path)
self.failed_paths.append(path)
fix_status = ''
if AUTOPEP8_CAPABLE:
if self.args.fix:
self.log.info('Trying to fix errors with autopep8')
try:
process.run('autopep8 --in-place --max-line-length=%s --ignore %s %s' % (self.args.max_line_length, self.args.disable, path))
process.run('autopep8 --in-place --max-line-length=%s --ignore %s %s' % (self.args.max_line_length, self.ignored_errors, path))
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a fix not related to this commit...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I'm a lazy bastard :) Sorry for that one.

fix_status = 'FIX OK'
except Exception:
self.log.error('Unable to fix errors')
exc_info = sys.exc_info()
stacktrace.log_exc_info(exc_info, 'inspektor.style')
fix_status = 'FIX NOT OK'
else:
self.log.error('Python library autopep8 not installed. '
'Please install it if you want to use --fix')
fix_status = 'FIX NOT OK'
checker.log_status(status='FAIL', extra=fix_status)
else:
checker.log_status(status='PASS')

return status == 0

Expand Down
12 changes: 12 additions & 0 deletions inspektor/utils/data_structures.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
#
# See LICENSE for more details.


class Borg:

"""
Expand Down
4 changes: 2 additions & 2 deletions inspektor/utils/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ def run(cmd, verbose=True, ignore_status=False, shell=False):
duration = time.time() - start
result = CmdResult(cmd)
result.exit_status = p.returncode
result.stdout = stdout
result.stderr = stderr
result.stdout = str(stdout)
result.stderr = str(stderr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this required? Isn't the return of communicate already string?

Copy link
Member Author

Choose a reason for hiding this comment

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

In py3 it's bytes :)

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

result.duration = duration
if p.returncode != 0 and not ignore_status:
raise exceptions.CmdError(cmd, result)
Expand Down