-
-
Notifications
You must be signed in to change notification settings - Fork 405
Issue 3241: CLI tool to explain violations #3256
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
Changes from 43 commits
784f7de
7ad0c7a
7bff6aa
8c469bf
daf71cc
b359f0e
a13e022
bdd8f41
fd99365
4ee0aed
9931697
b9074a1
6365a2b
9334b0b
f9b983b
ff23e7b
3a3d56a
5060ace
4eca1fa
c167f0b
c85ae45
c062e76
a065ad1
4533f63
a8695cb
54405e1
4ef4db1
fcf7350
815ffd2
da33339
e6fbf52
1755eb0
f27d726
c037010
173e821
d462de5
f864fed
4392a6f
88f7f82
2978cd7
9ea66b7
82100b2
ecf631a
8c68b6d
2a54118
b03371e
2969b6c
9769448
52ed4ff
4a976ca
017e271
d502cc2
56fc859
7950dc9
481212e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
Command line tool | ||
================= | ||
|
||
.. versionadded:: 1.1.0 | ||
|
||
WPS has a command-line utility named ``wps`` | ||
|
||
Here are listed all the subcommands it has. | ||
|
||
.. rubric:: ``wps explain`` | ||
|
||
This command can be used to get description of violation. | ||
It will be the same description that is located on the website. | ||
|
||
Syntax: ``wps explain <code>`` | ||
|
||
Examples: | ||
|
||
.. code:: bash | ||
|
||
$ wps explain WPS115 | ||
WPS115 (UpperCaseAttributeViolation) | ||
|
||
WPS115 - Require ``snake_case`` for naming class attributes. | ||
... | ||
|
||
.. code:: bash | ||
|
||
$ wps explain 116 | ||
WPS116 (ConsecutiveUnderscoresInNameViolation) | ||
|
||
WPS116 - Forbid using more than one consecutive underscore in variable names. |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# serializer version: 1 | ||
# name: test_command | ||
''' | ||
WPS123 — Forbid unused variables with multiple underscores. | ||
|
||
Reasoning: | ||
We only use ``_`` as a special definition for an unused variable. | ||
Other variables are hard to read. It is unclear why would one use it. | ||
|
||
Solution: | ||
Rename unused variables to ``_`` | ||
or give it some more context with an explicit name: ``_context``. | ||
|
||
Example:: | ||
|
||
# Correct: | ||
some_element, _next_element, _ = some_tuple() | ||
some_element, _, _ = some_tuple() | ||
some_element, _ = some_tuple() | ||
|
||
# Wrong: | ||
some_element, _, __ = some_tuple() | ||
|
||
.. versionadded:: 0.12.0 | ||
|
||
See at website: https://pyflak.es/WPS123 | ||
|
||
''' | ||
# --- | ||
# name: test_command_on_not_found[wps explain 10000] | ||
''' | ||
Violation "10000" not found | ||
|
||
''' | ||
# --- | ||
# name: test_command_on_not_found[wps explain NOT_A_CODE] | ||
''' | ||
Violation "NOT_A_CODE" not found | ||
|
||
''' | ||
# --- | ||
# name: test_command_on_not_found[wps explain WPS10000] | ||
''' | ||
Violation "WPS10000" not found | ||
|
||
''' | ||
# --- | ||
# name: test_no_command_specified | ||
''' | ||
usage: wps [-h] {explain} ... | ||
wps: error: the following arguments are required: {explain} | ||
|
||
''' | ||
# --- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
"""Test that wps explain command works fine.""" | ||
|
||
import os | ||
import platform | ||
import subprocess | ||
|
||
import pytest | ||
|
||
from wemake_python_styleguide.cli.commands.explain import ( | ||
message_formatter, | ||
violation_loader, | ||
) | ||
from wemake_python_styleguide.violations.best_practices import ( | ||
InitModuleHasLogicViolation, | ||
) | ||
from wemake_python_styleguide.violations.naming import ( | ||
UpperCaseAttributeViolation, | ||
) | ||
from wemake_python_styleguide.violations.oop import BuiltinSubclassViolation | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'violation_params', | ||
[ | ||
(115, UpperCaseAttributeViolation), | ||
(412, InitModuleHasLogicViolation), | ||
(600, BuiltinSubclassViolation), | ||
], | ||
) | ||
def test_violation_getter(violation_params): | ||
"""Test that violation loader can get violation by their codes.""" | ||
violation_code, expected_class = violation_params | ||
violation = violation_loader.get_violation(violation_code) | ||
assert violation.code is not None | ||
assert violation.docstring == expected_class.__doc__ | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'test_params', | ||
[ | ||
(' text\n text\n text', 'text\ntext\ntext'), | ||
(' text\n\ttext\r\n text', 'text\n text\ntext'), | ||
(' text\n \n\n text', 'text\n \n\ntext'), | ||
('\n\n', '\n\n'), | ||
('text', 'text'), | ||
('text\ntext', 'text\ntext'), | ||
('', ''), | ||
(' ', ' '), | ||
], | ||
) | ||
def test_indentation_removal(test_params): | ||
"""Test that indentation remover works in different conditions.""" | ||
input_text, expected = test_params | ||
actual = message_formatter._remove_indentation(input_text) # noqa: SLF001 | ||
assert actual == expected | ||
|
||
|
||
violation_mock = violation_loader.ViolationInfo( | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
identifier='Mock', | ||
code=100, | ||
docstring='docstring', | ||
section='mock', | ||
) | ||
violation_string = 'docstring\n\nSee at website: https://pyflak.es/WPS100' | ||
|
||
|
||
def test_formatter(): | ||
"""Test that formatter formats violations as expected.""" | ||
formatted = message_formatter.format_violation(violation_mock) | ||
assert formatted == violation_string | ||
|
||
|
||
def _popen_in_shell(args: str) -> subprocess.Popen: # pragma: no cover | ||
"""Run command in shell.""" | ||
encoding = 'utf-8' | ||
# Some encoding magic. Calling with shell=True on Windows | ||
# causes everything to be in cp1251. shell=True is needed | ||
# for subprocess.Popen to locate the installed wps command. | ||
if platform.system() == 'Windows': | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
encoding = 'cp1251' | ||
return subprocess.Popen( # noqa: S602 (insecure shell=True) | ||
args, | ||
stdout=subprocess.PIPE, | ||
stderr=subprocess.PIPE, | ||
universal_newlines=True, | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
encoding=encoding, | ||
env=os.environ, | ||
shell=True, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Marking as not really resolved. I think that we might need to use |
||
) | ||
|
||
|
||
def test_command(snapshot): | ||
"""Test that command works and formats violations as expected.""" | ||
process = _popen_in_shell('wps explain WPS123') | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
stdout, stderr = process.communicate() | ||
assert process.returncode == 0, (stdout, stderr) | ||
assert stdout == snapshot | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'command', | ||
[ | ||
'wps explain 10000', | ||
'wps explain NOT_A_CODE', | ||
'wps explain WPS10000', | ||
], | ||
) | ||
def test_command_on_not_found(command, snapshot): | ||
"""Test command works when violation code is wrong.""" | ||
process = _popen_in_shell(command) | ||
stdout, stderr = process.communicate() | ||
assert process.returncode == 1, (stdout, stderr) | ||
assert stderr == snapshot | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
def test_no_command_specified(snapshot): | ||
"""Test command displays error message when no subcommand provided.""" | ||
process = _popen_in_shell('wps') | ||
stdout, stderr = process.communicate() | ||
assert process.returncode != 0, (stdout, stderr) | ||
assert stderr == snapshot |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
"""Provides WPS CLI application class.""" | ||
|
||
from typing import final | ||
|
||
from wemake_python_styleguide.cli.commands.explain.command import ExplainCommand | ||
|
||
|
||
@final | ||
class Application: | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""WPS CLI application class.""" | ||
|
||
def run_explain(self, args) -> int: | ||
"""Run explain command.""" | ||
return ExplainCommand().run(args) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
"""Main CLI utility file.""" | ||
|
||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import argparse | ||
import sys | ||
|
||
from wemake_python_styleguide.cli.application import Application | ||
|
||
|
||
def _configure_arg_parser(app: Application) -> argparse.ArgumentParser: | ||
"""Configures CLI arguments and subcommands.""" | ||
parser = argparse.ArgumentParser( | ||
prog='wps', description='WPS command line tool' | ||
) | ||
sub_parsers = parser.add_subparsers(help='sub-command help') | ||
sub_parsers.required = True | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
parser_explain = sub_parsers.add_parser( | ||
'explain', | ||
help='Get violation description', | ||
) | ||
parser_explain.add_argument( | ||
'violation_code', | ||
help='Desired violation code', | ||
) | ||
parser_explain.set_defaults(func=app.run_explain) | ||
|
||
return parser | ||
|
||
|
||
def parse_args(app: Application) -> argparse.Namespace: | ||
"""Parse CLI arguments.""" | ||
parser = _configure_arg_parser(app) | ||
return parser.parse_args() | ||
|
||
|
||
def main() -> int: | ||
"""Main function.""" | ||
app = Application() | ||
args = parse_args(app) | ||
return int(args.func(args)) | ||
|
||
|
||
if __name__ == '__main__': | ||
sys.exit(main()) | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
"""Contains files common for all wps commands.""" | ||
|
||
from abc import ABC, abstractmethod | ||
|
||
|
||
class AbstractCommand(ABC): | ||
"""ABC for all commands.""" | ||
|
||
@abstractmethod | ||
def run(self, args) -> int: | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Run the command.""" | ||
raise NotImplementedError |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
"""Contains command implementation.""" | ||
|
||
from wemake_python_styleguide.cli.commands.base import AbstractCommand | ||
from wemake_python_styleguide.cli.commands.explain import ( | ||
message_formatter, | ||
violation_loader, | ||
) | ||
from wemake_python_styleguide.cli.output import print_stderr, print_stdout | ||
|
||
|
||
def _clean_violation_code(violation_str: str) -> int: | ||
"""Get int violation code from str violation code.""" | ||
violation_str = violation_str.removeprefix('WPS') | ||
try: | ||
return int(violation_str) | ||
except ValueError: | ||
return -1 | ||
|
||
|
||
class ExplainCommand(AbstractCommand): | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Explain command impl.""" | ||
|
||
def run(self, args) -> int: | ||
"""Run command.""" | ||
code = _clean_violation_code(args.violation_code) | ||
violation = violation_loader.get_violation(code) | ||
if violation is None: | ||
print_stderr(f'Violation "{args.violation_code}" not found') | ||
return 1 | ||
message = message_formatter.format_violation(violation) | ||
print_stdout(message) | ||
return 0 |
Uh oh!
There was an error while loading. Please reload this page.