diff --git a/crates/elp/src/bin/lint_cli.rs b/crates/elp/src/bin/lint_cli.rs index acc903517b..8f2ef1093c 100644 --- a/crates/elp/src/bin/lint_cli.rs +++ b/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 { 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::(&content) { - return config; + if let Ok(content) = fs::read_to_string(file_path.clone()) { + match toml::from_str::(&content) { + Ok(config) => return Ok(config), + Err(err) => bail!("failed to read {:?}:{err}", file_path), } } } break; } - LintConfig::default() + Ok(LintConfig::default()) } fn print_diagnostic( diff --git a/crates/elp/src/bin/main.rs b/crates/elp/src/bin/main.rs index 47a4afec31..109dcc6b9f 100644 --- a/crates/elp/src/bin/main.rs +++ b/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, + 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, ) -> 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)); diff --git a/test_projects/linter_bad_config/.elp.toml b/test_projects/linter_bad_config/.elp.toml new file mode 100644 index 0000000000..a887ab835c --- /dev/null +++ b/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" diff --git a/test_projects/linter_bad_config/.elp_lint.toml b/test_projects/linter_bad_config/.elp_lint.toml new file mode 100644 index 0000000000..77311d91ed --- /dev/null +++ b/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'] diff --git a/test_projects/linter_bad_config/.gitignore b/test_projects/linter_bad_config/.gitignore new file mode 100644 index 0000000000..d811070a54 --- /dev/null +++ b/test_projects/linter_bad_config/.gitignore @@ -0,0 +1,4 @@ +.idea +build_info.etf +/_build/ +rebar.lock diff --git a/test_projects/linter_bad_config/app_a/include/app_a.hrl b/test_projects/linter_bad_config/app_a/include/app_a.hrl new file mode 100644 index 0000000000..f4378955c0 --- /dev/null +++ b/test_projects/linter_bad_config/app_a/include/app_a.hrl @@ -0,0 +1 @@ +-typing([eqwalizer]). diff --git a/test_projects/linter_bad_config/app_a/src/app_a.app.src b/test_projects/linter_bad_config/app_a/src/app_a.app.src new file mode 100644 index 0000000000..1381435a46 --- /dev/null +++ b/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]}] +}. diff --git a/test_projects/linter_bad_config/app_a/src/app_a.erl b/test_projects/linter_bad_config/app_a/src/app_a.erl new file mode 100644 index 0000000000..9c00f2dd95 --- /dev/null +++ b/test_projects/linter_bad_config/app_a/src/app_a.erl @@ -0,0 +1 @@ +-module(app_a). diff --git a/test_projects/linter_bad_config/linter/.elp.toml b/test_projects/linter_bad_config/linter/.elp.toml new file mode 100644 index 0000000000..a887ab835c --- /dev/null +++ b/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" diff --git a/test_projects/linter_bad_config/linter/.elp_lint.toml b/test_projects/linter_bad_config/linter/.elp_lint.toml new file mode 100644 index 0000000000..0379322d88 --- /dev/null +++ b/test_projects/linter_bad_config/linter/.elp_lint.toml @@ -0,0 +1,2 @@ +enabled_lints =['L1268', 'W0011', 'P1700'] +disabled_lints = ['W0010'] diff --git a/test_projects/linter_bad_config/linter/.gitignore b/test_projects/linter_bad_config/linter/.gitignore new file mode 100644 index 0000000000..d811070a54 --- /dev/null +++ b/test_projects/linter_bad_config/linter/.gitignore @@ -0,0 +1,4 @@ +.idea +build_info.etf +/_build/ +rebar.lock diff --git a/test_projects/linter_bad_config/linter/app_a/include/app_a.hrl b/test_projects/linter_bad_config/linter/app_a/include/app_a.hrl new file mode 100644 index 0000000000..f4378955c0 --- /dev/null +++ b/test_projects/linter_bad_config/linter/app_a/include/app_a.hrl @@ -0,0 +1 @@ +-typing([eqwalizer]). diff --git a/test_projects/linter_bad_config/linter/app_a/src/app_a.app.src b/test_projects/linter_bad_config/linter/app_a/src/app_a.app.src new file mode 100644 index 0000000000..1381435a46 --- /dev/null +++ b/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]}] +}. diff --git a/test_projects/linter_bad_config/linter/app_a/src/app_a.erl b/test_projects/linter_bad_config/linter/app_a/src/app_a.erl new file mode 100644 index 0000000000..c47f70751a --- /dev/null +++ b/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. diff --git a/test_projects/linter_bad_config/linter/app_a/src/app_a_unused_param.erl b/test_projects/linter_bad_config/linter/app_a/src/app_a_unused_param.erl new file mode 100644 index 0000000000..5b9fb07d27 --- /dev/null +++ b/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. diff --git a/test_projects/linter_bad_config/linter/app_a/test/app_a_SUITE.erl b/test_projects/linter_bad_config/linter/app_a/test/app_a_SUITE.erl new file mode 100644 index 0000000000..ff17886c13 --- /dev/null +++ b/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(). + diff --git a/test_projects/linter_bad_config/linter/app_a/test/app_a_test_helpers.erl b/test_projects/linter_bad_config/linter/app_a/test/app_a_test_helpers.erl new file mode 100644 index 0000000000..52077306c7 --- /dev/null +++ b/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. + diff --git a/test_projects/linter_bad_config/linter/app_a/test/app_a_test_helpers_not_opted_in.erl b/test_projects/linter_bad_config/linter/app_a/test/app_a_test_helpers_not_opted_in.erl new file mode 100644 index 0000000000..dc280d10bb --- /dev/null +++ b/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. + diff --git a/test_projects/linter_bad_config/linter/app_a/test/app_test_helpers_no_errors.erl b/test_projects/linter_bad_config/linter/app_a/test/app_test_helpers_no_errors.erl new file mode 100644 index 0000000000..35af9911ed --- /dev/null +++ b/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. + diff --git a/test_projects/linter_bad_config/linter/app_b/src/app_b.app.src b/test_projects/linter_bad_config/linter/app_b/src/app_b.app.src new file mode 100644 index 0000000000..4112b4129f --- /dev/null +++ b/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]}] +}. diff --git a/test_projects/linter_bad_config/linter/app_b/src/app_b.erl b/test_projects/linter_bad_config/linter/app_b/src/app_b.erl new file mode 100644 index 0000000000..a71ef9123f --- /dev/null +++ b/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). diff --git a/test_projects/linter_bad_config/linter/app_b/src/app_b_unused_param.erl b/test_projects/linter_bad_config/linter/app_b/src/app_b_unused_param.erl new file mode 100644 index 0000000000..81f3f876aa --- /dev/null +++ b/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. diff --git a/test_projects/linter_bad_config/linter/rebar.config b/test_projects/linter_bad_config/linter/rebar.config new file mode 100644 index 0000000000..344eb71dd8 --- /dev/null +++ b/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, []}. diff --git a/test_projects/linter_bad_config/linter/wa_utils/src/wa_build_info_prv.erl b/test_projects/linter_bad_config/linter/wa_utils/src/wa_build_info_prv.erl new file mode 100644 index 0000000000..a085e0b548 --- /dev/null +++ b/test_projects/linter_bad_config/linter/wa_utils/src/wa_build_info_prv.erl @@ -0,0 +1,384 @@ +%% % @format +-module(wa_build_info_prv). +-oncall("whatsapp_elp"). + +-export([init/1, do/1]). + +-define(NUM_RETRIES_FETCH_DEPS, 5). + +-type app_build_opts() :: #{ + name := binary(), + dir := binary(), + ebin := binary(), + src_dirs := [binary()], + extra_src_dirs := [binary()], + include_dirs := [binary()], + macros := [atom() | {atom(), any()}], + parse_transforms := [any()] +}. + +init(State) -> + State1 = rebar_state:add_provider( + State, + providers:create([ + {name, build_info}, + {module, wa_build_info_prv}, + {bare, true}, + {deps, [app_discovery]}, + {example, "rebar3 build_info"}, + {short_desc, "Get build_info"}, + {desc, "Get build_info"}, + {opts, [ + {to, $t, "to", {string, undefined}, + "file to write buid_info in file (ETF for LAP, or erlang_ls.config format)"}, + {mode, $m, "mode", {string, "etf"}, + "build info format, can be etf (generic build info for ELP and Eqwalizer), els (Erlang LS), rl (Readme lint), wm (wiki_moduledocs lint)"}, + {els_config, $e, "els-config", {string, undefined}, "existing erlang_ls.config"} + ]} + ]) + ), + {ok, State1}. + +do(State0) -> + {RawOpts, _} = rebar_state:command_parsed_args(State0), + Mode = proplists:get_value(mode, RawOpts), + State1 = maybe_get_deps(State0, Mode), + case Mode of + "etf" -> etf_build_info(State1, RawOpts); + "els" -> els_build_info(State1, RawOpts); + "rl" -> rl_build_info(State1, RawOpts); + "wm" -> wm_build_info(State1, RawOpts) + end, + {ok, State1}. + +maybe_get_deps(State0, Mode) -> + %% fetching dependencies in CI may require multiple attemps due to + %% some unreliable source constrol infra + case Mode of + %% Dependencies not needed for "rl" or "wm" modes + Mode when Mode == "rl"; Mode == "wm" -> + State0; + _ -> + State1 = safe_get_install_deps(State0, ?NUM_RETRIES_FETCH_DEPS), + {ok, State2} = rebar_prv_lock:do(State1), + State2 + end. + +safe_get_install_deps(_State, 0) -> + rebar_log:log(error, "Unable to fetch dependencies after ~b tries, aborting", [ + ?NUM_RETRIES_FETCH_DEPS + ]), + throw(rebar_abort); +safe_get_install_deps(State, N) -> + try rebar_prv_install_deps:do(State) of + {ok, NewState} -> NewState + catch + E:R:ST -> + rebar_log:log(warn, "error fetching deps: {~p, ~p}~n~p", [E, R, ST]), + safe_get_install_deps(State, N - 1) + end. + +get_data(State) -> + ProjectApps = rebar_state:project_apps(State), + DepApps = rebar_state:all_deps(State), + #{ + apps => [app_build_opts(App) || App <- ProjectApps], + deps => [app_build_opts(App) || App <- DepApps], + otp_lib_dir => list_to_binary(code:lib_dir()), + source_root => list_to_binary(rebar_state:dir(State)) + }. + +etf_build_info(State, RawOpts) -> + Data = get_data(State), + + To = proplists:get_value(to, RawOpts), + case To of + undefined -> + rebar_log:log(info, "Build info:~n", []), + io:fwrite("~p.~n", [Data]); + File -> + ok = file:write_file(File, term_to_binary(Data)), + rebar_log:log(info, "Build info written to: ~ts", [File]) + end. + +write_output(Encoded, RawOpts, Name) -> + To = proplists:get_value(to, RawOpts), + case To of + undefined -> + rebar_log:log(info, "~s:", [Name]), + io:fwrite("~s~n", [Encoded]); + File -> + ok = file:write_file(File, [Encoded, "\n"]), + rebar_log:log(info, "~s written to: ~ts", [Name, File]) + end. + +all_include_dirs(Apps, Deps) -> + lists:concat( + [ + [filename:join(Dir, SrcDir) || SrcDir <- SrcDirs] ++ IncludeDirs + || #{include_dirs := IncludeDirs, src_dirs := SrcDirs, dir := Dir} <- Apps + ] ++ + [ + IncludeDirs + || #{include_dirs := IncludeDirs} <- Deps + ] + ). + +els_build_info(State, RawOpts) -> + #{deps := Deps, apps := Apps} = get_data(State), + + %% prepare data + BaseDir = rebar_state:dir(State), + IncludeDirs = [ + Dir + || IncludeDir <- all_include_dirs(Apps, Deps), + Dir <- [make_relative(BaseDir, IncludeDir), make_relative(BaseDir, enclosing_app_folder(IncludeDir))], + filelib:is_dir(Dir) + ], + AppsDirs = [make_relative(BaseDir, AppDir) || #{dir := AppDir} <- Apps], + + %% extract feature config from exsiting config file + Erlang_LS_Config = proplists:get_value(els_config, RawOpts), + Config = maybe_get_existing_config(Erlang_LS_Config), + + %% prepare JSON term + JSONData = [ + {<<"include_dirs">>, lists:usort(IncludeDirs)}, + {<<"apps_dirs">>, lists:usort(AppsDirs)} + | Config + ], + Encoded = jsone:encode(JSONData, [{indent, 2}, native_forward_slash]), + write_output(Encoded, RawOpts, <<"erlang_ls.config">>). + +rl_build_info(State, RawOpts) -> + Data = get_data(State), + + %% prepare data + #{apps := Apps} = Data, + BaseDir = rebar_state:dir(State), + AppsDirs = [ + make_relative(BaseDir, AppDir) + || #{ + dir := AppDir + } <- Apps + ], + + %% output + Encoded = string:join(lists:map(fun binary_to_list/1, AppsDirs), "\n"), + write_output(Encoded, RawOpts, <<"erlang_rl.config">>). + +%% report a guess as to what files would be created by running `wiki_moduledocs extract' +%% in the DOCS file format used by fbsphinx. +%% +%% the result should be an over-estimate; not every module/app will necessarily be +%% successfully processed (and some will be generated/private); fbsphinx will leniently +%% ignore any missing files. if TODO(T96305665) is ever implemented, this and the +%% associated linter should be removed. +%% +%% this is meant to be used with and support a single use case: updating the +%% `erl/docs/DOCS' file as part of an `arc lint'. it assumes presence of specific +%% template vars in that file. it assumes the presence of `erl/docs/DOCS.src' as a +%% template file. +wm_build_info(State, RawOpts) -> + #{apps := Apps} = get_data(State), + BaseDir = rebar_state:dir(State), + + %% find the source modules of each app: + AppSources = wm_app_sources(BaseDir, Apps), + + %% convert each source path to its corresponding DOCS path, and check each appdir for + %% presence of an existing readme to determine the final output readme name seen by + %% fbsphinx: + DocsPaths = wm_output_pathnames(AppSources), + + %% substitute the docs/readmes content for the relevant template variables: + FinalContent = wm_populate_template(BaseDir, DocsPaths), + + write_output(FinalContent, RawOpts, <<"wiki_moduledocs DOCS">>). + +%% given the build options for an app, return its name, its directory, and a list of its +%% source pathnames. each such source file has a corresponding entry in erl/docs/DOCS as +%% "erlang/AppName/modname.rst". each app has a README.md or README.rst in the same +%% location (if it had any modules). +-spec wm_app_sources(binary(), [app_build_opts()]) -> [{binary(), binary(), [binary()]}]. +wm_app_sources(BaseDir, Apps) -> + lists:foldl( + fun(#{name := AppName, dir := AppDir, src_dirs := SrcDirs}, Acc) -> + RelativeAppDir = make_relative(BaseDir, AppDir), + %% currently, wiki_moduledocs ignores most/all non-toplevel apps; this ignores + %% app paths containing more than a single "/": + ShouldIgnore = + case filename:split(RelativeAppDir) of + [<<".">>, _] -> + false; + [_] -> + false; + _ -> + rebar_log:log(debug, "ignoring appdir ~s", [RelativeAppDir]), + true + end, + case ShouldIgnore of + true -> + Acc; + false -> + [ + {AppName, RelativeAppDir, + lists:append([ + lists_to_binaries( + filelib:wildcard( + unicode:characters_to_list( + filename:join([RelativeAppDir, SrcDir, "*.erl"]) + ) + ) + ) + || SrcDir <- SrcDirs + ])} + | Acc + ] + end + end, + [], + Apps + ). + +%% given the output of `wm_app_sources', produce a list of relative pathnames to be +%% handled by fbsphinx corresponding to module docs and app readmes. +-spec wm_output_pathnames([{AppName :: binary(), AppDir :: binary(), Sources :: [binary()]}]) -> + {Docs :: [binary()], Readmes :: [binary()]}. +wm_output_pathnames(AppSources) -> + {A, B} = + lists:foldl( + fun({AppName, AppDir, Sources}, {DocsAcc, ReadmesAcc}) -> + DocsAcc1 = [ + [ + filename:join([ + "erlang", + AppName, + [filename:basename(Source, ".erl"), ".rst"] + ]) + || Source <- Sources + ] + | DocsAcc + ], + + %% readmes are handled in their own section to work around an fbsphinx + %% bug (parent paths must be created before same-named children): + ReadmeExt = + case filelib:is_regular(filename:join(AppDir, "README.md")) of + true -> + ".md"; + false -> + ".rst" + end, + ReadmesAcc1 = [[filename:join(["erlang", AppName, "README"]), ReadmeExt] | ReadmesAcc], + {DocsAcc1, ReadmesAcc1} + end, + {[], []}, + AppSources + ), + {lists:append(A), B}. + +%% given the output of `wm_output_pathnames', produce the final corresponding DOCS content +%% by substituting into the static source template file. +-spec wm_populate_template(binary(), {[binary()], [binary()]}) -> binary(). +wm_populate_template(BaseDir, {Docs, Readmes}) -> + DocsSrc = filename:join([BaseDir, "docs", "DOCS.src"]), + {ok, DocsSrcContent} = file:read_file(DocsSrc), + + %% items should be sorted, but readmes should occur before deeper children in order to + %% work around an fbsphinx bug: + Items = lists:sort(["erlang/README.rst" | Readmes]) ++ lists:sort(Docs), + Srcs = ["[", lists:join(",\n ", Items), "]"], + + %% substitute template variables for actual content. note: `re:split' with a fold is + %% ~6x faster than using `re:replace'. + Substitutions = #{<<"%%SRCS%%">> => Srcs}, + SplitPattern = ["(", lists:join("|", maps:keys(Substitutions)), ")"], + SplitContent = re:split(DocsSrcContent, SplitPattern, [{return, binary}, unicode]), + lists:foldl( + fun(Part, Acc) -> + case Substitutions of + #{Part := Replacement} -> + [Acc, Replacement]; + _ -> + [Acc, Part] + end + end, + [], + SplitContent + ). + +make_relative(Base, Full) -> + case string:split(Full, Base) of + [Prefix, Relative] -> + case string:is_empty(Prefix) of + true -> + unicode:characters_to_binary([".", Relative]); + _ -> + make_relative_error(Full, Base) + end; + _ -> + make_relative_error(Full, Base) + end. + +enclosing_app_folder(Dir) -> + filename:dirname(filename:dirname(Dir)). + +make_relative_error(Full, Base) -> + rebar_log:log(error, "the path ~s is not an extension of the base path ~s", [Full, Base]), + throw(rebar_abort). + +maybe_get_existing_config(undefined) -> + []; +maybe_get_existing_config(File) -> + case file:read_file(File) of + {ok, Json} -> + try + Data = jsone:decode(Json), + maps:to_list(Data) + catch + E:R -> + rebar_log:log(error, "error parsing erlang_ls config at ~s (~p)", [File, {E, R}]), + throw(rebar_abort) + end; + {error, _} = Error -> + rebar_log:log(error, "could not open erlang_ls config at ~s (~p)", [File, Error]), + throw(rebar_abort) + end. + +%% From rebar_compiler:context/1 + +-spec app_build_opts(rebar_app_info:t()) -> app_build_opts(). +app_build_opts(AppInfo) -> + Name = rebar_app_info:name(AppInfo), + AppDir = rebar_app_info:dir(AppInfo), + EbinDir = rebar_app_info:ebin_dir(AppInfo), + RebarOpts = rebar_app_info:opts(AppInfo), + SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]), + ExistingSrcDirs = [Dir || Dir <- SrcDirs, ec_file:is_dir(filename:join(AppDir, Dir))], + ExtraSrcDirs = rebar_dir:extra_src_dirs(RebarOpts), + ErlOpts = rebar_opts:erl_opts(RebarOpts), + ErlOptIncludes = proplists:get_all_values(i, ErlOpts), + InclDirs = + [filename:join(AppDir, "include")] ++ [filename:absname(Dir) || Dir <- ErlOptIncludes], + PTrans = proplists:get_all_values(parse_transform, ErlOpts), + Macros = macros(ErlOpts), + + #{ + name => Name, + dir => list_to_binary(AppDir), + ebin => list_to_binary(EbinDir), + src_dirs => lists_to_binaries(ExistingSrcDirs), + extra_src_dirs => lists_to_binaries(ExtraSrcDirs), + include_dirs => lists_to_binaries(InclDirs), + macros => Macros, + parse_transforms => PTrans + }. + +macros([{d, Name} | Rest]) -> [Name | macros(Rest)]; +macros([{d, Name, Value} | Rest]) -> [{Name, Value} | macros(Rest)]; +macros([_ | Rest]) -> macros(Rest); +macros([]) -> []. + +lists_to_binaries(Strings) -> + [list_to_binary(String) || String <- Strings]. diff --git a/test_projects/linter_bad_config/linter/wa_utils/src/wa_utils.app.src b/test_projects/linter_bad_config/linter/wa_utils/src/wa_utils.app.src new file mode 100644 index 0000000000..731eb1325d --- /dev/null +++ b/test_projects/linter_bad_config/linter/wa_utils/src/wa_utils.app.src @@ -0,0 +1,3 @@ +{application, wa_utils, + [{description, "wa_utils"}, {vsn, "inplace"}, {applications, [kernel, stdlib]}] +}. diff --git a/test_projects/linter_bad_config/linter/wa_utils/src/wa_utils.erl b/test_projects/linter_bad_config/linter/wa_utils/src/wa_utils.erl new file mode 100644 index 0000000000..9974dc25d8 --- /dev/null +++ b/test_projects/linter_bad_config/linter/wa_utils/src/wa_utils.erl @@ -0,0 +1,15 @@ +-module(wa_utils). + +-export([init/1]). + +init(State0) -> + Providers = [wa_build_info_prv], + State1 = lists:foldl( + fun(Provider, State) -> + {ok, NewState} = Provider:init(State), + NewState + end, + State0, + Providers + ), + {ok, State1}. diff --git a/test_projects/linter_bad_config/rebar.config b/test_projects/linter_bad_config/rebar.config new file mode 100644 index 0000000000..4dd09dc463 --- /dev/null +++ b/test_projects/linter_bad_config/rebar.config @@ -0,0 +1,8 @@ +{checkouts_dir, ["."]}. +{plugins, [wa_utils]}. +{project_app_dirs, [ + "app_a" +]}. + +{erl_opts, [debug_info]}. +{deps, []}. diff --git a/test_projects/linter_bad_config/wa_utils/src/wa_build_info_prv.erl b/test_projects/linter_bad_config/wa_utils/src/wa_build_info_prv.erl new file mode 100644 index 0000000000..a085e0b548 --- /dev/null +++ b/test_projects/linter_bad_config/wa_utils/src/wa_build_info_prv.erl @@ -0,0 +1,384 @@ +%% % @format +-module(wa_build_info_prv). +-oncall("whatsapp_elp"). + +-export([init/1, do/1]). + +-define(NUM_RETRIES_FETCH_DEPS, 5). + +-type app_build_opts() :: #{ + name := binary(), + dir := binary(), + ebin := binary(), + src_dirs := [binary()], + extra_src_dirs := [binary()], + include_dirs := [binary()], + macros := [atom() | {atom(), any()}], + parse_transforms := [any()] +}. + +init(State) -> + State1 = rebar_state:add_provider( + State, + providers:create([ + {name, build_info}, + {module, wa_build_info_prv}, + {bare, true}, + {deps, [app_discovery]}, + {example, "rebar3 build_info"}, + {short_desc, "Get build_info"}, + {desc, "Get build_info"}, + {opts, [ + {to, $t, "to", {string, undefined}, + "file to write buid_info in file (ETF for LAP, or erlang_ls.config format)"}, + {mode, $m, "mode", {string, "etf"}, + "build info format, can be etf (generic build info for ELP and Eqwalizer), els (Erlang LS), rl (Readme lint), wm (wiki_moduledocs lint)"}, + {els_config, $e, "els-config", {string, undefined}, "existing erlang_ls.config"} + ]} + ]) + ), + {ok, State1}. + +do(State0) -> + {RawOpts, _} = rebar_state:command_parsed_args(State0), + Mode = proplists:get_value(mode, RawOpts), + State1 = maybe_get_deps(State0, Mode), + case Mode of + "etf" -> etf_build_info(State1, RawOpts); + "els" -> els_build_info(State1, RawOpts); + "rl" -> rl_build_info(State1, RawOpts); + "wm" -> wm_build_info(State1, RawOpts) + end, + {ok, State1}. + +maybe_get_deps(State0, Mode) -> + %% fetching dependencies in CI may require multiple attemps due to + %% some unreliable source constrol infra + case Mode of + %% Dependencies not needed for "rl" or "wm" modes + Mode when Mode == "rl"; Mode == "wm" -> + State0; + _ -> + State1 = safe_get_install_deps(State0, ?NUM_RETRIES_FETCH_DEPS), + {ok, State2} = rebar_prv_lock:do(State1), + State2 + end. + +safe_get_install_deps(_State, 0) -> + rebar_log:log(error, "Unable to fetch dependencies after ~b tries, aborting", [ + ?NUM_RETRIES_FETCH_DEPS + ]), + throw(rebar_abort); +safe_get_install_deps(State, N) -> + try rebar_prv_install_deps:do(State) of + {ok, NewState} -> NewState + catch + E:R:ST -> + rebar_log:log(warn, "error fetching deps: {~p, ~p}~n~p", [E, R, ST]), + safe_get_install_deps(State, N - 1) + end. + +get_data(State) -> + ProjectApps = rebar_state:project_apps(State), + DepApps = rebar_state:all_deps(State), + #{ + apps => [app_build_opts(App) || App <- ProjectApps], + deps => [app_build_opts(App) || App <- DepApps], + otp_lib_dir => list_to_binary(code:lib_dir()), + source_root => list_to_binary(rebar_state:dir(State)) + }. + +etf_build_info(State, RawOpts) -> + Data = get_data(State), + + To = proplists:get_value(to, RawOpts), + case To of + undefined -> + rebar_log:log(info, "Build info:~n", []), + io:fwrite("~p.~n", [Data]); + File -> + ok = file:write_file(File, term_to_binary(Data)), + rebar_log:log(info, "Build info written to: ~ts", [File]) + end. + +write_output(Encoded, RawOpts, Name) -> + To = proplists:get_value(to, RawOpts), + case To of + undefined -> + rebar_log:log(info, "~s:", [Name]), + io:fwrite("~s~n", [Encoded]); + File -> + ok = file:write_file(File, [Encoded, "\n"]), + rebar_log:log(info, "~s written to: ~ts", [Name, File]) + end. + +all_include_dirs(Apps, Deps) -> + lists:concat( + [ + [filename:join(Dir, SrcDir) || SrcDir <- SrcDirs] ++ IncludeDirs + || #{include_dirs := IncludeDirs, src_dirs := SrcDirs, dir := Dir} <- Apps + ] ++ + [ + IncludeDirs + || #{include_dirs := IncludeDirs} <- Deps + ] + ). + +els_build_info(State, RawOpts) -> + #{deps := Deps, apps := Apps} = get_data(State), + + %% prepare data + BaseDir = rebar_state:dir(State), + IncludeDirs = [ + Dir + || IncludeDir <- all_include_dirs(Apps, Deps), + Dir <- [make_relative(BaseDir, IncludeDir), make_relative(BaseDir, enclosing_app_folder(IncludeDir))], + filelib:is_dir(Dir) + ], + AppsDirs = [make_relative(BaseDir, AppDir) || #{dir := AppDir} <- Apps], + + %% extract feature config from exsiting config file + Erlang_LS_Config = proplists:get_value(els_config, RawOpts), + Config = maybe_get_existing_config(Erlang_LS_Config), + + %% prepare JSON term + JSONData = [ + {<<"include_dirs">>, lists:usort(IncludeDirs)}, + {<<"apps_dirs">>, lists:usort(AppsDirs)} + | Config + ], + Encoded = jsone:encode(JSONData, [{indent, 2}, native_forward_slash]), + write_output(Encoded, RawOpts, <<"erlang_ls.config">>). + +rl_build_info(State, RawOpts) -> + Data = get_data(State), + + %% prepare data + #{apps := Apps} = Data, + BaseDir = rebar_state:dir(State), + AppsDirs = [ + make_relative(BaseDir, AppDir) + || #{ + dir := AppDir + } <- Apps + ], + + %% output + Encoded = string:join(lists:map(fun binary_to_list/1, AppsDirs), "\n"), + write_output(Encoded, RawOpts, <<"erlang_rl.config">>). + +%% report a guess as to what files would be created by running `wiki_moduledocs extract' +%% in the DOCS file format used by fbsphinx. +%% +%% the result should be an over-estimate; not every module/app will necessarily be +%% successfully processed (and some will be generated/private); fbsphinx will leniently +%% ignore any missing files. if TODO(T96305665) is ever implemented, this and the +%% associated linter should be removed. +%% +%% this is meant to be used with and support a single use case: updating the +%% `erl/docs/DOCS' file as part of an `arc lint'. it assumes presence of specific +%% template vars in that file. it assumes the presence of `erl/docs/DOCS.src' as a +%% template file. +wm_build_info(State, RawOpts) -> + #{apps := Apps} = get_data(State), + BaseDir = rebar_state:dir(State), + + %% find the source modules of each app: + AppSources = wm_app_sources(BaseDir, Apps), + + %% convert each source path to its corresponding DOCS path, and check each appdir for + %% presence of an existing readme to determine the final output readme name seen by + %% fbsphinx: + DocsPaths = wm_output_pathnames(AppSources), + + %% substitute the docs/readmes content for the relevant template variables: + FinalContent = wm_populate_template(BaseDir, DocsPaths), + + write_output(FinalContent, RawOpts, <<"wiki_moduledocs DOCS">>). + +%% given the build options for an app, return its name, its directory, and a list of its +%% source pathnames. each such source file has a corresponding entry in erl/docs/DOCS as +%% "erlang/AppName/modname.rst". each app has a README.md or README.rst in the same +%% location (if it had any modules). +-spec wm_app_sources(binary(), [app_build_opts()]) -> [{binary(), binary(), [binary()]}]. +wm_app_sources(BaseDir, Apps) -> + lists:foldl( + fun(#{name := AppName, dir := AppDir, src_dirs := SrcDirs}, Acc) -> + RelativeAppDir = make_relative(BaseDir, AppDir), + %% currently, wiki_moduledocs ignores most/all non-toplevel apps; this ignores + %% app paths containing more than a single "/": + ShouldIgnore = + case filename:split(RelativeAppDir) of + [<<".">>, _] -> + false; + [_] -> + false; + _ -> + rebar_log:log(debug, "ignoring appdir ~s", [RelativeAppDir]), + true + end, + case ShouldIgnore of + true -> + Acc; + false -> + [ + {AppName, RelativeAppDir, + lists:append([ + lists_to_binaries( + filelib:wildcard( + unicode:characters_to_list( + filename:join([RelativeAppDir, SrcDir, "*.erl"]) + ) + ) + ) + || SrcDir <- SrcDirs + ])} + | Acc + ] + end + end, + [], + Apps + ). + +%% given the output of `wm_app_sources', produce a list of relative pathnames to be +%% handled by fbsphinx corresponding to module docs and app readmes. +-spec wm_output_pathnames([{AppName :: binary(), AppDir :: binary(), Sources :: [binary()]}]) -> + {Docs :: [binary()], Readmes :: [binary()]}. +wm_output_pathnames(AppSources) -> + {A, B} = + lists:foldl( + fun({AppName, AppDir, Sources}, {DocsAcc, ReadmesAcc}) -> + DocsAcc1 = [ + [ + filename:join([ + "erlang", + AppName, + [filename:basename(Source, ".erl"), ".rst"] + ]) + || Source <- Sources + ] + | DocsAcc + ], + + %% readmes are handled in their own section to work around an fbsphinx + %% bug (parent paths must be created before same-named children): + ReadmeExt = + case filelib:is_regular(filename:join(AppDir, "README.md")) of + true -> + ".md"; + false -> + ".rst" + end, + ReadmesAcc1 = [[filename:join(["erlang", AppName, "README"]), ReadmeExt] | ReadmesAcc], + {DocsAcc1, ReadmesAcc1} + end, + {[], []}, + AppSources + ), + {lists:append(A), B}. + +%% given the output of `wm_output_pathnames', produce the final corresponding DOCS content +%% by substituting into the static source template file. +-spec wm_populate_template(binary(), {[binary()], [binary()]}) -> binary(). +wm_populate_template(BaseDir, {Docs, Readmes}) -> + DocsSrc = filename:join([BaseDir, "docs", "DOCS.src"]), + {ok, DocsSrcContent} = file:read_file(DocsSrc), + + %% items should be sorted, but readmes should occur before deeper children in order to + %% work around an fbsphinx bug: + Items = lists:sort(["erlang/README.rst" | Readmes]) ++ lists:sort(Docs), + Srcs = ["[", lists:join(",\n ", Items), "]"], + + %% substitute template variables for actual content. note: `re:split' with a fold is + %% ~6x faster than using `re:replace'. + Substitutions = #{<<"%%SRCS%%">> => Srcs}, + SplitPattern = ["(", lists:join("|", maps:keys(Substitutions)), ")"], + SplitContent = re:split(DocsSrcContent, SplitPattern, [{return, binary}, unicode]), + lists:foldl( + fun(Part, Acc) -> + case Substitutions of + #{Part := Replacement} -> + [Acc, Replacement]; + _ -> + [Acc, Part] + end + end, + [], + SplitContent + ). + +make_relative(Base, Full) -> + case string:split(Full, Base) of + [Prefix, Relative] -> + case string:is_empty(Prefix) of + true -> + unicode:characters_to_binary([".", Relative]); + _ -> + make_relative_error(Full, Base) + end; + _ -> + make_relative_error(Full, Base) + end. + +enclosing_app_folder(Dir) -> + filename:dirname(filename:dirname(Dir)). + +make_relative_error(Full, Base) -> + rebar_log:log(error, "the path ~s is not an extension of the base path ~s", [Full, Base]), + throw(rebar_abort). + +maybe_get_existing_config(undefined) -> + []; +maybe_get_existing_config(File) -> + case file:read_file(File) of + {ok, Json} -> + try + Data = jsone:decode(Json), + maps:to_list(Data) + catch + E:R -> + rebar_log:log(error, "error parsing erlang_ls config at ~s (~p)", [File, {E, R}]), + throw(rebar_abort) + end; + {error, _} = Error -> + rebar_log:log(error, "could not open erlang_ls config at ~s (~p)", [File, Error]), + throw(rebar_abort) + end. + +%% From rebar_compiler:context/1 + +-spec app_build_opts(rebar_app_info:t()) -> app_build_opts(). +app_build_opts(AppInfo) -> + Name = rebar_app_info:name(AppInfo), + AppDir = rebar_app_info:dir(AppInfo), + EbinDir = rebar_app_info:ebin_dir(AppInfo), + RebarOpts = rebar_app_info:opts(AppInfo), + SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]), + ExistingSrcDirs = [Dir || Dir <- SrcDirs, ec_file:is_dir(filename:join(AppDir, Dir))], + ExtraSrcDirs = rebar_dir:extra_src_dirs(RebarOpts), + ErlOpts = rebar_opts:erl_opts(RebarOpts), + ErlOptIncludes = proplists:get_all_values(i, ErlOpts), + InclDirs = + [filename:join(AppDir, "include")] ++ [filename:absname(Dir) || Dir <- ErlOptIncludes], + PTrans = proplists:get_all_values(parse_transform, ErlOpts), + Macros = macros(ErlOpts), + + #{ + name => Name, + dir => list_to_binary(AppDir), + ebin => list_to_binary(EbinDir), + src_dirs => lists_to_binaries(ExistingSrcDirs), + extra_src_dirs => lists_to_binaries(ExtraSrcDirs), + include_dirs => lists_to_binaries(InclDirs), + macros => Macros, + parse_transforms => PTrans + }. + +macros([{d, Name} | Rest]) -> [Name | macros(Rest)]; +macros([{d, Name, Value} | Rest]) -> [{Name, Value} | macros(Rest)]; +macros([_ | Rest]) -> macros(Rest); +macros([]) -> []. + +lists_to_binaries(Strings) -> + [list_to_binary(String) || String <- Strings]. diff --git a/test_projects/linter_bad_config/wa_utils/src/wa_utils.app.src b/test_projects/linter_bad_config/wa_utils/src/wa_utils.app.src new file mode 100644 index 0000000000..731eb1325d --- /dev/null +++ b/test_projects/linter_bad_config/wa_utils/src/wa_utils.app.src @@ -0,0 +1,3 @@ +{application, wa_utils, + [{description, "wa_utils"}, {vsn, "inplace"}, {applications, [kernel, stdlib]}] +}. diff --git a/test_projects/linter_bad_config/wa_utils/src/wa_utils.erl b/test_projects/linter_bad_config/wa_utils/src/wa_utils.erl new file mode 100644 index 0000000000..9974dc25d8 --- /dev/null +++ b/test_projects/linter_bad_config/wa_utils/src/wa_utils.erl @@ -0,0 +1,15 @@ +-module(wa_utils). + +-export([init/1]). + +init(State0) -> + Providers = [wa_build_info_prv], + State1 = lists:foldl( + fun(Provider, State) -> + {ok, NewState} = Provider:init(State), + NewState + end, + State0, + Providers + ), + {ok, State1}.