lpil / dogma

:closed_lock_with_key: A code style linter for Elixir
Other
471 stars 52 forks source link

New rule: InfixOperatorPadding #212

Closed mgartner closed 8 years ago

mgartner commented 8 years ago

Related to #165.

I think I finally got a solid implementation for this rule. It checks for padding around all infix operators, but handles - and / specially to allow things like a = -1 and &Thing.run/2.

There's no way to get this rule working for versions of Elixir less than 1.1, because column information is not provided by the tokenizer. I still think this rule should be included in Dogma, so I've disabled it from running if the version is less than 1.1. If there is a cleaner way to disable this rule for certain versions that fits more in line with Dogma's rule configs, let me know.

rrrene commented 8 years ago

@mgartner Nice work!

Obviously, I haven't been contributing to Dogma since my own little project started, but I want to give my 2 cents on this one, since I really like the fact that Dogma will have checks which require Elixir > v1.0.

What would you think about providing a way to define the Elixir requirement for a Rule in a consistent way. Returning [] in the test function seems a bit pragmatic. Maybe every check has to define its minimal version requirement, with the default pointing to 1.0. This way you would know how many rules were tested, how many produced "real" errors and how many were skipped due to version conflicts.

Just a suggestion. Keep up the good work! :+1:

lpil commented 8 years ago

I think that sounds like a grand idea. :)

mgartner commented 8 years ago

@lpil I addressed all your comments, lmk what you think.

lpil commented 8 years ago

Great! Thank you.