Skip to content

Commit 65ee394

Browse files
committed
refactor(rules): extract crawl logic into standalone functions
1 parent 31f1cf5 commit 65ee394

File tree

14 files changed

+70
-89
lines changed

14 files changed

+70
-89
lines changed

Cargo.lock

-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/lib/Cargo.toml

+2-3
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ python = ["pyo3", "sqruff-lib-core/serde"]
4444
sqruff-lib-core.workspace = true
4545
sqruff-lib-dialects.workspace = true
4646

47-
dyn-clone = "1"
4847
fancy-regex = "0.14.0"
4948
itertools = "0.14.0"
5049
regex = "1"
@@ -91,5 +90,5 @@ expect-test = "1.5"
9190
glob = "0.3"
9291
serde_json = "1"
9392
serde_with = "3.9"
94-
sqruff-lib-core = { path="../lib-core", features = ["serde"]}
95-
pyo3 = { version = "0.24.2", features=["auto-initialize"]}
93+
sqruff-lib-core = { path = "../lib-core", features = ["serde"] }
94+
pyo3 = { version = "0.24.2", features = ["auto-initialize"] }

crates/lib/src/core/linter/core.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ impl Linter {
326326
continue;
327327
}
328328

329-
let linting_errors = rule.crawl(
329+
let linting_errors = crate::core::rules::base::crawl(
330+
rule,
330331
tables,
331332
&self.config.dialect,
332333
templated_file,

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

+56-68
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ impl LintResult {
5656
}
5757
}
5858

59-
pub fn to_linting_error(&self, rule: ErasedRule, fixes: Vec<LintFix>) -> Option<SQLLintError> {
59+
pub fn to_linting_error(&self, rule: &ErasedRule) -> Option<SQLLintError> {
6060
let anchor = self.anchor.clone()?;
6161

6262
let description = self
@@ -66,7 +66,7 @@ impl LintResult {
6666

6767
let is_fixable = rule.is_fix_compatible();
6868

69-
SQLLintError::new(description.as_str(), anchor, is_fixable, fixes)
69+
SQLLintError::new(description.as_str(), anchor, is_fixable, self.fixes.clone())
7070
.config(|this| {
7171
this.rule = Some(ErrorStructRule {
7272
name: rule.name(),
@@ -107,23 +107,13 @@ impl Debug for LintResult {
107107
}
108108
}
109109

110-
pub trait CloneRule {
111-
fn erased(&self) -> ErasedRule;
112-
}
113-
114-
impl<T: Rule> CloneRule for T {
115-
fn erased(&self) -> ErasedRule {
116-
dyn_clone::clone(self).erased()
117-
}
118-
}
119-
120110
#[derive(Debug, Clone, PartialEq)]
121111
pub enum LintPhase {
122112
Main,
123113
Post,
124114
}
125115

126-
pub trait Rule: CloneRule + dyn_clone::DynClone + Debug + 'static + Send + Sync {
116+
pub trait Rule: Debug + 'static + Send + Sync {
127117
fn load_from_config(&self, _config: &AHashMap<String, Value>) -> Result<ErasedRule, String>;
128118

129119
fn lint_phase(&self) -> LintPhase {
@@ -171,74 +161,72 @@ pub trait Rule: CloneRule + dyn_clone::DynClone + Debug + 'static + Send + Sync
171161
}
172162

173163
fn crawl_behaviour(&self) -> Crawler;
164+
}
174165

175-
fn crawl(
176-
&self,
177-
tables: &Tables,
178-
dialect: &Dialect,
179-
templated_file: &TemplatedFile,
180-
tree: ErasedSegment,
181-
config: &FluffConfig,
182-
) -> Vec<SQLLintError> {
183-
let mut root_context = RuleContext::new(tables, dialect, config, tree.clone());
184-
let mut vs = Vec::new();
185-
186-
// TODO Will to return a note that rules were skipped
187-
if self.dialect_skip().contains(&dialect.name) && !self.force_enable() {
188-
return Vec::new();
189-
}
166+
pub fn crawl(
167+
rule: &ErasedRule,
168+
tables: &Tables,
169+
dialect: &Dialect,
170+
templated_file: &TemplatedFile,
171+
tree: ErasedSegment,
172+
config: &FluffConfig,
173+
) -> Vec<SQLLintError> {
174+
let mut root_context = RuleContext::new(tables, dialect, config, tree.clone());
175+
let mut vs = Vec::new();
176+
177+
// TODO Will to return a note that rules were skipped
178+
if rule.dialect_skip().contains(&dialect.name) && !rule.force_enable() {
179+
return Vec::new();
180+
}
190181

191-
self.crawl_behaviour().crawl(&mut root_context, &mut |context| {
192-
let resp =
193-
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| self.eval(context)));
182+
rule.crawl_behaviour().crawl(&mut root_context, &mut |context| {
183+
let resp =
184+
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| rule.eval(context)));
194185

195-
let resp = match resp {
196-
Ok(t) => t,
197-
Err(_) => {
198-
vs.push(SQLLintError::new("Unexpected exception. Could you open an issue at https://github.com/quarylabs/sqruff", tree.clone(), false, vec![]));
199-
Vec::new()
200-
}
201-
};
186+
let resp = match resp {
187+
Ok(t) => t,
188+
Err(_) => {
189+
vs.push(SQLLintError::new("Unexpected exception. Could you open an issue at https://github.com/quarylabs/sqruff", tree.clone(), false, vec![]));
190+
Vec::new()
191+
}
192+
};
202193

203-
let mut new_lerrs = Vec::new();
194+
let mut new_lerrs = Vec::new();
204195

205-
if resp.is_empty() {
206-
// Assume this means no problems (also means no memory)
207-
} else {
208-
for elem in resp {
209-
self.process_lint_result(elem, templated_file, &mut new_lerrs);
210-
}
196+
if resp.is_empty() {
197+
// Assume this means no problems (also means no memory)
198+
} else {
199+
for elem in resp {
200+
process_lint_result(rule, elem, templated_file, &mut new_lerrs);
211201
}
202+
}
212203

213-
// Consume the new results
214-
vs.extend(new_lerrs);
215-
});
204+
// Consume the new results
205+
vs.extend(new_lerrs);
206+
});
216207

217-
vs
218-
}
208+
vs
209+
}
219210

220-
fn process_lint_result(
221-
&self,
222-
res: LintResult,
223-
templated_file: &TemplatedFile,
224-
new_lerrs: &mut Vec<SQLLintError>,
225-
) {
226-
if res
227-
.fixes
228-
.iter()
229-
.any(|it| it.has_template_conflicts(templated_file))
230-
{
231-
return;
232-
}
211+
fn process_lint_result(
212+
rule: &ErasedRule,
213+
res: LintResult,
214+
templated_file: &TemplatedFile,
215+
new_lerrs: &mut Vec<SQLLintError>,
216+
) {
217+
if res
218+
.fixes
219+
.iter()
220+
.any(|it| it.has_template_conflicts(templated_file))
221+
{
222+
return;
223+
}
233224

234-
if let Some(lerr) = res.to_linting_error(self.erased(), res.fixes.clone()) {
235-
new_lerrs.push(lerr);
236-
}
225+
if let Some(lerr) = res.to_linting_error(rule) {
226+
new_lerrs.push(lerr);
237227
}
238228
}
239229

240-
dyn_clone::clone_trait_object!(Rule);
241-
242230
#[derive(Debug, Clone)]
243231
pub struct ErasedRule {
244232
erased: Arc<dyn Rule>,

crates/lib/src/rules/ambiguous/am01.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet};
33
use sqruff_lib_core::parser::segments::base::ErasedSegment;
44

55
use crate::core::config::Value;
6-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
6+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
77
use crate::core::rules::context::RuleContext;
88
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
99
use crate::utils::functional::context::FunctionalContext;

crates/lib/src/rules/ambiguous/am02.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use sqruff_lib_core::lint_fix::LintFix;
55
use sqruff_lib_core::parser::segments::base::SegmentBuilder;
66

77
use crate::core::config::Value;
8-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
8+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
99
use crate::core::rules::context::RuleContext;
1010
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1111

crates/lib/src/rules/ambiguous/am03.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use sqruff_lib_core::lint_fix::LintFix;
55
use sqruff_lib_core::parser::segments::base::{ErasedSegment, SegmentBuilder};
66

77
use crate::core::config::Value;
8-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
8+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
99
use crate::core::rules::context::RuleContext;
1010
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1111

crates/lib/src/rules/ambiguous/am06.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use ahash::AHashMap;
22
use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet};
33

44
use crate::core::config::Value;
5-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
5+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
66
use crate::core::rules::context::RuleContext;
77
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
88
use crate::utils::functional::context::FunctionalContext;

crates/lib/src/rules/capitalisation/cp04.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet};
44

55
use super::cp01::RuleCP01;
66
use crate::core::config::Value;
7-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
7+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
88
use crate::core::rules::context::RuleContext;
99
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1010

crates/lib/src/rules/references/rf02.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet};
77
use sqruff_lib_core::parser::segments::object_reference::ObjectReferenceSegment;
88

99
use crate::core::config::Value;
10-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
10+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
1111
use crate::core::rules::context::RuleContext;
1212
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1313
use crate::rules::aliasing::al04::RuleAL04;

crates/lib/src/rules/references/rf04.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use itertools::Itertools;
22
use regex::Regex;
33
use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet};
44

5-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
5+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
66
use crate::core::rules::context::RuleContext;
77
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
88
use crate::utils::identifers::identifiers_policy_applicable;

crates/lib/src/rules/structure/st04.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use sqruff_lib_core::parser::segments::base::{ErasedSegment, SegmentBuilder, Tab
77
use sqruff_lib_core::utils::functional::segments::Segments;
88

99
use crate::core::config::Value;
10-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
10+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
1111
use crate::core::rules::context::RuleContext;
1212
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1313
use crate::utils::functional::context::FunctionalContext;

crates/lib/src/rules/structure/st06.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use sqruff_lib_core::lint_fix::LintFix;
77
use sqruff_lib_core::parser::segments::base::ErasedSegment;
88

99
use crate::core::config::Value;
10-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
10+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
1111
use crate::core::rules::context::RuleContext;
1212
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1313

crates/lib/src/rules/structure/st07.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use sqruff_lib_core::utils::analysis::select::get_select_statement_info;
99
use sqruff_lib_core::utils::functional::segments::Segments;
1010

1111
use crate::core::config::Value;
12-
use crate::core::rules::base::{CloneRule, ErasedRule, LintResult, Rule, RuleGroups};
12+
use crate::core::rules::base::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
1313
use crate::core::rules::context::RuleContext;
1414
use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
1515
use crate::utils::functional::context::FunctionalContext;

0 commit comments

Comments
 (0)