For unresolved macros, hightlight only the last segment #7805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Canceled. |
matklad
commented
Feb 28, 2021
| ) | ||
| } | ||
| .into(); | ||
| Ok(res) |
There was a problem hiding this comment.
This was a case of https://matklad.github.io/2020/08/15/concrete-abstraction.html
| @@ -141,7 +140,7 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { | |||
| self.imp.original_range(node) | |||
| } | |||
|
|
|||
| pub fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { | |||
| pub fn diagnostics_display_range(&self, diagnostics: InFile<SyntaxNodePtr>) -> FileRange { | |||
There was a problem hiding this comment.
General code smell: if you need only a single thing, better to just require that single thing
| .and_then(|it| it.segment()) | ||
| .and_then(|it| it.name_ref()) | ||
| .map(|it| InFile::new(d.file, SyntaxNodePtr::new(it.syntax()))) | ||
| }); |
There was a problem hiding this comment.
cc @jonas-schievink, I worry a bit that the logic to narrow the range is different between UnresolvedMacroCall and UnresolvedProcMacro. In theory, UnresolvedMacroCall is how it's supposed to be done, but our diagnostics infra is a giant mess, so 馃し )
|
bors r+ |
bors bot
added a commit
that referenced
this pull request
Mar 17, 2021
8048: Fix missing unresolved macro diagnostic in function body r=edwin0cheng a=brandondong This was an issue I found while working on #7970. **Reproduction:** 1. Call a non-existent macro in a function body. ``` fn main() { foo!(); } ``` 2. No diagnostics are raised. An unresolved-macro-call diagnostic is expected. 3. If the macro call is instead outside of the function body, this works as expected. I believe this worked previously and regressed in #7805. **Behavior prior to #7805 - The unresolved-macro-call diagnostic did not exist. Instead, a macro-error diagnostic would be raised with the text "could not resolve macro [path]". - This was implemented by adding an error to the error sink (https://github.com/rust-analyzer/rust-analyzer/pull/7805/files#diff-50a326c5ae465bd9b31ee4310186380aa06e4fa1f6b41dbc0aed5bcc656a3cb8L657). - The error was propagated through https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body.rs#L123 eventually reaching https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body/lower.rs#L569. **Behavior after:** - Instead of writing to the error sink, an UnresolvedMacro error is now returned (https://github.com/rust-analyzer/rust-analyzer/pull/7805/files#diff-50a326c5ae465bd9b31ee4310186380aa06e4fa1f6b41dbc0aed5bcc656a3cb8R631). - The parent caller throws away the error as its function signature is `Option<MacroCallId>` (https://github.com/rust-analyzer/rust-analyzer/pull/7805/files#diff-50a326c5ae465bd9b31ee4310186380aa06e4fa1f6b41dbc0aed5bcc656a3cb8R604). - We instead now reach the warn condition (https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body.rs#L124) and no diagnostics are created in https://github.com/rust-analyzer/rust-analyzer/blob/1a82af3527e476d52410ff4dfd2fb4c57466abcb/crates/hir_def/src/body/lower.rs#L575. **Fix:** - Make sure to propagate the UnresolvedMacro error. Report the error using the new unresolved-macro-call diagnostic. Co-authored-by: Brandon <brandondong604@hotmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
bors r+
馃