From 07316967886aa8500b21837cf359cff45f655894 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Fri, 15 Sep 2023 06:58:40 -0700 Subject: [PATCH] Introduce `elp explain`, show links to Erlang Error Index Summary: Introduce a `elp explain` command which, given a diagnostic code, returns a link to the respective entry in the Erlang Error Index. Also enable links to the index from diagnostics. Later on we will probably want to show the actual content instead of just the link. Reviewed By: alanz Differential Revision: D49230473 fbshipit-source-id: 63b4d10eae3ed9f8ba31a52718db60be0e2be1e5 --- crates/elp/src/bin/args.rs | 15 +++++ crates/elp/src/bin/explain_cli.rs | 24 ++++++++ crates/elp/src/bin/main.rs | 33 +++++++++++ .../src/resources/test/explain_code.stdout | 1 + .../src/resources/test/explain_help.stdout | 5 ++ .../test/explain_unkwnown_code.stdout | 1 + crates/elp/src/resources/test/help.stdout | 1 + crates/ide/src/diagnostics.rs | 55 +++++++++++-------- 8 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 crates/elp/src/bin/explain_cli.rs create mode 100644 crates/elp/src/resources/test/explain_code.stdout create mode 100644 crates/elp/src/resources/test/explain_help.stdout create mode 100644 crates/elp/src/resources/test/explain_unkwnown_code.stdout diff --git a/crates/elp/src/bin/args.rs b/crates/elp/src/bin/args.rs index a98a778ecb..61ff45b654 100644 --- a/crates/elp/src/bin/args.rs +++ b/crates/elp/src/bin/args.rs @@ -246,6 +246,13 @@ pub struct Lint { pub ignore_apps: Vec, } +#[derive(Clone, Debug, Bpaf)] +pub struct Explain { + /// Error code to explain + #[bpaf(argument("CODE"))] + pub code: String, +} + #[derive(Clone, Debug, Bpaf)] pub struct Shell { /// Path to directory with project (defaults to `.`) @@ -269,6 +276,7 @@ pub enum Command { Lint(Lint), Version(Version), Shell(Shell), + Explain(Explain), Help(), } @@ -366,6 +374,12 @@ pub fn command() -> impl Parser { .command("shell") .help("Starts an interactive ELP shell"); + let explain = explain() + .map(Command::Explain) + .to_options() + .command("explain") + .help("Explain a diagnostic code"); + construct!([ eqwalize, eqwalize_all, @@ -381,6 +395,7 @@ pub fn command() -> impl Parser { version, shell, eqwalize_stats, + explain, ]) .fallback(Help()) } diff --git a/crates/elp/src/bin/explain_cli.rs b/crates/elp/src/bin/explain_cli.rs new file mode 100644 index 0000000000..3a3c785f4b --- /dev/null +++ b/crates/elp/src/bin/explain_cli.rs @@ -0,0 +1,24 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under both the MIT license found in the + * LICENSE-MIT file in the root directory of this source tree and the Apache + * License, Version 2.0 found in the LICENSE-APACHE file in the root directory + * of this source tree. + */ + +use anyhow::Result; +use elp::cli::Cli; +use elp_ide::diagnostics::DiagnosticCode; + +use crate::args::Explain; + +pub fn explain(args: &Explain, cli: &mut dyn Cli) -> Result<()> { + if let Some(code) = DiagnosticCode::maybe_from_string(&args.code) { + if let Some(uri) = DiagnosticCode::as_uri(&code) { + let label = code.as_label(); + return Ok(writeln!(cli, "{uri} ({label})")?); + } + } + Ok(writeln!(cli, "Unkwnown code: {}", args.code)?) +} diff --git a/crates/elp/src/bin/main.rs b/crates/elp/src/bin/main.rs index 4328133a8d..4c23b3637d 100644 --- a/crates/elp/src/bin/main.rs +++ b/crates/elp/src/bin/main.rs @@ -28,6 +28,7 @@ mod build_info_cli; mod elp_parse_cli; mod eqwalizer_cli; mod erlang_service_cli; +mod explain_cli; mod lint_cli; mod reporting; mod shell; @@ -86,6 +87,7 @@ fn try_main(cli: &mut dyn Cli, args: Args) -> Result<()> { let help = batteries::get_usage(args::args()); writeln!(cli, "{}", help)? } + args::Command::Explain(args) => explain_cli::explain(&args, cli)?, } log::logger().flush(); @@ -140,6 +142,7 @@ mod tests { use bpaf::Args; use elp::cli::Fake; + use elp_ide::diagnostics::BASE_URL; use expect_test::expect; use expect_test::expect_file; use expect_test::Expect; @@ -855,6 +858,36 @@ mod tests { expected.assert_eq(&stdout); } + #[test] + fn explain_help() { + let args = args::args() + .run_inner(Args::from(&["explain", "--help"])) + .unwrap_err(); + let expected = expect_file!["../resources/test/explain_help.stdout"]; + let stdout = args.unwrap_stdout(); + expected.assert_eq(&stdout); + } + + #[test] + fn explain_code() { + let args = args_vec!["explain", "--code", "W0005"]; + let (stdout, stderr, code) = elp(args); + let expected = expect_file!["../resources/test/explain_code.stdout"]; + expected.assert_eq(&stdout.strip_prefix(BASE_URL).unwrap()); + assert!(stderr.is_empty()); + assert_eq!(code, 0); + } + + #[test] + fn explain_unknown_code() { + let args = args_vec!["explain", "--code", "does_not_exist"]; + let (stdout, stderr, code) = elp(args); + let expected = expect_file!["../resources/test/explain_unkwnown_code.stdout"]; + expected.assert_eq(&stdout); + assert!(stderr.is_empty()); + assert_eq!(code, 0); + } + fn simple_snapshot( args: Vec, project: &str, diff --git a/crates/elp/src/resources/test/explain_code.stdout b/crates/elp/src/resources/test/explain_code.stdout new file mode 100644 index 0000000000..79c536ec92 --- /dev/null +++ b/crates/elp/src/resources/test/explain_code.stdout @@ -0,0 +1 @@ +/erlang-error-index/w/W0005 (mutable_variable_bug) diff --git a/crates/elp/src/resources/test/explain_help.stdout b/crates/elp/src/resources/test/explain_help.stdout new file mode 100644 index 0000000000..d4bc6e7086 --- /dev/null +++ b/crates/elp/src/resources/test/explain_help.stdout @@ -0,0 +1,5 @@ +Usage: --code CODE + +Available options: + --code Error code to explain + -h, --help Prints help information diff --git a/crates/elp/src/resources/test/explain_unkwnown_code.stdout b/crates/elp/src/resources/test/explain_unkwnown_code.stdout new file mode 100644 index 0000000000..a6ecf28309 --- /dev/null +++ b/crates/elp/src/resources/test/explain_unkwnown_code.stdout @@ -0,0 +1 @@ +Unkwnown code: does_not_exist diff --git a/crates/elp/src/resources/test/help.stdout b/crates/elp/src/resources/test/help.stdout index a0f9907beb..0fc5b16c0f 100644 --- a/crates/elp/src/resources/test/help.stdout +++ b/crates/elp/src/resources/test/help.stdout @@ -20,3 +20,4 @@ Available commands: version Print version shell Starts an interactive ELP shell eqwalize-stats Return statistics about code quality for eqWAlizer + explain Explain a diagnostic code diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 6026a558e1..e7da331997 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -97,6 +97,9 @@ pub struct Diagnostic { pub uri: Option, } +// @fb-only: pub const BASE_URL: &str = "https://www.internalfb.com/intern/staticdocs/elp/docs"; +// @oss-only pub const BASE_URL: &str = "https://whatsapp.github.io/erlang-language-platform/docs"; + impl Diagnostic { pub(crate) fn new( code: DiagnosticCode, @@ -105,14 +108,14 @@ impl Diagnostic { ) -> Diagnostic { let message = message.into(); Diagnostic { - code, + code: code.clone(), message, range, severity: Severity::Error, categories: HashSet::new(), fixes: None, related_info: None, - uri: None, + uri: code.as_uri(), } } @@ -406,6 +409,30 @@ impl DiagnosticCode { } } + pub fn namespace(code: &String) -> Option { + let first = code.to_string().chars().next()?; + Some(first.to_lowercase().to_string()) + } + + pub fn as_namespace(&self) -> Option { + match self { + DiagnosticCode::DefaultCodeForEnumIter => None, + DiagnosticCode::AdHoc(_) => None, + // @fb-only: DiagnosticCode::MetaOnly(_) => None, + DiagnosticCode::ErlangService(code) => Self::namespace(code), + _ => Self::namespace(&self.as_code()), + } + } + + pub fn as_uri(&self) -> Option { + let namespace = self.as_namespace()?; + let code = self.as_code(); + Some(format!( + "{}/erlang-error-index/{namespace}/{code}", + BASE_URL.to_string() + )) + } + /// Check if the diagnostic label is for an AdHoc one. fn is_adhoc(s: &str) -> Option { // Looking for something like "ad-hoc: ad-hoc-title-1" @@ -707,16 +734,9 @@ fn no_module_definition_diagnostic( parse: &Parse, ) { let mut report = |range| { - diagnostics.push(Diagnostic { - message: "no module definition".to_string(), - range, - severity: Severity::Error, - categories: HashSet::new(), - fixes: None, - related_info: None, - code: DiagnosticCode::MissingModule, - uri: None, - }); + let diagnostic = + Diagnostic::new(DiagnosticCode::MissingModule, "no module definition", range); + diagnostics.push(diagnostic); }; for form in parse.tree().forms() { match form { @@ -883,16 +903,7 @@ fn non_whitespace_prev_token(node: &SyntaxNode) -> Option { fn make_missing_diagnostic(range: TextRange, item: &'static str, code: String) -> Diagnostic { let message = format!("Missing '{}'", item); - Diagnostic { - message, - range, - severity: Severity::Warning, - categories: HashSet::new(), - fixes: None, - related_info: None, - code: DiagnosticCode::Missing(code), - uri: None, - } + Diagnostic::new(DiagnosticCode::Missing(code), message, range).severity(Severity::Warning) } pub fn erlang_service_diagnostics(