Skip to content

Commit 4dd13fd

Browse files
committed
incorrect_length_validation: Consider inclusion validations as an alternative
1 parent 998780b commit 4dd13fd

File tree

2 files changed

+88
-4
lines changed

2 files changed

+88
-4
lines changed

lib/active_record_doctor/detectors/incorrect_length_validation.rb

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,28 +33,44 @@ def message(model:, attribute:, table:, database_maximum:, model_maximum:)
3333
def detect
3434
each_model(except: config(:ignore_models), existing_tables_only: true) do |model|
3535
each_attribute(model, except: config(:ignore_attributes), type: [:string, :text]) do |column|
36-
model_maximum = maximum_allowed_by_validations(model, column.name.to_sym)
37-
next if model_maximum == column.limit
36+
model_maximum = maximum_allowed_by_length_validation(model, column.name.to_sym)
37+
database_maximum = column.limit
38+
next if model_maximum == database_maximum
39+
next if database_maximum && covered_by_inclusion_validation?(model, column.name.to_sym, database_maximum)
3840

3941
problem!(
4042
model: model.name,
4143
attribute: column.name,
4244
table: model.table_name,
43-
database_maximum: column.limit,
45+
database_maximum: database_maximum,
4446
model_maximum: model_maximum
4547
)
4648
end
4749
end
4850
end
4951

50-
def maximum_allowed_by_validations(model, column)
52+
def maximum_allowed_by_length_validation(model, column)
5153
length_validator = model.validators.find do |validator|
5254
validator.kind == :length &&
5355
validator.options.include?(:maximum) &&
5456
validator.attributes.include?(column)
5557
end
5658
length_validator ? length_validator.options[:maximum] : nil
5759
end
60+
61+
def covered_by_inclusion_validation?(model, column, limit)
62+
inclusion_validator = model.validators.find do |validator|
63+
validator.kind == :inclusion &&
64+
validator.attributes.include?(column) &&
65+
[:if, :unless].none? { |option| validator.options.key?(option) } &&
66+
[:allow_nil, :allow_blank].none? { |option| validator.options[option] == true }
67+
end
68+
69+
return false if !inclusion_validator
70+
71+
values = inclusion_validator.options[:in] || inclusion_validator.options[:within]
72+
values.is_a?(Array) && values.all? { |value| value.is_a?(String) && value.size <= limit }
73+
end
5874
end
5975
end
6076
end

test/active_record_doctor/detectors/incorrect_length_validation_test.rb

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,74 @@ def test_no_validation_and_no_limit_is_ok
6161
refute_problems
6262
end
6363

64+
def test_array_inclusion_validation_with_shorter_values_and_limit_is_ok
65+
Context.create_table(:users) do |t|
66+
t.string :status, limit: 64
67+
end.define_model do
68+
validates :status, inclusion: { in: ["new", "vip", "a" * 64] }
69+
end
70+
71+
refute_problems
72+
end
73+
74+
def test_array_inclusion_validation_with_longer_values_and_limit_is_error
75+
Context.create_table(:users) do |t|
76+
t.string :status, limit: 64
77+
end.define_model do
78+
validates :status, inclusion: { in: ["new", "vip", "a" * 65] }
79+
end
80+
81+
assert_problems(<<~OUTPUT)
82+
the schema limits users.status to 64 characters but there's no length validator on Context::User.status - remove the database limit or add the validator
83+
OUTPUT
84+
end
85+
86+
def test_proc_inclusion_validation_and_limit_is_error
87+
Context.create_table(:users) do |t|
88+
t.string :status, limit: 64
89+
end.define_model do
90+
validates :status, inclusion: { in: ->(user) { } }
91+
end
92+
93+
assert_problems(<<~OUTPUT)
94+
the schema limits users.status to 64 characters but there's no length validator on Context::User.status - remove the database limit or add the validator
95+
OUTPUT
96+
end
97+
98+
def test_array_inclusion_validation_with_shorter_values_and_if_is_error
99+
Context.create_table(:users) do |t|
100+
t.string :status, limit: 64
101+
end.define_model do
102+
validates :status, inclusion: { in: ["new", "vip"] }, if: :condition?
103+
end
104+
105+
assert_problems(<<~OUTPUT)
106+
the schema limits users.status to 64 characters but there's no length validator on Context::User.status - remove the database limit or add the validator
107+
OUTPUT
108+
end
109+
110+
def test_array_inclusion_validation_with_shorter_values_and_allow_nil_is_error
111+
Context.create_table(:users) do |t|
112+
t.string :status, limit: 64
113+
end.define_model do
114+
validates :status, inclusion: { in: ["new", "vip"] }, allow_nil: true
115+
end
116+
117+
assert_problems(<<~OUTPUT)
118+
the schema limits users.status to 64 characters but there's no length validator on Context::User.status - remove the database limit or add the validator
119+
OUTPUT
120+
end
121+
122+
def test_array_inclusion_validation_with_shorter_values_and_disallow_nil_is_ok
123+
Context.create_table(:users) do |t|
124+
t.string :status, limit: 64
125+
end.define_model do
126+
validates :status, inclusion: { in: ["new", "vip"] }, allow_nil: false
127+
end
128+
129+
refute_problems
130+
end
131+
64132
def test_config_ignore_models
65133
Context.create_table(:users) do |t|
66134
t.string :email, limit: 64

0 commit comments

Comments
 (0)