Skip to content
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

Add possible-forgotten-f-prefix checker #4787

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
16061e1
Add ``possible-f-string-as-string`` checker
DanielNoord Aug 2, 2021
92b8959
Remove walrus operator
DanielNoord Aug 2, 2021
240b4da
Apply suggestions from code review
DanielNoord Aug 2, 2021
3c2cf24
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 2, 2021
f7f4f89
Add review changes
DanielNoord Aug 2, 2021
2259aaf
Merge branch 'main' into f-strings-variable
Pierre-Sassoulas Aug 3, 2021
a75c186
Merge branch 'main' into f-strings-variable
Pierre-Sassoulas Aug 17, 2021
c91a14f
Fix bad conflict resolution
Pierre-Sassoulas Aug 17, 2021
2060037
Add more test cases with invalid python code
Pierre-Sassoulas Aug 17, 2021
8def6c5
Use ast.parse() work in progress
Pierre-Sassoulas Aug 17, 2021
f803caf
Fix pre-commit messages
DanielNoord Aug 17, 2021
971c29d
Update checks and tests
DanielNoord Aug 17, 2021
817bf99
Update tests/functional/p/possible_forgotten_f_prefix.py
DanielNoord Aug 17, 2021
b351fe9
Fix some tests
DanielNoord Aug 18, 2021
e755973
Merge branch 'main' into f-strings-variable
DanielNoord Aug 18, 2021
f042377
Fix tests
DanielNoord Aug 18, 2021
bfd075c
Fix hashing of list
DanielNoord Aug 18, 2021
8e01ff2
Apply suggestions from code review
DanielNoord Aug 18, 2021
5d13efd
Remove non-relevant changes
DanielNoord Aug 18, 2021
c61e834
Merge branch 'f-strings-variable' of https://github.com/DanielNoord/p…
DanielNoord Aug 18, 2021
f64e4fd
Additional tests..
DanielNoord Aug 18, 2021
4851d9e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ Release date: TBA

Closes #4681

* Added ``possible-f-string-as-string``: Emitted when variables are used in normal strings in between "{}"

Closes #2507


What's New in Pylint 2.9.6?
===========================
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/2.10.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ New checkers

Closes #3692

* Added ``possible-f-string-as-string``: Emitted when variables are used in normal strings in between "{}"

Closes #2507


Extensions
==========
Expand Down
56 changes: 56 additions & 0 deletions pylint/checkers/strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
# Copyright (c) 2020 Anthony <tanant@users.noreply.github.com>
# Copyright (c) 2021 Marc Mueller <30130371+cdce8p@users.noreply.github.com>
# Copyright (c) 2021 Peter Kolbus <peter.kolbus@garmin.com>
# Copyright (c) 2021 Daniel van Noord <13665637+DanielNoord@users.noreply.github.com>
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved


# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
Expand Down Expand Up @@ -204,6 +205,12 @@
"Used when we detect an f-string that does not use any interpolation variables, "
"in which case it can be either a normal string or a bug in the code.",
),
"W1310": (
"Using an string which should probably be a f-string",
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"possible-f-string-as-string",
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"Used when we detect a string that uses '{}' with a local variable inside. "
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"This string is probably meant to be an f-string.",
),
}

OTHER_NODES = (
Expand Down Expand Up @@ -892,6 +899,55 @@ def process_non_raw_string_token(
# character can never be the start of a new backslash escape.
index += 2

@check_messages("possible-f-string-as-string")
def visit_const(self, node: astroid.Const):
self._detect_possible_f_string(node)
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved

def _detect_possible_f_string(self, node: astroid.Const):
"""Check whether strings include local/global variables in '{}'
Those should probably be f-strings's
"""

def detect_if_used_in_format(node: astroid.Const, assign_name: str) -> bool:
"""Check if the node is used in a call to format() if so return True"""
# the skip_class is to make sure we don't go into inner scopes, but might not be needed per se
for attr in node.scope().nodes_of_class(
astroid.Attribute, skip_klass=(astroid.FunctionDef,)
):
if isinstance(attr.expr, astroid.Name):
if attr.expr.name == assign_name and attr.attrname == "format":
return True
return False

if node.pytype() == "builtins.str" and not isinstance(
node.parent, astroid.JoinedStr
):
# Find all pairs of '{}' within a string
inner_matches = re.findall(r"(?<=\{).*?(?=\})", node.value)
if len(inner_matches) != len(set(inner_matches)):
return
if inner_matches:
for match in inner_matches:
# Check if match is a local or global variable
if node.scope().locals.get(match) or node.root().locals.get(match):
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
assign_node = node
while not isinstance(assign_node, astroid.Assign):
assign_node = assign_node.parent
if isinstance(assign_node.value, astroid.Tuple):
node_index = assign_node.value.elts.index(node)
assign_name = assign_node.targets[0].elts[node_index].name
else:
assign_name = assign_node.targets[0].name
if not detect_if_used_in_format(node, assign_name):
self.add_message(
"possible-f-string-as-string",
line=node.lineno,
node=node,
)
return
else:
return


def register(linter):
"""required method to auto register this checker"""
Expand Down
12 changes: 12 additions & 0 deletions tests/functional/p/possible_f_string_as_string.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# pylint: disable=missing-module-docstring, invalid-name
var = "string"
var_two = "extra string"

x = f"This is a {var} which should be a f-string"
x = "This is a {var} used twice, see {var}"
x = "This is a {var} which should be a f-string" # [possible-f-string-as-string]
x = "This is a {var} and {var_two} which should be a f-string" # [possible-f-string-as-string]
x1, x2, x3 = (1, 2, "This is a {var} which should be a f-string") # [possible-f-string-as-string]

y = "This is a {var} used for formatting later"
z = y.format(var="string")
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions tests/functional/p/possible_f_string_as_string.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
possible-f-string-as-string:7:4::Using an string which should probably be a f-string:HIGH
possible-f-string-as-string:8:4::Using an string which should probably be a f-string:HIGH
possible-f-string-as-string:9:20::Using an string which should probably be a f-string:HIGH