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

Change end line and column for untyped function defs to line of function definition #17006

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

Conversation

mschoettle
Copy link

@mschoettle mschoettle commented Mar 9, 2024

Fixes #16746

Add extra attribures to FuncDef to keep track of a function definition's end excluding the body (end_line and end_column include the function body).

The definition's end is determined by either the return type hint's end or the last argument's end. Otherwise, the fall back is the function start (since it otherwise affects the output of --pretty). Potential alternatives (if desired) are either the end of the body or to include the length of the function name.

Before

image

After

image

@mschoettle mschoettle changed the title WIP: Add test for line and column end for missing function type annotation WIP: Change end line and column for untyped function defs to line of function definition Mar 9, 2024

This comment has been minimized.

mypy/messages.py Outdated

# set end line and column to same as start of context for function definitions
# this avoids errors being reported in IDEs for the whole function
# TODO: figure out if it's possible to find the end of the function definition line
Copy link
Member

Choose a reason for hiding this comment

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

One approach for this could be to add a new attribute to the FuncDef class and set it when we're creating the FuncDef object in fastparse.py.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

1 similar comment

This comment has been minimized.

@mschoettle mschoettle changed the title WIP: Change end line and column for untyped function defs to line of function definition Change end line and column for untyped function defs to line of function definition May 9, 2024
@mschoettle mschoettle marked this pull request as ready for review May 9, 2024 20:49

This comment has been minimized.

@@ -778,6 +780,9 @@ def __init__(
self.is_mypy_only = False
self.dataclass_transform_spec: DataclassTransformSpec | None = None
self.docstring: str | None = None
# track the end of the function definition itself
self.def_end_line: int | None = None
self.def_end_column: int | None = None
Copy link
Member

Choose a reason for hiding this comment

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

I think the new attributes will also need to be serialized, or they may get lost in incremental mode.

Copy link
Author

Choose a reason for hiding this comment

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

I checked in nodes.py and no node serializes end_line or end_column. The same for line and column with the exception of the TypeAlias node (it's unclear to me why).

I did a quick test invoking mypy with --incremental --disallow-untyped-defs --show-error-end on the same file twice which resulted in consistent results.

) -> int:
x = 1
[out]
main:2:1:4:11: error: Function is missing a return type annotation
Copy link
Member

Choose a reason for hiding this comment

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

It's unfortunate that the closing ): isn't included in the range. I imagine that's hard to fix though.

Copy link
Author

Choose a reason for hiding this comment

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

I agree. I unfortunately don't know how this could be fixed since it is unavailable in the AST.

For reference: I did find ast.get_source_segment. However, it also returns the source code for a function definition node including the body.

Copy link
Member

Choose a reason for hiding this comment

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

I understand. I'm still struggling with this case (as well as the one above where if the function has no annotations at all, we emit a one-character range); I'm worried this PR may lead to more confusing output in some cases. However, on balance it's probably fine and we can apply this change, but consider reverting it if there's a lot of negative feedback.

We could also consider looking at the first start position in the body of the function, but then the whitespace between the colon and the first statement would be included in the range.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Sep 9, 2024

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why report entire functions as an error on "missing return statement"
2 participants