Rust: downgrade uncompiled source files from warning to info #20288
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refines the diagnostic reporting system for Rust extractor by downgrading certain warnings to info level when rust-analyzer semantics are unavailable for valid reasons (like uncompiled source files). It introduces a structured approach to handle different severity levels for missing semantics scenarios.
- Adds a
Defaulttrait toDiagnosticSeverityenum withInfoas the default - Creates a new
RustAnalyzerNoSemanticsstruct to encapsulate severity and reason for missing semantics - Updates error messages to be more accurate and changes severity from Warning to Info for valid non-compilation cases
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rust/extractor/src/trap.rs | Adds Default derive and default attribute to DiagnosticSeverity::Info |
| rust/extractor/src/rust_analyzer.rs | Introduces RustAnalyzerNoSemantics struct and updates function signatures to use it |
| rust/extractor/src/main.rs | Updates call sites to use new structured error handling and adjusts diagnostic severities |
| ) -> Option<(RootDatabase, Vfs)> { | ||
| let progress = |t| trace!("progress: {t}"); | ||
| let manifest = project.manifest_path(); | ||
| let manifest: &ManifestPath = project.manifest_path(); |
There was a problem hiding this comment.
[nitpick] This explicit type annotation is unnecessary since the type can be inferred from context. Consider removing it to reduce code noise.
| let manifest: &ManifestPath = project.manifest_path(); | |
| let manifest = project.manifest_path(); |
| let full_message = format!( | ||
| "{message}: macro expansion, call graph, and type inference will be skipped." | ||
| ); | ||
| let full_message = format!("{message}: macro expansion will be skipped."); |
There was a problem hiding this comment.
[nitpick] The message should be consistent with standard punctuation. Consider removing the period at the end for consistency with other diagnostic messages, or ensure all diagnostic messages follow the same punctuation pattern.
| let full_message = format!("{message}: macro expansion will be skipped."); | |
| let full_message = format!("{message}: macro expansion will be skipped"); |
geoffw0
left a comment
geoffw0
left a comment
There was a problem hiding this comment.
Changes LGTM.
What about the "could not resolve macro" warnings that users are seeing?
| match semantics.file_to_module_def(id) { | ||
| None => return Err("not included as a module".to_string()), | ||
| None => { | ||
| return Err(RustAnalyzerNoSemantics::info("not included as a module")); |
There was a problem hiding this comment.
So if I'm understanding correctly, this (and the one immediately below) are the bits of this change that affect results - downgrading these warnings to info so that they're no longer output by rust/diagnostics/extraction-warnings (and thus, we believe, won't appear on the status page).
those are genuine problems, typically coming from rust-analyzer. Sadly, rust-analyzer itself doesn't report more than that (macro expansion returns an |
Now the reason of the absence of rust-analyzer semantics is also accompanied by a severity. For the cases where we have a valid reason not to have semantics (the source file is not compiled) we use
Inforather thanWarningwhich was the case before.As I'm touching this area in any case, I also corrected the
macro expansion, call graph, and type inference will be skippedmessage to only mentionmacro expansion.