On config. validation

[paulo-ferraz-oliveira]

Opening this as a general discussion to concentrate thoughts around it.

[paulo-ferraz-oliveira]

elvis_SUITE's custom_ruleset internally relies on custom rulesets to be reset every time we call e.g., from_file. My current implementation doesn't, and assumes these are constant (once read from the configuration) from start to end of process. What am I missing? Are we supposed to change rulesets midway through execution?

[paulo-ferraz-oliveira]

I added a "dump custom" function that can be called when pertinent. At the moment, aside from one test and the rock functions it's not called. (I think it only makes sense to reset once we start a new analysis, not every time we try to validate the config. and/or populate the rulesets)

[paulo-ferraz-oliveira]

We have dirs, files (not exposed), and filter. Food for thought, for the future: get rid of all but files (expose it), since we can express everything with that alone. If agreed on, lets create an issue...

[paulo-ferraz-oliveira]

#516

[paulo-ferraz-oliveira]

• if it's an elvis.config file we have a list at the top with a single entry, elvis. (note: by implicit implementation both rebar.config and "application env." already guarantee elvis where we need).

Validating an elvis section

• section is a non-empty list
• only known options are present
• output_format is oneOf [colors | plain | parsable]
• verbose is boolean()
• no_output is boolean()
• parallel is pos_integer()
• rulesets is valid
• config is (basically) valid
• config is 100% valid

Validating a rulesets section

• section is a map
• all keys of map are atom
• all values are list of 2-tuple or 3-tuple as per validation of rules
• none of the used keys can be a pre-defined ruleset
• custom rules (TBD: checking if these are supported) are not def-validated (they can still be disabled)

Validating a config section

• section is a list of maps
• per map, only known options are present
• dirs+filter are valid
• ignore is valid
• ruleset (if not undefined) is valid
• rules is (basically) valid
• rules is 100% valid

Validating dirs+filter

• elements are strings
• all dirs+filter combos lead to at least one file for analysis

Validating ignore

• elements are atoms
• all ignore entries refer to an existing module (or ignorable: Function + Arity)

Validating ruleset

• is atom
• refers to either a custom ruleset or one of the predefined sets

Validating rules

• element is a list
• all rules are either a 2-tuple (Module + Function) or a 3-tuple (Module + Function + Def)
• Def is either disable or a map with a valid config.

Validating rule config. (stretch)

• element is a map
• all used options are declared in the default map
• used options are of a given type (unless we want to tolerate crashes) or we apply a default

[paulo-ferraz-oliveira]

We could/should consider the following changes, while keeping retro-compat with a warning:

1. elvis.config does not need the top list or elvis
2. the content of elvis could be a map (instead of a proplist)

[paulo-ferraz-oliveira]

#517

[paulo-ferraz-oliveira]

We accept config, [#{}] and also config, #{} (maybe the latter for one the list is only one element). This is confusing. We could drop the no-list variant, with a deprecation warning, and support only the format. Note: we also have quite a few function clauses (that keep getting repeated) just to deal with the no-list variant. If deemed relevant we could create an issue to deal with it later.

[paulo-ferraz-oliveira]

#518

[paulo-ferraz-oliveira]

All of this can continue in #515 and the linked issues.