Skip to content

Commit f6ac633

Browse files
gvozdvmozgubenfdking
gvozdvmozgu
authored and
benfdking
committed
refactor: remove all RuleContext clones, use references instead
1 parent 123af47 commit f6ac633

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+138
-154
lines changed

‎crates/lib/src/core/rules/base.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ pub trait Rule: CloneRule + dyn_clone::DynClone + Debug + 'static + Send + Sync
164164
.unwrap_or(name)
165165
}
166166

167-
fn eval(&self, rule_cx: RuleContext) -> Vec<LintResult>;
167+
fn eval(&self, rule_cx: &RuleContext) -> Vec<LintResult>;
168168

169169
fn is_fix_compatible(&self) -> bool {
170170
false
@@ -180,15 +180,15 @@ pub trait Rule: CloneRule + dyn_clone::DynClone + Debug + 'static + Send + Sync
180180
tree: ErasedSegment,
181181
config: &FluffConfig,
182182
) -> (Vec<SQLLintError>, Vec<LintFix>) {
183-
let root_context = RuleContext::new(tables, dialect, config, tree.clone());
183+
let mut root_context = RuleContext::new(tables, dialect, config, tree.clone());
184184
let mut vs = Vec::new();
185185

186186
// TODO Will to return a note that rules were skipped
187187
if self.dialect_skip().contains(&dialect.name) && !self.force_enable() {
188188
return Vec::new();
189189
}
190190

191-
self.crawl_behaviour().crawl(root_context, &mut |context| {
191+
self.crawl_behaviour().crawl(&mut root_context, &mut |context| {
192192
let resp =
193193
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| self.eval(context)));
194194

‎crates/lib/src/core/rules/context.rs

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,8 @@ use sqruff_lib_core::templaters::base::TemplatedFile;
99

1010
use crate::core::config::FluffConfig;
1111

12-
#[derive(Clone, Debug)]
12+
#[derive(Debug)]
1313
pub struct RuleContext<'a> {
14-
data: Rc<RuleContextData<'a>>,
15-
}
16-
17-
#[derive(Clone, Debug)]
18-
pub struct RuleContextData<'a> {
1914
pub tables: &'a Tables,
2015
pub dialect: &'a Dialect,
2116
pub templated_file: Option<TemplatedFile>,
@@ -35,18 +30,9 @@ pub struct RuleContextData<'a> {
3530
pub segment_idx: usize,
3631
}
3732

38-
impl<'a> std::ops::Deref for RuleContext<'a> {
39-
type Target = RuleContextData<'a>;
40-
41-
fn deref(&self) -> &Self::Target {
42-
&self.data
43-
}
44-
}
45-
46-
impl std::ops::DerefMut for RuleContext<'_> {
47-
fn deref_mut(&mut self) -> &mut Self::Target {
48-
Rc::make_mut(&mut self.data)
49-
}
33+
pub struct Checkpoint {
34+
parent_stack: usize,
35+
raw_stack: usize,
5036
}
5137

5238
impl<'a> RuleContext<'a> {
@@ -57,22 +43,31 @@ impl<'a> RuleContext<'a> {
5743
segment: ErasedSegment,
5844
) -> Self {
5945
Self {
60-
data: RuleContextData {
61-
tables,
62-
dialect,
63-
config,
64-
segment,
65-
templated_file: <_>::default(),
66-
path: <_>::default(),
67-
parent_stack: <_>::default(),
68-
raw_stack: <_>::default(),
69-
memory: Rc::new(RefCell::new(AHashMap::new())),
70-
segment_idx: 0,
71-
}
72-
.into(),
46+
tables,
47+
dialect,
48+
config,
49+
segment,
50+
templated_file: <_>::default(),
51+
path: <_>::default(),
52+
parent_stack: <_>::default(),
53+
raw_stack: <_>::default(),
54+
memory: Rc::new(RefCell::new(AHashMap::new())),
55+
segment_idx: 0,
56+
}
57+
}
58+
59+
pub fn checkpoint(&self) -> Checkpoint {
60+
Checkpoint {
61+
parent_stack: self.parent_stack.len(),
62+
raw_stack: self.raw_stack.len(),
7363
}
7464
}
7565

66+
pub fn restore(&mut self, checkpoint: Checkpoint) {
67+
self.parent_stack.truncate(checkpoint.parent_stack);
68+
self.raw_stack.truncate(checkpoint.raw_stack);
69+
}
70+
7671
pub fn try_get<T: Clone + 'static>(&self) -> Option<T> {
7772
let id = TypeId::of::<T>();
7873

‎crates/lib/src/core/rules/crawlers.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub trait BaseCrawler {
1414
self.works_on_unparsable() || !segment.is_type(SyntaxKind::Unparsable)
1515
}
1616

17-
fn crawl<'a>(&self, context: RuleContext<'a>, f: &mut impl FnMut(RuleContext<'a>));
17+
fn crawl<'a>(&self, context: &mut RuleContext<'a>, f: &mut impl FnMut(&RuleContext<'a>));
1818
}
1919

2020
#[enum_dispatch(BaseCrawler)]
@@ -32,7 +32,7 @@ pub enum Crawler {
3232
pub struct RootOnlyCrawler;
3333

3434
impl BaseCrawler for RootOnlyCrawler {
35-
fn crawl<'a>(&self, context: RuleContext<'a>, f: &mut impl FnMut(RuleContext<'a>)) {
35+
fn crawl<'a>(&self, context: &mut RuleContext<'a>, f: &mut impl FnMut(&RuleContext<'a>)) {
3636
if self.passes_filter(&context.segment) {
3737
f(context);
3838
}
@@ -70,12 +70,12 @@ impl SegmentSeekerCrawler {
7070
}
7171

7272
impl BaseCrawler for SegmentSeekerCrawler {
73-
fn crawl<'a>(&self, mut context: RuleContext<'a>, f: &mut impl FnMut(RuleContext<'a>)) {
73+
fn crawl<'a>(&self, context: &mut RuleContext<'a>, f: &mut impl FnMut(&RuleContext<'a>)) {
7474
let mut self_match = false;
7575

7676
if self.is_self_match(&context.segment) {
7777
self_match = true;
78-
f(context.clone());
78+
f(context);
7979
}
8080

8181
if context.segment.segments().is_empty() || (self_match && !self.allow_recurse) {
@@ -96,26 +96,30 @@ impl BaseCrawler for SegmentSeekerCrawler {
9696
for (idx, child) in segment.segments().iter().enumerate() {
9797
context.segment = child.clone();
9898
context.segment_idx = idx;
99-
100-
self.crawl(context.clone(), f);
99+
let checkpoint = context.checkpoint();
100+
self.crawl(context, f);
101+
context.restore(checkpoint);
101102
}
102103
}
103104
}
104105

105106
pub struct TokenSeekerCrawler;
106107

107108
impl BaseCrawler for TokenSeekerCrawler {
108-
fn crawl<'a>(&self, mut context: RuleContext<'a>, f: &mut impl FnMut(RuleContext<'a>)) {
109+
fn crawl<'a>(&self, context: &mut RuleContext<'a>, f: &mut impl FnMut(&RuleContext<'a>)) {
109110
if context.segment.segments().is_empty() {
110-
f(context.clone());
111+
f(context);
111112
}
112113

113114
let segment = context.segment.clone();
114115
context.parent_stack.push(segment.clone());
115116
for (idx, child) in segment.segments().iter().enumerate() {
116117
context.segment = child.clone();
117118
context.segment_idx = idx;
118-
self.crawl(context.clone(), f);
119+
120+
let checkpoint = context.checkpoint();
121+
self.crawl(context, f);
122+
context.restore(checkpoint);
119123
}
120124
}
121125
}

‎crates/lib/src/rules/aliasing/al01.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ FROM foo AS voo
102102
&[RuleGroups::All, RuleGroups::Aliasing]
103103
}
104104

105-
fn eval(&self, rule_cx: RuleContext) -> Vec<LintResult> {
105+
fn eval(&self, rule_cx: &RuleContext) -> Vec<LintResult> {
106106
let last_seg = rule_cx.parent_stack.last().unwrap();
107107
let last_seg_ty = last_seg.get_type();
108108

‎crates/lib/src/rules/aliasing/al02.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ FROM foo
8383
&[RuleGroups::All, RuleGroups::Core, RuleGroups::Aliasing]
8484
}
8585

86-
fn eval(&self, context: RuleContext) -> Vec<LintResult> {
87-
if FunctionalContext::new(context.clone())
86+
fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
87+
if FunctionalContext::new(context)
8888
.segment()
8989
.children(None)
9090
.last()

‎crates/lib/src/rules/aliasing/al03.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ FROM foo
5555
&[RuleGroups::All, RuleGroups::Core, RuleGroups::Aliasing]
5656
}
5757

58-
fn eval(&self, context: RuleContext) -> Vec<LintResult> {
59-
let functional_context = FunctionalContext::new(context.clone());
58+
fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
59+
let functional_context = FunctionalContext::new(context);
6060
let segment = functional_context.segment();
6161
let children = segment.children(None);
6262

‎crates/lib/src/rules/aliasing/al04.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ FROM
100100
&[RuleGroups::All, RuleGroups::Core, RuleGroups::Aliasing]
101101
}
102102

103-
fn eval(&self, context: RuleContext) -> Vec<LintResult> {
103+
fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
104104
let Some(select_info) =
105105
get_select_statement_info(&context.segment, context.dialect.into(), true)
106106
else {

‎crates/lib/src/rules/aliasing/al05.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ FROM foo
7474
&[RuleGroups::All, RuleGroups::Core, RuleGroups::Aliasing]
7575
}
7676

77-
fn eval(&self, context: RuleContext) -> Vec<LintResult> {
77+
fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
7878
let mut violations = Vec::new();
7979
let select_info = get_select_statement_info(&context.segment, context.dialect.into(), true);
8080

‎crates/lib/src/rules/aliasing/al06.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,8 @@ JOIN
144144
&[RuleGroups::All, RuleGroups::Core, RuleGroups::Aliasing]
145145
}
146146

147-
fn eval(&self, context: RuleContext) -> Vec<LintResult> {
148-
let children = FunctionalContext::new(context.clone())
149-
.segment()
150-
.children(None);
147+
fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
148+
let children = FunctionalContext::new(context).segment().children(None);
151149
let from_expression_elements = children.recursive_crawl(
152150
const { &SyntaxSet::new(&[SyntaxKind::FromExpressionElement]) },
153151
true,

‎crates/lib/src/rules/aliasing/al07.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -271,14 +271,12 @@ FROM
271271
&[RuleGroups::All, RuleGroups::Aliasing]
272272
}
273273

274-
fn eval(&self, context: RuleContext) -> Vec<LintResult> {
274+
fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
275275
if !self.force_enable {
276276
return Vec::new();
277277
}
278278

279-
let children = FunctionalContext::new(context.clone())
280-
.segment()
281-
.children(None);
279+
let children = FunctionalContext::new(context).segment().children(None);
282280
let from_clause_segment = children
283281
.select(
284282
Some(|it: &ErasedSegment| it.is_type(SyntaxKind::FromClause)),

0 commit comments

Comments
 (0)