Skip to content

Commit

Permalink
Add Consider-using-f-string checker (#4796)
Browse files Browse the repository at this point in the history
* Add ``consider-using-f-string`` checker
This adds a checker for normal strings which are formatted
with ``.format()`` or '%'.
The message is a convention to nudge users towards using f-strings.
This closes #3592

* Update pylint code to use f-strings
After adding `consider-using-f-strings` the codebase showed numerous
cases of formatting which could be f-strings.
This commit changes most of these to become f-strings, or adds ignores.

* Apply suggestions from code review

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
  • Loading branch information
DanielNoord and Pierre-Sassoulas committed Aug 30, 2021
1 parent 1a19421 commit 66ffcbc
Show file tree
Hide file tree
Showing 84 changed files with 525 additions and 384 deletions.
5 changes: 4 additions & 1 deletion ChangeLog
Expand Up @@ -10,6 +10,10 @@ Release date: TBA
..
Put new features here and also in 'doc/whatsnew/2.11.rst'

* Added ``consider-using-f-string``: Emitted when .format() or '%' is being used to format a string.

Closes #3592


What's New in Pylint 2.10.3?
============================
Expand Down Expand Up @@ -193,7 +197,6 @@ Release date: 2021-08-20

* Allow ``true`` and ``false`` values in ``pylintrc`` for better compatibility with ``toml`` config.


* Class methods' signatures are ignored the same way as functions' with similarities "ignore-signatures" option enabled

Closes #4653
Expand Down
2 changes: 1 addition & 1 deletion doc/exts/pylint_extensions.py
Expand Up @@ -37,7 +37,7 @@ def builder_inited(app):
if name[0] == "_" or name in DEPRECATED_MODULES:
continue
if ext == ".py":
modules.append("pylint.extensions.%s" % name)
modules.append(f"pylint.extensions.{name}")
elif ext == ".rst":
doc_files["pylint.extensions." + name] = os.path.join(ext_path, filename)
modules.sort()
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/2.11.rst
Expand Up @@ -12,6 +12,10 @@ Summary -- Release highlights
New checkers
============

* Added ``consider-using-f-string``: Emitted when .format() or '%' is being used to format a string.

Closes #3592


Extensions
==========
Expand Down
4 changes: 2 additions & 2 deletions pylint/checkers/__init__.py
Expand Up @@ -65,8 +65,8 @@ def table_lines_from_stats(stats, old_stats, columns):
diff_str = diff_string(old, new)
else:
old, diff_str = "NC", "NC"
new = "%.3f" % new if isinstance(new, float) else str(new)
old = "%.3f" % old if isinstance(old, float) else str(old)
new = f"{new:.3f}" if isinstance(new, float) else str(new)
old = f"{old:.3f}" if isinstance(old, float) else str(old)
lines += (m_type.replace("_", " "), new, old, diff_str)
return lines

Expand Down
18 changes: 8 additions & 10 deletions pylint/checkers/base.py
Expand Up @@ -193,7 +193,7 @@ class AnyStyle(NamingStyle):
["set()", "{}", "[]"],
),
**{
x: "%s()" % x
x: f"{x}()"
for x in (
"collections.deque",
"collections.ChainMap",
Expand Down Expand Up @@ -404,12 +404,12 @@ def report_by_type_stats(sect, stats, old_stats):
try:
documented = total - stats["undocumented_" + node_type]
percent = (documented * 100.0) / total
nice_stats[node_type]["percent_documented"] = "%.2f" % percent
nice_stats[node_type]["percent_documented"] = f"{percent:.2f}"
except KeyError:
nice_stats[node_type]["percent_documented"] = "NC"
try:
percent = (stats["badname_" + node_type] * 100.0) / total
nice_stats[node_type]["percent_badname"] = "%.2f" % percent
nice_stats[node_type]["percent_badname"] = f"{percent:.2f}"
except KeyError:
nice_stats[node_type]["percent_badname"] = "NC"
lines = ("type", "number", "old number", "difference", "%documented", "%badname")
Expand Down Expand Up @@ -1703,8 +1703,7 @@ def _create_naming_options():
"type": "choice",
"choices": list(NAMING_STYLES.keys()),
"metavar": "<style>",
"help": "Naming style matching correct %s names."
% (human_readable_name,),
"help": f"Naming style matching correct {human_readable_name} names.",
},
)
)
Expand All @@ -1715,8 +1714,7 @@ def _create_naming_options():
"default": None,
"type": "regexp",
"metavar": "<regexp>",
"help": "Regular expression matching correct %s names. Overrides %s-naming-style."
% (human_readable_name, name_type),
"help": f"Regular expression matching correct {human_readable_name} names. Overrides {name_type}-naming-style.",
},
)
)
Expand Down Expand Up @@ -1888,9 +1886,9 @@ def _create_naming_rules(self):
regexps[name_type] = custom_regex

if custom_regex is not None:
hints[name_type] = "%r pattern" % custom_regex.pattern
hints[name_type] = f"{custom_regex.pattern!r} pattern"
else:
hints[name_type] = "%s naming style" % naming_style_name
hints[name_type] = f"{naming_style_name} naming style"

return regexps, hints

Expand Down Expand Up @@ -2023,7 +2021,7 @@ def _raise_name_warning(
type_label = HUMAN_READABLE_TYPES[node_type]
hint = self._name_hints[node_type]
if self.config.include_naming_hint:
hint += " (%r pattern)" % self._name_regexps[node_type].pattern
hint += f" ({self._name_regexps[node_type].pattern!r} pattern)"
args = (
(type_label.capitalize(), name, hint)
if warning == "invalid-name"
Expand Down
20 changes: 11 additions & 9 deletions pylint/checkers/base_checker.py
Expand Up @@ -70,35 +70,37 @@ def __str__(self):

def get_full_documentation(self, msgs, options, reports, doc=None, module=None):
result = ""
checker_title = "%s checker" % (self.name.replace("_", " ").title())
checker_title = f"{self.name.replace('_', ' ').title()} checker"
if module:
# Provide anchor to link against
result += ".. _%s:\n\n" % module
result += "%s\n" % get_rst_title(checker_title, "~")
result += f".. _{module}:\n\n"
result += f"{get_rst_title(checker_title, '~')}\n"
if module:
result += "This checker is provided by ``%s``.\n" % module
result += "Verbatim name of the checker is ``%s``.\n\n" % self.name
result += f"This checker is provided by ``{module}``.\n"
result += f"Verbatim name of the checker is ``{self.name}``.\n\n"
if doc:
# Provide anchor to link against
result += get_rst_title(f"{checker_title} Documentation", "^")
result += "%s\n\n" % cleandoc(doc)
result += f"{cleandoc(doc)}\n\n"
# options might be an empty generator and not be False when casted to boolean
options = list(options)
if options:
result += get_rst_title(f"{checker_title} Options", "^")
result += "%s\n" % get_rst_section(None, options)
result += f"{get_rst_section(None, options)}\n"
if msgs:
result += get_rst_title(f"{checker_title} Messages", "^")
for msgid, msg in sorted(
msgs.items(), key=lambda kv: (_MSG_ORDER.index(kv[0][0]), kv[1])
):
msg = self.create_message_definition_from_tuple(msgid, msg)
result += "%s\n" % msg.format_help(checkerref=False)
result += f"{msg.format_help(checkerref=False)}\n"
result += "\n"
if reports:
result += get_rst_title(f"{checker_title} Reports", "^")
for report in reports:
result += ":%s: %s\n" % report[:2]
result += (
":%s: %s\n" % report[:2] # pylint: disable=consider-using-f-string
)
result += "\n"
result += "\n"
return result
Expand Down
3 changes: 2 additions & 1 deletion pylint/checkers/classes.py
Expand Up @@ -2039,7 +2039,7 @@ class SpecialMethodsChecker(BaseChecker):
"__iter__ returns non-iterator",
"non-iterator-returned",
"Used when an __iter__ method returns something which is not an "
"iterable (i.e. has no `%s` method)" % NEXT_METHOD,
f"iterable (i.e. has no `{NEXT_METHOD}` method)",
{
"old_names": [
("W0234", "old-non-iterator-returned-1"),
Expand Down Expand Up @@ -2189,6 +2189,7 @@ def _check_unexpected_method_signature(self, node):
# tuple, although the user should implement the method
# to take all of them in consideration.
emit = mandatory not in expected_params
# pylint: disable-next=consider-using-f-string
expected_params = "between %d or %d" % expected_params
else:
# If the number of mandatory parameters doesn't
Expand Down
17 changes: 4 additions & 13 deletions pylint/checkers/exceptions.py
Expand Up @@ -271,7 +271,7 @@ class ExceptionsChecker(checkers.BaseChecker):
"default": OVERGENERAL_EXCEPTIONS,
"type": "csv",
"metavar": "<comma-separated class names>",
"help": "Exceptions that will emit a warning "
"help": "Exceptions that will emit a warning " # pylint: disable=consider-using-f-string
'when being caught. Defaults to "%s".'
% (", ".join(OVERGENERAL_EXCEPTIONS),),
},
Expand Down Expand Up @@ -488,20 +488,14 @@ def gather_exceptions_from_handler(
def visit_binop(self, node):
if isinstance(node.parent, nodes.ExceptHandler):
# except (V | A)
suggestion = "Did you mean '({}, {})' instead?".format(
node.left.as_string(),
node.right.as_string(),
)
suggestion = f"Did you mean '({node.left.as_string()}, {node.right.as_string()})' instead?"
self.add_message("wrong-exception-operation", node=node, args=(suggestion,))

@utils.check_messages("wrong-exception-operation")
def visit_compare(self, node):
if isinstance(node.parent, nodes.ExceptHandler):
# except (V < A)
suggestion = "Did you mean '({}, {})' instead?".format(
node.left.as_string(),
", ".join(operand.as_string() for _, operand in node.ops),
)
suggestion = f"Did you mean '({node.left.as_string()}, {', '.join(operand.as_string() for _, operand in node.ops)})' instead?"
self.add_message("wrong-exception-operation", node=node, args=(suggestion,))

@utils.check_messages(
Expand Down Expand Up @@ -561,10 +555,7 @@ def visit_tryexcept(self, node):

for previous_exc in exceptions_classes:
if previous_exc in exc_ancestors:
msg = "{} is an ancestor class of {}".format(
previous_exc.name,
exc.name,
)
msg = f"{previous_exc.name} is an ancestor class of {exc.name}"
self.add_message(
"bad-except-order", node=handler.type, args=msg
)
Expand Down
22 changes: 10 additions & 12 deletions pylint/checkers/imports.py
Expand Up @@ -151,16 +151,16 @@ def _repr_tree_defs(data, indent_str=None):
lines = []
nodes_items = data.items()
for i, (mod, (sub, files)) in enumerate(sorted(nodes_items, key=lambda x: x[0])):
files = "" if not files else "(%s)" % ",".join(sorted(files))
files = "" if not files else f"({','.join(sorted(files))})"
if indent_str is None:
lines.append(f"{mod} {files}")
sub_indent_str = " "
else:
lines.append(fr"{indent_str}\-{mod} {files}")
if i == len(nodes_items) - 1:
sub_indent_str = "%s " % indent_str
sub_indent_str = f"{indent_str} "
else:
sub_indent_str = "%s| " % indent_str
sub_indent_str = f"{indent_str}| "
if sub:
lines.append(_repr_tree_defs(sub, sub_indent_str))
return "\n".join(lines)
Expand Down Expand Up @@ -739,8 +739,8 @@ def _check_imports_order(self, _module_node):
"wrong-import-order",
node=node,
args=(
'standard import "%s"' % node.as_string(),
'"%s"' % wrong_import[0][0].as_string(),
f'standard import "{node.as_string()}"',
f'"{wrong_import[0][0].as_string()}"',
),
)
elif import_category == "THIRDPARTY":
Expand All @@ -754,8 +754,8 @@ def _check_imports_order(self, _module_node):
"wrong-import-order",
node=node,
args=(
'third party import "%s"' % node.as_string(),
'"%s"' % wrong_import[0][0].as_string(),
f'third party import "{node.as_string()}"',
f'"{wrong_import[0][0].as_string()}"',
),
)
elif import_category == "FIRSTPARTY":
Expand All @@ -769,8 +769,8 @@ def _check_imports_order(self, _module_node):
"wrong-import-order",
node=node,
args=(
'first party import "%s"' % node.as_string(),
'"%s"' % wrong_import[0][0].as_string(),
f'first party import "{node.as_string()}"',
f'"{wrong_import[0][0].as_string()}"',
),
)
elif import_category == "LOCALFOLDER":
Expand All @@ -787,9 +787,7 @@ def _get_imported_module(self, importnode, modname):
return None
self.add_message("relative-beyond-top-level", node=importnode)
except astroid.AstroidSyntaxError as exc:
message = "Cannot import {!r} due to syntax error {!r}".format(
modname, str(exc.error) # pylint: disable=no-member; false positive
)
message = f"Cannot import {modname!r} due to syntax error {str(exc.error)!r}" # pylint: disable=no-member; false positive
self.add_message("syntax-error", line=importnode.lineno, args=message)

except astroid.AstroidBuildingException:
Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/misc.py
Expand Up @@ -110,7 +110,7 @@ def open(self):
if self.config.notes_rgx:
regex_string = fr"#\s*({notes}|{self.config.notes_rgx})\b"
else:
regex_string = r"#\s*(%s)\b" % (notes)
regex_string = fr"#\s*({notes})\b"

self._fixme_pattern = re.compile(regex_string, re.I)

Expand Down
4 changes: 2 additions & 2 deletions pylint/checkers/raw_metrics.py
Expand Up @@ -29,7 +29,7 @@ def report_raw_stats(sect, stats, old_stats):
total_lines = stats["total_lines"]
if not total_lines:
raise EmptyReportError()
sect.description = "%s lines have been analyzed" % total_lines
sect.description = f"{total_lines} lines have been analyzed"
lines = ("type", "number", "%", "previous", "difference")
for node_type in ("code", "docstring", "comment", "empty"):
key = node_type + "_lines"
Expand All @@ -40,7 +40,7 @@ def report_raw_stats(sect, stats, old_stats):
diff_str = diff_string(old, total)
else:
old, diff_str = "NC", "NC"
lines += (node_type, str(total), "%.2f" % percent, str(old), diff_str)
lines += (node_type, str(total), f"{percent:.2f}", str(old), diff_str)
sect.append(Table(children=lines, cols=5, rheaders=1))


Expand Down
6 changes: 2 additions & 4 deletions pylint/checkers/refactoring/not_checker.py
Expand Up @@ -75,10 +75,8 @@ def visit_unaryop(self, node):
and _type.qname() in self.skipped_classnames
):
return
suggestion = "{} {} {}".format(
left.as_string(),
self.reverse_op[operator],
right.as_string(),
suggestion = (
f"{left.as_string()} {self.reverse_op[operator]} {right.as_string()}"
)
self.add_message(
"unneeded-not", node=node, args=(node.as_string(), suggestion)
Expand Down
6 changes: 1 addition & 5 deletions pylint/checkers/refactoring/refactoring_checker.py
Expand Up @@ -1406,11 +1406,7 @@ def visit_return(self, node: nodes.Return) -> None:
suggestion = false_value.as_string()
else:
message = "consider-using-ternary"
suggestion = "{truth} if {cond} else {false}".format(
truth=truth_value.as_string(),
cond=cond.as_string(),
false=false_value.as_string(),
)
suggestion = f"{truth_value.as_string()} if {cond.as_string()} else {false_value.as_string()}"
self.add_message(message, node=node, args=(suggestion,))

def _append_context_managers_to_stack(self, node: nodes.Assign) -> None:
Expand Down
8 changes: 2 additions & 6 deletions pylint/checkers/similar.py
Expand Up @@ -457,11 +457,7 @@ def _get_similarity_report(
report += f" {line.rstrip()}\n" if line.rstrip() else "\n"
duplicated_line_number += number * (len(couples_l) - 1)
total_line_number: int = sum(len(lineset) for lineset in self.linesets)
report += "TOTAL lines={} duplicates={} percent={:.2f}\n".format(
total_line_number,
duplicated_line_number,
duplicated_line_number * 100.0 / total_line_number,
)
report += f"TOTAL lines={total_line_number} duplicates={duplicated_line_number} percent={duplicated_line_number * 100.0 / total_line_number:.2f}\n"
return report

def _find_common(
Expand Down Expand Up @@ -676,7 +672,7 @@ def __init__(
)

def __str__(self):
return "<Lineset for %s>" % self.name
return f"<Lineset for {self.name}>"

def __len__(self):
return len(self._real_lines)
Expand Down
6 changes: 3 additions & 3 deletions pylint/checkers/spelling.py
Expand Up @@ -233,7 +233,7 @@ class SpellingChecker(BaseTokenChecker):
"metavar": "<dict name>",
"choices": dict_choices,
"help": "Spelling dictionary name. "
"Available dictionaries: %s.%s" % (dicts, instr),
f"Available dictionaries: {dicts}.{instr}",
},
),
(
Expand Down Expand Up @@ -400,14 +400,14 @@ def _check_spelling(self, msgid, line, line_num):
# Store word to private dict or raise a message.
if self.config.spelling_store_unknown_words:
if lower_cased_word not in self.unknown_words:
self.private_dict_file.write("%s\n" % lower_cased_word)
self.private_dict_file.write(f"{lower_cased_word}\n")
self.unknown_words.add(lower_cased_word)
else:
# Present up to N suggestions.
suggestions = self.spelling_dict.suggest(word)
del suggestions[self.config.max_spelling_suggestions :]
line_segment = line[word_start_at:]
match = re.search(r"(\W|^)(%s)(\W|$)" % word, line_segment)
match = re.search(fr"(\W|^)({word})(\W|$)", line_segment)
if match:
# Start position of second group in regex.
col = match.regs[2][0]
Expand Down

0 comments on commit 66ffcbc

Please sign in to comment.