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

Bug solving sprint #747

Merged
merged 21 commits into from
Sep 3, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4f01f11
requirements: Remove setuptools
AbdealiLoKo Sep 2, 2016
5a4c5c2
GitCommitBear: Improve assertion empty queue
AbdealiLoKo Sep 2, 2016
241726c
GitCommitBear: Add ignore_length_regex setting
AbdealiLoKo Sep 2, 2016
4e5a4d8
LocalBearTestHelper: Return results for checking
AbdealiLoKo Sep 3, 2016
dc3d2ec
YapfBear: Catch parse errors and give result
AbdealiLoKo Sep 2, 2016
ae85d9d
YapfBear: Add prefer line break setting
AbdealiLoKo Sep 2, 2016
c24be59
ESLintBear: Update version of eslint
AbdealiLoKo Sep 3, 2016
6f685f4
MarkdownBear: Update version of remark
AbdealiLoKo Sep 3, 2016
138c7aa
CSSLintBear: Update csslint version
AbdealiLoKo Sep 3, 2016
600ba0b
AlexBear: Update alex version
AbdealiLoKo Sep 3, 2016
f8c00cd
KeywordBear: Simplify config for case sensitivity
AbdealiLoKo Sep 3, 2016
35140b4
KeywordBear: Conform to style guide
AbdealiLoKo Sep 3, 2016
bfd61fb
KeywordBear: Simplify and optimize using regex
AbdealiLoKo Sep 3, 2016
fa70e92
AlexBear: Check if wrong alex is installed
AbdealiLoKo Sep 3, 2016
22cdc8a
JSHintBear: Deprecate "use_es6_syntax"
AbdealiLoKo Aug 31, 2016
db8a2de
JSHintBear: Deprecate - "globalstrict" -> "strict"
AbdealiLoKo Aug 31, 2016
ee34cdc
bears.configfile: Add new bear - PuppetLintBear
AbdealiLoKo Sep 3, 2016
c60e9f5
ESLintBear: Handle corner case if eslint fails
AbdealiLoKo Sep 2, 2016
5c72d4e
requirements: Remove redundant comment abot nltk
AbdealiLoKo Sep 3, 2016
8a3d7fd
generate_packageTest: Use correct casing
AbdealiLoKo Sep 3, 2016
b3be8f3
natural_language: Add SpellCheckBear
mr-karan Jul 25, 2016
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
4 changes: 3 additions & 1 deletion .coafile
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,7 @@ max_lines_per_file = 1000
enabled = False
bears = KeywordBear

ci_keywords, keywords_case_insensitive = \#TODO, \# TODO, \#FIXME, \# FIXME
# Note that the ci_keywords and cs_keywords are only used here because coala
# 0.8.x (current stable release) needs them.
ci_keywords, keywords = \#TODO, \# TODO, \#FIXME, \# FIXME
cs_keywords =
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ gem "rubocop"
gem "sqlint"
gem 'scss_lint', require: false# require flag is necessary https://github.com/brigade/scss-lint#installation
gem "reek"
gem "puppet-lint"
26 changes: 26 additions & 0 deletions bears/configfiles/PuppetLintBear.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from coalib.bearlib.abstractions.Linter import linter
from coalib.bears.requirements.GemRequirement import GemRequirement


@linter(executable='puppet-lint',
output_format='regex',
output_regex=r'(?P<line>\d+):(?P<column>\d+):'
r'(?P<severity>warning|error):(?P<message>.+)')
class PuppetLintBear:
'''
Check and correct puppet configuration files using ``puppet-lint``.

See <http://puppet-lint.com/> for details about the tool.
'''

LANGUAGES = {"Puppet"}
REQUIREMENTS = {GemRequirement('puppet-lint', '2')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
CAN_FIX = {'Syntax'}
Copy link
Member

Choose a reason for hiding this comment

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

can you add an asciinema URL or file an issue about making one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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


@staticmethod
def create_arguments(filename, file, config_file):
return ('--log-format', "%{line}:%{column}:%{kind}:%{message}",
filename)
2 changes: 1 addition & 1 deletion bears/css/CSSLintBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class CSSLintBear:
problems or inefficiencies.
"""
LANGUAGES = {"CSS"}
REQUIREMENTS = {NpmRequirement('csslint', '0')}
REQUIREMENTS = {NpmRequirement('csslint', '1')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
Expand Down
61 changes: 19 additions & 42 deletions bears/general/KeywordBear.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import re

from coalib.bearlib import deprecate_settings
from coalib.bears.LocalBear import LocalBear
from coalib.results.Result import RESULT_SEVERITY, Result
Expand All @@ -10,53 +12,28 @@ class KeywordBear(LocalBear):
LICENSE = 'AGPL-3.0'
CAN_DETECT = {'Documentation'}

@deprecate_settings(keywords_case_sensitive='cs_keywords')
def run(self,
filename,
file,
keywords_case_insensitive: list,
keywords_case_sensitive: list=()):
@deprecate_settings(keywords='ci_keywords')
def run(self, filename, file, keywords: list):
'''
Checks the code files for given keywords.

:param keywords_case_insensitive:
:param keywords:
A list of keywords to search for (case insensitive).
Usual examples are TODO and FIXME.
:param keywords_case_sensitive:
A list of keywords to search for (case sensitive).
'''
results = list()

for i in range(len(keywords_case_insensitive)):
keywords_case_insensitive[i] = keywords_case_insensitive[i].lower()
keywords_regex = re.compile(
'(' + '|'.join(re.escape(key) for key in keywords) + ')',
Copy link
Member

Choose a reason for hiding this comment

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

Would this not result in having a first empty group?
aka (|a|b|c...)

Copy link
Member

Choose a reason for hiding this comment

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

no as per explanation on gitter

re.IGNORECASE)

for line_number, line in enumerate(file):
for keyword in keywords_case_sensitive:
results += self.check_line_for_keyword(line,
filename,
line_number,
keyword)

for keyword in keywords_case_insensitive:
results += self.check_line_for_keyword(line.lower(),
filename,
line_number,
keyword)

return results

def check_line_for_keyword(self, line, filename, line_number, keyword):
pos = line.find(keyword)
if pos != -1:
return [Result.from_values(
origin=self,
message="The line contains the keyword `{}`."
.format(keyword),
file=filename,
line=line_number+1,
column=pos+1,
end_line=line_number+1,
end_column=pos+len(keyword)+1,
severity=RESULT_SEVERITY.INFO)]

return []
for keyword in keywords_regex.finditer(line):
yield Result.from_values(
origin=self,
message="The line contains the keyword '{}'."
.format(keyword.group()),
file=filename,
line=line_number + 1,
column=keyword.start() + 1,
end_line=line_number + 1,
end_column=keyword.end() + 1,
severity=RESULT_SEVERITY.INFO)
4 changes: 2 additions & 2 deletions bears/js/ESLintBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ESLintBear:
"""

LANGUAGES = {"JavaScript", "JSX"}
REQUIREMENTS = {NpmRequirement('eslint', '2')}
REQUIREMENTS = {NpmRequirement('eslint', '3')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
Expand Down Expand Up @@ -48,7 +48,7 @@ def generate_config(filename, file):
return '{"extends": "eslint:recommended"}'

def process_output(self, output, filename, file):
if not file:
if not file or not output:
return
Copy link
Member

Choose a reason for hiding this comment

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

IMO we'll rather want to trow a warning in that case with a nice message what's wrong instead of silently approving that all code is good!

Copy link
Member

Choose a reason for hiding this comment

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

i.e. the message will probably have a similar structure, make a regex and match the message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, but I do not have stderr here. SO, the message is empty ...
I will be solving coala/coala#2711 and #730 soon after. But this JSONDecodeError is another issue which Im fixing here


output = json.loads(output)
Expand Down
33 changes: 22 additions & 11 deletions bears/js/JSHintBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ class JSHintBear:
CAN_DETECT = {'Formatting', 'Syntax', 'Complexity', 'Unused Code'}

@staticmethod
@deprecate_settings(cyclomatic_complexity='maxcomplexity',
@deprecate_settings(es_version='use_es6_syntax',
javascript_strictness=(
"allow_global_strict",
lambda x: "global" if x else True),
Copy link
Member

Choose a reason for hiding this comment

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

ah your lambdas aren't covered by the tests

cyclomatic_complexity='maxcomplexity',
allow_unused_variables=('prohibit_unused', negate),
max_parameters='maxparams',
allow_missing_semicolon='allow_missing_semicol',
Expand Down Expand Up @@ -89,20 +93,19 @@ def generate_config(filename, file,
allow_debugger: bool=False,
allow_assignment_comparisions: bool=False,
allow_eval: bool=False,
allow_global_strict: bool=False,
allow_increment: bool=False,
allow_proto: bool=False,
allow_scripturls: bool=False,
allow_singleton: bool=False,
allow_this_statements: bool=False,
allow_with_statements: bool=False,
use_mozilla_extension: bool=False,
javascript_strictness: bool_or_str=True,
allow_noyield: bool=False,
allow_eqnull: bool=False,
allow_last_semicolon: bool=False,
allow_func_in_loop: bool=False,
allow_expr_in_assignments: bool=False,
use_es6_syntax: bool=False,
use_es3_array: bool=False,
environment_mootools: bool=False,
environment_couch: bool=False,
Expand Down Expand Up @@ -132,7 +135,7 @@ def generate_config(filename, file,
allow_variable_shadowing: bool_or_str=False,
allow_unused_variables: bool_or_str=False,
allow_latedef: bool_or_str=False,
es_version: int=5,
es_version: bool_or_int=5,
jshint_config: str=""):
"""
:param allow_bitwise_operators:
Expand Down Expand Up @@ -184,9 +187,6 @@ def generate_config(filename, file,
:param allow_eval:
This options suppresses warnings about the use of ``eval``
function.
:param allow_global_strict:
This option suppresses warnings about the use of global strict
mode.
:param allow_increment:
This option suppresses warnings about the use of unary increment
and decrement operators.
Expand All @@ -209,6 +209,14 @@ def generate_config(filename, file,
:param use_mozilla_extension:
This options tells JSHint that your code uses Mozilla JavaScript
extensions.
:param javascript_strictness:
Determines what sort of strictness to use in the JavaScript code.
The possible options are:

- "global" - there must be a ``"use strict";`` at global level
- "implied" - lint the code as if there is a ``"use strict";``
- "False" - disable warnings about strict mode
- "True" - there must be a ``"use strict";`` at function level
:param allow_noyield:
This option suppresses warnings about generator functions with no
``yield`` statement in them.
Expand All @@ -225,8 +233,6 @@ def generate_config(filename, file,
:param use_es3_array:
This option tells JSHintBear ES3 array elision elements, or empty
elements are used.
:param use_es3_array:
This option tells JSHint ECMAScript 6 specific syntax is used.
:param environment_mootools:
This option defines globals exposed by the Mootools.
:param environment_couch:
Expand Down Expand Up @@ -308,6 +314,12 @@ def generate_config(filename, file,
This option is used to specify the ECMAScript version to which the
code must adhere to.
"""
# Assume that when es_version is bool, it is intended for the
# deprecated use_es6_version
if es_version is True:
es_version = 6
elif es_version is False:
es_version = 5
Copy link
Member

Choose a reason for hiding this comment

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

actually you can put this into the conversion routine/annotation for es_version huh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not complete. If I put it there, people can still do es_version=True (As it's bool_or_int) which can throw errors.
Hence I had to move it down here.

Copy link
Member

Choose a reason for hiding this comment

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

isn't the conversion run on any given valu?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't gone into the dept of reading it and understanding what it does
exactly, but it did give me an error. I've filed an issue here
coala/coala#2715

On Sat, Sep 3, 2016 at 9:36 PM, Lasse Schuirmann [email protected]
wrote:

In bears/js/JSHintBear.py
#747 (comment)
:

@@ -308,6 +306,12 @@ def generate_config(filename, file,
This option is used to specify the ECMAScript version to which the
code must adhere to.
"""

  •    # Assume that when es_version is bool, it is intended for the
    
  •    # deprecated use_es6_version
    
  •    if es_version is True:
    
  •        es_version = 6
    
  •    elif es_version is False:
    
  •        es_version = 5
    

isn't the conversion run on any given valu?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/coala-analyzer/coala-bears/pull/747/files/09efa41e64d6a88201c94ad2b0bff9a8f6ea52ed..5fd63b2490955fd31c1ea01d950139a6836fab10#r77438715,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACGUp_Dm1Gw_ubLMcDJ5-PjpREWFzk4Jks5qmZrsgaJpZM4JxwAR
.

if not jshint_config:
options = {"bitwise": not allow_bitwise_operators,
"freeze": not allow_prototype_overwrite,
Expand All @@ -329,7 +341,7 @@ def generate_config(filename, file,
"debug": allow_debugger,
"boss": allow_assignment_comparisions,
"evil": allow_eval,
"globalstrict": allow_global_strict,
"strict": javascript_strictness,
"plusplus": allow_increment,
"proto": allow_proto,
"scripturl": allow_scripturls,
Expand All @@ -342,7 +354,6 @@ def generate_config(filename, file,
"lastsemic": allow_last_semicolon,
"loopfunc": allow_func_in_loop,
"expr": allow_expr_in_assignments,
"esnext": use_es6_syntax,
"elision": use_es3_array,
"mootools": environment_mootools,
"couch": environment_couch,
Expand Down
2 changes: 1 addition & 1 deletion bears/markdown/MarkdownBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class MarkdownBear:
"""

LANGUAGES = {"Markdown"}
REQUIREMENTS = {NpmRequirement('remark', '3')}
REQUIREMENTS = {NpmRequirement('remark-cli', '2')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
Expand Down
29 changes: 28 additions & 1 deletion bears/natural_language/AlexBear.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import subprocess
import sys

from coalib.bearlib.abstractions.Linter import linter
from coalib.bears.requirements.NpmRequirement import NpmRequirement

Expand All @@ -13,11 +16,35 @@ class AlexBear:
writing.
"""
LANGUAGES = {"Natural Language"}
REQUIREMENTS = {NpmRequirement('alex', '2')}
REQUIREMENTS = {NpmRequirement('alex', '3')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'

@classmethod
def check_prerequisites(cls):
parent_prereqs = super().check_prerequisites()
if parent_prereqs is not True: # pragma: no cover
return parent_prereqs

incorrect_pkg_msg = (
"Please ensure that the package that has been installed is the "
"one to 'Catch insensitive, inconsiderate writing'. This can be "
"verified by running `alex --help` and seeing what it does.")
try:
output = subprocess.check_output(("alex", "--help"),
stderr=subprocess.STDOUT)
except (OSError, subprocess.CalledProcessError):
return ("The `alex` package could not be verified. " +
incorrect_pkg_msg)
else:
output = output.decode(sys.getfilesystemencoding())
if "Catch insensitive, inconsiderate writing" in output:
Copy link
Member

Choose a reason for hiding this comment

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

@wooorm is this workaround ok or would you suggest something else?

Copy link

Choose a reason for hiding this comment

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

@sils1297 would it work to check if 'which alex', resolving symlinks, yields an '.js' file?

Copy link
Member

Choose a reason for hiding this comment

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

@AbdealiJK ^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find the current method to be cleaner as compared to following symlinks.
In a distributed environment like NFS/HDFS symlinks aren't well understood and errors out. Just yesterday I was writing a Python+Spark script which couldn't understand symlinks due to various FileSystem/configuration issues. But I guess npm itself would install things incorrectly in such cases ^_^ and AlexBear wouldn't work.
Secondly, if you're using a non-typical npm installation, you have these wrapper scripts that call other scripts (without symlinks). Something like how pyenv and setuptools does. Or if you're using a bash alias, that method will always fail.

Is there any downside of the current way ? Is there a plan to change the slogan in the help ?

Copy link

Choose a reason for hiding this comment

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

Is there any downside of the current way ? Is there a plan to change the slogan in the help ?

No, not to my knowledge. And there are no plans to change it!

return True
else:
return ("The `alex` package that's been installed seems to "
"be incorrect. " + incorrect_pkg_msg)

@staticmethod
def create_arguments(filename, file, config_file):
return filename,
24 changes: 24 additions & 0 deletions bears/natural_language/SpellCheckBear.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from coalib.bearlib.abstractions.Linter import linter
from coalib.bears.requirements.PipRequirement import PipRequirement


@linter(executable='scspell',
use_stderr=True,
output_format='regex',
output_regex=r'(?P<filename>.*):(?P<line>.\d*):\s*(?P<message>.*)')
class SpellCheckBear:
"""
Lints files to check for incorrect spellings using ``scspell``.

See <https://pypi.python.org/pypi/scspell> for more information.
"""
LANGUAGES = {"Natural Language"}
REQUIREMENTS = {PipRequirement('scspell3k', '2.0')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
CAN_DETECT = {'Spelling'}
Copy link
Member

Choose a reason for hiding this comment

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

asciinema URL missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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


@staticmethod
def create_arguments(filename, file, config_file):
return '--report-only', filename
32 changes: 25 additions & 7 deletions bears/python/YapfBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ def run(self, filename, file,
split_before_logical_operator: bool=False,
split_before_named_assigns: bool=True,
use_spaces: bool=True,
based_on_style: str='pep8'):
based_on_style: str='pep8',
prefer_line_break_after_opening_bracket: bool=True):
"""
:param max_line_length:
Maximum number of characters for a line.
Expand Down Expand Up @@ -112,6 +113,9 @@ def run(self, filename, file,
Uses spaces for indentation.
:param based_on_style:
The formatting style to be used as reference.
:param prefer_line_break_after_opening_bracket:
If True, splitting right after a open bracket will not be
preferred.
"""
if not file:
# Yapf cannot handle zero-byte files well, and adds a redundent
Expand Down Expand Up @@ -141,14 +145,28 @@ def run(self, filename, file,
space_between_ending_comma_and_closing_bracket= \
{space_between_ending_comma_and_closing_bracket}
"""
options += 'use_tabs = ' + str(not use_spaces)
options += 'use_tabs = ' + str(not use_spaces) + "\n"
options += ('split_penalty_after_opening_bracket = ' +
('30' if prefer_line_break_after_opening_bracket
else '0') + "\n")
options = options.format(**locals())

with prepare_file(options.splitlines(keepends=True),
None) as (file_, fname):
corrected = FormatFile(filename,
style_config=fname,
verify=False)[0].splitlines(True)
try:
with prepare_file(options.splitlines(keepends=True),
None) as (file_, fname):
corrected = FormatFile(filename,
style_config=fname,
verify=False)[0].splitlines(True)
except SyntaxError as err:
if isinstance(err, IndentationError):
error_type = "indentation errors (" + err.args[0] + ')'
else:
error_type = "syntax errors"
yield Result.from_values(
self,
"The code cannot be parsed due to {0}.".format(error_type),
filename, line=err.lineno, column=err.offset)
return
diffs = Diff.from_string_arrays(file, corrected).split_diff()
for diff in diffs:
yield Result(self,
Expand Down
Loading