trjhtr

I'm very new to Lua (and programming in general). I wanted to start out with a simple package that validates input based on a set of rules.

The code works as intended, but is what I have written is any good? I'd love to hear your feedback!

validator.lua

--- An input validation package written in Lua.
-- @module validator
local validator = 

--- Local table that holds all validation functions.
-- @within rules
local rules = 

--- Check if input holds a value.
-- @param input The input to check.
-- @return True if a input holds a value, false and error otherwise.
-- @within rules
function rules.required(input)
 if (type(input) == "string" and input:len() == 0) or
 (type(input) == "table" and not next(input))
 then
 return false, "%s has no value"
 end
 return true
end

--- Check if string or number length is greater than or equal to a given size.
-- @param input The input to check.
-- @param size The minimum size.
-- @return True if input is at least the given size, false and error otherwise.
-- @within rules
function rules.min(input, size)
 if (type(input) == "string" and input:len() < size) or
 (type(input) == "number" and input < size)
 then
 return false, "%s must be a minimum of "..size 
 end
 return true
end

--- Check if string or number length is less than or equal to a given size.
-- @param input The input to check.
-- @param size The maximum size.
-- @return True if input is at most the given size, false and error otherwise.
-- @within rules
function rules.max(input, size)
 if (type(input) == "string" and input:len() > size) or
 (type(input) == "number" and input > size)
 then
 return false, "%s must be a maximum of "..size
 end
 return true
end

--- Check if string only contains letters.
-- @param input The input to check.
-- @return True if input only contains letters, false and error otherwise.
-- @within rules
function rules.alpha(input)
 if input:find("%A") then
 return false, "%s must be alphabetic"
 end
 return true
end

--- Check if string only contains digits.
-- @param input The input to check.
-- @return True if input only contains digits, false and error otherwise.
-- @within rules
function rules.int(input)
 if input:find("%D") then
 return false, "%s must be a number"
 end
 return true
end

--- Check if string only contains letters or digits.
-- @param input The input to check.
-- @return True if input only contains letters or digits, false and error otherwise.
-- @within rules
function rules.alnum(input)
 if input:find("%W") then
 return false, "%s must be alphanumeric"
 end
 return true
end

function rules.__index()
 error("Trying to validate with non-existant rule")
end

--- Extracts all rules from a string.
-- @param[opt] rules_string The string of rules.
-- @return Table with all the rules.
-- @usage extract_rules("required|min(12)") --> "required", "min(12)"
local function extract_rules(rules_string)
 local rules = 
 if rules_string then
 for rule in rules_string:gmatch("[^|]+") do
 table.insert(rules, rule)
 end
 end
 return rules
end

--- Extracts the argument from a rule.
-- @param rule The rule to extract the argument from.
-- @return The name of the rule, and the argument or nil
-- @usage extract_rule_arg("min(12)") --> "min", 12
-- @usage extract_rule_arg("required") --> "required", nil
local function extract_rule_arg(rule)
 local func_name = rule:match("%a+")
 local func_arg = rule:match("%((.+)%)")
 func_arg = tonumber(func_arg) or func_arg
 return func_name, func_arg
end

--- Validate using input and rules
-- @param inputs A table of inputs
-- @param inputs_rules A table of rules
-- @return True if validation passes, false and table with errors otherwise.
function validator.validate(inputs, inputs_rules)
 local errors = 
 local has_errors = false

 for field, input in pairs(inputs) do
 errors[field] = 
 local input_rules = extract_rules(inputs_rules[field])
 for _, input_rule in pairs(input_rules) do
 local rule_func_name, rule_arg = extract_rule_arg(input_rule)
 local is_valid, err = rules[rule_func_name](input, rule_arg)
 if not is_valid then
 has_errors = true
 table.insert(errors[field], err:format(field))
 end
 end
 end

 if has_errors then
 return false, errors
 end
 return true
end

return validator

Here's an example of how to use it:

local validator = require("validator")

local inputs = 
 username = "CodeReviewer",
 password = "Password123",


local rules = max(20)",
 password = "required

local is_valid, errors = validator.validate(inputs, rules)

if is_valid then
 -- Success!
else
 for field, field_errors in pairs(errors) do
 print("> "..field)
 for i=1, #field_errors do print(field_errors[i]) end
 end
end

Any input would be highly appreciated! I want to focus on becoming a better programmer in any way I can.

I ran the example you provided with some invalid rules. The output I receive is:

> username
> password
password must be a minimum of 12
password must be a maximum of 6

which to me appears to mean that there was some error in the username field, but was not propagated to the errors list.

Instead of creating errors[field] table for all fields, create it lazily:

if not is_valid then
 has_errors = true
 place_error(errors, field, err)
end

and the helper:

local function place_error(error_list, field, template)
 if error_list[field] == nil then
 error_list[field] = 
 end
 --- can also write
 -- error_list[field] = error_list[field] or 
 table.insert(error_list[field], template:format(field))
end

The function name extract_rule_arg should only extract the argument from a rule, but it returns 2 values; which is an anti-pattern imo. Rename it to for eg. parse_rule or just get_rule.

Based on current patterns parsing, you are considering |min_max_2134(12) to be the same as |min(12). It should instead raise error for the same.

local name, arg = rule:match "[^(]+%s*%((.+)%)"

and for the required and other such similar rules, the next line:

if rules[rule] then name = rule end

should be enough. Although, in this method; you'd be again discarding of another imaginary no-arg based rule: a|min(x) since rules['a'] does not exist. If you really want to raise errors for such cases, you can chose to force your rule to become the name:

name = name or rule

putting that all together:

local function parse_rule(rule)
 local name, arg = rule:match "([^(]+)%s*%((.+)%)"
 if rules[rule] then name = rule end
 name = name or rule
 arg = tonumber(arg) or arg
 return name, arg
 -- or you may choose to return the function if you rename
 -- the function as `get_rule` for eg.
 -- return rules[name], arg
end