-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
New option: logging-format-style for logging checker #2521
Conversation
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.
Thanks for the PR @pinealan ! The code and the feature look good, but I'd suggest reusing parse_format_method_string
from checkers/strings.py
instead of adding a new function. Maybe we can also move that to utils
.
pylint/checkers/utils.py
Outdated
@@ -535,6 +535,67 @@ def next_char(i): | |||
return keys, num_args, key_types, pos_types | |||
|
|||
|
|||
def parse_brace_format_string(format_string: str) -> Tuple[Set[str], int]: |
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.
It's not totally obvious but we have parse_format_method_string
in checkers/strings.py
, which does the same thing. It's used by the string checkers for warning about passing invalid format keys when formatting with .format()
.
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.
Thanks for pointing that out. I will refactor accordingly.
pylint/checkers/utils.py
Outdated
@@ -535,6 +536,104 @@ def next_char(i): | |||
return keys, num_args, key_types, pos_types | |||
|
|||
|
|||
if PY3K: |
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.
Since we're now running only on Python 3, feel free to remove the else
clause of this check.
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.
Oh cool got it.
Thanks a lot @pinealan ! |
Steps
Description
logging-format-style
accepts one of '%' or '{', (defaults to '%'). When '{' is selected, logging checker assumes str.format() style format strings for calls to the logging.This PR came about when pylint threw E1205
logging-too-many-args
at my project that uses '{' style format strings. This was because pylint was unable to count the required number of args for the format string. The new feature indirectly fixes that by allowing the proper interpretation of that format string. The possibility for others to implement the '$' style template strings parsing is also open.