diff --git a/.rubocop.yml b/.rubocop.yml index de86888..1e3a840 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,13 @@ +require: + - ./lib/rubocop/cop/custom/explicit_select.rb + +Custom/ExplicitSelect: + Enabled: true + AllCops: - NewCops: enable Exclude: - "db/schema.rb" + - "db/migrate.rb" - "db/structure.sql" - "Gemfile" - "lib/tasks/*.rake" @@ -11,9 +17,13 @@ AllCops: - "config/environments/development.rb" - "config/environments/production.rb" - "spec/spec_helper.rb" + - 'tmp/**/*' + - 'docker/**/*' + - 'log/**/*' + - '.circleci/**/*' + - '.git/**/*' + SuggestExtensions: false + NewCops: enable + Style/Documentation: Enabled: false - -# Disable suggestions for rubocop-rails for now -AllCops: - SuggestExtensions: false diff --git a/README.md b/README.md index dc1fc8c..5e1744b 100644 --- a/README.md +++ b/README.md @@ -203,3 +203,13 @@ bin/rails server Once that's running, visit in your browser to see it. ![Screenshot of PgHero for Rideshare](https://i.imgur.com/VduvxSK.png) + +## Rubocop + +- +- Custom cop for detecting missing explicit select + +``` +app/controllers/api/trip_requests_controller.rb:3:5: C: [Correctable] Custom/ExplicitSelect: Avoid using all without an explicit .select(...). Autocorrecting to .select(:id). + Driver.all +``` diff --git a/lib/rubocop/cop/custom/explicit_select.rb b/lib/rubocop/cop/custom/explicit_select.rb new file mode 100644 index 0000000..1cfddbe --- /dev/null +++ b/lib/rubocop/cop/custom/explicit_select.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Custom + # This cop warns when Active Record queries use `all` or `where` + # without an explicit `.select(...)` clause. + # Autocorrects to `.select(:id)` which needs to be expanded to all fields needed + # by the developer. + class ExplicitSelect < Base + extend AutoCorrector + + MSG = 'Avoid using `%s` without an explicit `.select(...)`. Autocorrecting to `.select(:id)`.' + + RESTRICT_ON_SEND = %i[all where].freeze + + def on_send(node) + return unless inside_active_record_chain?(node) + + if node.method?(:all) || node.method?(:select) || node.method?(:where) + unless select_present_in_chain?(node) + add_offense(node, message: format(MSG, method: node.method_name)) do |corrector| + append_select_id(corrector, node) + end + end + end + end + + private + + def inside_active_record_chain?(node) + # Only apply if the receiver is a constant (e.g., `User.all`) + node.receiver&.const_type? + end + + def select_present_in_chain?(node) + visited = Set.new + + while node + return true if node.method?(:select) && node.arguments.any? + + # Avoid re-visiting the same node (paranoia guard) + break if visited.include?(node) + visited << node + + # Walk up the chain safely + node = node.parent if node.parent&.send_type? + break unless node&.send_type? + end + + false + end + + def append_select_id(corrector, node) + insertion_point = node.source_range.end + corrector.insert_after(insertion_point, '.select(:id)') # Developer must add additional fields beyond :id + end + end + end + end +end