combine related errors for eqwalizer cli reporting too

Summary:
The `DiagnosticsCollection` has functionality to combine related errors, so that if a function has a syntax error, the cascade of undefined functions from that reported by the erlang service compiler is collapsed, as they get added as related diagnostics to the syntax error.

Route the erlang service diagnostics reported by eqwalizer cli through this too.

Reviewed By: robertoaloi

Differential Revision: D56818059

fbshipit-source-id: 674d2622a12abf4a2d52339f0ec7fe8bb3c68791

crates/elp/src/bin/elp_parse_cli.rs

@@ -26,6 +26,7 @@ use elp_eqwalizer::Mode;
 use elp_ide::diagnostics;
 use elp_ide::diagnostics::DiagnosticsConfig;
 use elp_ide::diagnostics::LabeledDiagnostics;
+use elp_ide::diagnostics::RemoveElpReported;
 use elp_ide::diagnostics_collection::DiagnosticCollection;
 use elp_ide::elp_ide_db::elp_base_db::AbsPath;
 use elp_ide::elp_ide_db::elp_base_db::FileId;
@@ -305,7 +306,8 @@ fn do_parse_one(
 ) -> Result<Option<ParseResult>> {
     let url = file_id_to_url(vfs, file_id);
     let native = db.native_diagnostics(config, file_id)?;
-    let erlang_service_diagnostics = db.erlang_service_diagnostics(file_id, config)?;
+    let erlang_service_diagnostics =
+        db.erlang_service_diagnostics(file_id, config, RemoveElpReported::Yes)?;
     let line_index = db.line_index(file_id)?;
 
     // Should we return the included file diagnostics as well? Not doing so now.

crates/elp/src/bin/eqwalizer_cli.rs

@@ -19,12 +19,17 @@ use elp::cli::Cli;
 use elp::convert;
 use elp_eqwalizer::Mode;
 use elp_ide::diagnostics::Diagnostic;
+use elp_ide::diagnostics::DiagnosticsConfig;
+use elp_ide::diagnostics::LabeledDiagnostics;
+use elp_ide::diagnostics::RemoveElpReported;
+use elp_ide::diagnostics_collection::DiagnosticCollection;
 use elp_ide::elp_ide_db::elp_base_db::FileId;
 use elp_ide::elp_ide_db::elp_base_db::IncludeOtp;
 use elp_ide::elp_ide_db::elp_base_db::ModuleName;
 use elp_ide::elp_ide_db::elp_base_db::VfsPath;
 use elp_ide::elp_ide_db::EqwalizerDiagnostics;
 use elp_ide::elp_ide_db::LineIndex;
+use elp_ide::elp_ide_db::LineIndexDatabase;
 use elp_ide::erlang_service;
 use elp_ide::Analysis;
 use elp_project_model::AppName;
@@ -40,8 +45,8 @@ use crate::args::EqwalizeAll;
 use crate::args::EqwalizeApp;
 use crate::args::EqwalizeStats;
 use crate::args::EqwalizeTarget;
-use crate::erlang_service_cli;
 use crate::reporting;
+use crate::reporting::ParseDiagnostic;
 use crate::reporting::Reporter;
 
 struct EqwalizerInternalArgs<'a> {
@@ -361,14 +366,45 @@ fn eqwalize(
         }
         EqwalizerDiagnostics::NoAst { module } => {
             if let Some(file_id) = analysis.module_file_id(loaded.project_id, &module)? {
-                let parse_diagnostics = erlang_service_cli::do_parse_one(
-                    analysis,
-                    None,
-                    file_id,
-                    erlang_service::Format::OffsetEtf,
-                )?;
+                let config = DiagnosticsConfig::default();
+                let erlang_service_diagnostics =
+                    analysis.erlang_service_diagnostics(file_id, &config, RemoveElpReported::No)?;
+                let erlang_service = erlang_service_diagnostics
+                    .into_iter()
+                    .find(|(f, _diags)| f == &file_id)
+                    .map(|(_, diags)| diags)
+                    .unwrap_or(LabeledDiagnostics::default());
+                let mut diagnostics = DiagnosticCollection::default();
+                diagnostics.set_erlang_service(file_id, erlang_service);
+                // `diagnostics_for` will also combine related diagnostics
+                let diags = diagnostics.diagnostics_for(file_id);
+                let mut parse_diagnostics: Vec<ParseDiagnostic> = Vec::default();
+                let line_index = analysis.with_db(|db| db.file_line_index(file_id))?;
+                for diag in diags {
+                    let vfs_path = loaded.vfs.file_path(file_id);
+                    let analysis = loaded.analysis();
+                    let root_path = &analysis
+                        .project_data(file_id)
+                        .unwrap_or_else(|_err| panic!("could not find project data"))
+                        .unwrap_or_else(|| panic!("could not find project data"))
+                        .root_dir;
+                    let relative_path = reporting::get_relative_path(root_path, &vfs_path);
+
+                    let line_num = convert::position(&line_index, diag.range.start()).line + 1;
+                    parse_diagnostics.push(ParseDiagnostic {
+                        file_id,
+                        relative_path: relative_path.to_path_buf(),
+                        line_num,
+                        msg: diag.message,
+                        range: Some(diag.range),
+                    });
+                }
                 // The cached parse errors must be non-empty otherwise we wouldn't have `NoAst`
-                assert!(!parse_diagnostics.is_empty());
+                assert!(
+                    !parse_diagnostics.is_empty(),
+                    "Expecting erlang service diagnostics, but none found, for '{}'",
+                    module
+                );
                 let parse_diagnostics: Vec<_> = parse_diagnostics
                     .into_iter()
                     .sorted_by(|d1, d2| {

crates/elp/src/bin/main.rs

@@ -541,6 +541,18 @@ mod tests {
         );
     }
 
+    #[test_case(false ; "rebar")]
+    #[test_case(true  ; "buck")]
+    fn eqwalize_reports_cascading_syntax_errors(buck: bool) {
+        simple_snapshot(
+            args_vec!["eqwalize", "parse_error_a_cascade",],
+            "parse_error",
+            expect_file!("../resources/test/standard/eqwalize_all_parse_error_cascade.pretty"),
+            buck,
+            None,
+        );
+    }
+
     #[test_case(false ; "rebar")]
     #[test_case(true  ; "buck")]
     fn parse_all_diagnostics1(buck: bool) {

crates/elp/src/resources/test/standard/eqwalize_all_parse_error_cascade.pretty

@@ -0,0 +1,6 @@
+error: parse_error
+   ┌─ parse_error_a/src/parse_error_a_cascade.erl:11:5
+   │
+11 │     .
+   │     ^ syntax error before: '.'
+

crates/elp/src/snapshot.rs

@@ -18,6 +18,7 @@ use elp_ide::diagnostics;
 use elp_ide::diagnostics::DiagnosticsConfig;
 use elp_ide::diagnostics::LabeledDiagnostics;
 use elp_ide::diagnostics::LintsFromConfig;
+use elp_ide::diagnostics::RemoveElpReported;
 use elp_ide::diagnostics_collection::DiagnosticCollection;
 use elp_ide::elp_ide_db::elp_base_db::AbsPathBuf;
 use elp_ide::elp_ide_db::elp_base_db::FileId;
@@ -324,7 +325,7 @@ impl Snapshot {
 
         let diags = &*self
             .analysis
-            .erlang_service_diagnostics(file_id, config)
+            .erlang_service_diagnostics(file_id, config, RemoveElpReported::Yes)
             .ok()?;
 
         Some(

crates/ide/src/diagnostics.rs

@@ -880,10 +880,17 @@ pub(crate) fn make_unexpected_diagnostic(
         .with_severity(Severity::Warning)
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub enum RemoveElpReported {
+    Yes,
+    No,
+}
+
 pub fn erlang_service_diagnostics(
     db: &RootDatabase,
     file_id: FileId,
     config: &DiagnosticsConfig,
+    remove_elp_reported: RemoveElpReported,
 ) -> Vec<(FileId, LabeledDiagnostics)> {
     if config.include_generated || !db.is_generated(file_id) {
         // Use the same format as eqwalizer, so we can re-use the salsa cache entry
@@ -943,10 +950,14 @@ pub fn erlang_service_diagnostics(
 
         // Remove diagnostics kinds already reported by ELP
         let file_kind = db.file_kind(file_id);
-        let diags: Vec<(FileId, Diagnostic)> = diags
-            .into_iter()
-            .filter(|(_, d)| !is_implemented_in_elp(&d.code, file_kind))
-            .collect();
+        let diags: Vec<(FileId, Diagnostic)> = if remove_elp_reported == RemoveElpReported::Yes {
+            diags
+                .into_iter()
+                .filter(|(_, d)| !is_implemented_in_elp(&d.code, file_kind))
+                .collect()
+        } else {
+            diags
+        };
         let diags = if diags.is_empty() {
             // If there are no diagnostics reported, return an empty list
             // against the `file_id` to clear the list of diagnostics for
@@ -1951,6 +1962,28 @@ baz(1)->4.
         );
     }
 
+    #[test]
+    fn group_related_diagnostics_wtf() {
+        // Note: the cascade fails because ELP errors out to such an
+        // extent we do not even see bar() as a function.
+        check_diagnostics(
+            r#"
+             //- erlang_service
+             //- native
+             //- /src/a_mod.erl app:app_a
+             -module(a_mod).
+             -export([foo/0]).
+
+             foo() ->
+                  bar().
+             %%   ^^^^^ error: function bar/0 undefined
+
+             bar() -> !!! %% syntax error
+             %%<^^^^^^^^^ error: Syntax Error
+            "#,
+        );
+    }
+
     #[test]
     fn check_specific_fix_works() {
         check_specific_fix(

crates/ide/src/fixture.rs

@@ -21,6 +21,7 @@ use elp_project_model::test_fixture::DiagnosticsEnabled;
 use parking_lot::MutexGuard;
 
 use crate::diagnostics::DiagnosticsConfig;
+use crate::diagnostics::RemoveElpReported;
 use crate::diagnostics_collection::DiagnosticCollection;
 use crate::Analysis;
 use crate::AnalysisHost;
@@ -156,7 +157,7 @@ pub fn diagnostics_for(
     }
     if *use_erlang_service {
         let erlang_service_diagnostics = analysis
-            .erlang_service_diagnostics(file_id, config)
+            .erlang_service_diagnostics(file_id, config, RemoveElpReported::Yes)
             .unwrap();
         for (file_id, diags) in erlang_service_diagnostics {
             diagnostics.set_erlang_service(file_id, diags)

crates/ide/src/lib.rs

@@ -15,6 +15,7 @@ use call_hierarchy::CallItem;
 use diagnostics::Diagnostic;
 use diagnostics::DiagnosticsConfig;
 use diagnostics::LabeledDiagnostics;
+use diagnostics::RemoveElpReported;
 use diagnostics_collection::DiagnosticCollection;
 use elp_ide_assists::Assist;
 use elp_ide_assists::AssistConfig;
@@ -328,8 +329,11 @@ impl Analysis {
         &self,
         file_id: FileId,
         config: &DiagnosticsConfig,
+        remove_elp_reported: RemoveElpReported,
     ) -> Cancellable<Vec<(FileId, LabeledDiagnostics)>> {
-        self.with_db(|db| diagnostics::erlang_service_diagnostics(db, file_id, config))
+        self.with_db(|db| {
+            diagnostics::erlang_service_diagnostics(db, file_id, config, remove_elp_reported)
+        })
     }
 
     /// Low-level access to eqwalizer

test_projects/parse_error/parse_error_a/src/parse_error_a_cascade.erl

@@ -0,0 +1,11 @@
+-module(parse_error_a_cascade).
+
+-export([foo/0]).
+
+foo() ->
+     bar().
+
+bar() ->
+
+    %% syntax error
+    .