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

Cop ideas #2

Open
3 of 20 tasks
pocke opened this issue Jan 1, 2020 · 9 comments
Open
3 of 20 tasks

Cop ideas #2

pocke opened this issue Jan 1, 2020 · 9 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@pocke
Copy link
Owner

pocke commented Jan 1, 2020

Feel free to comment cop ideas with type.

  • Prefer find_each over each
    • We already have Rails/FindEach
  • Check method executes SQL usage in view.
    • AR::Base.where is not allowed in view
    • Detect method call that returns AR::Relation
  • Check bool type argument receives type that is always truthy or falsy.
    • e.g. after_update :foo, if: -> { 1 + 1 }
    • The if's block requires bool type, but 1 + 1 is always truthy.
  • Prefer Hash#dig over Hash#[]
  • Improve Rails/SaveBang
  • find_by with bang or conditional https://github.com/rubocop-hq/rubocop-rails/issues/167
  • Detect unreachable code with bot type
  • Rails/DynamicFindBy without false positives
  • truthy type in conditional
  • Replacement of Style/BracesAroundHashParameters
  • Method aliases
    • find vs detect, map vs collect, etc...
  • Replace String#first and comparison with String#start_with?.
  • AR::Relation#pluck vs AR::Relation#select for sub-query
    • rel.where(id: rel2.pluck(:id))
    • We may be able to reduce query with replacing pluck with select, but it depends on context.
  • send with public method name
    • foo.send(:foo_public_method) should be foo.foo_public_method
  • str[/substr/]should be str['substr']
    • Maybe typing is overkill?
  • Style/HashTransformValues with not-hash iteration
    • Model.pluck(:id, :val).map { |id, val| [id, foo(val)] }.to_h

I already search the following sources to find cop ideas.

@pocke pocke added enhancement New feature or request help wanted Extra attention is needed labels Jan 1, 2020
@koic
Copy link
Contributor

koic commented Feb 6, 2020

This comment is inspired by the following commit in the rails/rails repository.
rails/rails@051e349

Performance/StartWith cop would be extended with the following example if RuboCop can detect a receiver type is the String type.

# bad
receiver.first == 'str'

# good
receiver.start_with?('str')

https://github.com/rubocop-hq/rubocop-performance/blob/master/lib/rubocop/cop/performance/start_with.rb

@pocke
Copy link
Owner Author

pocke commented Feb 7, 2020

Thanks for your comment, @koic !

I'll add your idea to the checklist.

By the way, I guess your example code does not work well because String#first returns the first character, but it compares with 'str' in your example.
Probably it should be receiver.first == 's'.

And I confirmed String#first is added by ActiveSupport.

Thanks!

@yahonda
Copy link

yahonda commented Mar 16, 2020

Let me follow up rubocop/rubocop-minitest#68 (comment) here.

_ method is only defined by Minitest::Spec::DSL::InstanceMethods but rubocop-minitest is not able to handle the class of this method. I'm wondering if RuboCop knows the type it can skip auto-corrections.

cc @koic

@koic
Copy link
Contributor

koic commented Mar 31, 2020

I think Style/HashEachMethods cop can be strictly detected depending on the receiver type.
rubocop/rubocop#7832

values.each { |k, v| do_something(k, v) }

@koic
Copy link
Contributor

koic commented Apr 3, 2020

It would be useful to be able to know a full qualified class name that omit the namespace as follows:

module Gem
  def self.foo
    raise Exception
  end
end

Gem.foo #=> Gem::Exception

cf. rubocop/rubocop#7846

@koic
Copy link
Contributor

koic commented Jul 11, 2020

Performance/AncestorsInclude cop.
rubocop/rubocop-performance#149

@koic
Copy link
Contributor

koic commented Sep 16, 2020

It seems possible to detect without false positive by using receiver type (AR mode or not).
rubocop/rubocop-rails#149

@yahonda
Copy link

yahonda commented Feb 2, 2021

I'm wondering if warnings via ruby/ruby#4070 can be detected by RuboCop or RuboCop with type.

# frozen_string_literal: true

# bad
Struct.new(:a).new(a: 1)

foo = Struct.new(:x)
foo.new(x: 1)

# good
Struct.new(:b).new({ b: 1 })

bar = Struct.new(:y)
bar.new({ y: 1 })

@koic
Copy link
Contributor

koic commented Apr 27, 2021

Type information is required to determine whether the receiver has to_time method.
rubocop/rubocop-rails#288

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants