Boolean query param cop (#35)

This is a mistake I've made at least twice now. Basically, query params
like `?should_do_something=false` will result in the value of string
'false' when fetched from a params object.

```ruby
# Sending '?should_do_something=false' results in string 'false' here!
should_do_something = params.fetch(:should_do_something, false)
```

The only way that they can be falsey is if they are omitted from the
request and default to false. This is also an issue where we are
expecting exactly true and not just any truthy value. The solution is to
cast them explicitly

```ruby
# Always an actual boolean value
should_do_something = ActiveModel::Type::Boolean.new.cast(params.fetch(:should_do_something, false))
```

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    betterlint (1.7.0)
+    betterlint (1.8.0)
       rubocop (> 1.0)
       rubocop-performance
       rubocop-rails
@@ -63,6 +63,8 @@ GEM
     method_source (1.0.0)
     minitest (5.20.0)
     mutex_m (0.2.0)
+    nokogiri (1.16.2-arm64-darwin)
+      racc (~> 1.4)
     nokogiri (1.16.2-x86_64-darwin)
       racc (~> 1.4)
     nokogiri (1.16.2-x86_64-linux)
@@ -167,6 +169,7 @@ GEM
     zeitwerk (2.6.12)
 
 PLATFORMS
+  arm64-darwin-22
   x86_64-darwin-22
   x86_64-linux
 

betterlint.gemspec

@@ -5,7 +5,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 Gem::Specification.new do |s|
   s.name = "betterlint"
-  s.version = "1.7.0"
+  s.version = "1.8.0"
   s.authors = ["Development"]
   s.email = ["development@betterment.com"]
   s.summary = "Betterment rubocop configuration"

lib/rubocop/cop/betterment.rb

@@ -19,3 +19,4 @@
 require 'rubocop/cop/betterment/server_error_assertion'
 require 'rubocop/cop/betterment/hardcoded_id'
 require 'rubocop/cop/betterment/vague_serialize'
+require 'rubocop/cop/betterment/fetch_boolean'

lib/rubocop/cop/betterment/fetch_boolean.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Betterment
+      class FetchBoolean < Base
+        MSG = <<~MSG
+          A boolean fetched from query params or ENV will never be false when
+          explicitly specified on the request or env var. Please use a model
+          with a boolean attribute, or cast the value.
+        MSG
+
+        # @!method fetch_boolean?(node)
+        def_node_matcher :fetch_boolean?, <<-PATTERN
+          (send _ :fetch _ (boolean))
+        PATTERN
+
+        # @!method fetch_env_boolean?(node)
+        def_node_matcher :fetch_env_boolean?, <<-PATTERN
+          (send (const nil? :ENV) :fetch _ (boolean))
+        PATTERN
+
+        # @!method boolean_cast?(node)
+        def_node_search :boolean_cast?, <<-PATTERN
+          (send
+            (send
+              (const
+                (const
+                  (const nil? :ActiveModel) :Type) :Boolean) :new) :cast
+            ...)
+        PATTERN
+
+        # @!method action_controller?(node)
+        def_node_search :action_controller?, <<~PATTERN
+          {
+            (const {nil? cbase} :ApplicationController)
+            (const (const {nil? cbase} :ActionController) :Base)
+          }
+        PATTERN
+
+        def on_send(node)
+          return unless fetch_env_boolean?(node) ||
+            (fetch_boolean?(node) && inherit_action_controller_base?(node))
+
+          return if node.each_ancestor(:send).any? { |ancestor| boolean_cast?(ancestor) }
+
+          add_offense(node)
+        end
+
+        private
+
+        def inherit_action_controller_base?(node)
+          class_node = node.each_ancestor(:class).first
+          return false unless class_node
+
+          action_controller?(class_node)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/betterment/fetch_boolean_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Betterment::FetchBoolean, :config do
+  context 'when using parameters' do
+    it 'registers an offense when defaulting to a boolean value' do
+      expect_offense(<<~RUBY)
+        class UserController < ApplicationController
+          def taxable
+            params.fetch(:taxable, false)
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ A boolean fetched [...]
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense when defaulting to a boolean value in assignment from ENV' do
+      expect_offense(<<~RUBY)
+        class App
+          def do_thing
+            in_progress = ENV.fetch('IN_PROGRESS', true)
+                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ A boolean fetched [...]
+            check_for(in_progress)
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense when defaulting to a boolean value in method call' do
+      expect_offense(<<~RUBY)
+        class UserController < ApplicationController
+          def create
+            do_thing(
+              in_progress: create_params.permit(:in_progress, :taxable).fetch(:in_progress, false),
+                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ A boolean fetched [...]
+            )
+          end
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when wrapped with cast' do
+      expect_no_offenses(<<~RUBY)
+        class UserController < ApplicationController
+          def create
+            ActiveModel::Type::Boolean.new.cast(params.fetch(:taxable, false))
+            in_progress = ActiveModel::Type::Boolean.new.cast user_params.fetch(:in_progress, true)
+            do_thing(
+              in_progress: ActiveModel::Type::Boolean.new.cast(
+                            create_params.permit(:in_progress, :taxable).fetch(:in_progress, false)
+                          )
+            )
+          end
+        end
+      RUBY
+    end
+
+    it 'does not register an offense for params when not in a controller' do
+      expect_no_offenses(<<~RUBY)
+        class Application
+          def create
+            params.fetch(:taxable, false)
+            in_progress = other.fetch('thing', true)
+            do_thing(
+              in_progress: create_params.permit(:in_progress, :taxable).fetch(:in_progress, false),
+            )
+          end
+        end
+      RUBY
+    end
+  end
+end