Skip to content

Improve position of deprecation #23052

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

Merged
merged 1 commit into from
Apr 29, 2025
Merged

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Apr 25, 2025

Fixes #22795

This commit moves the caret to the site of inlining in the current file.

report.deprecationWarning omitted addInlineds, which yields the "uninlined" position.

It would be nice to correctly represent the inlined, spliced positions as well (which are shown in the explanation with the wrong file name, and which may be a source in a library).

It's clearly wrong to use a span from a different file, but the question remains how to represent a position from a dependency, which is not necessarily meaningful to the user.

This provisional tweak just shows the top-level splice location in the explanation, instead of what it quotes.

@som-snytt som-snytt force-pushed the issue/22795-msg-pos branch from 2eef13f to d51cf92 Compare April 25, 2025 21:51
@som-snytt som-snytt changed the title [WIP] Spliced position respects source file Spliced position respects source file Apr 25, 2025
@som-snytt som-snytt changed the title Spliced position respects source file Improve position of deprecation Apr 25, 2025
@som-snytt som-snytt marked this pull request as ready for review April 26, 2025 04:08
@Gedochao Gedochao requested review from lrytz and jchyb April 28, 2025 08:39
|---------------------------------------------------------------------------------------------------------------------
|Inline stack trace
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
|This location contains code that was inlined from test_1.scala:5
Copy link
Member

Choose a reason for hiding this comment

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

the code was inlined from lib_0.scala, not test_1.scala, or?

Copy link
Contributor Author

@som-snytt som-snytt Apr 29, 2025

Choose a reason for hiding this comment

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

yes, that was the improvement (to do) I mention in the comment. I hoped to book the progress before opening the can of worms. A can of worms has a long shelf life and can be opened at any time.

I'll improve the PR comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

note this is the (wrong) span it currently reports at.

Copy link
Member

Choose a reason for hiding this comment

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

👍 thanks

@som-snytt som-snytt merged commit 57807c6 into scala:main Apr 29, 2025
29 checks passed
@som-snytt som-snytt deleted the issue/22795-msg-pos branch April 29, 2025 18:49
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.

Deprecation warning generated via macro points to nonsense position
3 participants