RoR style guide
Rubocop is the default linter for Ruby on Rails. It is implacable and impartial, and won't take no for an answer when it comes to improving the style of your ruby code. What it does is enforce many of the guidelines outlined in the community Ruby Style Guide.
Here is Rubocop's doc.
Rubocop is a gem that should be in your gemfile. To install the atom plugin you simply need to run $apm install linter-rubocop in your terminal. You can configure the atom plugin by editing ~/.atom/config.cson.
The setting and config for Rubocop are in the .rubocop.yml file. This file can be different for each directory in which they are located. Meaning that two folders of the project can have different rubocop configs. It is very tweakable as seen in their .rubocop.yml default file.
The linter is simply run by typing $rubocop in the terminal. This will analyze and find all the offenses in the current directory. To run it on a single file simply type $rubocop name_of_the_file.rb
Each cop can be disabled in four ways depending on what's needed.
In the .rubocop.yml file for the whole project
Cop/NameOfTheActualCop:Enabled: false
In the .rubocop.yml file for a specific file
Cop/NameOfTheActualCop:Exclude:- 'file/**/*'- '**/*.rake'
In the file itsef with an inline comment :
# rubocop:disable Cop/NameOfTheCopIn the file itself for a block of code situated between
# rubocop:disable Cop/NameOfTheCopand# rubocop:enable Cop/NameOfTheCop
Now let's dive in the actual coding part of this ruby on rails style guide according to Rubocop's rules.
You can run Rubocop to see only one of the three kind of errors. ($rubocop only --lint for example)
Those are the red E's, the errors that will break your code. A missing comma or a misplaced parenthesis that have to be fixed as soon as possible. Rubocop tells you exactly where they are. They are the more breaking kind of errors.
These are important offenses that, while not breaking, often denotes a wrong logic. It is mostly for unused argument or weird variable assignement. These offenses should also be fixed as soon as possible.
These are the most common errors. They are sound from a coding point of view, however they are not in line with the Ruby Style Guide. These types of errors pushes the coder to use more readable, simpler and more efficient code. They should definitely be reduced to a minimum, even though the requirement of the logic sometimes doesn't allow to be perfectly respectful of these guidelines.
Here are the most common minor offenses that even a veteran ruby developer codes from time to time.
These offenses are very common and related to the length of classes methods and lines of code.
Following the best practice guide, a method shouldn't be longer than 10 lines of code, a line shouldn't be longer than 150 characters and a class shouldn't be longer than a 100 lines of codes. These requirements are obviously nigh impossible to fulfill, but should be respected as much as possible. A very divided code is easier to read and use than a very convoluted monolithic one.
The complexity of method is also taken into account and Rubocop really doesn't like complex ones. ABC size measures the assignements, branches and conditionality of a method and issues an offense when it is too high. The perceived compelxity and cyclomatic complexity are also exigent cops that strive for simpler methods.
Those errors are hard to fix and often require rethinking the logic or arrangement of the code.
This type of offense warns you for all the layout mistakes of the code. It warns for useless empty lines, trailing whitespaces that should be removed, spaces insides parenthesis, lack of spaces inside/outside brackets, unaligned hashes arguments, etc...
According to these offenses, it is also necessary to put an empty line after a guard close.
These offenses are usually very simple fixes.
Style offenses pushes the coder to use more elegant methods in their code. For example it will tell to use .eql? instead of ==, to use && instead of and, etc...
It will also warn that the coder should use ternary operator instead of if, or use if on one line if possible. It often prefers the use of a guard close than the use of an if.
Moreover, the use of redundant returns is not in line with the best practice, so the last line of a method should not include a return.
It also worth to note that an array of strings should be written %w[string string] and an array of symbol %i[symbol symbol] .
According to Rubocop's style, every constant must be frozen so that they cannot be modified inadvertently in another part of the code. It would also want a magic comment as well as documentation comments at the beginning of each file.
Naming conventions in coding are different from one project to another, the important part being the consistency. However, to follow the ruby best practices Rubocop has a few cops on naming. The most recurrent one is the predicate name cop, which does not allow for boolean method in the form of is_a ? or has_? . These is_ and has_ should simply be removed as they add nothing to clarity.
Each project respect their own convention and they might sometimes differ consciously from hat Rubocop would want us to do. This is the case for several conventions used in SeraphinWeb.
Exceptions are easily noticeable by the comment # rubocop:disable Cop/NameOfTheCop that is either inline with the offense it tell Rubocop to ignore or that is followed by # rubocop:enable Cop/NameOfTheCop somewhere below to tell Rubocop to ignore a snippet of code.
As said previously lint errors are of high impact and should be fixed quite quickly. However, some of the lint offense detected are actually needed by some obscure logic that the great Rubocop does not see.
This is the case for a few UselessAssignment which are actually needed or UnusedMethodArgument that cannot be removed nor be changed with an underscore in front.
At Seraphin we took the decision to put two lines before every private section of the code. This violate the empty lines policy of Rubocop. Therefore it has been decided to not fix these offenses in order for the code to remain clear in our eyes.
The use of double negation is formally reprimanded by our tough linter. However double negation does not return the same value as no negation at all, and are therefore required in some cases. Even though they should be avoided as much as possible, some of them are left in the code with a Rubocop disabling comment giving them a free pass.
Moreover, we find it also acceptable for some methods to keep the has_ or is_ predicate in some cases, where it really adds value to the clarity.
Finally, in some rare cases, we prefer using if syntaxes even though Rubocop would want us to use guard closes, as they make it a lot easier for a human to comprehend what is going on in the method.
ABC size, perceived compelxity and cyclomatic complexity are often disregarded as our logic is often very complex. These cops stays enabled as they should be respected whenever possible, however they are often disregarded by necessity.
Some place of the code make use of the dangerous eval method, which Rubocop abhors. Having found no other better method than this one, we chose to disable the Security/Eval and the Style/EvalWithLocation every time the eval method is called.
The method Open is considered a significant security risk by Rubocop. Likewise the eval function, other ways of doing are not suited for our project and we decided to manually disable Rubocop's Security/Open cop each time it is used in the code.
Please note that there may be a few offense that we chose not to resolve as they are useful or misinterpreted by Rubocop. Sometimes it might be better not to please Rubocop, and sometimes we have no other choice.
Some cops are entirely disabled for the whole of seraphinWeb as they are cluttering the offenses count and are regulation we chose not to implement in the code. The config file is the .rubocop.yml situated directly in seraphinWeb.
This is the case for the magic comment as well as the documentation comments that we didn't see fit to include in each file.
We also lengthened or even disabled most of the Metrics cops as we found they were too restricting by default. The config for metrics is specified in each folder in the .rubocp.yml file. Ideally the metrics spec should not be defined at all in the config file, and there should be a comment disabling Rubocop for each method, class, block or module that we deem worthy enough to outsize Rubocop's requirements.
We find it not necessary to lint each and every file of the project. This is why we excluded some of them in the .rubocop.yml file. The files of the bin are better left untouched, as well as all the old migrations, the schema (which is auto generated) and the various rake tasks of seraphinWeb.
The specs represent a huge number of files and need a special treatment of the linter. Indeed, the tests are not structured in the exact same way as the rest of the code and this is why we decided to disable some cops for the specs only. This is the case of the metrics/length of the blocks as all specs are inside huge blocks. It is also the case of the Variable Number cop which is disabled for the spec files. Then the eval cops are disabled as well, as we chose to use eval in a lot of place in the spec even though we are conscious of the risks of using it.
Rubocop should be run before putting your code in review. It indeed does the work of a reviewer beforehand, telling you all the mistakes that may be difficult to see from the naked eye. This is why at Seraphin we never fail to run Rubocop before we put a card in review. That way the reviewer can merge into master with confidence.
Rubocop is also embedded in codeship, which will run it with the tests and find any lint offenses that might still be in your branch. It is therefore important to either correct the offense or put a comment # rubocop:disable Cop/NameOfTheCop inline to make sure Rubocop doesn't take it into account.
Dig: The dig method is preferred for nested hashes. For flat hashes we use [], and in the case when we need to safely navigate, we use object&.[](:key)
Happy Linting