-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Merged
Merged
Changes from all commits
Commits
Show all changes
55 commits
Select commit
Hold shift + click to select a range
784f7de
WIP #3241
Tapeline 7ad0c7a
Tidy up so all checks could pass. Update CHANGELOG.md
Tapeline 7bff6aa
Update docs
Tapeline 8c469bf
Remove unused docutils
Tapeline daf71cc
WIP #3241
Tapeline b359f0e
Tidy up so all checks could pass. Update CHANGELOG.md
Tapeline a13e022
Update docs
Tapeline bdd8f41
Remove unused docutils
Tapeline fd99365
Apply suggestions from code review
Tapeline 4ee0aed
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 9931697
Ruff fixes
Tapeline b9074a1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 6365a2b
WIP #3241
Tapeline 9334b0b
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline f9b983b
Re-lock poetry
Tapeline ff23e7b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 3a3d56a
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 5060ace
Update tests accordingly. Fix docs
Tapeline 4eca1fa
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] c167f0b
Try fixing docs again. Add more tests
Tapeline c85ae45
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] c062e76
More tests to cover almost all cli utility
Tapeline a065ad1
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 4533f63
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] a8695cb
Cover 100% CLI
Tapeline 54405e1
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 4ef4db1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] fcf7350
Finally, found and fixed what breaks my pytest snapshots
Tapeline 815ffd2
Bring back whitespaces in snapshot
Tapeline da33339
Merge branch 'master' into issue-3241
Tapeline e6fbf52
Re-lock poetry
Tapeline 1755eb0
Remove unnecessary text and newlines in wps explain
Tapeline f27d726
Remove duplicated versionadded label from cli.rst
Tapeline c037010
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 173e821
Remove Writable abstraction. Cosmetic changes to formatting. More tes…
Tapeline d462de5
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline f864fed
Merge branch 'master' into issue-3241
Tapeline 4392a6f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 88f7f82
Remove unused pragma no cover from cli_app.py
Tapeline 2978cd7
Delegate no argument corner-case to argparse. Move doc link to consta…
Tapeline 9ea66b7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 82100b2
Update conditions so python 3.10 tests pass too
Tapeline ecf631a
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 8c68b6d
Update so python 3.10 tests pass too
Tapeline 2a54118
Add pragma no cover to previous fix as it only runs on python 3.10
Tapeline b03371e
Separate unit and integration tests. Delegate indentation removal to …
Tapeline 2969b6c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 9769448
Separate unit and integration tests. Delegate indentation removal to …
Tapeline 52ed4ff
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 4a976ca
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 017e271
Clean up redundant abstractions. Move to old generics syntax to suppo…
Tapeline d502cc2
Merge remote-tracking branch 'origin/issue-3241' into issue-3241
Tapeline 56fc859
Remove application that came back after merging
Tapeline 7950dc9
Fix some things that broke after merge
Tapeline 481212e
Apply suggestions from code review
sobolevn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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,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.
Oops, something went wrong.
This file contains hidden or 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 hidden or 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,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} | ||
|
||
''' | ||
# --- |
This file contains hidden or 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,53 @@ | ||
"""Integration testing of wps explain command.""" | ||
|
||
import subprocess | ||
|
||
import pytest | ||
|
||
|
||
def _popen_in_shell(args: str) -> tuple[subprocess.Popen, str, str]: | ||
"""Run command in shell.""" | ||
# shell=True is needed for subprocess.Popen to | ||
# locate the installed wps command. | ||
process = subprocess.Popen( # noqa: S602 (insecure shell=True) | ||
args, | ||
stdout=subprocess.PIPE, | ||
stderr=subprocess.PIPE, | ||
text=True, | ||
shell=True, | ||
) | ||
stdin, stdout = process.communicate() | ||
return process, stdin, stdout | ||
|
||
|
||
def test_command(snapshot): | ||
"""Test that command works and formats violations as expected.""" | ||
process, stdout, stderr = _popen_in_shell('wps explain WPS123') | ||
assert process.returncode == 0, (stdout, stderr) | ||
assert stdout == snapshot | ||
assert not stderr | ||
|
||
|
||
@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, stdout, stderr = _popen_in_shell(command) | ||
assert process.returncode == 1, (stdout, stderr) | ||
assert not stdout | ||
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, stdout, stderr = _popen_in_shell('wps') | ||
stdout, stderr = process.communicate() | ||
assert process.returncode != 0, (stdout, stderr) | ||
assert not stdout | ||
assert stderr == snapshot |
This file contains hidden or 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,30 @@ | ||
"""Unit testing of wps explain command.""" | ||
|
||
import pytest | ||
|
||
from wemake_python_styleguide.cli.commands.explain import ( | ||
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__ |
Empty file.
This file contains hidden or 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,38 @@ | ||
import argparse | ||
|
||
from wemake_python_styleguide.cli.commands.explain.command import ExplainCommand | ||
|
||
|
||
def _configure_arg_parser() -> argparse.ArgumentParser: | ||
"""Configures CLI arguments and subcommands.""" | ||
parser = argparse.ArgumentParser( | ||
prog='wps', description='WPS command line tool' | ||
) | ||
sub_parsers = parser.add_subparsers( | ||
help='sub-parser for exact wps commands', | ||
required=True, | ||
) | ||
|
||
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=ExplainCommand()) | ||
|
||
return parser | ||
|
||
|
||
def parse_args() -> argparse.Namespace: | ||
"""Parse CLI arguments.""" | ||
parser = _configure_arg_parser() | ||
return parser.parse_args() | ||
|
||
|
||
def main() -> int: | ||
"""Main function.""" | ||
args = parse_args() | ||
return int(args.func(args=args)) |
Empty file.
This file contains hidden or 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,25 @@ | ||
"""Contains files common for all wps commands.""" | ||
|
||
from abc import ABC, abstractmethod | ||
from argparse import Namespace | ||
from typing import Generic, TypeVar | ||
|
||
_ArgsT = TypeVar('_ArgsT') | ||
|
||
|
||
class AbstractCommand(ABC, Generic[_ArgsT]): | ||
"""ABC for all commands.""" | ||
|
||
_args_type: type[_ArgsT] | ||
|
||
def __call__(self, args: Namespace) -> int: | ||
"""Parse arguments into the generic namespace.""" | ||
args_dict = vars(args) # noqa: WPS421 | ||
args_dict.pop('func') # argument classes do not expect that | ||
cmd_args = self._args_type(**args_dict) | ||
return self._run(cmd_args) | ||
|
||
@abstractmethod | ||
def _run(self, args: _ArgsT) -> int: | ||
"""Run the command.""" | ||
raise NotImplementedError |
Empty file.
This file contains hidden or 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,47 @@ | ||
"""Contains command implementation.""" | ||
|
||
from typing import final | ||
|
||
from attrs import frozen | ||
|
||
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 | ||
|
||
|
||
@final | ||
@frozen | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
class ExplainCommandArgs: | ||
"""Arguments for wps explain command.""" | ||
|
||
violation_code: str | ||
|
||
|
||
@final | ||
class ExplainCommand(AbstractCommand[ExplainCommandArgs]): | ||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Explain command impl.""" | ||
|
||
_args_type = ExplainCommandArgs | ||
|
||
def _run(self, args: ExplainCommandArgs) -> 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 |
22 changes: 22 additions & 0 deletions
22
wemake_python_styleguide/cli/commands/explain/message_formatter.py
This file contains hidden or 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,22 @@ | ||
"""Provides tools for formatting explanations.""" | ||
|
||
import textwrap | ||
|
||
from wemake_python_styleguide.cli.commands.explain.violation_loader import ( | ||
ViolationInfo, | ||
) | ||
from wemake_python_styleguide.constants import SHORTLINK_TEMPLATE | ||
|
||
|
||
def _remove_newlines_at_ends(text: str) -> str: | ||
"""Remove leading and trailing newlines.""" | ||
return text.strip('\n\r') | ||
|
||
|
||
def format_violation(violation: ViolationInfo) -> str: | ||
"""Format violation information.""" | ||
cleaned_docstring = _remove_newlines_at_ends( | ||
textwrap.dedent(violation.docstring) | ||
) | ||
violation_url = SHORTLINK_TEMPLATE.format(f'WPS{violation.code}') | ||
return f'{cleaned_docstring}\n\nSee at website: {violation_url}' |
38 changes: 38 additions & 0 deletions
38
wemake_python_styleguide/cli/commands/explain/module_loader.py
This file contains hidden or 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,38 @@ | ||
import importlib | ||
from collections.abc import Collection | ||
from pathlib import Path | ||
from types import ModuleType | ||
from typing import Final | ||
|
||
_VIOLATION_MODULE_BASE: Final = 'wemake_python_styleguide.violations' | ||
|
||
|
||
def get_violation_submodules() -> Collection[ModuleType]: | ||
"""Get all possible violation submodules.""" | ||
submodule_names = _get_all_possible_submodule_names(_VIOLATION_MODULE_BASE) | ||
return [ | ||
importlib.import_module(submodule_name) | ||
for submodule_name in submodule_names | ||
] | ||
|
||
|
||
def _get_all_possible_submodule_names(module_name: str) -> Collection[str]: | ||
"""Get .py submodule names listed in given module.""" | ||
root_module = importlib.import_module(module_name) | ||
root_paths = root_module.__path__ | ||
names = [] | ||
for root in root_paths: | ||
names.extend([ | ||
f'{module_name}.{name}' | ||
for name in _get_all_possible_names_in_root(root) | ||
]) | ||
return names | ||
|
||
|
||
def _get_all_possible_names_in_root(root: str) -> Collection[str]: | ||
"""Get .py submodule names listed in given root path.""" | ||
return [ | ||
path.name.removesuffix('.py') | ||
for path in Path(root).glob('*.py') | ||
if '__' not in path.name # filter dunder files like __init__.py | ||
] |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need
shell=
andenv=
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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
shell=True
becausewps
is not locatable without thePATH
, right?