Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(grit): fix matching multiple args #3797

Merged
merged 1 commit into from
Sep 5, 2024
Merged

fix(grit): fix matching multiple args #3797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions crates/biome_grit_patterns/src/grit_binding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,17 @@ impl<'a> Binding<'a, GritQueryContext> for GritBinding<'a> {
/// more than one associated node.
fn singleton(&self) -> Option<GritTargetNode<'a>> {
match self {
Self::Node(node) => Some(node.clone()),
Self::Node(node) => {
if node.is_list() {
let mut children = node.named_children();
match (children.next(), children.next()) {
(Some(only_child), None) => Some(only_child),
_ => None,
}
} else {
Some(node.clone())
}
}
Self::File(..) | Self::Range(..) | Self::Empty(..) | Self::Constant(..) => None,
}
}
Expand Down Expand Up @@ -189,7 +199,10 @@ impl<'a> Binding<'a, GritQueryContext> for GritBinding<'a> {
}

fn is_list(&self) -> bool {
self.as_node().map_or(false, |node| node.is_list())
match self {
Self::Node(node) => node.is_list(),
_ => false,
}
}

fn list_items(&self) -> Option<impl Iterator<Item = GritTargetNode<'a>> + Clone> {
Expand Down Expand Up @@ -268,15 +281,34 @@ fn are_equivalent(node1: &GritTargetNode, node2: &GritTargetNode) -> bool {
return true;
}

// If the node kinds are different, then the nodes are not equivalent.
// But if one of them is a list with a single node, we may still find a
// match against that node.
// If the node kinds are different, then the nodes are not equivalent,
// except in the presence of lists:
// - If both are a list, we don't care about the kind of the list, we just
// compare the nodes individually.
// - If one of them is a list with a single node, we may still find a
// match against that node.
if node1.kind() != node2.kind() {
return if node1.is_list() {
let mut children1 = node1.named_children();
match (children1.next(), children1.next()) {
(Some(only_child), None) => are_equivalent(&only_child, node2),
_ => false,
if node2.is_list() {
let mut children1 = node1.named_children();
let mut children2 = node2.named_children();
loop {
match (children1.next(), children2.next()) {
(Some(child1), Some(child2)) => {
if !are_equivalent(&child1, &child2) {
break false;
}
}
(None, None) => break true,
_ => break false,
}
}
} else {
let mut children1 = node1.named_children();
match (children1.next(), children1.next()) {
(Some(only_child), None) => are_equivalent(&only_child, node2),
_ => false,
}
}
} else if node2.is_list() {
let mut children2 = node2.named_children();
Expand Down
20 changes: 13 additions & 7 deletions crates/biome_grit_patterns/src/grit_node_patterns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ impl Matcher<GritQueryContext> for GritNodePattern {
let Some(node) = binding.singleton() else {
return Ok(false);
};
if binding.is_list() {
return self.execute(
&ResolvedPattern::from_node_binding(node),
init_state,
context,
logs,
);
}

if node.kind() != self.kind {
return Ok(false);
Expand Down Expand Up @@ -158,13 +166,11 @@ impl Matcher<GritQueryContext> for GritLeafNodePattern {
let Some(node) = binding.get_last_binding().and_then(Binding::singleton) else {
return Ok(false);
};
if let Some(class) = &self.equivalence_class {
Ok(class.are_equivalent(node.kind(), node.text()))
} else if self.kind != node.kind() {
Ok(false)
} else {
Ok(node.text() == self.text)
}

Ok(match &self.equivalence_class {
Some(class) => class.are_equivalent(node.kind(), node.text()),
None => node.kind() == self.kind && node.text() == self.text,
})
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/biome_grit_patterns/tests/specs/ts/functionToArrow.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ SnapshotResult {
messages: [],
matched_ranges: [
"1:1-2:2",
"4:1-5:2",
],
rewritten_files: [],
created_files: [],
Expand Down
6 changes: 6 additions & 0 deletions crates/biome_grit_patterns/tests/specs/ts/functionToArrow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
function foo(apple) {
}

function bar(apple, pear) {
}

function baz() {
}
3 changes: 2 additions & 1 deletion crates/biome_js_analyze/src/lint/complexity/no_useless_constructor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ impl Rule for NoUselessConstructor {
for parameter in constructor.parameters().ok()?.parameters() {
let decorators = match parameter.ok()? {
AnyJsConstructorParameter::AnyJsFormalParameter(
AnyJsFormalParameter::JsBogusParameter(_),
AnyJsFormalParameter::JsBogusParameter(_)
| AnyJsFormalParameter::JsMetavariable(_),
)
| AnyJsConstructorParameter::TsPropertyParameter(_) => {
// Ignore constructors with Bogus parameters or parameter properties
Expand Down
6 changes: 4 additions & 2 deletions crates/biome_js_analyze/src/lint/correctness/no_unused_private_class_members.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,8 @@ impl AnyMember {
},
AnyMember::TsPropertyParameter(ts_property) => {
match ts_property.formal_parameter().ok()? {
AnyJsFormalParameter::JsBogusParameter(_) => None,
AnyJsFormalParameter::JsBogusParameter(_)
| AnyJsFormalParameter::JsMetavariable(_) => None,
AnyJsFormalParameter::JsFormalParameter(param) => Some(
param
.binding()
Expand Down Expand Up @@ -348,7 +349,8 @@ impl AnyMember {
},
AnyMember::TsPropertyParameter(ts_property) => {
match ts_property.formal_parameter().ok()? {
AnyJsFormalParameter::JsBogusParameter(_) => None,
AnyJsFormalParameter::JsBogusParameter(_)
| AnyJsFormalParameter::JsMetavariable(_) => None,
AnyJsFormalParameter::JsFormalParameter(param) => Some(
param
.binding()
Expand Down
4 changes: 3 additions & 1 deletion crates/biome_js_analyze/src/lint/style/no_parameter_assign.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ impl Rule for NoParameterAssign {
fn binding_of(param: &AnyJsParameter) -> Option<AnyJsBindingPattern> {
match param {
AnyJsParameter::AnyJsFormalParameter(formal_param) => match &formal_param {
AnyJsFormalParameter::JsBogusParameter(_) => None,
AnyJsFormalParameter::JsBogusParameter(_) | AnyJsFormalParameter::JsMetavariable(_) => {
None
}
AnyJsFormalParameter::JsFormalParameter(param) => param.binding().ok(),
},
AnyJsParameter::JsRestParameter(param) => param.binding().ok(),
Expand Down
1 change: 1 addition & 0 deletions crates/biome_js_formatter/src/js/any/formal_parameter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ impl FormatRule<AnyJsFormalParameter> for FormatAnyJsFormalParameter {
match node {
AnyJsFormalParameter::JsBogusParameter(node) => node.format().fmt(f),
AnyJsFormalParameter::JsFormalParameter(node) => node.format().fmt(f),
AnyJsFormalParameter::JsMetavariable(node) => node.format().fmt(f),
}
}
}
4 changes: 3 additions & 1 deletion crates/biome_js_formatter/src/js/bindings/parameters.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,9 @@ pub(crate) fn should_hug_function_parameters(
}
}
}
AnyJsFormalParameter::JsBogusParameter(_) => return Err(FormatError::SyntaxError),
AnyJsFormalParameter::JsBogusParameter(_) | AnyJsFormalParameter::JsMetavariable(_) => {
return Err(FormatError::SyntaxError)
}
};

Ok(result)
Expand Down
7 changes: 4 additions & 3 deletions crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,10 @@ fn has_rest_object_or_array_parameter(parameters: &AnyJsArrowFunctionParameters)
| AnyJsBindingPattern::JsObjectBindingPattern(_))
)
}
AnyJsParameter::AnyJsFormalParameter(AnyJsFormalParameter::JsBogusParameter(_)) => {
false
}
AnyJsParameter::AnyJsFormalParameter(
AnyJsFormalParameter::JsBogusParameter(_)
| AnyJsFormalParameter::JsMetavariable(_),
) => false,
AnyJsParameter::TsThisParameter(_) => false,
AnyJsParameter::JsRestParameter(_) => true,
}),
Expand Down
6 changes: 6 additions & 0 deletions crates/biome_js_parser/src/syntax/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use biome_js_syntax::{JsSyntaxKind, TextRange, T};
use biome_parser::ParserProgress;
use biome_rowan::SyntaxKind;

use super::metavariable::parse_metavariable;

/// A function declaration, this could be async and or a generator. This takes a marker
/// because you need to first advance over async or start a marker and feed it in.
// test js function_decl
Expand Down Expand Up @@ -1368,6 +1370,10 @@ pub(super) fn parse_parameters_list(

progress.assert_progressing(p);

if parse_metavariable(p).is_present() {
break;
}

let parameter = parse_parameter(
p,
ExpressionContext::default().and_object_expression_allowed(!first || has_l_paren),
Expand Down
27 changes: 24 additions & 3 deletions crates/biome_js_syntax/src/generated/nodes.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions crates/biome_js_syntax/src/parameter_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,15 +537,19 @@ impl AnyJsFormalParameter {
/// Returns the list of decorators of the parameter if the parameter is decorated.
pub fn decorators(&self) -> Option<JsDecoratorList> {
match self {
AnyJsFormalParameter::JsBogusParameter(_) => None,
AnyJsFormalParameter::JsBogusParameter(_) | AnyJsFormalParameter::JsMetavariable(_) => {
None
}
AnyJsFormalParameter::JsFormalParameter(parameter) => Some(parameter.decorators()),
}
}

/// Returns the type annotation of the parameter if any.
pub fn type_annotation(&self) -> Option<TsTypeAnnotation> {
match self {
AnyJsFormalParameter::JsBogusParameter(_) => None,
AnyJsFormalParameter::JsBogusParameter(_) | AnyJsFormalParameter::JsMetavariable(_) => {
None
}
AnyJsFormalParameter::JsFormalParameter(parameter) => parameter.type_annotation(),
}
}
Expand Down
1 change: 1 addition & 0 deletions xtask/codegen/js.ungram
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1602,6 +1602,7 @@ JsParameterList = (AnyJsParameter (',' AnyJsParameter)* ','?)
AnyJsFormalParameter =
JsFormalParameter
| JsBogusParameter
| JsMetavariable

AnyJsParameter =
AnyJsFormalParameter
Expand Down
Loading