From 70276816bb6122e6b6989444faab6bbdf09e59db Mon Sep 17 00:00:00 2001
From: Masataka Kuwabara <p.ck.t22@gmail.com>
Date: Mon, 19 Sep 2016 15:52:01 +0900
Subject: [PATCH] Fix false negatives in `Rails/NotNullColumn` cop (#3508)

---
 CHANGELOG.md                                  |  1 +
 lib/rubocop/cop/rails/not_null_column.rb      |  2 +-
 .../rubocop/cop/rails/not_null_column_spec.rb | 30 ++++++++++---------
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 965a03ee3748..930534a76aea 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,6 +43,7 @@
 * [#3450](https://github.com/bbatsov/rubocop/issues/3450): Prevent `Style/TernaryParentheses` cop from making unsafe corrections. ([@drenmi][])
 * [#3460](https://github.com/bbatsov/rubocop/issues/3460): Fix false positives in `Style/InlineComment` cop. ([@drenmi][])
 * [#3485](https://github.com/bbatsov/rubocop/issues/3485): Make OneLineConditional cop not register offense for empty else. ([@tejasbubane][])
+* [#3508](https://github.com/bbatsov/rubocop/pull/3508): Fix false negatives in `Rails/NotNullColumn`. ([@pocke][])
 
 ### Changes
 
diff --git a/lib/rubocop/cop/rails/not_null_column.rb b/lib/rubocop/cop/rails/not_null_column.rb
index 52dd276cc761..8de0b80a87ca 100644
--- a/lib/rubocop/cop/rails/not_null_column.rb
+++ b/lib/rubocop/cop/rails/not_null_column.rb
@@ -26,7 +26,7 @@ class NotNullColumn < Cop
         PATTERN
 
         def_node_matcher :has_default?, <<-PATTERN
-          (pair (sym :default) _)
+          (pair (sym :default) !(:nil))
         PATTERN
 
         def on_send(node)
diff --git a/spec/rubocop/cop/rails/not_null_column_spec.rb b/spec/rubocop/cop/rails/not_null_column_spec.rb
index 20b74ded8f44..2dbe64d6dac7 100644
--- a/spec/rubocop/cop/rails/not_null_column_spec.rb
+++ b/spec/rubocop/cop/rails/not_null_column_spec.rb
@@ -26,23 +26,29 @@
       let(:source) do
         'add_column :users, :name, :string, null: false, default: ""'
       end
-      it 'accepts' do
-        expect(cop.offenses).to be_empty
+      include_examples 'accepts'
+    end
+
+    context 'with null: false and default: nil' do
+      let(:source) do
+        'add_column :users, :name, :string, null: false, default: nil'
+      end
+      it 'reports an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.messages).to eq(
+          ['Do not add a NOT NULL column without a default value']
+        )
       end
     end
 
     context 'with null: true' do
       let(:source) { 'add_column :users, :name, :string, null: true' }
-      it 'accepts' do
-        expect(cop.offenses).to be_empty
-      end
+      include_examples 'accepts'
     end
 
     context 'without any options' do
       let(:source) { 'add_column :users, :name, :string' }
-      it 'accepts' do
-        expect(cop.offenses).to be_empty
-      end
+      include_examples 'accepts'
     end
   end
 
@@ -54,9 +60,7 @@
         'change_column :users, :name, :string, null: false'
       ]
     end
-    it 'accepts' do
-      expect(cop.offenses).to be_empty
-    end
+    include_examples 'accepts'
   end
 
   context 'with create_table call' do
@@ -70,8 +74,6 @@
        '  end',
        'end']
     end
-    it 'accepts' do
-      expect(cop.offenses).to be_empty
-    end
+    include_examples 'accepts'
   end
 end