Skip to content

[bot] Fix Rails/Blank#177

Closed
6[bot] wants to merge 1 commit intomainfrom
fix/rails-blank-23511805278
Closed

[bot] Fix Rails/Blank#177
6[bot] wants to merge 1 commit intomainfrom
fix/rails-blank-23511805278

Conversation

@6
Copy link
Copy Markdown
Contributor

@6 6 bot commented Mar 24, 2026

Status: Agent is working on this fix...

Cop: Rails/Blank | Backend: codex / normal | Model: gpt-5.3-codex (high) | Mode: fix
Code bugs: 9 | Run: https://github.com/6/nitrocop/actions/runs/23511805278

Closes #160

Task prompt (10619 tokens)

Fix Rails/Blank — 0 FP, 14 FN

Instructions

You are fixing ONE cop in nitrocop, a Rust Ruby linter that uses Prism for parsing.

Current state: 6,472 matches, 0 false positives, 14 false negatives.
Focus on: FN (RuboCop flags code nitrocop misses).

Workflow

  1. Read the Pre-diagnostic Results and Corpus FP/FN Examples sections below first
  2. Verify with RuboCop first (for FP fixes): before writing any code, confirm RuboCop's
    behavior on BOTH the specific FP case AND the general pattern:
    echo '<specific FP case>' > /tmp/test.rb && rubocop --only Rails/Blank /tmp/test.rb
    echo '<general pattern>' > /tmp/test.rb && rubocop --only Rails/Blank /tmp/test.rb
    If RuboCop flags the general pattern, your fix must be narrow enough to not suppress it.
  3. Add a test case FIRST:
    • FN fix: add the missed pattern to tests/fixtures/cops/rails/blank/offense.rb with ^ annotation
    • FP fix: add the false-positive pattern to tests/fixtures/cops/rails/blank/no_offense.rb
  4. Verify test fails: cargo test --lib -- cop::rails::blank
  5. Fix src/cop/rails/blank.rs
  6. Verify test passes: cargo test --lib -- cop::rails::blank
  7. Add a /// doc comment on the cop struct documenting what you found and fixed
  8. Commit only your cop's files

Fixture Format

Mark offenses with ^ markers on the line AFTER the offending source line:

x = 1
     ^^ Rails/Blank: Trailing whitespace detected.

The ^ characters must align with the offending columns. The message format is Rails/Blank: <message text>.

If your test passes immediately

If you add a test case and it passes without code changes, the corpus mismatch is
caused by config/context differences, not a detection bug.
Do NOT loop trying to make the test fail. Instead:

  1. Investigate config resolution (Include/Exclude, cop enablement, disable comments)
  2. The fix is likely in src/config/ or the cop's config handling, not detection logic
  3. If you cannot determine the root cause within 5 minutes, document your findings as
    a /// comment on the cop struct and commit

CRITICAL: Avoid regressions in the opposite direction

When fixing FPs, your change MUST NOT suppress legitimate detections. When fixing FNs,
your change MUST NOT flag code that RuboCop accepts. A fix that eliminates a few issues
in one direction but introduces hundreds in the other is a catastrophic regression.

Before exempting a category of patterns, verify with RuboCop that the general case
is still an offense:

rubocop --only Rails/Blank /tmp/test.rb

If RuboCop flags the general pattern but not your specific case, the difference is in
a narrow context (e.g., enclosing structure, receiver type, argument count) — your fix
must target that specific context, not the broad category.

Rule of thumb: if your fix adds an early return or continue that skips a whole
node type, operator class, or naming pattern, it's probably too broad. Prefer adding a
condition that matches the SPECIFIC differentiating context.

Rules

  • Only modify src/cop/rails/blank.rs and tests/fixtures/cops/rails/blank/
  • Run cargo test --lib -- cop::rails::blank to verify your fix (do NOT run the full test suite)
  • Do NOT touch unrelated files
  • Do NOT use git stash

Start Here

Use the existing corpus data to focus on the most concentrated regressions first.

Helpful local commands:

  • python3 scripts/investigate-cop.py Rails/Blank --repos-only
  • python3 scripts/investigate-cop.py Rails/Blank --context
  • python3 scripts/verify-cop-locations.py Rails/Blank

Top FN repos:

  • databasically__lowdown__d593927 (5 FN) — example vendor/rails/actionmailer/lib/action_mailer/vendor/text-format-0.6.3/text/format.rb:594
  • cjstewart88__Tubalr__f6956c8 (4 FN) — example heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/parser/c.rb:784
  • pitluga__supply_drop__d64c50c (4 FN) — example examples/vendored-puppet/vendor/puppet-2.7.8/lib/puppet/util/adsi.rb:161

Representative FN examples:

  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/parser/c.rb:784 — Use elements.blank? instead of elements.nil? or elements.empty?.
  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/ri/driver.rb:865 — Use name.blank? instead of name.nil? or name.empty?.
  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.9.4/lib/rdoc/parser/c.rb:782 — Use elements.blank? instead of elements.nil? or elements.empty?.

Pre-diagnostic Results

Diagnosis Summary

Each example was tested by running nitrocop on the extracted source in isolation
with --force-default-config to determine if the issue is a code bug or config issue.
Note: source context is truncated and may not parse perfectly. If a diagnosis
seems wrong (e.g., your test passes immediately for a 'CODE BUG'), treat it as
a config/context issue instead.

  • FN: 14 code bug(s), 0 config/context issue(s)

FN #1: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/parser/c.rb:784

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: if branch (line: if type.downcase == 'const' then)
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Use elements.blank?instead ofelements.nil? or elements.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

      if elements.nil? or elements.empty? then
^ Rails/Blank: Use `elements.blank?` instead of `elements.nil? or elements.empty?`.

Full source context:

    # In the case of rb_define_const, the definition and comment are in
    # "/* definition: comment */" form.  The literal ':' and '\' characters
    # can be escaped with a backslash.
    if type.downcase == 'const' then
      elements = comment.split ':'

      if elements.nil? or elements.empty? then
        con = RDoc::Constant.new const_name, definition, comment
      else
        new_definition = elements[0..-2].join(':')

        if new_definition.empty? then # Default to literal C definition
          new_definition = definition
        else

FN #2: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/ri/driver.rb:865

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use name.blank?instead ofname.nil? or name.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

      return if name.nil? or name.empty?
^ Rails/Blank: Use `name.blank?` instead of `name.nil? or name.empty?`.

Full source context:

      name = if defined? Readline then
               Readline.readline ">> "
             else
               print ">> "
               $stdin.gets
             end

      return if name.nil? or name.empty?

      name = expand_name name.strip

      begin
        display_name name
      rescue NotFoundError => e
        puts e.message

FN #3: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.9.4/lib/rdoc/parser/c.rb:782

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: if branch (line: if type.downcase == 'const' then)
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Use elements.blank?instead ofelements.nil? or elements.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

      if elements.nil? or elements.empty? then
^ Rails/Blank: Use `elements.blank?` instead of `elements.nil? or elements.empty?`.

Full source context:

    # In the case of rb_define_const, the definition and comment are in
    # "/* definition: comment */" form.  The literal ':' and '\' characters
    # can be escaped with a backslash.
    if type.downcase == 'const' then
      elements = comment.split ':'

      if elements.nil? or elements.empty? then
        con = RDoc::Constant.new const_name, definition, comment
      else
        new_definition = elements[0..-2].join(':')

        if new_definition.empty? then # Default to literal C definition
          new_definition = definition
        else

FN #4: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.9.4/lib/rdoc/ri/driver.rb:878

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use name.blank?instead ofname.nil? or name.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

      return if name.nil? or name.empty?
^ Rails/Blank: Use `name.blank?` instead of `name.nil? or name.empty?`.

Full source context:

      name = if defined? Readline then
               Readline.readline ">> "
             else
               print ">> "
               $stdin.gets
             end

      return if name.nil? or name.empty?

      name = expand_name name.strip

      begin
        display_name name
      rescue NotFoundError => e
        puts e.message

FN #5: databasically__lowdown__d593927: vendor/rails/actionmailer/lib/action_mailer/vendor/text-format-0.6.3/text/format.rb:594

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: method body (line: def __format(to_wrap) #:nodoc:)
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Use words[0].blank?instead ofwords[0].nil? or words[0].empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

      words.shift if words[0].nil? or words[0].empty?
^ Rails/Blank: Use `words[0].blank?` instead of `words[0].nil? or words[0].empty?`.

Full source context:

  private
    def __do_split_word(word, size) #:nodoc:
      [word[0 .. (size - 1)], word[size .. -1]]
    end

    def __format(to_wrap) #:nodoc:
      words = to_wrap.split(/\s+/).compact
      words.shift if words[0].nil? or words[0].empty?
      to_wrap = []

      abbrev = false
      width = @columns - @first_indent - @left_margin - @right_margin
      indent_str = ' ' * @first_indent
      first_line = true
      line = words.shift

FN #6: databasically__lowdown__d593927: vendor/rails/actionmailer/lib/action_mailer/vendor/text-format-0.6.3/text/format.rb:602

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use line.blank?instead ofline.nil? || line.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

      abbrev = __is_abbrev(line) unless line.nil? || line.empty?
^ Rails/Blank: Use `line.blank?` instead of `line.nil? || line.empty?`.

Full source context:

      to_wrap = []

      abbrev = false
      width = @columns - @first_indent - @left_margin - @right_margin
      indent_str = ' ' * @first_indent
      first_line = true
      line = words.shift
      abbrev = __is_abbrev(line) unless line.nil? || line.empty?

      while w = words.shift
        if (w.size + line.size < (width - 1)) ||
           ((line !~ LEQ_RE || abbrev) && (w.size + line.size < width))
          line << " " if (line =~ LEQ_RE) && (not abbrev)
          line << " #{w}"
        else

FN #7: databasically__lowdown__d593927: vendor/rails/actionmailer/lib/action_mailer/vendor/text-format-0.6.3/text/format.rb:630

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: block (do..end) (line: loop do)
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Use line.blank?instead ofline.nil? or line.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

        break if line.nil? or line.empty?
^ Rails/Blank: Use `line.blank?` instead of `line.nil? or line.empty?`.

Full source context:

          line = w
        end

        abbrev = __is_abbrev(w) unless w.nil?
      end

      loop do
        break if line.nil? or line.empty?
        line, w = __do_hyphenate(line, w, width) if @hard_margins
        to_wrap << __make_line(line, indent_str, width, w.nil?)
        line = w
      end

      if (@tag_paragraph && (to_wrap.size > 0)) then
        clr = %r{`(\w+)'}.match([caller(1)].flatten[0])[1]

FN #8: databasically__lowdown__d593927: vendor/rails/actionmailer/lib/action_mailer/vendor/text-format-0.6.3/text/format.rb:790

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: enclosing line: else
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Use rnext.blank?instead ofrnext.nil? or rnext.empty?.

Ready-made test snippet (add to offense.rb, adjust ^ count):

          break if rnext.nil? or rnext.empty? or rline.nil? or rline.empty?
^ Rails/Blank: Use `rnext.blank?` instead of `rnext.nil? or rnext.empty?`.

Full source context:

            words[-1] = first
            @split_words << SplitWord.new(word, first, rest)
          end
          rline = words.join(' ').strip
          rnext = "#{rest} #{rnext}".strip
          break
        else
          break if rnext.nil? or rnext.empty? or rline.nil? or rline.empty?
          words = rnext.split(/\s+/)
          word = words.shift
          size = width - rline.size - 1

          if (size <= 0)
            rnext = "#{word} #{words.join(' ')}".strip
            break

Omitted 6 additional diagnosed FN example(s) for brevity.

Current Rust Implementation

src/cop/rails/blank.rs

use crate::cop::{Cop, CopConfig};
use crate::diagnostic::{Diagnostic, Severity};
use crate::parse::codemap::CodeMap;
use crate::parse::source::SourceFile;
use ruby_prism::Visit;

/// Rails/Blank - flags code that can be simplified using `Object#blank?` from Active Support.
///
/// ## Investigation findings (2026-03-08)
///
/// **FN root cause (1095 FN):** The `UnlessPresent` pattern was not implemented at all.
/// The config option was read but the variable was prefixed with `_` and never used.
/// RuboCop's `on_if` handler flags `unless foo.present?` (both modifier and block forms)
/// suggesting `if foo.blank?` instead.
///
/// **Fix:** Implemented `UnlessPresent` check via a custom AST visitor in `check_source`.
/// The visitor handles both modifier (`something unless foo.present?`) and block
/// (`unless foo.present? ... end`) forms. Skips unless-with-else when Style/UnlessElse
/// would conflict (conservative: always skip when else clause is present).
///
/// **FP root cause (27 FP, first batch):** Missing `defining_blank?` check. RuboCop skips
/// `!present?` when it appears inside `def blank?` (defining blank? in terms of present?).
/// Nitrocop was incorrectly flagging these as offenses.
///
/// **Fix:** Added parent context tracking in the visitor. When inside a `def blank?` method,
/// `!present?` calls are suppressed.
///
/// **FP root cause (34 FP, second batch, 2026-03-10):** `nil_check_receiver` was matching
/// `!foo` (boolean negation, method name `!`) as a nil-check pattern. This caused
/// `!foo || foo.empty?` to be flagged as NilOrEmpty, but RuboCop's `nil_or_empty?`
/// NodePattern only matches explicit nil checks: `nil?`, `== nil`, `nil ==`. It does NOT
/// match `!foo`. The `!foo` branch was incorrectly added to `nil_check_receiver`.
///
/// **Fix:** Removed the `!` method name branch from `nil_check_receiver`. Only `nil?`,
/// `== nil`, and `nil == foo` are valid nil-check patterns for the NilOrEmpty check.
///
/// ## Investigation (2026-03-14)
///
/// **FP root cause (34 FP, third batch):** `present?` called WITH arguments was flagged.
/// RuboCop's NodePattern `(send (send $_ :present?) :!)` only matches when `present?` has
/// NO arguments. Calls like `!Helpers.present?(value)` or `unless Helpers.present?(value)`
/// use `present?` as a class method with an argument — RuboCop skips these.
///
/// Fix: Added argument count check in `check_not_present` and `check_unless_present`.
///
/// ## Investigation (2026-03-15)
///
/// **FP root cause (6 FP remaining):** Safe navigation calls were flagged.
/// - `unless response&.strip&.present?` → `check_unless_present` was matching `&.present?`
///   but RuboCop's `(send $_ :present?)` only matches `send` not `csend`.
/// - `foo.nil? || foo&.empty?` → `check_nil_or_empty` was matching `&.empty?` right side
///   but RuboCop's `(send $_ :empty?)` only matches `send` not `csend`.
///
/// Fix: Added `call_operator_loc() == &.` check to skip safe navigation calls in
/// `check_unless_present` and `check_nil_or_empty`.
///
/// ## Investigation (2026-03-15, second pass)
///
/// **FP root cause (2 FP remaining):** Pattern match guards `in "div" unless element.at("div").present?`
/// are parsed by Prism as `UnlessNode` inside `InNode`. RuboCop's `on_if` handler does not
/// visit these guard nodes. The cop was incorrectly visiting them.
///
/// Fix: Added `inside_in_node` context tracking. When inside an `InNode`, `check_unless_present`
/// is skipped.
///
/// ## Investigation (2026-03-15, round 3)
///
/// **FN root cause (2 FN):** `nil? || empty?` with implicit receiver (self)
/// was not matched. `nil_check_receiver` required an explicit receiver via
/// `receiver_source` which returns None for receiverless calls.
/// Fix: Changed `nil_check_receiver` return type to `Option<(Option<&[u8]>, &[u8])>` so
/// None receiver (implicit self) is preserved. Updated `check_nil_or_empty` to match
/// when both nil-check and empty? have None receivers (both implicit self).
///
/// **FN root cause (112 FN):** The `!foo || foo.empty?` pattern was not matched by
/// `nil_check_receiver`. RuboCop's `nil_or_empty?` NodePattern includes `(send $_ :!)` as one
/// of the left-side alternatives, meaning `!foo || foo.empty?` is a valid NilOrEmpty offense.
/// The previous fix (2026-03-10) incorrectly removed this pattern, because the old
/// implementation was treating `!foo` as the full left source text instead of extracting `foo`
/// (the receiver of `!`) as the variable to compare with `empty?`'s receiver.
///
/// Fix: Re-added `!` method name to `nil_check_receiver`. Now correctly extracts the receiver
/// of `!` (i.e., `foo` from `!foo`) as the variable, matching RuboCop's `(send $_ :!)` capture.
///
/// ## Investigation (2026-03-19)
///
/// **FP root cause (1 FP):** `!pkey_cols&.present?` uses safe navigation on `present?`.
/// RuboCop's `not_present?` NodePattern `(send (send $_ :present?) :!)` only matches `send`,
/// not `csend`. When `pkey_cols` is nil, `nil&.present?` returns `nil` and `!nil` is `true`,
/// but `pkey_cols&.blank?` would return `nil` (falsy). So the replacement changes semantics.
///
/// Fix: Added `call_operator_loc() == &.` check in `check_not_present` to skip safe navigation.
pub struct Blank;

/// Extract the receiver source text from a CallNode, returning None if absent.
fn receiver_source<'a>(call: &ruby_prism::CallNode<'a>) -> Option<&'a [u8]> {
    call.receiver().map(|r| r.location().as_slice())
}

/// Check if the left side of an OR node matches a nil-check-like pattern:
/// - `foo.nil?` or `nil?` (implicit self)
/// - `foo == nil`
/// - `nil == foo`
/// - `!foo` (boolean negation — RuboCop's `(send $_ :!)` pattern)
///
/// Returns (receiver source bytes option, left side source bytes) if matched.
/// The receiver is `None` when using implicit self (e.g., bare `nil?`).
/// RuboCop's `nil_or_empty?` NodePattern matches all four forms.
fn nil_check_receiver<'a>(node: &ruby_prism::Node<'a>) -> Option<(Option<&'a [u8]>, &'a [u8])> {
    let call = node.as_call_node()?;
    let method = call.name().as_slice();
    let left_src = node.location().as_slice();

    if method == b"nil?" {
        // foo.nil? or bare nil? (implicit self)
        return Some((receiver_source(&call), left_src));
    }

    if method == b"!" {
        // !foo — boolean negation. RuboCop's `(send $_ :!)` captures the receiver.
        return receiver_source(&call).map(|r| (Some(r), left_src));
    }

    if method == b"==" {
        // foo == nil  or  nil == foo
        let recv = call.receiver()?;
        let args = call.arguments()?;
        let arg_list: Vec<_> = args.arguments().iter().collect();
        if arg_list.len() != 1 {
            return None;
        }
        let arg = &arg_list[0];

        if arg.as_nil_node().is_some() {
            // foo == nil → receiver is foo
            return Some((Some(recv.location().as_slice()), left_src));
        }
        if recv.as_nil_node().is_some() {
            // nil == foo → receiver is arg
            return Some((Some(arg.location().as_slice()), left_src));
        }
    }

    None
}

impl Cop for Blank {
    fn name(&self) -> &'static str {
        "Rails/Blank"
    }

    fn default_severity(&self) -> Severity {
        Severity::Convention
    }

    fn check_source(
        &self,
        source: &SourceFile,
        parse_result: &ruby_prism::ParseResult<'_>,
        _code_map: &CodeMap,
        config: &CopConfig,
        diagnostics: &mut Vec<Diagnostic>,
        _corrections: Option<&mut Vec<crate::correction::Correction>>,
    ) {
        let nil_or_empty = config.get_bool("NilOrEmpty", true);
        let not_present = config.get_bool("NotPresent", true);
        let unless_present = config.get_bool("UnlessPresent", true);

        let mut visitor = BlankVisitor {
            cop: self,
            source,
            nil_or_empty,
            not_present,
            unless_present,
            inside_def_blank: false,
            inside_in_node: false,
            diagnostics: Vec::new(),
        };
        visitor.visit(&parse_result.node());
        diagnostics.extend(visitor.diagnostics);
    }
}

struct BlankVisitor<'a, 'src> {
    cop: &'a Blank,
    source: &'src SourceFile,
    nil_or_empty: bool,
    not_present: bool,
    unless_present: bool,
    inside_def_blank: bool,
    inside_in_node: bool,
    diagnostics: Vec<Diagnostic>,
}

impl<'pr> BlankVisitor<'_, '_> {
    /// Check NilOrEmpty: `foo.nil? || foo.empty?` or `nil? || empty?` (implicit self)
    fn check_nil_or_empty(&mut self, or_node: &ruby_prism::OrNode<'pr>) {
        let left = or_node.left();
        let right = or_node.right();

        if let Some((nil_recv, left_src)) = nil_check_receiver(&left) {
            // Right side must be `<same>.empty?` — NOT safe navigation (`&.empty?`)
            // RuboCop's NodePattern `(send $_ :empty?)` only matches send, not csend.
            if let Some(right_call) = right.as_call_node() {
                let is_safe_nav = right_call
                    .call_operator_loc()
                    .is_some_and(|loc| loc.as_slice() == b"&.");
                if right_call.name().as_slice() == b"empty?" && !is_safe_nav {
                    let empty_recv = receiver_source(&right_call);
                    // Both receivers must match: both Some with same source, or both None (implicit self)
                    if nil_recv == empty_recv {
                        let loc = or_node.location();
                        let (line, column) = self.source.offset_to_line_col(loc.start_offset());
                        let left_str = std::str::from_utf8(left_src).unwrap_or("nil?");
                        let right_str =
                            std::str::from_utf8(right.location().as_slice()).unwrap_or("empty?");
                        let message = match nil_recv {
                            Some(recv_bytes) => {
                                let recv_str = std::str::from_utf8(recv_bytes).unwrap_or("object");
                                format!(
                                    "Use `{recv_str}.blank?` instead of `{left_str} || {right_str}`."
                                )
                            }
                            None => {
                                format!("Use `blank?` instead of `{left_str} || {right_str}`.")
                            }
                        };
                        self.diagnostics.push(self.cop.diagnostic(
                            self.source,
                            line,
                            column,
                            message,
                        ));
                    }
                }
            }
        }
    }

    /// Check NotPresent: `!foo.present?`
    fn check_not_present(&mut self, call: &ruby_prism::CallNode<'pr>) {
        if call.name().as_slice() != b"!" {
            return;
        }

        let receiver = match call.receiver() {
            Some(r) => r,
            None => return,
        };

        let inner_call = match receiver.as_call_node() {
            Some(c) => c,
            None => return,
        };

        if inner_call.name().as_slice() != b"present?" {
            return;
        }

        // RuboCop's NodePattern `(send (send $_ :present?) :!)` only matches `send`, not `csend`.
        // `!pkey_cols&.present?` uses safe navigation — semantics differ from `!pkey_cols.blank?`
        // because `nil&.present?` returns `nil`, `!nil` is `true`, but `nil&.blank?` returns `nil`.
        if inner_call
            .call_operator_loc()
            .is_some_and(|loc| loc.as_slice() == b"&.")
        {
            return;
        }

        // RuboCop's NodePattern `(send (send $_ :present?) :!)` only matches present? with
        // NO arguments. `!Helpers.present?(value)` (class method style) must not be flagged.
        if inner_call
            .arguments()
            .is_some_and(|a| !a.arguments().is_empty())
        {
            return;
        }

        // Skip !present? inside def blank? (defining blank? in terms of present?)
        if self.inside_def_blank {
            return;
        }

        let loc = call.location();
        let (line, column) = self.source.offset_to_line_col(loc.start_offset());
        self.diagnostics.push(self.cop.diagnostic(
            self.source,
            line,
            column,
            "Use `blank?` instead of `!present?`.".to_string(),
        ));
    }

    /// Check UnlessPresent: `unless foo.present?` or `something unless foo.present?`
    fn check_unless_present(&mut self, unless_node: &ruby_prism::UnlessNode<'pr>) {
        // Skip pattern match guards: `in pattern unless condition`
        // In Prism, pattern match guards are represented as UnlessNodes inside InNodes.
        // RuboCop's `on_if` handler does not visit these guards.
        if self.inside_in_node {
            return;
        }

        // Skip unless-with-else (Style/UnlessElse interaction)
        // Conservative: always skip when else clause is present
        if unless_node.else_clause().is_some() {
            return;
        }

        let predicate = unless_node.predicate();
        let pred_call = match predicate.as_call_node() {
            Some(c) => c,
            None => return,
        };

        if pred_call.name().as_slice() != b"present?" {
            return;
        }

        // RuboCop's NodePattern `(send $_ :present?)` only matches send (not csend).
        // `unless obj&.present?` uses safe navigation and must NOT be flagged.
        if pred_call
            .call_operator_loc()
            .is_some_and(|loc| loc.as_slice() == b"&.")
        {
            return;
        }

        // RuboCop's NodePattern `(send $_ :present?)` only matches present? with NO arguments.
        if pred_call
            .arguments()
            .is_some_and(|a| !a.arguments().is_empty())
        {
            return;
        }

        // Build the receiver string for the message
        let recv_str = match pred_call.receiver() {
            Some(r) => {
                let src = r.location().as_slice();
                format!("{}.blank?", std::str::from_utf8(src).unwrap_or("object"))
            }
            None => "blank?".to_string(),
        };

        // Build the "current" string for the message
        let predicate_src =
            std::str::from_utf8(predicate.location().as_slice()).unwrap_or("present?");
        let current = format!("unless {predicate_src}");

        // Determine offense location based on modifier vs block form
        // For modifier form: `something unless foo.present?` → offense on `unless foo.present?`
        // For block form: `unless foo.present?\n...\nend` → offense on `unless foo.present?`
        let unless_loc = unless_node.location();
        let pred_loc = predicate.location();

        // The offense covers from the start of `unless` keyword to the end of the predicate
        // For modifier form, the keyword is in the middle; for block form, it's at the start
        let keyword_loc = unless_node.keyword_loc();
        let offense_start = keyword_loc.start_offset();
        let offense_end = pred_loc.end_offset();

        // Check if this is modifier form by comparing keyword start to node start
        let is_modifier = keyword_loc.start_offset() > unless_loc.start_offset();

        let (line, column) = if is_modifier {
            self.source.offset_to_line_col(offense_start)
        } else {
            // Block form: offense starts at the `unless` keyword (= node start)
            self.source.offset_to_line_col(offense_start)
        };

        // For the offense range length, count from keyword to end of predicate
        let _ = offense_end; // used implicitly via the annotation range

        self.diagnostics.push(self.cop.diagnostic(
            self.source,
            line,
            column,
            format!("Use `if {recv_str}` instead of `{current}`."),
        ));
    }
}

impl<'pr> Visit<'pr> for BlankVisitor<'_, '_> {
    fn visit_or_node(&mut self, node: &ruby_prism::OrNode<'pr>) {
        if self.nil_or_empty {
            self.check_nil_or_empty(node);
        }
        // Continue visiting children
        self.visit(&node.left());
        self.visit(&node.right());
    }

    fn visit_call_node(&mut self, node: &ruby_prism::CallNode<'pr>) {
        if self.not_present {
            self.check_not_present(node);
        }
        // Visit children (receiver, arguments, block)
        if let Some(recv) = node.receiver() {
            self.visit(&recv);
        }
        if let Some(args) = node.arguments() {
            for arg in args.arguments().iter() {
                self.visit(&arg);
            }
        }
        if let Some(block) = node.block() {
            self.visit(&block);
        }
    }

    fn visit_unless_node(&mut self, node: &ruby_prism::UnlessNode<'pr>) {
        if self.unless_present {
            self.check_unless_present(node);
        }
        // Visit children
        self.visit(&node.predicate());
        if let Some(stmts) = node.statements() {
            self.visit(&stmts.as_node());
        }
        if let Some(else_clause) = node.else_clause() {
            self.visit(&else_clause.as_node());
        }
    }

    fn visit_in_node(&mut self, node: &ruby_prism::InNode<'pr>) {
        let was_inside = self.inside_in_node;
        self.inside_in_node = true;
        ruby_prism::visit_in_node(self, node);
        self.inside_in_node = was_inside;
    }

    fn visit_def_node(&mut self, node: &ruby_prism::DefNode<'pr>) {
        let is_blank = node.name().as_slice() == b"blank?";
        let was_inside = self.inside_def_blank;
        if is_blank {
            self.inside_def_blank = true;
        }

        // Visit children: parameters and body
        if let Some(params) = node.parameters() {
            self.visit(&params.as_node());
        }
        if let Some(body) = node.body() {
            self.visit(&body);
        }

        self.inside_def_blank = was_inside;
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    crate::cop_fixture_tests!(Blank, "cops/rails/blank");
}

RuboCop Ruby Implementation (ground truth)

vendor/rubocop-rails/lib/rubocop/cop/rails/blank.rb

# frozen_string_literal: true

module RuboCop
  module Cop
    module Rails
      # Checks for code that can be written with simpler conditionals
      # using `Object#blank?` defined by Active Support.
      #
      # Interaction with `Style/UnlessElse`:
      # The configuration of `NotPresent` will not produce an offense in the
      # context of `unless else` if `Style/UnlessElse` is enabled. This is
      # to prevent interference between the autocorrection of the two cops.
      #
      # @safety
      #   This cop is unsafe autocorrection, because `' '.empty?` returns false,
      #   but `' '.blank?` returns true. Therefore, autocorrection is not compatible
      #   if the receiver is a non-empty blank string, tab, or newline meta characters.
      #
      # @example NilOrEmpty: true (default)
      #   # Converts usages of `nil? || empty?` to `blank?`
      #
      #   # bad
      #   foo.nil? || foo.empty?
      #   foo == nil || foo.empty?
      #
      #   # good
      #   foo.blank?
      #
      # @example NotPresent: true (default)
      #   # Converts usages of `!present?` to `blank?`
      #
      #   # bad
      #   !foo.present?
      #
      #   # good
      #   foo.blank?
      #
      # @example UnlessPresent: true (default)
      #   # Converts usages of `unless present?` to `if blank?`
      #
      #   # bad
      #   something unless foo.present?
      #
      #   # good
      #   something if foo.blank?
      #
      #   # bad
      #   unless foo.present?
      #     something
      #   end
      #
      #   # good
      #   if foo.blank?
      #     something
      #   end
      #
      #   # good
      #   def blank?
      #     !present?
      #   end
      class Blank < Base
        extend AutoCorrector

        MSG_NIL_OR_EMPTY = 'Use `%<prefer>s` instead of `%<current>s`.'
        MSG_NOT_PRESENT = 'Use `%<prefer>s` instead of `%<current>s`.'
        MSG_UNLESS_PRESENT = 'Use `if %<prefer>s` instead of `%<current>s`.'
        RESTRICT_ON_SEND = %i[!].freeze

        # `(send nil $_)` is not actually a valid match for an offense. Nodes
        # that have a single method call on the left hand side
        # (`bar || foo.empty?`) will blow up when checking
        # `(send (:nil) :== $_)`.
        def_node_matcher :nil_or_empty?, <<~PATTERN
          (or
              {
                (send $_ :!)
                (send $_ :nil?)
                (send $_ :== nil)
                (send nil :== $_)
              }
              {
                (send $_ :empty?)
                (send (send (send $_ :empty?) :!) :!)
              }
          )
        PATTERN

        def_node_matcher :not_present?, '(send (send $_ :present?) :!)'

        def_node_matcher :defining_blank?, '(def :blank? (args) ...)'

        def_node_matcher :unless_present?, <<~PATTERN
          (:if $(send $_ :present?) {nil? (...)} ...)
        PATTERN

        def on_send(node)
          return unless cop_config['NotPresent']

          not_present?(node) do |receiver|
            # accepts !present? if its in the body of a `blank?` method
            next if defining_blank?(node.parent)

            message = format(MSG_NOT_PRESENT, prefer: replacement(receiver), current: node.source)
            add_offense(node, message: message) do |corrector|
              autocorrect(corrector, node)
            end
          end
        end

        def on_or(node)
          return unless cop_config['NilOrEmpty']

          nil_or_empty?(node) do |var1, var2|
            return unless var1 == var2

            message = format(MSG_NIL_OR_EMPTY, prefer: replacement(var1), current: node.source)
            add_offense(node, message: message) do |corrector|
              autocorrect(corrector, node)
            end
          end
        end

        def on_if(node)
          return unless cop_config['UnlessPresent']
          return unless node.unless?
          return if node.else? && config.cop_enabled?('Style/UnlessElse')

          unless_present?(node) do |method_call, receiver|
            range = unless_condition(node, method_call)

            message = format(MSG_UNLESS_PRESENT, prefer: replacement(receiver), current: range.source)
            add_offense(range, message: message) do |corrector|
              autocorrect(corrector, node)
            end
          end
        end

        private

        def autocorrect(corrector, node)
          method_call, variable1 = unless_present?(node)

          if method_call
            corrector.replace(node.loc.keyword, 'if')
            range = method_call.source_range
          else
            variable1, _variable2 = nil_or_empty?(node) || not_present?(node)
            range = node.source_range
          end

          corrector.replace(range, replacement(variable1))
        end

        def unless_condition(node, method_call)
          if node.modifier_form?
            node.loc.keyword.join(node.source_range.end)
          else
            node.source_range.begin.join(method_call.source_range)
          end
        end

        def replacement(node)
          node.respond_to?(:source) ? "#{node.source}.blank?" : 'blank?'
        end
      end
    end
  end
end

RuboCop Test Excerpts

vendor/rubocop-rails/spec/rubocop/cop/rails/blank_spec.rb

    it 'registers an offense and corrects' do

      expect_offense(<<~RUBY, source: source, message: message)
        #{source}
        ^{source} #{message}
      RUBY

  context 'NilOrEmpty set to true' do

    it 'accepts checking nil?' do

    it 'accepts checking empty?' do

    it 'accepts checking nil? || empty? on different objects' do

    it 'does not break when RHS of `or` is a naked falsiness check' do

    it 'does not break when LHS of `or` is a naked falsiness check' do

    it 'does not break when LHS of `or` is a send node with an argument' do

    context 'nil or empty' do

    context 'checking all variable types' do

  context 'NotPresent set to true' do

    it 'accepts !present? if its in the body of a `blank?` method' do

  context 'UnlessPresent set to true' do

    it 'accepts modifier if present?' do

    it 'accepts modifier unless blank?' do

    it 'accepts normal if present?' do

      expect_no_offenses(<<~RUBY)
        if foo.present?
          something
        end
      RUBY

    it 'accepts normal unless blank?' do

      expect_no_offenses(<<~RUBY)
        unless foo.blank?
          something
        end
      RUBY

Current Fixture: offense.rb

tests/fixtures/cops/rails/blank/offense.rb

!x.present?
^^^^^^^^^^^ Rails/Blank: Use `blank?` instead of `!present?`.

!name.present?
^^^^^^^^^^^^^^ Rails/Blank: Use `blank?` instead of `!present?`.

!user.email.present?
^^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `blank?` instead of `!present?`.

x.nil? || x.empty?
^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `x.blank?` instead of `x.nil? || x.empty?`.

name.nil? || name.empty?
^^^^^^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `name.blank?` instead of `name.nil? || name.empty?`.

foo == nil || foo.empty?
^^^^^^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `foo.blank?` instead of `foo == nil || foo.empty?`.

something unless foo.present?
          ^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `if foo.blank?` instead of `unless foo.present?`.

something unless present?
          ^^^^^^^^^^^^^^^ Rails/Blank: Use `if blank?` instead of `unless present?`.

unless foo.present?
^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `if foo.blank?` instead of `unless foo.present?`.
  something
end

!foo || foo.empty?
^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `foo.blank?` instead of `!foo || foo.empty?`.

!methods || methods.empty?
^^^^^^^^^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `methods.blank?` instead of `!methods || methods.empty?`.

!url || url.empty?
^^^^^^^^^^^^^^^^^^ Rails/Blank: Use `url.blank?` instead of `!url || url.empty?`.

return self if nil? || empty?
               ^^^^^^^^^^^^^^ Rails/Blank: Use `blank?` instead of `nil? || empty?`.
return [] if nil? || empty?
             ^^^^^^^^^^^^^^ Rails/Blank: Use `blank?` instead of `nil? || empty?`.

Current Fixture: no_offense.rb

tests/fixtures/cops/rails/blank/no_offense.rb

x.blank?
x.present?
!x.empty?
x.nil?
name.present? && name.length > 0
x.nil? || y.empty?
x.nil? && x.empty?
x.nil? || x.zero?
something if foo.present?
something unless foo.blank?
def blank?
  !present?
end
unless foo.present?
  something
else
  something_else
end

# present? called with argument (class method style) should NOT be flagged
# RuboCop's NodePattern `(send (send $_ :present?) :!)` requires present? with no arguments
!Helpers.present?(value)
!Vagrant::Util::Presence.present?(directory)
unless Helpers.present?(value)
  do_something
end

# safe navigation on present?/empty? — RuboCop's NodePattern matches send not csend
# so &.present? and &.empty? should NOT be flagged
return [] unless response&.strip&.present?
unless object&.present?
  do_something
end
foo.nil? || foo&.empty?

# pattern match guard: `in pattern unless condition` is not a regular unless
# RuboCop's on_if handler does not visit pattern match guards
case element.name
in "div" unless element.at("div").present?
  element.name = "p"
end

# safe navigation with !present? — semantics differ:
# !pkey_cols&.present? when pkey_cols is nil → !nil → true
# pkey_cols&.blank? when pkey_cols is nil → nil (falsy)
# RuboCop skips this pattern because `(send (send $_ :present?) :!)` doesn't
# match csend (safe navigation).
id_option = if pk_is_also_fk || !pkey_cols&.present?

@6 6 closed this Mar 24, 2026
@6 6 deleted the fix/rails-blank-23511805278 branch March 25, 2026 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cop] Rails/Blank

1 participant