Skip to content

Commit a614445

Browse files
alanzmeta-codesync[bot]
authored andcommitted
CLI ssr: allow specifying macro and parens strategy
Summary: Structural Search (SSR) matches the syntax tree used internally in ELP, called HIR. This is conceptually similar to Erlang Abstract Forms. When we lower the surface Erlang language into this, we expand macros and strip of parentheses, as the precedence of items is captured in the actual HIR representation. But sometimes when we are searching over Erlang code we want to explicitly match parentheses, e.g. if we want to find all instances of redundant duplicate ones. This diff allows you to do this by issue the command ``` elp ssr --parens "((_A))" ``` Likewise, sometimes you are interested in occurrences of macros. This can now be provided using a match like ``` elp ssr --macros no-expand "?BAR(_X)" ``` Reviewed By: TD5 Differential Revision: D85876784 fbshipit-source-id: ba62d83ed8ea9c7ac30cceda2cac446ecdeefb06
1 parent 0e9a488 commit a614445

15 files changed

+331
-9
lines changed

crates/elp/src/bin/args.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,17 @@ use std::env;
1313
use std::fs;
1414
use std::path::PathBuf;
1515

16+
use anyhow::Result;
17+
use anyhow::bail;
1618
use bpaf::Bpaf;
1719
use bpaf::Parser;
1820
use bpaf::construct;
1921
use bpaf::long;
2022
use elp_ide::elp_ide_db::DiagnosticCode;
2123
use elp_project_model::buck::BuckQueryConfig;
24+
use hir::fold::MacroStrategy;
25+
use hir::fold::ParenStrategy;
26+
use hir::fold::Strategy;
2227
use itertools::Itertools;
2328
use serde::Deserialize;
2429

@@ -367,6 +372,20 @@ pub struct Ssr {
367372
)]
368373
pub format: Option<String>,
369374

375+
/// Macro expansion strategy: expand | no-expand | visible-expand (default expand)
376+
#[bpaf(
377+
long("macros"),
378+
argument("STRATEGY"),
379+
complete(macros_completer),
380+
fallback(None),
381+
guard(macros_guard, "Please supply a valid macro expansion value")
382+
)]
383+
pub macro_strategy: Option<String>,
384+
385+
/// Explicitly match parentheses. If omitted, they are ignored.
386+
#[bpaf(long("parens"))]
387+
pub paren_strategy: bool,
388+
370389
/// Report system memory usage and other statistics
371390
#[bpaf(long("report-system-stats"))]
372391
pub report_system_stats: bool,
@@ -706,6 +725,21 @@ fn format_guard(format: &Option<String>) -> bool {
706725
}
707726
}
708727

728+
fn macros_completer(_: &Option<String>) -> Vec<(String, Option<String>)> {
729+
vec![
730+
("expand".to_string(), None),
731+
("no-expand".to_string(), None),
732+
("visible-expand".to_string(), None),
733+
]
734+
}
735+
736+
fn macros_guard(format: &Option<String>) -> bool {
737+
match format {
738+
None => true,
739+
Some(_) => parse_macro_strategy(format).is_ok(),
740+
}
741+
}
742+
709743
#[allow(clippy::ptr_arg)] // This is needed in the BPAF macros
710744
fn at_least_1(data: &Vec<String>) -> bool {
711745
!data.is_empty()
@@ -788,6 +822,19 @@ impl Lint {
788822
}
789823
}
790824

825+
fn parse_macro_strategy(macro_strategy: &Option<String>) -> Result<MacroStrategy> {
826+
match macro_strategy.as_deref() {
827+
Some("no-expand") => Ok(MacroStrategy::DoNotExpand),
828+
Some("expand") => Ok(MacroStrategy::Expand),
829+
Some("visible-expand") => Ok(MacroStrategy::ExpandButIncludeMacroCall),
830+
None => Ok(MacroStrategy::Expand),
831+
Some(s) => bail!(
832+
"Invalid macro strategy '{}'. Valid options are: expand, no-expand, visible-expand",
833+
s
834+
),
835+
}
836+
}
837+
791838
impl Ssr {
792839
pub fn is_format_normal(&self) -> bool {
793840
self.format.is_none()
@@ -796,6 +843,16 @@ impl Ssr {
796843
pub fn is_format_json(&self) -> bool {
797844
self.format == Some("json".to_string())
798845
}
846+
847+
pub fn parse_strategy(&self) -> Result<Strategy> {
848+
let macros = parse_macro_strategy(&self.macro_strategy)?;
849+
let parens = if self.paren_strategy {
850+
ParenStrategy::VisibleParens
851+
} else {
852+
ParenStrategy::InvisibleParens
853+
};
854+
Ok(Strategy { macros, parens })
855+
}
799856
}
800857

801858
impl ParseAllElp {

crates/elp/src/bin/main.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2047,6 +2047,73 @@ mod tests {
20472047
)
20482048
}
20492049

2050+
#[test]
2051+
fn lint_ssr_as_cli_parens_visible() {
2052+
simple_snapshot(
2053+
args_vec!["ssr", "--parens", "(_@A)",],
2054+
"linter",
2055+
expect_file!("../resources/test/linter/ssr_ad_hoc_cli_parens_visible.stdout"),
2056+
true,
2057+
None,
2058+
)
2059+
}
2060+
2061+
#[test]
2062+
fn lint_ssr_as_cli_parens_invisible() {
2063+
// Invisible parens are the default
2064+
simple_snapshot(
2065+
args_vec!["ssr", "(((3)))",],
2066+
"linter",
2067+
expect_file!("../resources/test/linter/ssr_ad_hoc_cli_parens_invisible.stdout"),
2068+
true,
2069+
None,
2070+
)
2071+
}
2072+
2073+
#[test]
2074+
fn lint_ssr_as_cli_macros_expand() {
2075+
simple_snapshot(
2076+
args_vec!["ssr", "--macros", "expand", "?BAR(_@AA)", "{4}"],
2077+
"linter",
2078+
expect_file!("../resources/test/linter/ssr_ad_hoc_cli_macros_expand.stdout"),
2079+
true,
2080+
None,
2081+
)
2082+
}
2083+
2084+
#[test]
2085+
fn lint_ssr_as_cli_macros_expand_is_default() {
2086+
simple_snapshot(
2087+
args_vec!["ssr", "?BAR(_@AA)", "{4}"],
2088+
"linter",
2089+
expect_file!("../resources/test/linter/ssr_ad_hoc_cli_macros_expand.stdout"),
2090+
true,
2091+
None,
2092+
)
2093+
}
2094+
2095+
#[test]
2096+
fn lint_ssr_as_cli_macros_visible_expand() {
2097+
simple_snapshot(
2098+
args_vec!["ssr", "--macros", "visible-expand", "?BAR(_@AA)", "{4}"],
2099+
"linter",
2100+
expect_file!("../resources/test/linter/ssr_ad_hoc_cli_macros_visible_expand.stdout"),
2101+
true,
2102+
None,
2103+
)
2104+
}
2105+
2106+
#[test]
2107+
fn lint_ssr_as_cli_macros_no_expand() {
2108+
simple_snapshot(
2109+
args_vec!["ssr", "--macros", "no-expand", "?BAR(_@AA)", "{4}"],
2110+
"linter",
2111+
expect_file!("../resources/test/linter/ssr_ad_hoc_cli_macros_no_expand.stdout"),
2112+
true,
2113+
None,
2114+
)
2115+
}
2116+
20502117
#[test_case(false ; "rebar")]
20512118
#[test_case(true ; "buck")]
20522119
fn eqwalizer_tests_check(buck: bool) {

crates/elp/src/bin/ssr_cli.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,17 @@ pub fn run_ssr_command(
7878
}
7979
}
8080

81+
// Parse the strategy from CLI arguments
82+
let strategy = args.parse_strategy()?;
83+
8184
// Create the lint config with all SSR patterns
8285
let mut lint_config = LintConfig::default();
8386
for pattern in &args.ssr_specs {
8487
let normalized_pattern = normalize_ssr_pattern(pattern);
8588
let ssr_lint = diagnostics::Lint::LintMatchSsr(MatchSsr {
8689
ssr_pattern: normalized_pattern,
8790
message: None,
91+
strategy: Some(strategy),
8892
});
8993
lint_config.ad_hoc_lints.lints.push(ssr_lint);
9094
}

crates/elp/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ mod tests {
192192
Lint::LintMatchSsr(MatchSsr {
193193
ssr_pattern: "ssr: _@A = 10.".to_string(),
194194
message: None,
195+
strategy: None,
195196
}),
196197
],
197198
},

crates/elp/src/resources/test/linter/parse_elp_lint_explicit_enable_output.stdout

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
Diagnostics reported in 6 modules:
1+
Diagnostics reported in 7 modules:
22
app_a: 2
33
4:4-4:34::[Warning] [W0011] module `app_a` belongs to app `app_a`, but reads env for `misc`
44
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
55
app_a_edoc: 1
66
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
7+
app_a_ssr: 1
8+
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
79
app_a_unused_param: 2
810
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
911
4:4-4:5::[Warning] [L1268] variable 'X' is unused

crates/elp/src/resources/test/linter/parse_elp_no_lint_specified_json_output.stdout

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
{"path":"app_a/src/app_a.erl","line":16,"char":1,"code":"ELP","severity":"warning","name":"L1230 (L1230)","original":null,"replacement":null,"description":"function baz/2 is unused\n\nFor more information see: /erlang-error-index/l/L1230","docPath":null}
99
{"path":"app_a/src/app_a_edoc.erl","line":1,"char":1,"code":"ELP","severity":"error","name":"W0012 (compile-warn-missing-spec)","original":null,"replacement":null,"description":"Please add \"-compile(warn_missing_spec_all).\" to the module. If exported functions are not all specced, they need to be specced.\n\nFor more information see: /erlang-error-index/w/W0012","docPath":"website/docs/erlang-error-index/w/W0012.md"}
1010
{"path":"app_a/src/app_a_edoc.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
11+
{"path":"app_a/src/app_a_ssr.erl","line":1,"char":1,"code":"ELP","severity":"error","name":"W0012 (compile-warn-missing-spec)","original":null,"replacement":null,"description":"Please add \"-compile(warn_missing_spec_all).\" to the module. If exported functions are not all specced, they need to be specced.\n\nFor more information see: /erlang-error-index/w/W0012","docPath":"website/docs/erlang-error-index/w/W0012.md"}
12+
{"path":"app_a/src/app_a_ssr.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
1113
{"path":"app_a/src/app_a_unused_param.erl","line":1,"char":1,"code":"ELP","severity":"error","name":"W0012 (compile-warn-missing-spec)","original":null,"replacement":null,"description":"Please add \"-compile(warn_missing_spec_all).\" to the module. If exported functions are not all specced, they need to be specced.\n\nFor more information see: /erlang-error-index/w/W0012","docPath":"website/docs/erlang-error-index/w/W0012.md"}
1214
{"path":"app_a/src/app_a_unused_param.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
1315
{"path":"app_a/src/app_a_unused_param.erl","line":5,"char":5,"code":"ELP","severity":"warning","name":"L1268 (L1268)","original":null,"replacement":null,"description":"variable 'X' is unused\n\nFor more information see: /erlang-error-index/l/L1268","docPath":null}

crates/elp/src/resources/test/linter/parse_elp_no_lint_specified_output.stdout

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Reporting all diagnostics codes
2-
Diagnostics reported in 8 modules:
2+
Diagnostics reported in 9 modules:
33
app_a: 8
44
4:4-4:34::[Warning] [W0011] module `app_a` belongs to app `app_a`, but reads env for `misc`
55
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
@@ -12,6 +12,9 @@ Diagnostics reported in 8 modules:
1212
app_a_edoc: 2
1313
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
1414
0:8-0:18::[WeakWarning] [W0046] The module is not documented.
15+
app_a_ssr: 2
16+
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
17+
0:8-0:17::[WeakWarning] [W0046] The module is not documented.
1518
app_a_unused_param: 3
1619
0:0-0:0::[Error] [W0012] Please add "-compile(warn_missing_spec_all)." to the module. If exported functions are not all specced, they need to be specced.
1720
0:8-0:26::[WeakWarning] [W0046] The module is not documented.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Matches found in 1 modules:
2+
app_a_ssr: 1
3+
10:10-10:17::[WeakWarning] [ad-hoc: ssr-match] SSR pattern matched: ssr: {4}.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Matches found in 1 modules:
2+
app_a_ssr: 1
3+
10:10-10:17::[WeakWarning] [ad-hoc: ssr-match] SSR pattern matched: ssr: ?BAR(_@AA).
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Matches found in 1 modules:
2+
app_a_ssr: 2
3+
10:10-10:17::[WeakWarning] [ad-hoc: ssr-match] SSR pattern matched: ssr: ?BAR(_@AA).
4+
10:10-10:17::[WeakWarning] [ad-hoc: ssr-match] SSR pattern matched: ssr: {4}.

0 commit comments

Comments
 (0)