enkessler / cuke_linter

A linting tool for Cucumber
MIT License
32 stars 8 forks source link

Returned problems from Linters #1

Closed BillyRuffian closed 5 years ago

BillyRuffian commented 5 years ago

Hi, firstly, thank you for picking up the baton from gherkin_lint.

There's much to like about the new gem; I especially like basing it around cuke_modeller.

I'd like to politely ask about the return values from linters. It feels a bit un-rubyish to be returning empty arrays if no problems are detected. In the linters which currently exist, each has a least two returns of empty arrays in addition to the return of arrays of problems. Could I suggest that the cuke_linter test for nil returns in event that no lint issues are detected? This would simplify all of the linters.

Could I also suggest that, since each linter returns an array containing a hash of structured values, that the preferred implementation is to return an array of stucts?

Happy to help contribute.

enkessler commented 5 years ago

In general, I have found it's easier to handle return values if they maintain their 'type' when having nothing to return. So a method that is meant to find a single object would return either an object or nil, whereas a method that is meant to find many things (i.e. a collection) would return either an array of objects or an empty array.

It allows for code like

thing = find_thing
do_something if thing

things = find_things
do-something if things.any?

instead of

thing = find_thing
do_something if thing

things = find_things
do-something if things && things.any?

Although, with the advent of the 'safe navigation' operator &., the second form can be simplified more.


In any case, I'm probably going to do away with linters returning a collection and instead move towards a 'one linter, one problem' paradigm. Having a linter potentially return multiple problems hasn't helped so far and it is more cumbersome than I'd like. So...yes, I will shortly have liners be returning either nil or a problem object.

Regarding this shape of the returned problem, what's the benefit of going the struct route instead of just a hash?

BillyRuffian commented 5 years ago

Thanks for taking the time to reply.

I’d be inclined to have some kind of data structure (struct, class, whatever) to hold the ‘problem’ because, well, it has structure — two well-known fields which must be present — but mostly because it reduces the cognitive burden for anyone implementing linters and can be clearly documented with RDoc.

If you’re open to contributions, I’m going to make a wild guess that it’s the linters that people will want to add. The simpler the internal API the lower the cognitive burden there is just to integrate it into the framework.

It’s all stylistic at the end of the day. I’ll hang fire on trying to coerce some of gherkin_lint’s rules to cuke_lint’s framework for the time being then.

Again, thanks for starting the project.

enkessler commented 5 years ago

Again, thanks for starting the project.

You are quite welcome.

Happy to help contribute.

Thanks! It's always nice to have more than just me on a project. Right now, the two biggest things that are needed are ideas (because the gem is still in the 0.x versions) and any extra hands that want to help port over rules from cuke_sniffer and gherkin_lint.

As you hinted at, the process to make a new linter is still less than ideal. I had to make a custom one recently and, while straight forward, it was a little bit cumbersome. I'd like to preserve the fact that any object that has the needed methods can be a linter but I would also like to make the boilerplate linter code be more or less provided for the user. Maybe use a factory or maybe have a base class that custom linters can inherit. When you get down to it, all a linter really needs is a message to display and a lambda that returns true/false. The rest of the code is the same for 90%+ of linters.

enkessler commented 5 years ago

@BillyRuffian The base class approach seems to be going well in my efforts of toying around with it. I should have a branch pushed up this evening (I'm UTC-5), if you would like to take a look at a new way of making a custom linter.

enkessler commented 5 years ago

@BillyRuffian have a look at #2, if you would be so kind, and let me know what you think. When using that base class to create custom linters, the shape of the returned problem is completely handled without the user needing to know anything.

BillyRuffian commented 5 years ago

That looks great, it makes the linters really clean IMHO.

enkessler commented 5 years ago

Thanks! Are we good to close this issue?