Class: RuboCop::Cop::DevDoc::Migration::AvoidNonNull

Inherits:

Base

Object
Base
RuboCop::Cop::DevDoc::Migration::AvoidNonNull

show all

Includes:: Auth::AdjacentJustification

Defined in:: lib/rubocop/cop/dev_doc/migration/avoid_non_null.rb

Overview

Avoid null: false on regular columns.

Rationale

null: false on a regular column bakes a business rule (presence) into the schema. Presence belongs in the application layer (model validations), where it is easy to change.

The test for whether null: false is justified is "what would NULL mean for this column?":

If NULL is — or could become — a meaningful business state, presence is a business decision: keep it in the model. email NULL = a phone-only user; organization_id NULL = an unowned template.
If NULL is never a meaningful state by the nature of the data, it is a data-integrity concern and belongs in the schema (see Exception).

The line is drawn for standardization and non-subjectivity. Whether a regular column is "required" is subjective and invites per-column debate (email looks required until phone signup makes it optional), so the schema should not bake in that debatable call.

❌ Regular column
add_column :users, :profile_completion_rate, :float, null: false

❌ Tightening an existing column to NOT NULL
change_column_null :users, :name, false

❌ Same tightening, via change_column
change_column :users, :name, :string, null: false

✔️ Regular column
add_column :users, :profile_completion_rate, :float

Exception

null: false IS the right choice where NULL is never a meaningful state:

Required foreign keys — NOT flagged: this cop never looks at belongs_to, references, or add_reference. A required FK bundles two things: foreign_key: true is pure referential integrity (never a business decision), while null: false on the FK is a mandatory-ness decision that can flip (a document may later be an unowned template). Both are allowed in the schema pragmatically — the referential-integrity guarantee carries the mandatory-ness with it.
Enum columns — NULL is outside the enum's domain (a type violation), so null: false is required, and enforced from the model side by DevDoc/Rails/EnumColumnNotNull. But an enum is a plain integer column, statically indistinguishable from any other integer, so THIS cop cannot detect it and WILL flag it. Mark the line with an adjacent # enum comment (see Markers below) so the migration is self-documenting: a reader sees at a glance that the column is an enum.
Boolean columns justified via DevDoc/Migration/AvoidBooleanColumn — once the developer has justified a boolean through that cop's escape hatch, NULL is outside false (just as it is outside an enum's domain), so null: false is required and enforced by the sibling cop DevDoc/Migration/BooleanColumnNotNull. Mark the line with an adjacent # boolean comment.

Markers

The marker is a comment containing the word enum or boolean, either trailing the flagged line itself or in the contiguous COMMENT-ONLY block ending on the line directly above it. A marker trailing a previous code line deliberately does not count — otherwise one marked column would silence the unmarked column defined on the next line. Matching is case-insensitive and word-bounded (exactly enum / boolean): a prose comment like # enum — NULL is outside the enum's domain satisfies the marker, while enumeration, enums, or enum_type do not. Adjacency is the point — a marker elsewhere in the file justifies nothing.

✔️ Required foreign key (never flagged)
t.belongs_to :user, null: false, foreign_key: true

✔️ Enum, marker trailing the line
add_column :orders, :status, :integer, null: false # enum

✔️ Enum, marker in the comment above
# enum — NULL is outside the enum's domain
add_column :orders, :status, :integer, null: false

✔️ Boolean (justified through AvoidBooleanColumn's escape hatch)
# rubocop:disable DevDoc/Migration/AvoidBooleanColumn -- true binary preference
add_column :things, :flag, :boolean, null: false # boolean
# rubocop:enable DevDoc/Migration/AvoidBooleanColumn

A # rubocop:disable directive still silences the cop like any other (RuboCop-level behavior), but the marker is the sanctioned form — consumer projects that police inline disables (e.g. via Style/DisableCopsWithinSourceCodeDirective's AllowedCops) can drop this cop from their allowed-disables list once existing sites carry markers.

NOTE: This cop is deliberately NOT enum-aware. It could read the model's enum declarations and skip those columns, but requiring an explicit per-line marker is intentional: it forces the developer to signal that the column is an enum, which documents the migration. A silent skip would hide that intent.

NOTE: This cop also flags NOT NULL set on an existing column, by either API: change_column_null(table, column, false) and change_column(table, column, type, null: false). Both express the same constraint as null: false on a definition (the add-nullable -> backfill -> tighten step), so a legit enum/boolean tightening carries the same # enum/# boolean marker, exactly as for a new column. When the tightening runs inside a loop, put the marker on the line of the change_column_null call itself.

NOTE: This cop only flags null: false (and the equivalent false arg of change_column_null). It does not flag null: true (redundant but harmless on regular columns), and it never requires anything of foreign keys — whether a reference is mandatory is a genuine per-reference decision, so the sibling cop DevDoc/Migration/ExplicitNullOnReference requires that decision to be STATED (either value passes).

Examples:

# bad
add_column :users, :name, :string, null: false

# bad (tightening an existing column to NOT NULL)
change_column_null :users, :name, false

# bad (same tightening, via change_column)
change_column :users, :name, :string, null: false

# bad (enum without a marker — the cop flags it)
t.integer :processing_status, null: false

# good
add_column :users, :name, :string

# good (enum, marked)
t.integer :processing_status, null: false # enum

# good (required foreign key — never flagged)
t.belongs_to :user, null: false, foreign_key: true

Constant Summary collapse

MSG =

'Avoid `null: false` on regular columns; enforce presence in the model layer. ' \
'If this is an enum or justified boolean column, mark the line with an adjacent ' \
'`# enum` / `# boolean` comment.'.freeze

KIND_MARKER = Word-bounded so an incidental "enumeration" in a comment cannot satisfy the marker. Comment text is downcased before matching.

/\b(?:enum|boolean)\b/

COLUMN_METHODS = Column-definition helpers that take a null: option. Deliberately EXCLUDES references / belongs_to (and the separate add_reference method): a required foreign key SHOULD carry null: false, so those are never flagged.

%i[
  string integer float boolean datetime date text binary decimal
  json jsonb bigint
].freeze

RESTRICT_ON_SEND =

(COLUMN_METHODS + %i[add_column change_column change_column_null]).freeze

Instance Method Summary collapse

#on_send(node) ⇒ Object

Instance Method Details

#on_send(node) ⇒ `Object`

# File 'lib/rubocop/cop/dev_doc/migration/avoid_non_null.rb', line 181

def on_send(node)
  flag = offense_node(node)
  return unless flag
  return if kind_marker?(node, flag)

  add_offense(flag)
end