Skip to content

Commit

Permalink
Fix WPS323 false positive thrown when using some psycopg2 and datetim…
Browse files Browse the repository at this point in the history
…e functions (wemake-services#1676)

* Fix WPS323 false positive thrown when using some psycopg2 and datetime functions

* Added changes to the changelog

* Reuse `get_parent()`

* Add test with dt.strftime

* Use `given_function_called`. Waiting for the issue with this function to be solved.

* Use new `given_function_called`

* Delete pointless log in changelog

* Made the exceptions a class-level frozenset

* fixed my dumbness
  • Loading branch information
Jrryy committed Oct 22, 2020
1 parent acd0727 commit 2afc9a7
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ Semantic versioning in our case means:
- WPS531: Forbids testing conditions to just return booleans when it is possible to simply return the condition itself
- Forbids to use unsafe infinite loops
- Forbids to use raw strings `r''` when not necessary
- Add parameter to `given_function_called` so that it checks if a function has been called by its name regardless of the modules or classes it is in
- Forbids to use too complex f-strings

### Bugfixes
Expand All @@ -39,6 +38,7 @@ Semantic versioning in our case means:
- Allowed yield statements in call method
- Allow to use `^` with `1`
- Fixes false positives in WPS513
- Fixes false positives in WPS323

### Misc

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,33 @@ def test_regular_modulo_string(
))


@pytest.mark.parametrize('code', [
'dt.strftime("%A, %d. %B %Y %I:%M%p")',
'datetime.strftime("%d-%m-%Y (%H:%M:%S)")',
'datetime.strptime("01-01-2020 (10:20:30)", "%d-%m-%Y (%H:%M:%S)")',
'date.strftime("%d-%m-%Y (%H:%M:%S)")',
'date.strptime("01-01-2020", "%d-%m-%Y")',
'time.strftime("%H:%M:%S")',
'time.strptime("10:20:30", "%H:%M:%S")',
'strptime("01-01-2020 (10:20:30)", "%d-%m-%Y (%H:%M:%S)")',
'cur.execute("SELECT * FROM table WHERE column = %s", ("some_column"))',
'execute("SELECT * FROM table WHERE column = %s", ("some_column"))',
])
def test_functions_modulo_string(
assert_errors,
parse_ast_tree,
code,
default_options,
):
"""Testing that the strings violate the rules."""
tree = parse_ast_tree(code)

visitor = WrongStringVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [], (FormattedStringViolation,))


@pytest.mark.parametrize('code', [
'x % 1',
'"str" % 1',
Expand Down
35 changes: 33 additions & 2 deletions wemake_python_styleguide/visitors/ast/builtins.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
)
from wemake_python_styleguide.logic import nodes, safe_eval, source, walk
from wemake_python_styleguide.logic.naming.name_nodes import extract_name
from wemake_python_styleguide.logic.tree import attributes, operators, strings
from wemake_python_styleguide.logic.tree import (
attributes,
functions,
operators,
strings,
)
from wemake_python_styleguide.types import (
AnyChainable,
AnyFor,
Expand Down Expand Up @@ -85,6 +90,13 @@ class WrongStringVisitor(base.BaseNodeVisitor):
flags=re.X, # flag to ignore comments and whitespaces.
)

#: Names of functions in which we allow strings with modulo patterns.
_modulo_pattern_exceptions: ClassVar[FrozenSet[str]] = frozenset((
'strftime', # For date, time, and datetime.strftime()
'strptime', # For date, time, and datetime.strptime()
'execute', # For psycopg2's cur.execute()
))

def visit_any_string(self, node: AnyText) -> None:
"""
Forbids incorrect usage of strings.
Expand All @@ -111,6 +123,22 @@ def _check_is_alphatbet(
),
)

def _is_modulo_pattern_exception(self, parent: Optional[ast.AST]) -> bool:
"""
Check if the string with modulo patterns is in an exceptional situation.
Basically we have some function names in which we allow strings with
modulo patterns because they must have them for the functions to work
properly.
"""
if parent and isinstance(parent, ast.Call):
return bool(functions.given_function_called(
parent,
self._modulo_pattern_exceptions,
split_modules=True,
))
return False

def _check_modulo_patterns(
self,
node: AnyText,
Expand All @@ -121,7 +149,10 @@ def _check_modulo_patterns(
return # we allow `%s` in docstrings: they cannot be formatted.

if self._modulo_string_pattern.search(text_data):
self.add_violation(consistency.ModuloStringFormatViolation(node))
if not self._is_modulo_pattern_exception(parent):
self.add_violation(
consistency.ModuloStringFormatViolation(node),
)


@final
Expand Down

0 comments on commit 2afc9a7

Please sign in to comment.