Skip to content

Migrate checkstrformat to use ErrorMessage class #7

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Migrate checkstrformat to use ErrorMessage class #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91c46c7
Migrate checkstrformat to use `ErrorMessage` class
tusharsadhwani Apr 24, 2023
45242d8
Merge branch 'master' into msg-regostry-checkstrformat
tusharsadhwani Jun 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 25 additions & 91 deletions mypy/checkstrformat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,11 @@ def parse_format_value(
custom_match, start_pos=start_pos, non_standard_format_spec=True
)
else:
msg.fail(
"Invalid conversion specifier in format string",
ctx,
code=codes.STRING_FORMATTING,
)
msg.fail(message_registry.FORMAT_STR_INVALID_SPECIFIER, ctx)
return None

if conv_spec.key and ("{" in conv_spec.key or "}" in conv_spec.key):
msg.fail("Conversion value must not contain { or }", ctx, code=codes.STRING_FORMATTING)
msg.fail(message_registry.FORMAT_STR_BRACES_IN_SPECIFIER, ctx)
return None
result.append(conv_spec)

Expand All @@ -218,11 +214,7 @@ def parse_format_value(
and ("{" in conv_spec.format_spec or "}" in conv_spec.format_spec)
):
if nested:
msg.fail(
"Formatting nesting must be at most two levels deep",
ctx,
code=codes.STRING_FORMATTING,
)
msg.fail(message_registry.FORMAT_STR_NESTING_ATMOST_TWO_LEVELS, ctx)
return None
sub_conv_specs = parse_format_value(conv_spec.format_spec, ctx, msg, nested=True)
if sub_conv_specs is None:
Expand Down Expand Up @@ -260,11 +252,7 @@ def find_non_escaped_targets(
if pos < len(format_value) - 1 and format_value[pos + 1] == "}":
pos += 1
else:
msg.fail(
"Invalid conversion specifier in format string: unexpected }",
ctx,
code=codes.STRING_FORMATTING,
)
msg.fail(message_registry.FORMAT_STR_UNEXPECTED_RBRACE, ctx)
return None
else:
# Adjust nesting level, then either continue adding chars or move on.
Expand All @@ -279,11 +267,7 @@ def find_non_escaped_targets(
next_spec = ""
pos += 1
if nesting:
msg.fail(
"Invalid conversion specifier in format string: unmatched {",
ctx,
code=codes.STRING_FORMATTING,
)
msg.fail(message_registry.FORMAT_STR_UNMATCHED_LBRACE, ctx)
return None
return result

Expand Down Expand Up @@ -369,9 +353,8 @@ def check_specs_in_format_call(
):
# TODO: add support for some custom specs like datetime?
self.msg.fail(
"Unrecognized format" ' specification "{}"'.format(spec.format_spec[1:]),
message_registry.UNRECOGNIZED_FORMAT_SPEC.format(spec.format_spec[1:]),
call,
code=codes.STRING_FORMATTING,
)
continue
Comment on lines 353 to 359
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider adding a code to the fail call, for consistency with the other calls in this file. This will also allow users to filter this error specifically.

                    self.msg.fail(
                        message_registry.UNRECOGNIZED_FORMAT_SPEC.format(spec.format_spec[1:])
                        call,
                        code=codes.STRING_FORMATTING,
                    )
                    continue

# Adjust expected and actual types.
Expand All @@ -390,10 +373,10 @@ def check_specs_in_format_call(
# If the explicit conversion is given, then explicit conversion is called _first_.
if spec.conversion[1] not in "rsa":
self.msg.fail(
'Invalid conversion type "{}",'
' must be one of "r", "s" or "a"'.format(spec.conversion[1]),
message_registry.FORMAT_STR_INVALID_CONVERSION_TYPE.format(
spec.conversion[1]
),
call,
code=codes.STRING_FORMATTING,
)
actual_type = self.named_type("builtins.str")

Expand Down Expand Up @@ -433,13 +416,7 @@ def perform_special_format_checks(
if has_type_component(actual_type, "builtins.bytes") and not custom_special_method(
actual_type, "__str__"
):
self.msg.fail(
'If x = b\'abc\' then f"{x}" or "{}".format(x) produces "b\'abc\'", '
'not "abc". If this is desired behavior, use f"{x!r}" or "{!r}".format(x). '
"Otherwise, decode the bytes",
call,
code=codes.STR_BYTES_PY3,
)
self.msg.fail(message_registry.FORMAT_STR_BYTES_USE_REPR, call)
if spec.flags:
numeric_types = UnionType(
[self.named_type("builtins.int"), self.named_type("builtins.float")]
Expand All @@ -451,11 +428,7 @@ def perform_special_format_checks(
and not is_subtype(actual_type, numeric_types)
and not custom_special_method(actual_type, "__format__")
):
self.msg.fail(
"Numeric flags are only allowed for numeric types",
call,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_INVALID_NUMERIC_FLAG, call)

def find_replacements_in_call(self, call: CallExpr, keys: list[str]) -> list[Expression]:
"""Find replacement expression for every specifier in str.format() call.
Expand All @@ -469,19 +442,14 @@ def find_replacements_in_call(self, call: CallExpr, keys: list[str]) -> list[Exp
expr = self.get_expr_by_position(int(key), call)
if not expr:
self.msg.fail(
"Cannot find replacement for positional"
" format specifier {}".format(key),
call,
code=codes.STRING_FORMATTING,
message_registry.FORMAT_STR_REPLACEMENT_NOT_FOUND.format(key), call
)
expr = TempNode(AnyType(TypeOfAny.from_error))
else:
expr = self.get_expr_by_name(key, call)
if not expr:
self.msg.fail(
"Cannot find replacement for named" ' format specifier "{}"'.format(key),
call,
code=codes.STRING_FORMATTING,
message_registry.FORMAT_STR_NAMED_REPLACEMENT_NOT_FOUND.format(key), call
)
expr = TempNode(AnyType(TypeOfAny.from_error))
result.append(expr)
Expand Down Expand Up @@ -555,11 +523,7 @@ def auto_generate_keys(self, all_specs: list[ConversionSpecifier], ctx: Context)
some_defined = any(s.key and s.key.isdecimal() for s in all_specs)
all_defined = all(bool(s.key) for s in all_specs)
if some_defined and not all_defined:
self.msg.fail(
"Cannot combine automatic field numbering and manual field specification",
ctx,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_PARTIAL_FIELD_NUMBERING, ctx)
return False
if all_defined:
return True
Expand Down Expand Up @@ -594,11 +558,7 @@ def apply_field_accessors(
dummy, fnam="<format>", module=None, options=self.chk.options, errors=temp_errors
)
if temp_errors.is_errors():
self.msg.fail(
f'Syntax error in format specifier "{spec.field}"',
ctx,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_SYNTAX_ERROR.format(spec.field), ctx)
return TempNode(AnyType(TypeOfAny.from_error))

# These asserts are guaranteed by the original regexp.
Expand Down Expand Up @@ -637,10 +597,7 @@ class User(TypedDict):
"""
if not isinstance(temp_ast, (MemberExpr, IndexExpr)):
self.msg.fail(
"Only index and member expressions are allowed in"
' format field accessors; got "{}"'.format(spec.field),
ctx,
code=codes.STRING_FORMATTING,
message_registry.FORMAT_STR_INVALID_ACCESSOR_EXPR.format(spec.field), ctx
)
return False
if isinstance(temp_ast, MemberExpr):
Expand All @@ -651,10 +608,10 @@ class User(TypedDict):
assert spec.key, "Call this method only after auto-generating keys!"
assert spec.field
self.msg.fail(
"Invalid index expression in format field"
' accessor "{}"'.format(spec.field[len(spec.key) :]),
message_registry.FORMAT_STR_INVALID_INDEX_ACCESSOR.format(
spec.field[len(spec.key) :]
),
ctx,
code=codes.STRING_FORMATTING,
)
return False
if isinstance(temp_ast.index, NameExpr):
Expand Down Expand Up @@ -683,11 +640,7 @@ def check_str_interpolation(self, expr: FormatStringExpr, replacements: Expressi
specifiers = parse_conversion_specifiers(expr.value)
has_mapping_keys = self.analyze_conversion_specifiers(specifiers, expr)
if isinstance(expr, BytesExpr) and self.chk.options.python_version < (3, 5):
self.msg.fail(
"Bytes formatting is only supported in Python 3.5 and later",
replacements,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_BYTES_ABOVE_PY35, replacements)
return AnyType(TypeOfAny.from_error)

if has_mapping_keys is None:
Expand Down Expand Up @@ -794,9 +747,7 @@ def check_mapping_str_interpolation(
# Special case: for bytes formatting keys must be bytes.
if not isinstance(k, BytesExpr):
self.msg.fail(
"Dictionary keys in bytes formatting must be bytes, not strings",
expr,
code=codes.STRING_FORMATTING,
message_registry.FORMAT_STR_BYTES_DICT_KEYS_MUST_BE_BYTES, expr
)
key_str = cast(FormatStringExpr, k).value
mapping[key_str] = self.accept(v)
Expand Down Expand Up @@ -948,21 +899,12 @@ def check_s_special_cases(self, expr: FormatStringExpr, typ: Type, context: Cont
if isinstance(expr, StrExpr):
# Couple special cases for string formatting.
if has_type_component(typ, "builtins.bytes"):
self.msg.fail(
'If x = b\'abc\' then "%s" % x produces "b\'abc\'", not "abc". '
'If this is desired behavior use "%r" % x. Otherwise, decode the bytes',
context,
code=codes.STR_BYTES_PY3,
)
self.msg.fail(message_registry.FORMAT_STR_BYTES_USE_REPR_OLD, context)
return False
if isinstance(expr, BytesExpr):
# A special case for bytes formatting: b'%s' actually requires bytes on Python 3.
if has_type_component(typ, "builtins.str"):
self.msg.fail(
"On Python 3 b'%s' requires bytes, not string",
context,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_BYTES_REQUIRED_PY3, context)
return False
return True

Expand Down Expand Up @@ -1024,18 +966,10 @@ def conversion_type(
INT_TYPES = REQUIRE_INT_NEW if format_call else REQUIRE_INT_OLD
if p == "b" and not format_call:
if self.chk.options.python_version < (3, 5):
self.msg.fail(
'Format character "b" is only supported in Python 3.5 and later',
context,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_INVALID_BYTES_SPECIFIER_PY35, context)
return None
if not isinstance(expr, BytesExpr):
self.msg.fail(
'Format character "b" is only supported on bytes patterns',
context,
code=codes.STRING_FORMATTING,
)
self.msg.fail(message_registry.FORMAT_STR_INVALID_BYTES_SPECIFIER, context)
return None
return self.named_type("builtins.bytes")
elif p == "a":
Expand Down
4 changes: 2 additions & 2 deletions mypy/errorcodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ def __str__(self) -> str:
VALID_NEWTYPE: Final = ErrorCode(
"valid-newtype", "Check that argument 2 to NewType is valid", "General"
)
STRING_FORMATTING: Final = ErrorCode(
STRING_FORMATTING: Final[ErrorCode] = ErrorCode(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhancement: Adding Final[ErrorCode] type annotation provides better type safety than just Final.

"str-format", "Check that string formatting/interpolation is type-safe", "General"
)
STR_BYTES_PY3: Final = ErrorCode(
STR_BYTES_PY3: Final[ErrorCode] = ErrorCode(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency: Good job applying the same Final[ErrorCode] type annotation to STR_BYTES_PY3 as well for consistency.

"str-bytes-safe", "Warn about implicit coercions related to bytes and string types", "General"
)
EXIT_RETURN: Final = ErrorCode(
Expand Down
71 changes: 71 additions & 0 deletions mypy/message_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,74 @@ def with_additional_msg(self, info: str) -> ErrorMessage:
ARG_NAME_EXPECTED_STRING_LITERAL: Final = ErrorMessage(
"Expected string literal for argument name, got {}", codes.SYNTAX
)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improvement: Good refactoring to centralize error messages in message_registry.py. This makes error messages more consistent and easier to maintain.

FORMAT_STR_INVALID_SPECIFIER: Final = ErrorMessage(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider adding a docstring for this new section of error messages to explain that these are all related to string formatting.

"Invalid conversion specifier in format string", codes.STRING_FORMATTING
)
FORMAT_STR_BRACES_IN_SPECIFIER: Final = ErrorMessage(
"Conversion value must not contain { or }", codes.STRING_FORMATTING
)
FORMAT_STR_NESTING_ATMOST_TWO_LEVELS: Final = ErrorMessage(
"Formatting nesting must be at most two levels deep", codes.STRING_FORMATTING
)
FORMAT_STR_UNEXPECTED_RBRACE: Final = ErrorMessage(
"Invalid conversion specifier in format string: unexpected }", codes.STRING_FORMATTING
)
FORMAT_STR_UNMATCHED_LBRACE: Final = ErrorMessage(
"Invalid conversion specifier in format string: unmatched {", codes.STRING_FORMATTING
)
UNRECOGNIZED_FORMAT_SPEC: Final = ErrorMessage(
'Unrecognized format specification "{}"', codes.STRING_FORMATTING
)
FORMAT_STR_INVALID_CONVERSION_TYPE: Final = ErrorMessage(
'Invalid conversion type "{}", must be one of "r", "s" or "a"', codes.STRING_FORMATTING
)
FORMAT_STR_BYTES_USE_REPR: Final = ErrorMessage(
'If x = b\'abc\' then f"{x}" or "{}".format(x) produces "b\'abc\'", '
'not "abc". If this is desired behavior, use f"{x!r}" or "{!r}".format(x). '
"Otherwise, decode the bytes",
codes.STR_BYTES_PY3,
)
FORMAT_STR_BYTES_USE_REPR_OLD: Final = ErrorMessage(
'If x = b\'abc\' then "%s" % x produces "b\'abc\'", not "abc". '
'If this is desired behavior use "%r" % x. Otherwise, decode the bytes',
codes.STR_BYTES_PY3,
)
FORMAT_STR_INVALID_NUMERIC_FLAG: Final = ErrorMessage(
"Numeric flags are only allowed for numeric types", codes.STRING_FORMATTING
)
FORMAT_STR_REPLACEMENT_NOT_FOUND: Final = ErrorMessage(
"Cannot find replacement for positional format specifier {}", codes.STRING_FORMATTING
)
FORMAT_STR_NAMED_REPLACEMENT_NOT_FOUND: Final = ErrorMessage(
'Cannot find replacement for named format specifier "{}"', codes.STRING_FORMATTING
)
FORMAT_STR_PARTIAL_FIELD_NUMBERING: Final = ErrorMessage(
"Cannot combine automatic field numbering and manual field specification",
codes.STRING_FORMATTING,
)
FORMAT_STR_SYNTAX_ERROR: Final = ErrorMessage(
'Syntax error in format specifier "{}"', codes.STRING_FORMATTING
)
FORMAT_STR_INVALID_ACCESSOR_EXPR: Final = ErrorMessage(
'Only index and member expressions are allowed in format field accessors; got "{}"',
codes.STRING_FORMATTING,
)
FORMAT_STR_INVALID_INDEX_ACCESSOR: Final = ErrorMessage(
'Invalid index expression in format field accessor "{}"', codes.STRING_FORMATTING
)
FORMAT_STR_BYTES_ABOVE_PY35: Final = ErrorMessage(
"Bytes formatting is only supported in Python 3.5 and later", codes.STRING_FORMATTING
)
FORMAT_STR_BYTES_DICT_KEYS_MUST_BE_BYTES: Final = ErrorMessage(
"Dictionary keys in bytes formatting must be bytes, not strings", codes.STRING_FORMATTING
)
FORMAT_STR_BYTES_REQUIRED_PY3: Final = ErrorMessage(
"On Python 3 b'%s' requires bytes, not string", codes.STRING_FORMATTING
)
FORMAT_STR_INVALID_BYTES_SPECIFIER_PY35: Final = ErrorMessage(
'Format character "b" is only supported in Python 3.5 and later', codes.STRING_FORMATTING
)
FORMAT_STR_INVALID_BYTES_SPECIFIER: Final = ErrorMessage(
'Format character "b" is only supported on bytes patterns', codes.STRING_FORMATTING
)
35 changes: 33 additions & 2 deletions mypy/messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import re
from contextlib import contextmanager
from textwrap import dedent
from typing import Any, Callable, Collection, Iterable, Iterator, List, Sequence, cast
from typing import Any, Callable, Collection, Iterable, Iterator, List, Sequence, cast, overload
from typing_extensions import Final

import mypy.typeops
Expand Down Expand Up @@ -269,6 +269,7 @@ def span_from_context(ctx: Context) -> Iterable[int]:
allow_dups=allow_dups,
)

@overload
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhancement: The @overload decorator for fail() method is a good addition that improves type checking when using the new ErrorMessage objects.

def fail(
self,
msg: str,
Expand All @@ -278,10 +279,40 @@ def fail(
file: str | None = None,
allow_dups: bool = False,
secondary_context: Context | None = None,
) -> None:
...

@overload
def fail(
self,
msg: message_registry.ErrorMessage,
context: Context | None,
*,
file: str | None = None,
allow_dups: bool = False,
secondary_context: Context | None = None,
) -> None:
...

def fail(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improvement: The implementation of fail() now handles both string messages and ErrorMessage objects, which is a good design that maintains backward compatibility.

self,
msg: str | message_registry.ErrorMessage,
context: Context | None,
*,
code: ErrorCode | None = None,
file: str | None = None,
allow_dups: bool = False,
secondary_context: Context | None = None,
) -> None:
"""Report an error message (unless disabled)."""
if isinstance(msg, message_registry.ErrorMessage):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: Consider adding a type annotation for the local variables msg_str to make the code more explicit about types.

msg_str = msg.value
code = msg.code
else:
msg_str = msg

self.report(
msg,
msg_str,
context,
"error",
code=code,
Expand Down