-
Notifications
You must be signed in to change notification settings - Fork 53
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
cEP-0031: Improve Generic Bear Quality
Closes #183
- Loading branch information
1 parent
b8a2d2b
commit 5b0e720
Showing
2 changed files
with
321 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,320 @@ | ||
# Improve Generic Bear Quality | ||
|
||
| Metadata | | | ||
| -------- | ------------------------------------------- | | ||
| cEP | 31 | | ||
| Version | 0.1 | | ||
| Title | Improve Generic Bear Quality | | ||
| Authors | Bhushan Khanale <mailto:[email protected]> | | ||
| Status | Proposed | | ||
| Type | Feature | | ||
|
||
## Abstract | ||
|
||
This cEP describes the improvement in the generic bears currently available | ||
along with the implementation of the new bears as a part of the | ||
[GSoC 2019 Project][project]. | ||
|
||
## Introduction | ||
|
||
coala has few generic bears which have the potential to perform better | ||
fixing some of the issues in them. Some of these issues are as follows: | ||
|
||
- [coala/coala-bears#644][644]: Ignore doc comments/doc strings | ||
Comments or docstrings do not have any set standards while the | ||
IndentationBear does not ignore them in a file. IndentationBear checks for | ||
correct indentation in the program statements since that is crucial for | ||
proper code style. Docstrings/comments, on the other hand, do not have any | ||
such compulsion. So ideally docstrings should be ignored by the bear. coala | ||
already uses documentation extraction algorithms in DocumentationStyleBear. | ||
Using the existing algorithms and constructing ignore ranges out of the | ||
extracted comments inside the IndentationBear, this issue can be solved. | ||
|
||
- [coala/coala-bears#1897][1897]: Show PEP8 error description | ||
On running PEP8Bear the default message is 'The code does not comply to | ||
PEP8.' which is not very helpful for the user. This issue can be solved | ||
by improving autopep8 itself. One of the alternative approach is to | ||
invoke pycodestyle inside PEP8Bear and pass the issues to the autopep8 | ||
and generate a fix for them using `--line-range`, `--column-range` and | ||
`--select` options. | ||
|
||
- [hhatto/autopep8#227][227]: Create diff for a specific pep8 issue | ||
User should have a choice of selecting the issue he wants to generate a fix | ||
for. This issue is related to the above issue since the fix for this issue | ||
will introduce a new setting `--column-range` in autopep8. | ||
|
||
Along with fixing these issues, new bears including `OutdatedDependencyBear`, | ||
`RegexLintBear`, `FileModeBear` and `RequirementsCheckBear` | ||
will be implemented. | ||
|
||
In place of `FilesExistBear` we will introduce two new settings in coala, | ||
`require_files_not_empty` and `require_files_for_each_glob` which will check | ||
`files` option doesn't evaluate to empty and there is at least one match for | ||
each glob in `files`. | ||
|
||
## Implementation of new bears | ||
|
||
### 1. OutdatedDependencyBear | ||
|
||
Issue: [coala/coala-bears#2445][2445] | ||
|
||
The bear will be looking for the outdated dependencies in the project. The | ||
dependencies can be specified explicitly with the files or by default the bear | ||
will look for files like requirements.txt, package.json, etc. | ||
|
||
The bear will require changes to the existing [package_manager][package_manager] | ||
with the help of new additions to the requirement classes. | ||
|
||
- For pip requirements, we could use the PyPI JSON API which provides the | ||
version info by specifying the `package_name`. | ||
|
||
```py | ||
from xmlrpc import client | ||
|
||
class PipRequirement(PackageRequirement): | ||
def get_latest_version(self): | ||
""" | ||
Gets the latest version available for the package. | ||
:return: | ||
A string with the latest version of the package. | ||
""" | ||
pypi = client.ServerProxy(PYPI_URL) | ||
return pypi.package_releases(self.package)[0] | ||
``` | ||
|
||
- For npm dependencies, we can use the npm cli command `npm outdated` which | ||
produces an output as below: | ||
|
||
``` | ||
$ npm outdated | ||
Package Current Wanted Latest Location | ||
glob 5.0.15 5.0.15 6.0.1 test-outdated-output | ||
nothingness 0.0.3 git git test-outdated-output | ||
npm 3.5.1 3.5.2 3.5.1 test-outdated-output | ||
local-dev 0.0.3 linked linked test-outdated-output | ||
once 1.3.2 1.3.3 1.3.3 test-outdated-output | ||
``` | ||
|
||
The output can be directly parsed and can be used to get the list of all | ||
outdated dependencies. | ||
|
||
For pip, the bear can be constructed as below: | ||
|
||
```py | ||
class OutdatedDependencyBear(LocalBear): | ||
def run(self, filename, file, requirement_type: str,): | ||
""" | ||
Checks for the outdated dependencies in a project. | ||
:param requirement_type: | ||
One of the requirement types supported by coala's package manager. | ||
:param requirements_file: | ||
Requirements file can be specified to look for the requirements. | ||
""" | ||
requirement_types = ['pip'] | ||
|
||
if requirement_type not in requirement_types: | ||
raise ValueError('Currently the bear only supports {} as ' | ||
'requirement_type.' | ||
.format(', '.join( | ||
_type for _type in requirement_types))) | ||
|
||
message = ('The requirement {} with version {} is not ' | ||
'pinned to its latest version {}.') | ||
|
||
out = run('pip-compile -n --allow-unsafe {}'.format(filename), | ||
stdout=Capture()) | ||
|
||
data = [line for line in out.stdout.text.splitlines() | ||
if '#' not in line and line] | ||
|
||
for requiremenent in data: | ||
package, version = requiremenent.split('==') | ||
pip_requirement = PipRequirement(package) | ||
latest_ver = pip_requirement.get_latest_version() | ||
line_number = [num for num, line in enumerate(file, 1) | ||
if package in line.lower()] | ||
|
||
if LooseVersion(version) < LooseVersion(latest_ver): | ||
yield Result.from_values(origin=self, | ||
message=message.format( | ||
package, | ||
version, | ||
latest_ver), | ||
file=filename, | ||
line=line_number[0], | ||
end_line=line_number[0], | ||
) | ||
``` | ||
|
||
### 2. RegexLintBear | ||
|
||
Issue: [coala/coala-bears#1532][1532] | ||
|
||
The task of the bear is to check the regex in strings. This can be done using | ||
the AnnotationBear to detect all the regex strings and then check for the valid | ||
regex through the prepared algorithm for each type. | ||
|
||
For Python, the bear can be written as follows: | ||
|
||
```py | ||
class RegexLintBear(LocalBear): | ||
def run(self, filename, file, language: str): | ||
""" | ||
Bear for linting regex through regexlint. | ||
:param language: | ||
The programming language of the file(s). | ||
""" | ||
section = Section('') | ||
section.append(Setting('language', language)) | ||
bear = AnnotationBear(section, Queue()) | ||
|
||
with execute_bear(bear, filename, file) as result: | ||
for src_range in result[0].contents['strings']: | ||
src_line = src_range.affected_source({filename: file})[0] | ||
regex = src_line[src_range.start.column:src_range.end.column-1] | ||
with suppress(re.error): | ||
re.compile(regex) | ||
out = run('regexlint --regex "{}"'.format(regex), | ||
stdout=Capture()).stdout.text | ||
if out[-3:-1] != 'OK': | ||
yield Result.from_values( | ||
origin=self, | ||
message=out, | ||
file=filename, | ||
line=src_range.start.line, | ||
column=src_range.start.column, | ||
end_line=src_range.end.line, | ||
end_column=src_range.end.column, | ||
) | ||
``` | ||
|
||
### 3. FileModeBear | ||
|
||
Issue: [coala/coala-bears#2370][2370] | ||
|
||
The bear will check the permissions on the files provided by the user | ||
and to ensure that the file permissions are the one that is expected. | ||
|
||
The bear will be used as follows: | ||
|
||
``` | ||
[all.mode] | ||
bears = FileModeBear | ||
filemode = rw | ||
[all.shell.mode] | ||
bears = FileModeBear | ||
filemode = rwx | ||
``` | ||
|
||
The bear will first find the permissions of the files specified by the user and | ||
then if the permissions are not suitable, the bear will try to change those | ||
permissions into the expected ones. If the bear doesn't have enough permissions | ||
to do so then the bear will let the user know about this. | ||
|
||
```py | ||
class FileModeBear(LocalBear): | ||
def run(self, | ||
filename, | ||
file, | ||
filemode: str, | ||
): | ||
""" | ||
The bear will check if the file has required permissions provided by | ||
the user. | ||
:param filemode: | ||
Filemode to check, e.g. `rw`, `rwx`, etc. | ||
""" | ||
st = os.stat(filename) | ||
permissions = {'r': stat.S_IRUSR, | ||
'w': stat.S_IWUSR, | ||
'x': stat.S_IXUSR, | ||
} | ||
|
||
invalid_chars = [ch for ch in filemode if ch not in permissions] | ||
|
||
if invalid_chars: | ||
raise ValueError('Unable to recognize character `{}` in filemode ' | ||
'`{}`.'.format(''.join(invalid_chars), filemode)) | ||
|
||
mode = st.st_mode | ||
for char in filemode: | ||
if not mode & permissions[char]: | ||
message = ('The file permissions are not adequate. ' | ||
'The permissions are set to {}' | ||
.format(stat.filemode(mode))) | ||
return [Result.from_values(origin=self, | ||
message=message, | ||
severity=RESULT_SEVERITY.INFO, | ||
file=filename)] | ||
``` | ||
|
||
### 4. RequirementsCheckBear | ||
|
||
Issue: [coala/coala-bears#1113][1113] | ||
|
||
The bear will be focused on Python only since they are most prone for | ||
conflicting requirements. | ||
|
||
The implementation is based on the recursive check for the requirements of each | ||
package in requirements.txt file of the project. The requirements are checked | ||
through PyPI's JSON API. | ||
|
||
```py | ||
class RequirementsCheckBear(GlobalBear): | ||
""" | ||
The bear to check and find any conflicting pip dependencies. | ||
""" | ||
def run(self, require_files: tuple): | ||
""" | ||
:param require_files: | ||
Tuple of requirements files. | ||
""" | ||
data = '' | ||
orig_file = '' | ||
|
||
for require_file in require_files: | ||
if not os.path.isfile(os.path.abspath(require_file)): | ||
raise ValueError('The file \'{}\' doesn\'t exist.' | ||
.format(require_file)) | ||
|
||
with open(require_file) as _file: | ||
content = _file.read() | ||
if not orig_file: | ||
orig_file = content | ||
else: | ||
data += content | ||
|
||
with open(require_files[0], 'a+') as temp_file: | ||
temp_file.write(data) | ||
|
||
out = capture_both('pip-compile {} -r -n --no-annotate --no-header ' | ||
'--no-index --allow-unsafe'.format(require_files[0])) | ||
|
||
if out.stderr.text and not out.stdout.text: | ||
pip_warning = 'Cache entry deserialization failed, entry ignored' | ||
lines = out.stderr.text.splitlines() | ||
lines = [line for line in lines if line not in pip_warning] | ||
yield Result(self, | ||
message=lines[0], | ||
severity=RESULT_SEVERITY.MAJOR, | ||
) | ||
|
||
with open(require_files[0], 'w+') as _file: | ||
_file.write(orig_file) | ||
``` | ||
|
||
[project]: https://summerofcode.withgoogle.com/projects/#4866569388163072 | ||
|
||
[227]: https://github.com/hhatto/autopep8/issues/227 | ||
|
||
[644]: https://github.com/coala/coala-bears/issues/644 | ||
|
||
[1113]: https://github.com/coala/coala-bears/issues/1113 | ||
|
||
[1532]: https://github.com/coala/coala-bears/issues/1532 | ||
|
||
[1897]: https://github.com/coala/coala-bears/issues/1897 | ||
|
||
[2445]: https://github.com/coala/coala-bears/issues/2445 |