Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More cops(rules) #60

Closed
28 of 29 tasks
bbatsov opened this issue Apr 17, 2013 · 17 comments
Closed
28 of 29 tasks

More cops(rules) #60

bbatsov opened this issue Apr 17, 2013 · 17 comments
Milestone

Comments

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 17, 2013

Hey guys,

Here's a list of the more important rules that we should implement in the future.

  • Use Kernel#loop with break rather than begin/end/until or begin/end/while for post-loop tests.
  • Avoid return where not required for flow of control.
  • Avoid self where not required. (It is only required when calling a self write accessor.)
  • Avoid line continuation (\) where not required. In practice, avoid using line continuations at all~~.
  • Use only ascii symbols in identifiers and comments
  • When using reduce with short blocks, name the arguments |a, e| (accumulator, element).
  • When defining binary operators, name the argument other. (like def +(other))
  • Comment annotation would be all upcase and at the beginning of the comment like (# TODO improve performance).
  • Favor the use of module_function over extend self when you want to turn a module's instance methods into class methods.
  • Use the attr family of functions to define trivial accessors or mutators.
  • Use def self.method to define singleton methods. This makes the code easier to refactor since the class name is not repeated.
  • Signal exceptions using the fail method. Use raise only when catching an exception and re-raising it (because here you're not failing, but explicitly and purposefully raising an exception).
  • Never return from an ensure block. If you explicitly return from a method inside an ensure block, the return will take precedence over any exception being raised, and the method will return as if no exception had been raised at all. In effect, the exception will be silently thrown away.
  • Use implicit begin blocks where possible.
  • Don't suppress exceptions.
  • Avoid rescuing the Exception class. This will trap signals and calls to exit, requiring you to kill -9 the process.
  • Prefer literal array and hash creation notation (unless you need to pass parameters to their constructors, that is).
  • Prefer %w to the literal array syntax when you need an array of strings.
  • Use %r only for regular expressions matching more than one '/' character.
  • Favor %r if you have more than 1 '/' characters in your regexp.
  • Avoid %q, %Q, %x, %s, and %W.
  • Prefer () as delimiters for all % literals.
  • Avoid methods longer than 10 LOC (lines of code). Ideally, most methods will be shorter than 5 LOC. Empty lines do not contribute to the relevant LOC. (this cop should have configurable max length)
  • Avoid alias when alias_method will do.
  • Avoid more than three levels of block nesting.
  • One space after if, unless, etc.
  • Use two spaces for indentation.
  • Add empty lines around private and protected.
  • Indent private and protected as much as method definitions.

Details and examples about most rules are available in the guide itself.

I'm opening this ticket mostly so we can discuss who would like to implement what, so that we don't end up with two people working on the same cop. When somebody(me included) starts to work on a cop he'd write a comment here; when he's done - we'd scratch the rule from the list.

So, who'd like to pick a task first? :-)

@jonas054, @vsakarov, @emou

@jonas054
Copy link
Collaborator

I want to do "Use only ascii symbols in identifiers and comments". The list is a great idea, by the way.

@bbatsov
Copy link
Collaborator Author

bbatsov commented Apr 19, 2013

It's yours then.

@jdanielnd
Copy link

I've written a cop for "Prefer %w to the literal array syntax when you need an array of strings." and now it detects that style rule on various code files from the project. Should I correct them to match this new rule too?

@bolandrm
Copy link
Contributor

This can be crossed off the list:

  • Avoid methods longer than 10 LOC (lines of code). Ideally, most methods will be shorter than 5 LOC. Empty lines do not contribute to the relevant LOC. (this cop should have configurable max length)

I'm going to work on "When using reduce with short blocks, name the arguments |a, e|"

@bbatsov
Copy link
Collaborator Author

bbatsov commented Apr 20, 2013

I guess I crossed it off just as you were typing this comment. :-)

Great, looking forward to it.

@jonas054
Copy link
Collaborator

I'll do "Use %r only for regular expressions matching more than one '/' character".

@emou
Copy link
Contributor

emou commented Apr 21, 2013

I'll look into "Avoid more than three levels of block nesting."

@bbatsov
Copy link
Collaborator Author

bbatsov commented Apr 21, 2013

Great, @emou. I'd suggest to actually use a customizable param for that particular check(instead of hardcoding 3), since a lot of people might disagree with the number prescribed by default.

@bbatsov
Copy link
Collaborator Author

bbatsov commented Apr 22, 2013

I'll do 'Favor %r if you have more than 1 '/' characters in your regexp'.

bbatsov added a commit that referenced this issue May 5, 2013
#60 : Use the attr family of functions to define trivial accessors or mutators.
@jurriaan
Copy link
Contributor

jurriaan commented May 7, 2013

What do you think about an indentation Cop?

ps. Could you use a task list? I think that's more clear.

@bbatsov
Copy link
Collaborator Author

bbatsov commented May 7, 2013

Yep, that would be a great addition. The ruby interpreter detect misaligned ends, but nothing more than that. An indentation cop would be an awesome addition. Do you volunteer to write it? :-)

ps. Will do!

@bbatsov
Copy link
Collaborator Author

bbatsov commented May 7, 2013

Done.

@bbatsov
Copy link
Collaborator Author

bbatsov commented Jul 8, 2013

@jonas054 @yujinakayama I think we should try to wrap this issue before 1.0. I'd suggest we continue the old practice of doing a cop at time until we're out of cops. I'll start with the unneeded return check.

@jonas054
Copy link
Collaborator

jonas054 commented Jul 8, 2013

I'll do Avoid self where not required.

bbatsov pushed a commit that referenced this issue Jul 8, 2013
@bbatsov
Copy link
Collaborator Author

bbatsov commented Jul 8, 2013

I'll do the implicit begin next.

@jonas054
Copy link
Collaborator

I'll do "Comment annotation should be all upper case and at the beginning of the comment".

@jonas054
Copy link
Collaborator

I'll do "Prefer modules to classes with only class methods" next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants