[Fix #3743] Add Rails/SkipsModelValidations cop

[rahulcs]

warn against Rails ActiveRecord methods which skip validations.

Related issue: #3743

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Used the same coding conventions as the rest of the project.
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• All tests are passing.
• The new code doesn't generate RuboCop offenses.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Updated cop documentation with rake generate_cops_documentation (required only when you've added a new cop or changed the configuration/documentation of an existing cop).

[bbatsov]

Just use a single multiline string literal here. And wrap %s in backquotes.

Also use a single sentence instead - e.g. "Don't use %s because it skips model validations." or something like this.

[bbatsov]

I'd just write an array literal of symbols instead. :-)

[bbatsov]

Due to the nature of the checks this is bound to produce false positives, which I guess is ok.

It'd be nice if this was added as a rule to the rails style guide.

One more thing - you're not checking whether there's an explicit receiver to those method calls or not. This might reduce the false positives a bit (or not). :-)

[bbatsov]

I'd name the cop "SkipsModelValidations".

[rahulcs]

There is already mention about the update_attribute method here in the rails-style-guide. Shall i update it?

[Drenmi]

Due to the nature of the checks this is bound to produce false positives, which I guess is ok.

Either adding the methods to an array in config/default.yml to allow overriding, or having a whitelist option would probably be good. 😀

[rahulcs]

@bbatsov rake generate_cops_documentation is generating more documentation than i added.
attached diff. what do i need to do?

[rahulcs]

One more thing - you're not checking whether there's an explicit receiver to those method calls or not. This might reduce the false positives a bit (or not). :-)

if I do

return unless receiver.nil? && BLACKLIST.include?(method_name)

I get lesser number of rubocop errors on my project.

do I need to check explicit receiver?

[rahulcs]

@Drenmi added picking methods from whitelist.

[bbatsov]

Btw, why don't we simply make the blacklist configurable? This would eliminate the need for a whitelist and would make the operation of the cop more transparent.

[bbatsov]

@rahulcs ping :-)

[rahulcs]

@bbatsov Hi, i was not able to make to figure out how to make blacklist configurable.
if all the methods are initialized to be blacklisted, then how will the user remove the method from blacklist in .rubocop.yml?

[bbatsov]

They'll just override it. Not sure what your exact issue is, as we have several cops that employ configurable blacklists - e.g. Style/PredicateName.

[rahulcs]

@bbatsov thank you will check that.

[rahulcs]

@bbatsov figured it out :) thanks a lot.

[dpisarewski]

Why would you blacklist the touch method. Called without arguments it does not change any data. What is a better alternative for touch?