-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Better error location for incompatible override due to argument incompatibility #8298
Comments
@JukkaL I'd like to help with this issue but not quite sure about where to start, any suggestions? |
@DenisOH Look for calls to |
@DenisOH Can I work on this? |
@Arshaan-256 it looks like Denis has already made a PR for this! |
Oh! I am so sorry. I thought that the PR didn't work since the issue was still open. Sorry! |
@Arshaan-256 I'm still waiting on that PR to be reviewed. However, if it's not good or needs improvement, feel free to take over the issue. |
Modifies arguments parsed from the AST to use the AST's row and column information. Modifies function definitions to not overwrite their arguments' locations. Modifies incorrect override messages to use the locations of arguments instead of the method itself. Modifies tests to expect the new locations. I'm not sure whether this handles bound arguments correctly; it passes tests but I don't know whether there's some edge case I'm missing. Fixes #8298.
An incompatible override gets reported on the first line of a method header, even though it would be better to point it at the line with an argument that is incompatible, when possible. Example:
This would improve usability, especially for methods with many arguments on many lines. This would also make it cleaner to ignore these errors, as the location of the
# type: ignore
comment would document which argument is incompatible (if there is one argument per line, which is a common convention).The text was updated successfully, but these errors were encountered: