Skip to content

Commit 89305a1

Browse files
committed
The diagnostic gets a little worse, but that is because it is captured as a parse failure, and tree-sitter doesn't give us specifics about WHY a parse fails
1 parent f4dd8ed commit 89305a1

6 files changed

+21
-84
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ark/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ stdext = { path = "../stdext" }
4747
tokio = { version = "1.26.0", features = ["full"] }
4848
tower-lsp = "0.19.0"
4949
tree-sitter = "0.23.0"
50-
tree-sitter-r = { git = "https://github.com/r-lib/tree-sitter-r", rev = "c094bd57652f8a08edc31d79a31222268fe798ee" }
50+
tree-sitter-r = { git = "https://github.com/r-lib/tree-sitter-r", rev = "9a63f592f0d8736912b474260a49931766542ae4" }
5151
uuid = "1.3.0"
5252
url = "2.4.1"
5353
walkdir = "2"

crates/ark/src/lsp/diagnostics.rs

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -711,61 +711,6 @@ fn recurse_parenthesized_expression(
711711
().ok()
712712
}
713713

714-
// TODO: This should be a syntax check, as the grammar should not allow
715-
// two `Argument` nodes side by side
716-
fn check_call_like_next_sibling(
717-
child: Node,
718-
parent_type: &NodeType,
719-
context: &mut DiagnosticContext,
720-
diagnostics: &mut Vec<Diagnostic>,
721-
) -> Result<()> {
722-
let Some(next) = child.next_sibling() else {
723-
return ().ok();
724-
};
725-
726-
let close = match parent_type {
727-
NodeType::Call => ")",
728-
NodeType::Subset => "]",
729-
NodeType::Subset2 => "]]",
730-
_ => bail!("Parent must be a call, subset, or subset2 node."),
731-
};
732-
733-
let ok = match next.node_type() {
734-
NodeType::Comma => true,
735-
NodeType::Anonymous(kind) if kind.as_str() == close => true,
736-
NodeType::Comment => true,
737-
// Should be handled elsewhere
738-
NodeType::Error => true,
739-
_ => false,
740-
};
741-
742-
if ok {
743-
return ().ok();
744-
}
745-
746-
// Children can be arbitrarily large, so report the issue between the end of `child`
747-
// and the start of `next` (it's not really the child's fault anyways,
748-
// it's the fault of the thing right after it).
749-
let start_byte = child.end_byte();
750-
let start_point = child.end_position();
751-
let end_byte = next.start_byte();
752-
let end_point = next.start_position();
753-
754-
let range = Range {
755-
start_byte,
756-
start_point,
757-
end_byte,
758-
end_point,
759-
};
760-
761-
let range = convert_tree_sitter_range_to_lsp_range(context.contents, range);
762-
let message = "Expected ',' between expressions.";
763-
let diagnostic = Diagnostic::new_simple(range, message.into());
764-
diagnostics.push(diagnostic);
765-
766-
().ok()
767-
}
768-
769714
/// Default recursion for arguments of a call-like node
770715
///
771716
/// This applies for function calls, subset, and subset2, all of which are guaranteed
@@ -792,8 +737,6 @@ fn recurse_call_like_arguments_default(
792737
// every function behaves like `list()`, which is our default model of
793738
// strict evaluation.
794739
with_in_call_like_arguments(context, |context| {
795-
let call_type = node.node_type();
796-
797740
let Some(arguments) = node.child_by_field_name("arguments") else {
798741
return Ok(());
799742
};
@@ -803,9 +746,6 @@ fn recurse_call_like_arguments_default(
803746
let children = arguments.children_by_field_name("argument", &mut cursor);
804747

805748
for child in children {
806-
// Warn if the next sibling is neither a comma nor the correct closing delimiter
807-
check_call_like_next_sibling(child, &call_type, context, diagnostics)?;
808-
809749
// Recurse into `value`s
810750
if let Some(value) = child.child_by_field_name("value") {
811751
recurse(value, context, diagnostics)?;
@@ -1111,23 +1051,6 @@ foo
11111051
})
11121052
}
11131053

1114-
#[test]
1115-
fn test_expression_after_call_argument() {
1116-
r_task(|| {
1117-
let text = "match(1, 2 3)";
1118-
let document = Document::new(text, None);
1119-
1120-
let diagnostics = generate_diagnostics(document, DEFAULT_STATE.clone());
1121-
assert_eq!(diagnostics.len(), 1);
1122-
1123-
// Diagnostic highlights between the `2` and `3`
1124-
let diagnostic = diagnostics.get(0).unwrap();
1125-
insta::assert_snapshot!(diagnostic.message);
1126-
assert_eq!(diagnostic.range.start, Position::new(0, 10));
1127-
assert_eq!(diagnostic.range.end, Position::new(0, 11));
1128-
})
1129-
}
1130-
11311054
#[test]
11321055
fn test_no_diagnostic_for_dot_dot_i() {
11331056
r_task(|| {

crates/ark/src/lsp/diagnostics_syntax.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,4 +526,18 @@ function(x {
526526
assert_eq!(diagnostic.range.start, Position::new(3, 0));
527527
assert_eq!(diagnostic.range.end, Position::new(3, 1));
528528
}
529+
530+
#[test]
531+
fn test_repeated_call_arguments_without_delimiter() {
532+
let text = "match(1, 2 3)";
533+
534+
let diagnostics = text_diagnostics(text);
535+
assert_eq!(diagnostics.len(), 1);
536+
537+
// Diagnostic highlights the `2`
538+
let diagnostic = diagnostics.get(0).unwrap();
539+
insta::assert_snapshot!(diagnostic.message);
540+
assert_eq!(diagnostic.range.start, Position::new(0, 9));
541+
assert_eq!(diagnostic.range.end, Position::new(0, 10));
542+
}
529543
}

crates/ark/src/lsp/snapshots/ark__lsp__diagnostics__tests__expression_after_call_argument.snap

Lines changed: 0 additions & 5 deletions
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: crates/ark/src/lsp/diagnostics_syntax.rs
3+
expression: diagnostic.message
4+
---
5+
Syntax error

0 commit comments

Comments
 (0)