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

chore: reduce Binding variant usages #132

Merged
merged 3 commits into from
Apr 2, 2024
Merged

chore: reduce Binding variant usages #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6edf1d2
Reduce Binding variant usages
arendjr Apr 2, 2024
8321813
One more case
arendjr Apr 2, 2024
dce29c7
Oops
arendjr Apr 2, 2024
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
77 changes: 43 additions & 34 deletions crates/core/src/binding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ pub enum Binding<'a> {
impl PartialEq for Binding<'_> {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Binding::Empty(_, _, _), Binding::Empty(_, _, _)) => true,
(Binding::Node(src1, n1), Binding::Node(src2, n2)) => {
(Self::Empty(_, _, _), Self::Empty(_, _, _)) => true,
(Self::Node(src1, n1), Self::Node(src2, n2)) => {
n1.utf8_text(src1.as_bytes()) == n2.utf8_text(src2.as_bytes())
}
(Binding::String(src1, r1), Binding::String(src2, r2)) => {
(Self::String(src1, r1), Self::String(src2, r2)) => {
src1[r1.start_byte as usize..r1.end_byte as usize]
== src2[r2.start_byte as usize..r2.end_byte as usize]
}
(Binding::List(_, n1, f1), Binding::List(_, n2, f2)) => n1 == n2 && f1 == f2,
(Binding::ConstantRef(c1), Binding::ConstantRef(c2)) => c1 == c2,
(Self::List(_, n1, f1), Self::List(_, n2, f2)) => n1 == n2 && f1 == f2,
(Self::ConstantRef(c1), Self::ConstantRef(c2)) => c1 == c2,
_ => false,
}
}
Expand Down Expand Up @@ -307,10 +307,22 @@ pub(crate) fn linearize_binding<'a>(
}

impl<'a> Binding<'a> {
pub(crate) fn from_constant(constant: &'a Constant) -> Self {
Self::ConstantRef(constant)
}

pub(crate) fn from_node(node: NodeWithSource<'a>) -> Self {
Self::Node(node.source, node.node)
}

pub(crate) fn from_path(path: &'a Path) -> Self {
Self::FileName(path)
}

pub(crate) fn from_range(range: Range, source: &'a str) -> Self {
Self::String(source, range)
}

/// Returns the only node bound by this binding.
///
/// This includes list bindings that only match a single child.
Expand Down Expand Up @@ -339,8 +351,8 @@ impl<'a> Binding<'a> {

pub fn get_sexp(&self) -> Option<String> {
match self {
Binding::Node(_, node) => Some(node.to_sexp().to_string()),
Binding::List(_, parent_node, field_id) => {
Self::Node(_, node) => Some(node.to_sexp().to_string()),
Self::List(_, parent_node, field_id) => {
let mut cursor = parent_node.walk();
let mut children = parent_node.children_by_field_id(*field_id, &mut cursor);
let mut result = String::new();
Expand All @@ -353,20 +365,17 @@ impl<'a> Binding<'a> {
}
Some(result)
}
Binding::String(..)
| Binding::FileName(_)
| Binding::Empty(..)
| Binding::ConstantRef(_) => None,
Self::String(..) | Self::FileName(_) | Self::Empty(..) | Self::ConstantRef(_) => None,
}
}

// todo implement for empty and empty list
pub fn position(&self) -> Option<Range> {
match self {
Binding::Empty(_, _, _) => None,
Binding::Node(_, node) => Some(Range::from(node.range())),
Binding::String(_, range) => Some(range.to_owned()),
Binding::List(_, parent_node, field_id) => {
Self::Empty(_, _, _) => None,
Self::Node(_, node) => Some(Range::from(node.range())),
Self::String(_, range) => Some(range.to_owned()),
Self::List(_, parent_node, field_id) => {
let mut cursor = parent_node.walk();
let mut children = parent_node.children_by_field_id(*field_id, &mut cursor);

Expand Down Expand Up @@ -408,8 +417,8 @@ impl<'a> Binding<'a> {
}
}
}
Binding::FileName(_) => None,
Binding::ConstantRef(_) => None,
Self::FileName(_) => None,
Self::ConstantRef(_) => None,
}
}

Expand All @@ -423,8 +432,8 @@ impl<'a> Binding<'a> {
logs: &mut AnalysisLogs,
) -> Result<Cow<'a, str>> {
let res: Result<Cow<'a, str>> = match self {
Binding::Empty(_, _, _) => Ok(Cow::Borrowed("")),
Binding::Node(source, node) => {
Self::Empty(_, _, _) => Ok(Cow::Borrowed("")),
Self::Node(source, node) => {
let range = CodeRange::from_node(source, node);
linearize_binding(
language,
Expand All @@ -440,11 +449,11 @@ impl<'a> Binding<'a> {
}
// can't linearize until we update source to point to the entire file
// otherwise file file pointers won't match
Binding::String(s, r) => Ok(Cow::Owned(
Self::String(s, r) => Ok(Cow::Owned(
s[r.start_byte as usize..r.end_byte as usize].into(),
)),
Binding::FileName(s) => Ok(Cow::Owned(s.to_string_lossy().into())),
Binding::List(source, _parent_node, _field_id) => {
Self::FileName(s) => Ok(Cow::Owned(s.to_string_lossy().into())),
Self::List(source, _parent_node, _field_id) => {
if let Some(pos) = self.position() {
let range = CodeRange::new(pos.start_byte, pos.end_byte, source);
linearize_binding(
Expand All @@ -462,37 +471,37 @@ impl<'a> Binding<'a> {
Ok("".into())
}
}
Binding::ConstantRef(c) => Ok(Cow::Owned(c.to_string())),
Self::ConstantRef(c) => Ok(Cow::Owned(c.to_string())),
};
res
}

pub fn text(&self) -> String {
match self {
Binding::Empty(_, _, _) => "".to_string(),
Binding::Node(source, node) => {
Self::Empty(_, _, _) => "".to_string(),
Self::Node(source, node) => {
NodeWithSource::new(node.clone(), source).text().to_string()
}
Binding::String(s, r) => s[r.start_byte as usize..r.end_byte as usize].into(),
Binding::FileName(s) => s.to_string_lossy().into(),
Binding::List(source, _, _) => {
Self::String(s, r) => s[r.start_byte as usize..r.end_byte as usize].into(),
Self::FileName(s) => s.to_string_lossy().into(),
Self::List(source, _, _) => {
if let Some(pos) = self.position() {
source[pos.start_byte as usize..pos.end_byte as usize].to_string()
} else {
"".to_string()
}
}
Binding::ConstantRef(c) => c.to_string(),
Self::ConstantRef(c) => c.to_string(),
}
}

pub fn source(&self) -> Option<&'a str> {
match self {
Binding::Empty(source, _, _) => Some(source),
Binding::Node(source, _) => Some(source),
Binding::String(source, _) => Some(source),
Binding::List(source, _, _) => Some(source),
Binding::FileName(..) | Binding::ConstantRef(..) => None,
Self::Empty(source, _, _) => Some(source),
Self::Node(source, _) => Some(source),
Self::String(source, _) => Some(source),
Self::List(source, _, _) => Some(source),
Self::FileName(..) | Self::ConstantRef(..) => None,
}
}

Expand Down
5 changes: 2 additions & 3 deletions crates/core/src/pattern/after.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ use super::{
variable::VariableSourceLocations,
Node, State,
};
use crate::{binding::Binding, context::Context, resolve};
use crate::{binding::Constant, errors::debug};
use crate::{context::Context, resolve};
use anyhow::{anyhow, bail, Result};
use core::fmt::Debug;
use grit_util::AstNode;
use im::vector;
use marzano_util::analysis_logs::AnalysisLogs;
use std::collections::BTreeMap;

Expand Down Expand Up @@ -61,7 +60,7 @@ impl After {
};

if let Some(next) = node.next_non_trivia_node() {
Ok(ResolvedPattern::Binding(vector![Binding::from_node(next)]))
Ok(ResolvedPattern::from_node(next))
} else {
debug(
logs,
Expand Down
12 changes: 3 additions & 9 deletions crates/core/src/pattern/ast_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::{
variable::VariableSourceLocations,
State,
};
use crate::{binding::Binding, context::Context, resolve};
use crate::{context::Context, resolve};
use anyhow::{anyhow, Result};
use itertools::Itertools;
use marzano_language::language::{FieldId, Language, SortId};
Expand Down Expand Up @@ -163,12 +163,7 @@ impl Matcher for ASTNode {
return Ok(false);
};
if binding.is_list() {
return self.execute(
&ResolvedPattern::from_binding(Binding::from_node(node)),
init_state,
context,
logs,
);
return self.execute(&ResolvedPattern::from_node(node), init_state, context, logs);
}

let NodeWithSource { node, source } = node;
Expand All @@ -181,10 +176,9 @@ impl Matcher for ASTNode {
if context.language().is_comment(self.sort) {
let content = context.language().comment_text(&node, source);
let content = resolve!(content);
let content = Binding::String(source, content.1);

return self.args[0].2.execute(
&ResolvedPattern::from_binding(content),
&ResolvedPattern::from_range(content.1, source),
init_state,
context,
logs,
Expand Down
5 changes: 2 additions & 3 deletions crates/core/src/pattern/before.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ use super::{
variable::VariableSourceLocations,
Node, State,
};
use crate::{binding::Binding, context::Context, resolve};
use crate::{binding::Constant, errors::debug};
use crate::{context::Context, resolve};
use anyhow::{anyhow, bail, Result};
use core::fmt::Debug;
use grit_util::AstNode;
use im::vector;
use marzano_util::analysis_logs::AnalysisLogs;
use std::collections::BTreeMap;

Expand Down Expand Up @@ -61,7 +60,7 @@ impl Before {
};

if let Some(prev) = node.previous_non_trivia_node() {
Ok(ResolvedPattern::Binding(vector![Binding::from_node(prev)]))
Ok(ResolvedPattern::from_node(prev))
} else {
debug(
logs,
Expand Down
4 changes: 2 additions & 2 deletions crates/core/src/pattern/list_index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@

let len = list_items.clone().count();
let index = resolve_opt!(to_unsigned(index, len));
return Ok(list_items
Ok(list_items
.nth(index)
.map(|_| PatternOrResolvedMut::_ResolvedBinding));
.map(|_| PatternOrResolvedMut::_ResolvedBinding))

Check failure on line 173 in crates/core/src/pattern/list_index.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement 

error: unneeded `return` statement
   --> crates/core/src/pattern/list_index.rs:171:21
    |
171 | /                     return Ok(list_items
172 | |                         .nth(index)
173 | |                         .map(|_| PatternOrResolvedMut::_ResolvedBinding));
    | |_________________________________________________________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
    = note: `-D clippy::needless-return` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::needless_return)]`
help: remove `return`
    |
171 ~                     Ok(list_items
172 +                         .nth(index)
173 ~                         .map(|_| PatternOrResolvedMut::_ResolvedBinding))
    |
}
Some(PatternOrResolvedMut::Resolved(ResolvedPattern::List(l))) => {
let index = resolve_opt!(to_unsigned(index, l.len()));
Expand Down
6 changes: 3 additions & 3 deletions crates/core/src/pattern/or.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ impl Matcher for Or {
if let ResolvedPattern::Binding(binding_vector) = &binding {
for p in self.patterns.iter() {
// filter out pattern which cannot match because of a mismatched node type
if let (Binding::Node(_src, binding_node), Pattern::ASTNode(node_pattern)) =
(binding_vector.last().unwrap(), p)
if let (Some(binding_node), Pattern::ASTNode(node_pattern)) =
(binding_vector.last().and_then(Binding::as_node), p)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not documented but the binding vector in a resolved pattern should be wrapped in a non-empty-vec struct as the vector is always created with an initial element, and only ever appended to, so last shouldn't really be an optional here .

{
if node_pattern.sort != binding_node.kind_id() {
if node_pattern.sort != binding_node.node.kind_id() {
continue;
}
}
Expand Down
5 changes: 2 additions & 3 deletions crates/core/src/pattern/regex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ use super::{
variable::{Variable, VariableSourceLocations},
State,
};
use crate::{binding::Binding, context::Context};
use crate::context::Context;
use anyhow::{anyhow, bail, Result};
use core::fmt::Debug;
use im::vector;
use marzano_language::{language::Language, target_language::TargetLanguage};
use marzano_util::analysis_logs::{AnalysisLogBuilder, AnalysisLogs};
use marzano_util::position::Range;
Expand Down Expand Up @@ -201,7 +200,7 @@ impl RegexPattern {
// have a Range<usize> for String bindings?
position.end_byte = position.start_byte + range.end as u32;
position.start_byte += range.start as u32;
ResolvedPattern::Binding(vector![Binding::String(source, position)])
ResolvedPattern::from_range(position, source)
} else {
ResolvedPattern::from_string(value.to_string())
}
Expand Down
38 changes: 23 additions & 15 deletions crates/core/src/pattern/resolved_pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use marzano_util::{
use std::{
borrow::Cow,
collections::{BTreeMap, HashMap},
path::Path,
};
use tree_sitter::Node;

Expand Down Expand Up @@ -56,9 +57,7 @@ impl<'a> File<'a> {
pub(crate) fn name(&self, files: &FileRegistry<'a>) -> ResolvedPattern<'a> {
match self {
File::Resolved(resolved) => resolved.name.clone(),
File::Ptr(ptr) => {
ResolvedPattern::Binding(vector![Binding::FileName(&files.get_file(*ptr).name)])
}
File::Ptr(ptr) => ResolvedPattern::from_path(&files.get_file(*ptr).name),
}
}

Expand All @@ -69,9 +68,9 @@ impl<'a> File<'a> {
let absolute_path = absolutize(name.as_ref())?;
Ok(ResolvedPattern::Constant(Constant::String(absolute_path)))
}
File::Ptr(ptr) => Ok(ResolvedPattern::Binding(vector![Binding::FileName(
&files.get_file(*ptr).absolute_path
)])),
File::Ptr(ptr) => Ok(ResolvedPattern::from_path(
&files.get_file(*ptr).absolute_path,
)),
}
}

Expand All @@ -81,7 +80,7 @@ impl<'a> File<'a> {
File::Ptr(ptr) => {
let file = &files.get_file(*ptr);
let range = file.tree.root_node().range().into();
ResolvedPattern::Binding(vector![Binding::String(&file.source, range)])
ResolvedPattern::from_range(range, &file.source)
}
}
}
Expand All @@ -92,7 +91,7 @@ impl<'a> File<'a> {
File::Ptr(ptr) => {
let file = &files.get_file(*ptr);
let node = file.tree.root_node();
ResolvedPattern::Binding(vector![Binding::Node(&file.source, node)])
ResolvedPattern::from_node(NodeWithSource::new(node, &file.source))
}
}
}
Expand Down Expand Up @@ -418,19 +417,27 @@ impl<'a> ResolvedPattern<'a> {
}

pub fn from_constant(constant: &'a Constant) -> Self {
Self::Binding(vector![Binding::ConstantRef(constant)])
Self::from_binding(Binding::from_constant(constant))
}

pub(crate) fn from_node(node: NodeWithSource<'a>) -> Self {
Self::Binding(vector![Binding::Node(node.source, node.node)])
Self::from_binding(Binding::from_node(node))
}

pub(crate) fn from_list(src: &'a str, node: Node<'a>, field_id: FieldId) -> Self {
Self::Binding(vector![Binding::List(src, node, field_id)])
Self::from_binding(Binding::List(src, node, field_id))
}

pub(crate) fn empty_field(src: &'a str, node: Node<'a>, field_id: FieldId) -> Self {
Self::Binding(vector![Binding::Empty(src, node, field_id)])
Self::from_binding(Binding::Empty(src, node, field_id))
}

pub(crate) fn from_path(path: &'a Path) -> Self {
Self::from_binding(Binding::from_path(path))
}

pub(crate) fn from_range(range: Range, src: &'a str) -> Self {
Self::from_binding(Binding::from_range(range, src))
}

pub fn from_string(string: String) -> Self {
Expand Down Expand Up @@ -871,9 +878,10 @@ impl<'a> ResolvedPattern<'a> {

pub(crate) fn matches_undefined(&self) -> bool {
match self {
ResolvedPattern::Binding(b) => {
matches!(b.last(), Some(Binding::ConstantRef(Constant::Undefined)))
}
ResolvedPattern::Binding(b) => b
.last()
.and_then(Binding::as_constant)
.map_or(false, |c| c == &Constant::Undefined),
ResolvedPattern::Constant(Constant::Undefined) => true,
ResolvedPattern::Constant(_)
| ResolvedPattern::Snippets(_)
Expand Down
Loading
Loading