Skip to content

Commit

Permalink
chore: clean up long description setup
Browse files Browse the repository at this point in the history
- remove the None trait implementation
- make descriptions non optional
  • Loading branch information
benfdking committed Jun 25, 2024
1 parent e50f905 commit cc3da38
Show file tree
Hide file tree
Showing 55 changed files with 467 additions and 471 deletions.
2 changes: 1 addition & 1 deletion crates/cli/src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct Rule {
pub code: &'static str,
pub description: &'static str,
pub fixable: bool,
pub long_description: Option<&'static str>,
pub long_description: &'static str,
}

impl From<ErasedRule> for Rule {
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/docs/generate_rule_docs_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ The following rules are available in this create. This list is generated from th

**Fixable:** {% if rule.fixable %}Yes{% else %}No{% endif %}

{% if rule.long_description %}{{ rule.long_description }}{% endif %}
{{ rule.long_description }}
{% endfor %}
2 changes: 1 addition & 1 deletion crates/lib/benches/depth_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ SELECT construct_depth_info('uuid-3');"#;

fn depth_map(c: &mut Criterion) {
let linter = Linter::new(FluffConfig::default(), None, None);
let tree = linter.parse_string(COMPLEX_QUERY.into(), None, None, None).unwrap().tree.unwrap();
let tree = linter.parse_string(COMPLEX_QUERY, None, None, None).unwrap().tree.unwrap();

c.bench_function("DepthMap::from_parent", |b| {
b.iter(|| black_box(DepthMap::from_parent(&tree)));
Expand Down
4 changes: 2 additions & 2 deletions crates/lib/benches/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ fn mk_segments<'a>(
config: &FluffConfig,
source: &str,
) -> (ParseContext<'a>, std::sync::Arc<dyn Matchable>, Vec<ErasedSegment>) {
let ctx = ParseContext::new(&dialect, <_>::default());
let ctx = ParseContext::new(dialect, <_>::default());
let segment = dialect.r#ref("FileSegment");
let mut segments = lex(&config, source);
let mut segments = lex(config, source);

if segments.last().unwrap().get_type() == "end_of_file" {
segments.pop();
Expand Down
4 changes: 4 additions & 0 deletions crates/lib/src/cli/formatters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,10 @@ mod tests {
""
}

fn long_description(&self) -> &'static str {
"Ghost rule for testing purposes"
}

fn code(&self) -> &'static str {
"A"
}
Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/core/linter/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ mod tests {
#[test]
fn test__linter__empty_file() {
let linter = Linter::new(FluffConfig::new(<_>::default(), None, None), None, None);
let parsed = linter.parse_string("".into(), None, None, None).unwrap();
let parsed = linter.parse_string("", None, None, None).unwrap();

assert!(parsed.violations.is_empty());
}
Expand Down
4 changes: 2 additions & 2 deletions crates/lib/src/core/parser/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ mod tests {
/// particular string or negative matching (that it explicitly)
/// doesn't match.
fn assert_matches(in_string: &str, matcher: &Matcher, match_string: Option<&str>) {
let res = matcher.matches(&in_string);
let res = matcher.matches(in_string);
if let Some(match_string) = match_string {
assert_eq!(res.forward_string, &in_string[match_string.len()..]);
assert_eq!(res.elements.len(), 1);
Expand Down Expand Up @@ -663,7 +663,7 @@ mod tests {
];

for (raw, reg, res) in tests {
let matcher = Matcher::regex("test", &reg, |_, _| unimplemented!());
let matcher = Matcher::regex("test", reg, |_, _| unimplemented!());

assert_matches(raw, &matcher, Some(res));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/core/parser/parsers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ mod tests {

assert_eq!(
parser.simple(&parse_cx, None),
(AHashSet::new(), ["single_quote".into()].into()).into()
(AHashSet::new(), ["single_quote"].into()).into()
);
}

Expand Down
6 changes: 2 additions & 4 deletions crates/lib/src/core/rules/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,16 +248,14 @@ pub trait Rule: CloneRule + dyn_clone::DynClone + Debug + 'static + Send + Sync

fn name(&self) -> &'static str;

fn long_description(&self) -> Option<&'static str> {
None
}

fn config_ref(&self) -> &'static str {
self.name()
}

fn description(&self) -> &'static str;

fn long_description(&self) -> &'static str;

fn groups(&self) -> &'static [&'static str] {
&["all"]
}
Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/dialects/ansi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6662,7 +6662,7 @@ mod tests {
}

fn parse_sql(linter: &Linter, sql: &str) -> ErasedSegment {
let parsed = linter.parse_string(sql.into(), None, None, None).unwrap();
let parsed = linter.parse_string(sql, None, None, None).unwrap();
parsed.tree.unwrap()
}

Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/dialects/bigquery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2840,7 +2840,7 @@ mod tests {
use crate::helpers;

fn parse_sql(linter: &Linter, sql: &str) -> ErasedSegment {
let parsed = linter.parse_string(sql.into(), None, None, None).unwrap();
let parsed = linter.parse_string(sql, None, None, None).unwrap();
parsed.tree.unwrap()
}

Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/dialects/clickhouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1307,7 +1307,7 @@ mod tests {
use crate::helpers;

fn parse_sql(linter: &Linter, sql: &str) -> ErasedSegment {
let parsed = linter.parse_string(sql.into(), None, None, None).unwrap();
let parsed = linter.parse_string(sql, None, None, None).unwrap();
parsed.tree.unwrap()
}

Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/dialects/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7012,7 +7012,7 @@ mod tests {
use crate::helpers;

fn parse_sql(linter: &Linter, sql: &str) -> ErasedSegment {
let parsed = linter.parse_string(sql.into(), None, None, None).unwrap();
let parsed = linter.parse_string(sql, None, None, None).unwrap();
parsed.tree.unwrap()
}

Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/dialects/snowflake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8616,7 +8616,7 @@ mod tests {
use crate::helpers;

fn parse_sql(linter: &Linter, sql: &str) -> ErasedSegment {
let parsed = linter.parse_string(sql.into(), None, None, None).unwrap();
let parsed = linter.parse_string(sql, None, None, None).unwrap();
parsed.tree.unwrap()
}

Expand Down
53 changes: 26 additions & 27 deletions crates/lib/src/rules/aliasing/AL01.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,28 @@ impl Rule for RuleAL01 {
"Implicit/explicit aliasing of table."
}

fn is_fix_compatible(&self) -> bool {
true
fn long_description(&self) -> &'static str {
r#"
**Anti-pattern**
In this example, the alias `voo` is implicit.
```sql
SELECT
voo.a
FROM foo voo
```
**Best practice**
Add `AS` to make the alias explicit.
```sql
SELECT
voo.a
FROM foo AS voo
```
"#
}

fn eval(&self, rule_cx: RuleContext) -> Vec<LintResult> {
Expand Down Expand Up @@ -129,33 +149,12 @@ impl Rule for RuleAL01 {
Vec::new()
}

fn crawl_behaviour(&self) -> Crawler {
SegmentSeekerCrawler::new(["alias_expression"].into()).into()
fn is_fix_compatible(&self) -> bool {
true
}

fn long_description(&self) -> Option<&'static str> {
r#"
**Anti-pattern**
In this example, the alias `voo` is implicit.
```sql
SELECT
voo.a
FROM foo voo
```
**Best practice**
Add `AS` to make the alias explicit.
```sql
SELECT
voo.a
FROM foo AS voo
```
"#
.into()
fn crawl_behaviour(&self) -> Crawler {
SegmentSeekerCrawler::new(["alias_expression"].into()).into()
}
}

Expand Down
13 changes: 6 additions & 7 deletions crates/lib/src/rules/aliasing/AL02.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ impl Rule for RuleAL02 {
"aliasing.column"
}

fn long_description(&self) -> Option<&'static str> {
fn description(&self) -> &'static str {
"Implicit/explicit aliasing of columns."
}

fn long_description(&self) -> &'static str {
r#"
**Anti-pattern**
Expand All @@ -56,11 +60,6 @@ SELECT
FROM foo
```
"#
.into()
}

fn description(&self) -> &'static str {
"Implicit/explicit aliasing of columns."
}

fn eval(&self, context: RuleContext) -> Vec<LintResult> {
Expand Down Expand Up @@ -103,7 +102,7 @@ mod tests {
#[test]
fn test_fail_explicit_column_default() {
assert_eq!(
fix("select 1 bar from table1 b".into(), vec![RuleAL02::default().erased()]),
fix("select 1 bar from table1 b", vec![RuleAL02::default().erased()]),
"select 1 AS bar from table1 b"
);
}
Expand Down
11 changes: 5 additions & 6 deletions crates/lib/src/rules/aliasing/AL03.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ impl Rule for RuleAL03 {
"aliasing.expression"
}

fn long_description(&self) -> Option<&'static str> {
fn description(&self) -> &'static str {
"Column expression without alias. Use explicit `AS` clause."
}

fn long_description(&self) -> &'static str {
r#"
**Anti-pattern**
Expand All @@ -44,11 +48,6 @@ SELECT
FROM foo
```
"#
.into()
}

fn description(&self) -> &'static str {
"Column expression without alias. Use explicit `AS` clause."
}

fn eval(&self, context: RuleContext) -> Vec<LintResult> {
Expand Down
11 changes: 5 additions & 6 deletions crates/lib/src/rules/aliasing/AL04.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ impl Rule for RuleAL04 {
"aliasing.unique.table"
}

fn long_description(&self) -> Option<&'static str> {
fn description(&self) -> &'static str {
"Table aliases should be unique within each clause."
}

fn long_description(&self) -> &'static str {
r#"
**Anti-pattern**
Expand Down Expand Up @@ -64,11 +68,6 @@ FROM
2021.foo AS f2
```
"#
.into()
}

fn description(&self) -> &'static str {
"Table aliases should be unique within each clause."
}

fn eval(&self, context: RuleContext) -> Vec<LintResult> {
Expand Down
32 changes: 16 additions & 16 deletions crates/lib/src/rules/aliasing/AL05.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ impl Rule for RuleAL05 {
"aliasing.unused"
}

fn long_description(&self) -> Option<&'static str> {
fn description(&self) -> &'static str {
"Tables should not be aliased if that alias is not used."
}

fn long_description(&self) -> &'static str {
r#"
**Anti-pattern**
Expand All @@ -60,11 +64,7 @@ SELECT
a
FROM foo
```
"#.into()
}

fn description(&self) -> &'static str {
"Tables should not be aliased if that alias is not used."
"#
}

fn eval(&self, context: RuleContext) -> Vec<LintResult> {
Expand Down Expand Up @@ -144,15 +144,15 @@ impl RuleAL05 {
fn is_alias_required(from_expression_element: &ErasedSegment) -> bool {
for segment in from_expression_element.iter_segments(Some(&["bracketed"]), false) {
if segment.is_type("table_expression") {
if segment.child(&["values_clause"]).is_some() {
return false;
return if segment.child(&["values_clause"]).is_some() {
false
} else {
return segment.iter_segments(Some(&["bracketed"]), false).iter().any(|seg| {
segment.iter_segments(Some(&["bracketed"]), false).iter().any(|seg| {
["select_statement", "set_expression", "with_compound_statement"]
.iter()
.any(|it| seg.is_type(it))
});
}
})
};
}
}
false
Expand Down Expand Up @@ -198,7 +198,7 @@ mod tests {
let fail_str = "SELECT * FROM my_tbl AS foo";
let fix_str = "SELECT * FROM my_tbl";

let result = fix(fail_str.into(), rules());
let result = fix(fail_str, rules());
assert_eq!(fix_str, result);
}

Expand All @@ -207,7 +207,7 @@ mod tests {
let fail_str = "SELECT * FROM (SELECT * FROM my_tbl AS foo)";
let fix_str = "SELECT * FROM (SELECT * FROM my_tbl)";

let result = fix(fail_str.into(), rules());
let result = fix(fail_str, rules());
assert_eq!(fix_str, result);
}

Expand Down Expand Up @@ -295,7 +295,7 @@ mod tests {
let fail_str = "SELECT * FROM my_tbl foo";
let fix_str = "SELECT * FROM my_tbl";

let result = fix(fail_str.into(), rules());
let result = fix(fail_str, rules());
assert_eq!(fix_str, result);
}

Expand All @@ -304,7 +304,7 @@ mod tests {
let fail_str = "SELECT * FROM (SELECT * FROM my_tbl foo)";
let fix_str = "SELECT * FROM (SELECT * FROM my_tbl)";

let result = fix(fail_str.into(), rules());
let result = fix(fail_str, rules());
assert_eq!(fix_str, result);
}

Expand Down Expand Up @@ -443,7 +443,7 @@ mod tests {
FROM my_table
"#;

let result = fix(fail_str.into(), rules());
let result = fix(fail_str, rules());
assert_eq!(fix_str, result);
}

Expand Down
Loading

0 comments on commit cc3da38

Please sign in to comment.