Skip to content

Commit

Permalink
[Fix rubocop#952] Fix a false positive for Rails/NotNullColumn
Browse files Browse the repository at this point in the history
Fixes rubocop#952.

This PR fixes a false positive for `Rails/NotNullColumn`
when using `null: false` for MySQL's TEXT type.
  • Loading branch information
koic committed Oct 7, 2023
1 parent ebfb3ef commit 532fb23
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#952](https://github.com/rubocop/rubocop-rails/issues/952): Fix a false positive for `Rails/NotNullColumn` when using `null: false` for MySQL's TEXT type. ([@koic][])
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,10 @@ Rails/NotNullColumn:
Enabled: true
VersionAdded: '0.43'
VersionChanged: '2.20'
Database: null
SupportedDatabases:
- mysql
- postgresql
Include:
- db/**/*.rb

Expand Down
16 changes: 13 additions & 3 deletions lib/rubocop/cop/rails/not_null_column.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
module RuboCop
module Cop
module Rails
# Checks for add_column call with NOT NULL constraint
# in migration file.
# Checks for add_column call with NOT NULL constraint in migration file.
#
# `TEXT` can have default values in PostgreSQL, but not in MySQL.
# It will automatically detect an adapter from `development` environment
# in `config/database.yml` or the environment variable `DATABASE_URL`
# when the `Database` option is not set. If the database is MySQL,
# this cop ignores offenses for the `TEXT`.
#
# @example
# # bad
Expand All @@ -17,6 +22,8 @@ module Rails
# add_reference :products, :category
# add_reference :products, :category, null: false, default: 1
class NotNullColumn < Base
include DatabaseTypeResolvable

MSG = 'Do not add a NOT NULL column without a default value.'
RESTRICT_ON_SEND = %i[add_column add_reference].freeze

Expand Down Expand Up @@ -45,7 +52,10 @@ def on_send(node)

def check_add_column(node)
add_not_null_column?(node) do |type, pairs|
return if type.respond_to?(:value) && (type.value == :virtual || type.value == 'virtual')
if type.respond_to?(:value)
return if type.value == :virtual || type.value == 'virtual'
return if (type.value == :text || type.value == 'text') && database == MYSQL
end

check_pairs(pairs)
end
Expand Down
37 changes: 37 additions & 0 deletions spec/rubocop/cop/rails/not_null_column_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,41 @@ def change
end
end
end

context 'when database is MySQL' do
let(:cop_config) do
{ 'Database' => 'mysql' }
end

it 'does not register an offense when using `null: false` for `:text` type' do
expect_no_offenses(<<~RUBY)
def change
add_column :articles, :content, :text, null: false
end
RUBY
end

it "does not register an offense when using `null: false` for `'text'` type" do
expect_no_offenses(<<~RUBY)
def change
add_column :articles, :content, 'text', null: false
end
RUBY
end
end

context 'when database is PostgreSQL' do
let(:cop_config) do
{ 'Database' => 'postgresql' }
end

it 'registers an offense when using `null: false` for `:text` type' do
expect_offense(<<~RUBY)
def change
add_column :articles, :content, :text, null: false
^^^^^^^^^^^ Do not add a NOT NULL column without a default value.
end
RUBY
end
end
end

0 comments on commit 532fb23

Please sign in to comment.