Report errors reading 'elp lint' config file

Summary: Do not silently swallow errors in the `.elp_lint.toml` configuration file.

Reviewed By: ir-regular

Differential Revision: D48831490

fbshipit-source-id: 78e4cbbfc909f5cc8a826aa68182af8f2cb7d211

crates/elp/src/bin/lint_cli.rs

@@ -172,7 +172,7 @@ pub fn do_codemod(cli: &mut dyn Cli, loaded: &mut LoadResult, args: &Lint) -> Re
             ..
         } => {
             let cfg_from_file = if *read_config {
-                config_file(project)
+                config_file(project)?
             } else {
                 LintConfig::default()
             };
@@ -352,7 +352,7 @@ pub fn do_codemod(cli: &mut dyn Cli, loaded: &mut LoadResult, args: &Lint) -> Re
 
 const LINT_CONFIG_FILE: &str = ".elp_lint.toml";
 
-fn config_file(project: &PathBuf) -> LintConfig {
+fn config_file(project: &PathBuf) -> Result<LintConfig> {
     let mut potential_path = Some(project.as_path());
     while let Some(path) = potential_path {
         let file_path = path.join(LINT_CONFIG_FILE);
@@ -361,15 +361,16 @@ fn config_file(project: &PathBuf) -> LintConfig {
             potential_path = path.parent();
             continue;
         } else {
-            if let Ok(content) = fs::read_to_string(file_path) {
-                if let Ok(config) = toml::from_str::<LintConfig>(&content) {
-                    return config;
+            if let Ok(content) = fs::read_to_string(file_path.clone()) {
+                match toml::from_str::<LintConfig>(&content) {
+                    Ok(config) => return Ok(config),
+                    Err(err) => bail!("failed to read {:?}:{err}", file_path),
                 }
             }
         }
         break;
     }
-    LintConfig::default()
+    Ok(LintConfig::default())
 }
 
 fn print_diagnostic(

crates/elp/src/bin/main.rs

@@ -140,7 +140,9 @@ mod tests {
 
     use bpaf::Args;
     use elp::cli::Fake;
+    use expect_test::expect;
     use expect_test::expect_file;
+    use expect_test::Expect;
     use expect_test::ExpectFile;
     use tempfile::Builder;
     use tempfile::TempDir;
@@ -582,6 +584,30 @@ mod tests {
         .expect("bad test");
     }
 
+    #[test_case(false ; "rebar")]
+    #[test_case(true  ; "buck")]
+    fn lint_config_file_parse_error(buck: bool) {
+        let tmp_dir = TempDir::new().expect("Could not create temporary directory");
+        let tmp_path = tmp_dir.path();
+        fs::create_dir_all(tmp_path).expect("Could not create temporary directory path");
+        check_lint_fix_stderr(
+            args_vec!["lint", "--experimental", "--read-config"],
+            "linter_bad_config",
+            expect_file!("../resources/test/linter/parse_elp_lint_bad_config_output.stdout"),
+            101,
+            buck,
+            None,
+            &tmp_path,
+            Path::new("../resources/test/lint/lint_recursive"),
+            &[],
+            false,
+            Some(expect![[r#"
+                failed to read "../../test_projects/linter_bad_config/.elp_lint.toml":expected a right bracket, found an identifier at line 6 column 4
+            "#]]),
+        )
+        .expect("bad test");
+    }
+
     #[test_case(false ; "rebar")]
     #[test_case(true  ; "buck")]
     fn lint_no_diagnostics_enabled(buck: bool) {
@@ -857,6 +883,34 @@ mod tests {
         expected_dir: &Path,
         files: &[(&str, &str)],
         backup_files: bool,
+    ) -> Result<()> {
+        check_lint_fix_stderr(
+            args,
+            project,
+            expected,
+            expected_code,
+            buck,
+            file,
+            actual_dir,
+            expected_dir,
+            files,
+            backup_files,
+            None,
+        )
+    }
+
+    fn check_lint_fix_stderr(
+        args: Vec<OsString>,
+        project: &str,
+        expected: ExpectFile,
+        expected_code: i32,
+        buck: bool,
+        file: Option<&str>,
+        actual_dir: &Path,
+        expected_dir: &Path,
+        files: &[(&str, &str)],
+        backup_files: bool,
+        expected_stderr: Option<Expect>,
     ) -> Result<()> {
         if !buck || cfg!(feature = "buck") {
             let (mut args, path) = add_project(args, project, file);
@@ -876,6 +930,9 @@ mod tests {
                 "Expected exit code {expected_code}, got: {}\nstdout:\n{}\nstderr:\n{}",
                 code, stdout, stderr
             );
+            if let Some(expected_stderr) = expected_stderr {
+                expected_stderr.assert_eq(&stderr);
+            }
             assert_normalised_file(expected, &stdout, path);
             for (expected_file, file) in files {
                 let expected = expect_file!(expected_dir.join(expected_file));

test_projects/linter_bad_config/.elp.toml

@@ -0,0 +1,5 @@
+[buck]
+enabled = true
+build_deps = false
+included_targets = [ "fbcode//whatsapp/elp/test_projects/linter/..." ]
+source_root = "whatsapp/elp/test_projects/linter"

test_projects/linter_bad_config/.elp_lint.toml

@@ -0,0 +1,8 @@
+# @lint-ignore-every TOMLSYNTAX
+enabled_lints =[
+   'L1268',
+   'W0011',
+   'P1700'
+   syntax error
+   ]
+disabled_lints = ['W0010']

test_projects/linter_bad_config/.gitignore

@@ -0,0 +1,4 @@
+.idea
+build_info.etf
+/_build/
+rebar.lock

test_projects/linter_bad_config/app_a/include/app_a.hrl

@@ -0,0 +1 @@
+-typing([eqwalizer]).

test_projects/linter_bad_config/app_a/src/app_a.app.src

@@ -0,0 +1,3 @@
+{application, app_a,
+  [{description, "example app A"}, {vsn, "inplace"}, {applications, [kernel, stdlib]}]
+}.

test_projects/linter_bad_config/app_a/src/app_a.erl

@@ -0,0 +1 @@
+-module(app_a).

test_projects/linter_bad_config/linter/.elp.toml

@@ -0,0 +1,5 @@
+[buck]
+enabled = true
+build_deps = false
+included_targets = [ "fbcode//whatsapp/elp/test_projects/linter/..." ]
+source_root = "whatsapp/elp/test_projects/linter"

test_projects/linter_bad_config/linter/.elp_lint.toml

@@ -0,0 +1,2 @@
+enabled_lints =['L1268', 'W0011', 'P1700']
+disabled_lints = ['W0010']

test_projects/linter_bad_config/linter/.gitignore

@@ -0,0 +1,4 @@
+.idea
+build_info.etf
+/_build/
+rebar.lock

test_projects/linter_bad_config/linter/app_a/include/app_a.hrl

@@ -0,0 +1 @@
+-typing([eqwalizer]).

test_projects/linter_bad_config/linter/app_a/src/app_a.app.src

@@ -0,0 +1,3 @@
+{application, app_a,
+  [{description, "example app A"}, {vsn, "inplace"}, {applications, [kernel, stdlib]}]
+}.

test_projects/linter_bad_config/linter/app_a/src/app_a.erl

@@ -0,0 +1,10 @@
+-module(app_a).
+-export([application_env_error_app_a/0]).
+
+application_env_error_app_a() ->
+    application:get_env(misc, key).
+
+food(0) ->
+    ok;
+fooX(X) ->
+    no.

test_projects/linter_bad_config/linter/app_a/src/app_a_unused_param.erl

@@ -0,0 +1,6 @@
+-module(app_a_unused_param).
+
+-export([foo/1]).
+
+foo(X) ->
+    ok.

test_projects/linter_bad_config/linter/app_a/test/app_a_SUITE.erl

@@ -0,0 +1,19 @@
+-module(app_a_SUITE).
+-compile([export_all, nowarn_export_all]).
+-typing([eqwalizer]).
+
+-include_lib("stdlib/include/assert.hrl").
+-include("app_a.hrl").
+
+-spec ok() -> ok.
+ok() ->
+    ?assert(true),
+    case rand:uniform(1) of
+        1 -> app_a_test_helpers:ok();
+        _ -> app_a_test_helpers_not_opted_in:ok()
+    end.
+
+-spec fail() -> ok.
+fail() ->
+    app_a_test_helpers:fail().
+

test_projects/linter_bad_config/linter/app_a/test/app_a_test_helpers.erl

@@ -0,0 +1,10 @@
+-module(app_a_test_helpers).
+-typing([eqwalizer]).
+-compile([export_all, nowarn_export_all]).
+
+-spec fail() -> error.
+fail() -> wrong_ret.
+
+-spec ok() -> ok.
+ok() -> ok.
+

test_projects/linter_bad_config/linter/app_a/test/app_a_test_helpers_not_opted_in.erl

@@ -0,0 +1,9 @@
+-module(app_a_test_helpers_not_opted_in).
+-compile([export_all, nowarn_export_all]).
+
+-spec fail() -> ok.
+fail() -> error.
+
+-spec ok() -> ok.
+ok() -> ok.
+

test_projects/linter_bad_config/linter/app_a/test/app_test_helpers_no_errors.erl

@@ -0,0 +1,6 @@
+-module(app_a_test_helpers_no_errors).
+-compile([export_all, nowarn_export_all]).
+
+-spec ok() -> ok.
+ok() -> ok.
+

test_projects/linter_bad_config/linter/app_b/src/app_b.app.src

@@ -0,0 +1,3 @@
+{application, app_b,
+  [{description, "example app B"}, {vsn, "inplace"}, {applications, [kernel, stdlib]}]
+}.

test_projects/linter_bad_config/linter/app_b/src/app_b.erl

@@ -0,0 +1,5 @@
+-module(app_b).
+-export([application_env_error/0]).
+
+application_env_error() ->
+    application:get_env(misc, key).

test_projects/linter_bad_config/linter/app_b/src/app_b_unused_param.erl

@@ -0,0 +1,6 @@
+-module(app_b_unused_param).
+
+-export([foo/1]).
+
+foo(X) ->
+    ok.

test_projects/linter_bad_config/linter/rebar.config

@@ -0,0 +1,9 @@
+{checkouts_dir, ["."]}.
+{plugins, [wa_utils]}.
+{project_app_dirs, [
+    "app_a",
+    "app_b"
+]}.
+
+{erl_opts, [debug_info]}.
+{deps, []}.