Skip to content

Commit

Permalink
Standardize type comments to always have one space (#2698)
Browse files Browse the repository at this point in the history
  • Loading branch information
Pedro-Muller29 committed Sep 30, 2024
1 parent 67119b9 commit aeca4e4
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 52 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

- Fix type annotation spacing between * and more complex type variable tuple (i.e. `def
fn(*args: *tuple[*Ts, T]) -> None: pass`) (#4440)
- Standardize type comments to always have one space (#4467)

### Caching

Expand Down
3 changes: 3 additions & 0 deletions docs/the_black_code_style/future_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ Currently, the following features are included in the preview style:
blocks when the line is too long
- `pep646_typed_star_arg_type_var_tuple`: fix type annotation spacing between * and more
complex type variable tuple (i.e. `def fn(*args: *tuple[*Ts, T]) -> None: pass`)
- `type_comments_standardization`: type comments with zero or more empty spaces between
`#` and `type:`, or between `type:` and the type itself will be formatted to
`# type: (type itself)`.

(labels/unstable-features)=

Expand Down
67 changes: 44 additions & 23 deletions src/black/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ProtoComment:
leading_whitespace: str # leading whitespace before the comment, if any


def generate_comments(leaf: LN) -> Iterator[Leaf]:
def generate_comments(leaf: LN, mode: Mode) -> Iterator[Leaf]:
"""Clean the prefix of the `leaf` and generate comments from it, if any.
Comments in lib2to3 are shoved into the whitespace prefix. This happens
Expand All @@ -69,15 +69,17 @@ def generate_comments(leaf: LN) -> Iterator[Leaf]:
are emitted with a fake STANDALONE_COMMENT token identifier.
"""
total_consumed = 0
for pc in list_comments(leaf.prefix, is_endmarker=leaf.type == token.ENDMARKER):
for pc in list_comments(
leaf.prefix, is_endmarker=leaf.type == token.ENDMARKER, mode=mode
):
total_consumed = pc.consumed
prefix = make_simple_prefix(pc.newlines, pc.form_feed)
yield Leaf(pc.type, pc.value, prefix=prefix)
normalize_trailing_prefix(leaf, total_consumed)


@lru_cache(maxsize=4096)
def list_comments(prefix: str, *, is_endmarker: bool) -> list[ProtoComment]:
def list_comments(prefix: str, *, is_endmarker: bool, mode: Mode) -> list[ProtoComment]:
"""Return a list of :class:`ProtoComment` objects parsed from the given `prefix`."""
result: list[ProtoComment] = []
if not prefix or "#" not in prefix:
Expand Down Expand Up @@ -108,7 +110,7 @@ def list_comments(prefix: str, *, is_endmarker: bool) -> list[ProtoComment]:
comment_type = token.COMMENT # simple trailing comment
else:
comment_type = STANDALONE_COMMENT
comment = make_comment(line)
comment = make_comment(line, mode)
result.append(
ProtoComment(
type=comment_type,
Expand Down Expand Up @@ -139,7 +141,7 @@ def normalize_trailing_prefix(leaf: LN, total_consumed: int) -> None:
leaf.prefix = ""


def make_comment(content: str) -> str:
def make_comment(content: str, mode: Mode) -> str:
"""Return a consistently formatted comment from the given `content` string.
All comments (except for "##", "#!", "#:", '#'") should have a single
Expand All @@ -153,13 +155,30 @@ def make_comment(content: str) -> str:

if content[0] == "#":
content = content[1:]

NON_BREAKING_SPACE = " "
if (
content
and content[0] == NON_BREAKING_SPACE
and not content.lstrip().startswith("type:")
):

is_type_comment = (
re.match(r"^\s*type:", content)
if Preview.type_comments_standardization in mode
else content.lstrip().startswith("type:")
)

if content and content[0] == NON_BREAKING_SPACE and not is_type_comment:
content = " " + content[1:] # Replace NBSP by a simple space
elif (
Preview.type_comments_standardization in mode
and is_type_comment
and re.match(
r"^\s*type:(?!\s*ignore\b)", content
) # Check if it is type: ignore
and NON_BREAKING_SPACE not in content
):
content = content.strip()
parts = content.split(":")
key = parts[0].strip() # Remove extra spaces around "type"
value = parts[1].strip() # Remove extra spaces around the value part
content = f" {key}: {value}"
if content and content[0] not in COMMENT_EXCEPTIONS:
content = " " + content
return "#" + content
Expand All @@ -183,7 +202,7 @@ def convert_one_fmt_off_pair(
"""
for leaf in node.leaves():
previous_consumed = 0
for comment in list_comments(leaf.prefix, is_endmarker=False):
for comment in list_comments(leaf.prefix, is_endmarker=False, mode=mode):
is_fmt_off = comment.value in FMT_OFF
is_fmt_skip = _contains_fmt_skip_comment(comment.value, mode)
if (not is_fmt_off and not is_fmt_skip) or (
Expand Down Expand Up @@ -273,17 +292,17 @@ def generate_ignored_nodes(
Stops at the end of the block.
"""
if _contains_fmt_skip_comment(comment.value, mode):
yield from _generate_ignored_nodes_from_fmt_skip(leaf, comment)
yield from _generate_ignored_nodes_from_fmt_skip(leaf, comment, mode=mode)
return
container: Optional[LN] = container_of(leaf)
while container is not None and container.type != token.ENDMARKER:
if is_fmt_on(container):
if is_fmt_on(container, mode=mode):
return

# fix for fmt: on in children
if children_contains_fmt_on(container):
if children_contains_fmt_on(container, mode=mode):
for index, child in enumerate(container.children):
if isinstance(child, Leaf) and is_fmt_on(child):
if isinstance(child, Leaf) and is_fmt_on(child, mode=mode):
if child.type in CLOSING_BRACKETS:
# This means `# fmt: on` is placed at a different bracket level
# than `# fmt: off`. This is an invalid use, but as a courtesy,
Expand All @@ -294,12 +313,14 @@ def generate_ignored_nodes(
if (
child.type == token.INDENT
and index < len(container.children) - 1
and children_contains_fmt_on(container.children[index + 1])
and children_contains_fmt_on(
container.children[index + 1], mode=mode
)
):
# This means `# fmt: on` is placed right after an indentation
# level, and we shouldn't swallow the previous INDENT token.
return
if children_contains_fmt_on(child):
if children_contains_fmt_on(child, mode=mode):
return
yield child
else:
Expand All @@ -312,14 +333,14 @@ def generate_ignored_nodes(


def _generate_ignored_nodes_from_fmt_skip(
leaf: Leaf, comment: ProtoComment
leaf: Leaf, comment: ProtoComment, mode: Mode
) -> Iterator[LN]:
"""Generate all leaves that should be ignored by the `# fmt: skip` from `leaf`."""
prev_sibling = leaf.prev_sibling
parent = leaf.parent
# Need to properly format the leaf prefix to compare it to comment.value,
# which is also formatted
comments = list_comments(leaf.prefix, is_endmarker=False)
comments = list_comments(leaf.prefix, is_endmarker=False, mode=mode)
if not comments or comment.value != comments[0].value:
return
if prev_sibling is not None:
Expand Down Expand Up @@ -353,24 +374,24 @@ def _generate_ignored_nodes_from_fmt_skip(
yield from iter(ignored_nodes)


def is_fmt_on(container: LN) -> bool:
def is_fmt_on(container: LN, mode: Mode) -> bool:
"""Determine whether formatting is switched on within a container.
Determined by whether the last `# fmt:` comment is `on` or `off`.
"""
fmt_on = False
for comment in list_comments(container.prefix, is_endmarker=False):
for comment in list_comments(container.prefix, is_endmarker=False, mode=mode):
if comment.value in FMT_ON:
fmt_on = True
elif comment.value in FMT_OFF:
fmt_on = False
return fmt_on


def children_contains_fmt_on(container: LN) -> bool:
def children_contains_fmt_on(container: LN, mode: Mode) -> bool:
"""Determine if children have formatting switched on."""
for child in container.children:
leaf = first_leaf_of(child)
if leaf is not None and is_fmt_on(leaf):
if leaf is not None and is_fmt_on(leaf, mode=mode):
return True

return False
Expand Down
4 changes: 2 additions & 2 deletions src/black/linegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def visit_default(self, node: LN) -> Iterator[Line]:
"""Default `visit_*()` implementation. Recurses to children of `node`."""
if isinstance(node, Leaf):
any_open_brackets = self.current_line.bracket_tracker.any_open_brackets()
for comment in generate_comments(node):
for comment in generate_comments(node, mode=self.mode):
if any_open_brackets:
# any comment within brackets is subject to splitting
self.current_line.append(comment)
Expand Down Expand Up @@ -1352,7 +1352,7 @@ def normalize_invisible_parens( # noqa: C901
Standardizes on visible parentheses for single-element tuples, and keeps
existing visible parentheses for other tuples and generator expressions.
"""
for pc in list_comments(node.prefix, is_endmarker=False):
for pc in list_comments(node.prefix, is_endmarker=False, mode=mode):
if pc.value in FMT_OFF:
# This `node` has a prefix with `# fmt: off`, don't mess with parens.
return
Expand Down
4 changes: 2 additions & 2 deletions src/black/lines.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ def contains_uncollapsable_type_comments(self) -> bool:
comment_seen = False
for leaf_id, comments in self.comments.items():
for comment in comments:
if is_type_comment(comment):
if is_type_comment(comment, mode=self.mode):
if comment_seen or (
not is_type_ignore_comment(comment)
and leaf_id not in ignored_ids
Expand Down Expand Up @@ -401,7 +401,7 @@ def append_comment(self, comment: Leaf) -> bool:
and not last_leaf.value
and last_leaf.parent
and len(list(last_leaf.parent.leaves())) <= 3
and not is_type_comment(comment)
and not is_type_comment(comment, self.mode)
):
# Comments on an optional parens wrapping a single leaf should belong to
# the wrapped node except if it's a type comment. Pinning the comment like
Expand Down
23 changes: 16 additions & 7 deletions src/black/mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ class Preview(Enum):
remove_redundant_guard_parens = auto()
parens_for_long_if_clauses_in_case_block = auto()
pep646_typed_star_arg_type_var_tuple = auto()
type_comments_standardization = auto()


UNSTABLE_FEATURES: set[Preview] = {
Expand Down Expand Up @@ -247,13 +248,6 @@ class Mode:
enabled_features: set[Preview] = field(default_factory=set)

def __contains__(self, feature: Preview) -> bool:
"""
Provide `Preview.FEATURE in Mode` syntax that mirrors the ``preview`` flag.
In unstable mode, all features are enabled. In preview mode, all features
except those in UNSTABLE_FEATURES are enabled. Any features in
`self.enabled_features` are also enabled.
"""
if self.unstable:
return True
if feature in self.enabled_features:
Expand Down Expand Up @@ -294,3 +288,18 @@ def get_cache_key(self) -> str:
features_and_magics,
]
return ".".join(parts)

def __hash__(self) -> int:
return hash((
frozenset(self.target_versions),
self.line_length,
self.string_normalization,
self.is_pyi,
self.is_ipynb,
self.skip_source_first_line,
self.magic_trailing_comma,
frozenset(self.python_cell_magics),
self.preview,
self.unstable,
frozenset(self.enabled_features),
))
11 changes: 7 additions & 4 deletions src/black/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -905,16 +905,19 @@ def is_async_stmt_or_funcdef(leaf: Leaf) -> bool:
)


def is_type_comment(leaf: Leaf) -> bool:
def is_type_comment(leaf: Leaf, mode: Mode) -> bool:
"""Return True if the given leaf is a type comment. This function should only
be used for general type comments (excluding ignore annotations, which should
use `is_type_ignore_comment`). Note that general type comments are no longer
used in modern version of Python, this function may be deprecated in the future."""
t = leaf.type
v = leaf.value
return (
t in {token.COMMENT, STANDALONE_COMMENT}
and bool(re.match(r"#\s*type:", v))

type_comment_with_extra_spaces = bool(re.match(r"#\s*type:", v))

return t in {token.COMMENT, STANDALONE_COMMENT} and (
v.startswith("# type:")
or (Preview.type_comments_standardization and type_comment_with_extra_spaces)
)


Expand Down
3 changes: 2 additions & 1 deletion src/black/resources/black.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@
"docstring_check_for_newline",
"remove_redundant_guard_parens",
"parens_for_long_if_clauses_in_case_block",
"pep646_typed_star_arg_type_var_tuple"
"pep646_typed_star_arg_type_var_tuple",
"type_comments_standardization"
]
},
"description": "Enable specific features included in the `--unstable` style. Requires `--preview`. No compatibility guarantees are provided on the behavior or existence of any unstable features."
Expand Down
10 changes: 10 additions & 0 deletions tests/data/cases/type_comment_syntax.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
def f(
a, # type: int
):
pass


# test type comments
def f(a, b, c, d, e, f, g, h, i):
# type: (int, int, int, int, int, int, int, int, int) -> None
pass
11 changes: 0 additions & 11 deletions tests/data/cases/type_comment_syntax_error.py

This file was deleted.

57 changes: 57 additions & 0 deletions tests/data/cases/type_comment_syntax_preview.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# flags: --preview


def f(
a, # type: int
):
pass


# test type comments
def f(a, b, c, d, e, f, g, h, i):
# type: (int, int, int, int, int, int, int, int, int) -> None
pass


def f(
a, # type : int
b, # type : int
c, #type : int
d, # type: int
e, # type: int
f, # type : int
g, #type:int
h, # type: int
i, # type: int
):
# type: (...) -> None
pass



# output
def f(
a, # type: int
):
pass


# test type comments
def f(a, b, c, d, e, f, g, h, i):
# type: (int, int, int, int, int, int, int, int, int) -> None
pass


def f(
a, # type : int
b, # type : int
c, # type : int
d, # type: int
e, # type: int
f, # type : int
g, # type: int
h, # type: int
i, # type: int
):
# type: (...) -> None
pass
Loading

0 comments on commit aeca4e4

Please sign in to comment.