diff --git a/birdie_snapshots/snap_dummy_src_no_trailing_underscore_gleam.accepted b/birdie_snapshots/snap_dummy_src_no_trailing_underscore_gleam.accepted new file mode 100644 index 0000000..2ec6f52 --- /dev/null +++ b/birdie_snapshots/snap_dummy_src_no_trailing_underscore_gleam.accepted @@ -0,0 +1,10 @@ +--- +version: 1.1.2 +title: ./snap_dummy/src/no_trailing_underscore.gleam +--- +Path: ./snap_dummy/src/no_trailing_underscore.gleam + +Location Identifier: with_trailing_ +Rule: NoTrailingUnderscore +Error: Trailing underscore in function name +Details: We don't like no trailing underscores. \ No newline at end of file diff --git a/snap_dummy/src/no_trailing_underscore.gleam b/snap_dummy/src/no_trailing_underscore.gleam new file mode 100644 index 0000000..129170c --- /dev/null +++ b/snap_dummy/src/no_trailing_underscore.gleam @@ -0,0 +1,7 @@ +pub fn with_trailing_() { + 1 +} + +pub fn without_trailing() { + 1 +} diff --git a/src/code_review.gleam b/src/code_review.gleam index 7309092..5c014af 100644 --- a/src/code_review.gleam +++ b/src/code_review.gleam @@ -143,84 +143,98 @@ fn read_project(project_root_path: String) -> Result(KnowledgeBase, WhingeError) fn visit_knowledge_base(kb: KnowledgeBase, rules: List(Rule)) -> List(RuleError) { use acc, Module(path, module) <- list.fold(kb.src_modules, []) - visit_module(path, rules, module) + visit_module(module, rules) + |> list.map(fn(error) { RuleError(..error, path: path) }) |> list.append(acc) } -fn visit_module( - path: String, - rules: List(Rule), - input_module: glance.Module, -) -> List(RuleError) { - visit_expressions(input_module, rules) - |> list.map(fn(error) { RuleError(..error, path: path) }) +fn visit_module(input: glance.Module, rules: List(Rule)) -> List(RuleError) { + let glance.Module(constants: constants, functions: functions, ..) = input + + // Visit all constants + let results_after_const: List(RuleError) = visit_constants(constants, rules) + let results_after_functions: List(RuleError) = + visit_functions(functions, rules, results_after_const) + + results_after_functions } -// Extracts all the top level functions out of a glance module. -fn extract_functions(from input: glance.Module) -> List(glance.Function) { - let glance.Module(functions: function_defs, ..) = input - let _functions = - list.map(function_defs, fn(def) { - let glance.Definition(_, func) = def - func +fn visit_constants( + constants: List(glance.Definition(glance.Constant)), + rules: List(Rule), +) { + let f = fn(location_identifier, expr) { + apply_visitor(expr, rules, fn(rule) { rule.expression_visitors }) + |> list.map(fn(error) { + RuleError(..error, location_identifier: location_identifier) }) -} + } -fn extract_constants(from input: glance.Module) -> List(glance.Constant) { - let glance.Module(constants: consts, ..) = input - list.map(consts, fn(const_) { - let glance.Definition(_, c) = const_ - c + list.fold(constants, [], fn(const_acc, constant_with_definition) { + let glance.Definition(_, c) = constant_with_definition + do_visit_expressions(c.value, const_acc, fn(expr) { f(c.name, expr) }) }) } -fn visit_expressions(input: glance.Module, rules: List(Rule)) -> List(RuleError) { - let funcs = extract_functions(input) - let consts = extract_constants(input) +fn visit_functions( + functions: List(glance.Definition(glance.Function)), + rules: List(Rule), + acc: List(RuleError), +) { + use acc0: List(RuleError), glance.Definition(_, func) <- list.fold( + functions, + acc, + ) - let f = fn(location_identifier, expr) { - list.flat_map(rules, fn(rule) { - list.flat_map(rule.expression_visitors, fn(visitor) { visitor(expr) }) - |> list.map(fn(error) { - RuleError( - ..error, - rule: rule.name, - location_identifier: location_identifier, - ) - }) - }) - } + let errors_for_func: List(RuleError) = + apply_visitor(func, rules, fn(rule) { rule.function_visitors }) + |> list.map(fn(error) { RuleError(..error, location_identifier: func.name) }) - // Visit all the expressions in top level functions - let func_results: List(RuleError) = { - use func <- list.flat_map(funcs) - use stmt <- list.flat_map(func.body) + use acc1: List(RuleError), stmt <- list.fold( + func.body, + list.append(errors_for_func, acc0), + ) - let expr = case stmt { - glance.Use(_, expr) -> expr - glance.Assignment(value: val, ..) -> val - glance.Expression(expr) -> expr - } + list.append( + visit_statement(stmt, rules) + |> list.map(fn(error) { + RuleError(..error, location_identifier: func.name) + }), + acc1, + ) +} - do_visit_expressions(expr, [], fn(expr) { f(func.name, expr) }) - |> list.flatten +fn visit_statement( + statement: glance.Statement, + rules: List(Rule), +) -> List(RuleError) { + let expr: glance.Expression = case statement { + glance.Use(_, expr) -> expr + glance.Assignment(value: val, ..) -> val + glance.Expression(expr) -> expr } + do_visit_expressions(expr, [], fn(expr) { + apply_visitor(expr, rules, fn(rule) { rule.expression_visitors }) + }) +} - // Visit all the expressions in constants - let const_results: List(RuleError) = - list.flat_map(consts, fn(c) { - do_visit_expressions(c.value, [], fn(expr) { f(c.name, expr) }) - }) - |> list.flatten - list.append(func_results, const_results) +fn apply_visitor( + a: a, + rules: List(Rule), + visitor_fn: fn(Rule) -> List(fn(a) -> List(RuleError)), +) { + list.flat_map(rules, fn(rule) { + list.flat_map(visitor_fn(rule), fn(visitor) { visitor(a) }) + |> list.map(fn(error) { RuleError(..error, rule: rule.name) }) + }) } fn do_visit_expressions( input: glance.Expression, - acc: List(a), - do f: fn(glance.Expression) -> a, -) -> List(a) { - let acc = [f(input), ..acc] + acc: List(RuleError), + do f: fn(glance.Expression) -> List(RuleError), +) -> List(RuleError) { + let acc: List(RuleError) = list.append(f(input), acc) case input { glance.Todo(_) | glance.Panic(_) diff --git a/src/review_config.gleam b/src/review_config.gleam index d8e24cb..c5be35c 100644 --- a/src/review_config.gleam +++ b/src/review_config.gleam @@ -1,7 +1,12 @@ import rule.{type Rule, Rule} import rules/no_panic +import rules/no_trailing_underscore import rules/no_unnecessary_string_concatenation pub fn config() -> List(Rule) { - [no_panic.rule(), no_unnecessary_string_concatenation.rule()] + [ + no_panic.rule(), + no_unnecessary_string_concatenation.rule(), + no_trailing_underscore.rule(), + ] } diff --git a/src/rule.gleam b/src/rule.gleam index 5590c75..089ccd7 100644 --- a/src/rule.gleam +++ b/src/rule.gleam @@ -1,21 +1,49 @@ import glance +import gleam/list pub type Rule { Rule( name: String, + function_visitors: List(fn(glance.Function) -> List(RuleError)), expression_visitors: List(fn(glance.Expression) -> List(RuleError)), ) } pub fn new(name: String) { - Rule(name: name, expression_visitors: []) + Rule(name: name, function_visitors: [], expression_visitors: []) +} + +pub fn with_function_visitor( + rule: Rule, + visitor: fn(glance.Function) -> List(RuleError), +) { + Rule( + ..rule, + function_visitors: [ + set_rule_name_on_errors(rule.name, visitor), + ..rule.function_visitors + ], + ) } pub fn with_expression_visitor( rule: Rule, visitor: fn(glance.Expression) -> List(RuleError), ) { - Rule(..rule, expression_visitors: [visitor, ..rule.expression_visitors]) + Rule( + ..rule, + expression_visitors: [ + set_rule_name_on_errors(rule.name, visitor), + ..rule.expression_visitors + ], + ) +} + +fn set_rule_name_on_errors(name: String, visitor: fn(a) -> List(RuleError)) { + fn(a: a) { + visitor(a) + |> list.map(fn(error) { RuleError(..error, rule: name) }) + } } // Represents an error reported by a rule. diff --git a/src/rules/no_trailing_underscore.gleam b/src/rules/no_trailing_underscore.gleam new file mode 100644 index 0000000..58bde1f --- /dev/null +++ b/src/rules/no_trailing_underscore.gleam @@ -0,0 +1,19 @@ +import glance +import gleam/string +import rule.{type Rule, type RuleError} + +pub fn rule() -> Rule { + rule.new("NoTrailingUnderscore") + |> rule.with_function_visitor(function_visitor) +} + +pub fn function_visitor(function: glance.Function) -> List(RuleError) { + case string.ends_with(function.name, "_") { + True -> [ + rule.error(message: "Trailing underscore in function name", details: [ + "We don't like no trailing underscores.", + ]), + ] + False -> [] + } +} diff --git a/test/code_review_test.gleam b/test/code_review_test.gleam index ad607a7..8c51871 100644 --- a/test/code_review_test.gleam +++ b/test/code_review_test.gleam @@ -2,6 +2,7 @@ import birdie import code_review import gleam/list import gleeunit +import rule pub fn main() { gleeunit.main() @@ -11,8 +12,8 @@ pub fn main() { // while there are lots of moving pieces. // pub fn smoke_test() { - let assert Ok(rules) = code_review.run(on: "./snap_dummy") - use rule <- list.each(rules) + let assert Ok(rule_errors) = code_review.run(on: "./snap_dummy") + use rule: rule.RuleError <- list.each(rule_errors) rule |> code_review.display_rule_error