summaryrefslogtreecommitdiff
path: root/rubocop/cop
diff options
context:
space:
mode:
authorMichael Kozono <mkozono@gmail.com>2017-07-27 13:54:48 -0700
committerMichael Kozono <mkozono@gmail.com>2017-08-14 07:57:57 -0700
commit9beef1848791ac86d1b3eca083049cc2752b6713 (patch)
treee2ab5af2bd8e256013f4c73bdd2e63d7b6bc2b59 /rubocop/cop
parent12cdc4616df600edef92f417aecce217f2e5326d (diff)
downloadgitlab-ce-9beef1848791ac86d1b3eca083049cc2752b6713.tar.gz
Add SaferBooleanColumn cop
Diffstat (limited to 'rubocop/cop')
-rw-r--r--rubocop/cop/migration/safer_boolean_column.rb94
1 files changed, 94 insertions, 0 deletions
diff --git a/rubocop/cop/migration/safer_boolean_column.rb b/rubocop/cop/migration/safer_boolean_column.rb
new file mode 100644
index 00000000000..0335c25d85d
--- /dev/null
+++ b/rubocop/cop/migration/safer_boolean_column.rb
@@ -0,0 +1,94 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+ module Cop
+ module Migration
+ # This cop requires a default value and disallows nulls for boolean
+ # columns on small tables.
+ #
+ # In general, this prevents 3-state-booleans.
+ # https://robots.thoughtbot.com/avoid-the-threestate-boolean-problem
+ #
+ # In particular, for the `application_settings` table, this ensures that
+ # upgraded installations get a proper default for the new boolean setting.
+ # A developer might otherwise mistakenly assume that a value in
+ # `ApplicationSetting.defaults` is sufficient.
+ #
+ # See https://gitlab.com/gitlab-org/gitlab-ee/issues/2750 for more
+ # information.
+ class SaferBooleanColumn < RuboCop::Cop::Cop
+ include MigrationHelpers
+
+ DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default. You may wish to use `add_column_with_default`.'.freeze
+ NULL_OFFENSE = 'Boolean columns on the `%s` table should disallow nulls.'.freeze
+ DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls. You may wish to use `add_column_with_default`.'.freeze
+
+ SMALL_TABLES = %i[
+ application_settings
+ ].freeze
+
+ def_node_matcher :add_column?, <<~PATTERN
+ (send nil :add_column $...)
+ PATTERN
+
+ def on_send(node)
+ return unless in_migration?(node)
+
+ matched = add_column?(node)
+
+ return unless matched
+
+ table, _, type = matched.to_a.take(3).map(&:children).map(&:first)
+ opts = matched[3]
+
+ return unless SMALL_TABLES.include?(table) && type == :boolean
+
+ no_default = no_default?(opts)
+ nulls_allowed = nulls_allowed?(opts)
+
+ offense = if no_default && nulls_allowed
+ DEFAULT_AND_NULL_OFFENSE
+ elsif no_default
+ DEFAULT_OFFENSE
+ elsif nulls_allowed
+ NULL_OFFENSE
+ end
+
+ add_offense(node, :expression, format(offense, table)) if offense
+ end
+
+ def no_default?(opts)
+ return true unless opts
+
+ each_hash_node_pair(opts) do |key, value|
+ return value == 'nil' if key == :default
+ end
+ end
+
+ def nulls_allowed?(opts)
+ return true unless opts
+
+ each_hash_node_pair(opts) do |key, value|
+ return value != 'false' if key == :null
+ end
+ end
+
+ def each_hash_node_pair(hash_node, &block)
+ hash_node.each_node(:pair) do |pair|
+ key = hash_pair_key(pair)
+ value = hash_pair_value(pair)
+ yield(key, value)
+ end
+ end
+
+ def hash_pair_key(pair)
+ pair.children[0].children[0]
+ end
+
+ def hash_pair_value(pair)
+ pair.children[1].source
+ end
+ end
+ end
+ end
+end