Skip to content

Commit 9e87e98

Browse files
committed
[Fix #952] Fix a false positive for Rails/NotNullColumn
Fixes #952. This PR fixes a false positive for `Rails/NotNullColumn` when using `null: false` for MySQL's TEXT type.
1 parent ebfb3ef commit 9e87e98

File tree

4 files changed

+54
-3
lines changed

4 files changed

+54
-3
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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][])

config/default.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,9 @@ Rails/NotNullColumn:
694694
Enabled: true
695695
VersionAdded: '0.43'
696696
VersionChanged: '2.20'
697+
Database: null
698+
SupportedDatabases:
699+
- mysql
697700
Include:
698701
- db/**/*.rb
699702

lib/rubocop/cop/rails/not_null_column.rb

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@
33
module RuboCop
44
module Cop
55
module Rails
6-
# Checks for add_column call with NOT NULL constraint
7-
# in migration file.
6+
# Checks for add_column call with NOT NULL constraint in migration file.
7+
#
8+
# `TEXT` can have default values in PostgreSQL, but not in MySQL.
9+
# It will automatically detect an adapter from `development` environment
10+
# in `config/database.yml` or the environment variable `DATABASE_URL`
11+
# when the `Database` option is not set. If the database is MySQL,
12+
# this cop ignores offenses for the `TEXT`.
813
#
914
# @example
1015
# # bad
@@ -17,6 +22,8 @@ module Rails
1722
# add_reference :products, :category
1823
# add_reference :products, :category, null: false, default: 1
1924
class NotNullColumn < Base
25+
include DatabaseTypeResolvable
26+
2027
MSG = 'Do not add a NOT NULL column without a default value.'
2128
RESTRICT_ON_SEND = %i[add_column add_reference].freeze
2229

@@ -45,7 +52,10 @@ def on_send(node)
4552

4653
def check_add_column(node)
4754
add_not_null_column?(node) do |type, pairs|
48-
return if type.respond_to?(:value) && (type.value == :virtual || type.value == 'virtual')
55+
if type.respond_to?(:value)
56+
return if type.value == :virtual || type.value == 'virtual'
57+
return if (type.value == :text || type.value == 'text') && database == MYSQL
58+
end
4959

5060
check_pairs(pairs)
5161
end

spec/rubocop/cop/rails/not_null_column_spec.rb

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,41 @@ def change
113113
end
114114
end
115115
end
116+
117+
context 'when database is MySQL' do
118+
let(:cop_config) do
119+
{ 'Database' => 'mysql' }
120+
end
121+
122+
it 'does not register an offense when using `null: false` for `:text` type' do
123+
expect_no_offenses(<<~RUBY)
124+
def change
125+
add_column :articles, :content, :text, null: false
126+
end
127+
RUBY
128+
end
129+
130+
it "does not register an offense when using `null: false` for `'text'` type" do
131+
expect_no_offenses(<<~RUBY)
132+
def change
133+
add_column :articles, :content, 'text', null: false
134+
end
135+
RUBY
136+
end
137+
end
138+
139+
context 'when database is PostgreSQL' do
140+
let(:cop_config) do
141+
{ 'Database' => 'postgresql' }
142+
end
143+
144+
it 'registers an offense when using `null: false` for `:text` type' do
145+
expect_offense(<<~RUBY)
146+
def change
147+
add_column :articles, :content, :text, null: false
148+
^^^^^^^^^^^ Do not add a NOT NULL column without a default value.
149+
end
150+
RUBY
151+
end
152+
end
116153
end

0 commit comments

Comments
 (0)