3/n: includes: convert HIR diagnostics into ide Diagnostic W0057

Summary:
Create a new diagnostic W0057 (hir_unresolved_macro) to report macros not resolved during HIR lowering.

For a properly configured buck2 project this simply shadows the existing Erlang service `E1507` / `E1508` diagnostics, but for an incorrectly configured one they show up alone.

The next diff filters them out if they are already reported.

It also ensures it does not report on usages of `?FUNCTION_NAME` or ?FUNCTION_ARITY` in the replacement part of a `-define()` attribute.

Reviewed By: TD5

Differential Revision: D86963336

fbshipit-source-id: 08f8d1b6c58dde6ee0a106f9151b237709dc56e0

Author: alanz

crates/elp/src/resources/test/buck_tests_2/resolves_generated_includes.stdout

@@ -9,12 +9,10 @@ Diagnostics reported in 5 modules:
       0:8-0:18::[WeakWarning] [W0046] The module is not documented.
   eqwalizer: 1
       1:8-1:17::[WeakWarning] [W0046] The module is not documented.
-  top_includer: 5
-      9:0-9:3::[WeakWarning] [W0040] The function is non-trivial, exported, but not documented.
+  top_includer: 3
+      12:4-12:10::[Warning] [W0057] undefined macro 'THIRD/2'
       0:8-0:20::[WeakWarning] [W0046] The module is not documented.
       12:4-12:10::[Error] [E1508] undefined macro 'THIRD/2'
-      4:9-4:14::[Error] [L1227] function foo/0 undefined
-      8:6-8:9::[Error] [L1308] spec for undefined function foo/0
   wa_buck2_module_search: 6
       19:0-19:18::[WeakWarning] [W0040] The function is non-trivial, exported, but not documented.
       2:8-2:30::[WeakWarning] [W0046] The module is not documented.

crates/elp/src/resources/test/include_lib_dependency_test/include_lib_non_dependency_fails.stdout

@@ -1,8 +1,9 @@
 module specified: main_app
 Diagnostics reported in 1 modules:
-  main_app: 5
+  main_app: 6
       4:13-4:55::[Error] [E1516] can't find include lib "external_app/include/external_header.hrl"
       11:0-11:42::[Warning] [W0020] Unused file: stdlib/include/assert.hrl
       14:9-14:21::[Hint] [W0037] Unspecific include.
       16:9-16:24::[Error] [L1227] function test_function/0 undefined
+      23:4-23:19::[Warning] [W0057] undefined macro 'EXTERNAL_MACRO'
       23:4-23:19::[Error] [E1507] undefined macro 'EXTERNAL_MACRO'

crates/elp/src/resources/test/include_lib_dependency_test/include_lib_non_dependency_rebar.stdout

@@ -1,7 +1,8 @@
 module specified: main_app
 Diagnostics reported in 1 modules:
-  main_app: 4
+  main_app: 5
       11:0-11:42::[Warning] [W0020] Unused file: stdlib/include/assert.hrl
       14:9-14:21::[Error] [E1516] can't find include file "assert.hrl"
       16:9-16:24::[Error] [L1227] function test_function/0 undefined
+      20:4-20:25::[Warning] [W0057] undefined macro 'normalDepAssertEqual/2'
       20:4-20:25::[Error] [E1508] undefined macro 'normalDepAssertEqual/2'

crates/hir/src/body/lower.rs

@@ -118,6 +118,8 @@ pub struct Ctx<'a> {
     // This means we need to lower a Var specially when processing a SSR
     // template, where if it has a prefix of `_@` it is a placeholder.
     in_ssr: bool,
+    // Track when we're lowering the RHS of a define preprocessor directive
+    in_macro_rhs: bool,
     // Diagnostics collected during body lowering
     diagnostics: Vec<BodyDiagnostic>,
 }
@@ -156,6 +158,7 @@ impl<'a> Ctx<'a> {
             starting_stack_size: 1,
             macro_source_map: FxHashMap::default(),
             in_ssr: false,
+            in_macro_rhs: false,
             diagnostics: Vec::new(),
         }
     }
@@ -407,7 +410,9 @@ impl<'a> Ctx<'a> {
         let replacement = define.replacement();
         match replacement {
             Some(MacroDefReplacement::Expr(expr)) => {
+                self.in_macro_rhs = true;
                 let expr = self.lower_expr(&expr);
+                self.in_macro_rhs = false;
                 let (body, source_map) = self.finish();
                 (DefineBody { body, expr }, source_map)
             }
@@ -3023,6 +3028,17 @@ impl<'a> Ctx<'a> {
     }
 
     fn record_unresolved_macro(&mut self, call: &ast::MacroCallExpr) {
+        // If we're in a macro RHS and this is ?FUNCTION_NAME or ?FUNCTION_ARITY, skip the diagnostic
+        if self.in_macro_rhs {
+            let macro_name = self.macro_call_name(call.name());
+            if let MacroCallName::Var(var) = macro_name {
+                let name_str = var.as_string(self.db.upcast());
+                if name_str == "FUNCTION_NAME" || name_str == "FUNCTION_ARITY" {
+                    return;
+                }
+            }
+        }
+
         let source = InFileAstPtr::new(self.curr_file_id(), AstPtr::new(call).cast().unwrap());
         self.diagnostics.push(BodyDiagnostic {
             source,

crates/ide/src/diagnostics.rs

@@ -67,7 +67,9 @@ use elp_text_edit::TextEdit;
 use elp_types_db::TypedSemantic;
 use fxhash::FxHashMap;
 use fxhash::FxHashSet;
+use hir::FormIdx;
 use hir::InFile;
+use hir::PPDirective;
 use hir::Semantic;
 use hir::db::DefDatabase;
 use hir::fold::MacroStrategy;
@@ -1400,6 +1402,7 @@ pub fn native_diagnostics(
         }
 
         res.append(&mut form_missing_separator_diagnostics(&parse));
+        res.extend(get_hir_diagnostics(db, file_id));
 
         adhoc_semantic_diagnostics
             .iter()
@@ -1743,6 +1746,143 @@ pub fn filter_diagnostics(diagnostics: Vec<Diagnostic>, code: DiagnosticCode) ->
     diagnostics.into_iter().filter(|d| d.code == code).collect()
 }
 
+/// Retrieve all BodyDiagnostic values from the BodySourceMaps from lowering
+/// all the bodies for a given FileId.
+///
+/// This function iterates through all forms in the file and collects diagnostics
+/// from the BodySourceMaps associated with each form's body.
+pub fn collect_body_diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<hir::BodyDiagnostic> {
+    let sema = Semantic::new(db);
+    let form_list = sema.form_list(file_id);
+    let mut diagnostics = Vec::new();
+
+    for form_idx in form_list.forms().iter() {
+        let body_map = match form_idx {
+            FormIdx::FunctionClause(function_clause_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .function_clause_body_with_source(InFile::new(file_id, *function_clause_id));
+                Some(body_map)
+            }
+            FormIdx::TypeAlias(type_alias_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .type_body_with_source(InFile::new(file_id, *type_alias_id));
+                Some(body_map)
+            }
+            FormIdx::Spec(spec_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .spec_body_with_source(InFile::new(file_id, *spec_id));
+                Some(body_map)
+            }
+            FormIdx::Callback(callback_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .callback_body_with_source(InFile::new(file_id, *callback_id));
+                Some(body_map)
+            }
+            FormIdx::Record(record_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .record_body_with_source(InFile::new(file_id, *record_id));
+                Some(body_map)
+            }
+            FormIdx::Attribute(attribute_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .attribute_body_with_source(InFile::new(file_id, *attribute_id));
+                Some(body_map)
+            }
+            FormIdx::CompileOption(compile_option_id) => {
+                let (_, body_map) = sema
+                    .db
+                    .compile_body_with_source(InFile::new(file_id, *compile_option_id));
+                Some(body_map)
+            }
+            FormIdx::PPDirective(idx) => {
+                if let PPDirective::Define(define_id) = &form_list[*idx] {
+                    let (_, body_map) = sema
+                        .db
+                        .define_body_with_source(InFile::new(file_id, *define_id));
+                    Some(body_map)
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        };
+
+        if let Some(body_map) = body_map {
+            diagnostics.extend_from_slice(body_map.diagnostics());
+        }
+    }
+
+    diagnostics
+}
+
+/// Convert HIR body diagnostics to IDE Diagnostics.
+/// This function takes the diagnostics collected during HIR lowering and converts
+/// them to the IDE Diagnostic format for display to users.
+pub fn get_hir_diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic> {
+    let body_diagnostics = collect_body_diagnostics(db, file_id);
+
+    body_diagnostics
+        .into_iter()
+        .filter_map(|body_diag| {
+            let full_range = body_diag.source.range();
+
+            // Only include diagnostics for the requested file
+            if full_range.file_id != file_id {
+                return None;
+            }
+
+            let (code, message, range) = match body_diag.kind {
+                hir::BodyDiagnosticKind::UnresolvedMacro => {
+                    // Get the macro call AST node to extract name and arity
+                    let macro_call = body_diag.source.to_ast(db);
+                    let macro_name = macro_call
+                        .name()
+                        .map(|name| name.to_string())
+                        .unwrap_or_else(|| "?".to_string());
+
+                    let message = match macro_call.arity() {
+                        Some(arity) => format!("undefined macro '{}/{}'", macro_name, arity),
+                        None => format!("undefined macro '{}'", macro_name),
+                    };
+
+                    // For macros with arguments, only highlight the name part, not the full call
+                    let range = macro_call
+                        .name()
+                        .map(|name| {
+                            // Get the syntax range of just the macro name
+                            let name_range = name.syntax().text_range();
+                            // Include the '?' prefix by extending one character to the left
+                            if name_range.start() > 0.into() {
+                                TextRange::new(
+                                    name_range.start() - TextSize::from(1),
+                                    name_range.end(),
+                                )
+                            } else {
+                                name_range
+                            }
+                        })
+                        .unwrap_or(full_range.range);
+
+                    (DiagnosticCode::HirUnresolvedMacro, message, range)
+                }
+            };
+
+            Some(
+                Diagnostic::new(code, message, range)
+                    // We set the severity to Warning for now, until we have cleaned
+                    // up the code base from this diagnostic
+                    .with_severity(Severity::Warning),
+            )
+        })
+        .collect()
+}
+
 fn no_module_definition_diagnostic(
     diagnostics: &mut Vec<Diagnostic>,
     parse: &Parse<ast::SourceFile>,
@@ -3750,4 +3890,22 @@ baz(1)->4.
             "#,
         );
     }
+
+    #[test]
+    fn no_unused_macro_in_macro_rhs_for_function_name() {
+        let config = DiagnosticsConfig::default()
+            .set_experimental(true)
+            .disable(DiagnosticCode::UnspecificInclude)
+            .disable(DiagnosticCode::BinaryStringToSigil);
+        check_diagnostics_with_config(
+            config,
+            r#"
+            //- /my_app/src/a_file.erl
+              -module(a_file).
+              -define(A_MACRO, ?FUNCTION_NAME).
+              foo() -> ?A_MACRO.
+
+            "#,
+        );
+    }
 }

crates/ide/src/diagnostics/unused_macro.rs

@@ -123,7 +123,9 @@ mod tests {
 
     #[track_caller]
     pub(crate) fn check_diagnostics(fixture: &str) {
-        let config = DiagnosticsConfig::default().disable(DiagnosticCode::UndefinedFunction);
+        let config = DiagnosticsConfig::default()
+            .disable(DiagnosticCode::UndefinedFunction)
+            .disable(DiagnosticCode::HirUnresolvedMacro);
         check_diagnostics_with_config(config, fixture)
     }
 

crates/ide/src/tests.rs

@@ -376,7 +376,8 @@ pub(crate) fn check_diagnostics(fixture: &str) {
     let config = DiagnosticsConfig::default()
         .set_experimental(true)
         .disable(DiagnosticCode::UnspecificInclude)
-        .disable(DiagnosticCode::BinaryStringToSigil);
+        .disable(DiagnosticCode::BinaryStringToSigil)
+        .disable(DiagnosticCode::HirUnresolvedMacro);
     check_diagnostics_with_config(config, fixture)
 }
 
@@ -524,6 +525,7 @@ pub fn check_no_parse_errors(analysis: &Analysis, file_id: FileId) {
     let config = DiagnosticsConfig::default()
         .disable(DiagnosticCode::UnspecificInclude)
         .disable(DiagnosticCode::UndefinedFunction)
+        .disable(DiagnosticCode::HirUnresolvedMacro)
         .disable(DiagnosticCode::NoCatch);
     check_no_parse_errors_with_config(analysis, file_id, &config, &vec![]);
 }

crates/ide_db/src/diagnostic_code.rs

@@ -90,6 +90,7 @@ pub enum DiagnosticCode {
     NoNoWarnSuppressions,
     CouldBeAStringLiteral,
     ListsReverseAppend,
+    HirUnresolvedMacro,
 
     // Wrapper for erlang service diagnostic codes
     ErlangService(String),
@@ -250,6 +251,7 @@ impl DiagnosticCode {
             DiagnosticCode::NoNoWarnSuppressions => "W0054".to_string(),
             DiagnosticCode::CouldBeAStringLiteral => "W0055".to_string(),
             DiagnosticCode::ListsReverseAppend => "W0056".to_string(),
+            DiagnosticCode::HirUnresolvedMacro => "W0057".to_string(),
             DiagnosticCode::ErlangService(c) => c.to_string(),
             DiagnosticCode::Eqwalizer(c) => format!("eqwalizer: {c}"),
             DiagnosticCode::AdHoc(c) => format!("ad-hoc: {c}"),
@@ -347,6 +349,7 @@ impl DiagnosticCode {
             DiagnosticCode::NoNoWarnSuppressions => "no_nowarn_suppressions".to_string(),
             DiagnosticCode::CouldBeAStringLiteral => "could_be_a_binary_string_literal".to_string(),
             DiagnosticCode::ListsReverseAppend => "lists_reverse_append".to_string(),
+            DiagnosticCode::HirUnresolvedMacro => "hir_unresolved_macro".to_string(),
 
             DiagnosticCode::ErlangService(c) => c.to_string(),
             DiagnosticCode::Eqwalizer(c) => c.to_string(),
@@ -524,6 +527,7 @@ impl DiagnosticCode {
             DiagnosticCode::NoErrorLogger => false,
             DiagnosticCode::NoNoWarnSuppressions => false,
             DiagnosticCode::ListsReverseAppend => false,
+            DiagnosticCode::HirUnresolvedMacro => false,
 
             DiagnosticCode::BinaryStringToSigil => false,
             DiagnosticCode::ErlangService(_) => false,

test_projects/buck_tests_2/check_include/src/top_includer.erl

@@ -2,13 +2,13 @@
 
 -compile(warn_missing_spec_all).
 
--export([foo/0]).
-
 -include_lib("check_include_separate_1/include/top_includer.hrl").
 
--spec foo() -> ok.
+-define(A_MACRO, ?FUNCTION_NAME).
+
 foo() ->
     ?FIRST,
+    ?A_MACRO
     ?SECOND,
     ?THIRD(41,34),
     ?FUNCTION_NAME.