Skip to content

[bot] Fix Rails/RakeEnvironment#178

Closed
6[bot] wants to merge 1 commit intomainfrom
fix/rails-rake_environment-23511804121
Closed

[bot] Fix Rails/RakeEnvironment#178
6[bot] wants to merge 1 commit intomainfrom
fix/rails-rake_environment-23511804121

Conversation

@6
Copy link
Copy Markdown
Contributor

@6 6 bot commented Mar 24, 2026

Status: Agent is working on this fix...

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

Closes #170

Task prompt (6272 tokens)

Fix Rails/RakeEnvironment — 1 FP, 3 FN

Instructions

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

Current state: 7,740 matches, 1 false positives, 3 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/RakeEnvironment /tmp/test.rb
    echo '<general pattern>' > /tmp/test.rb && rubocop --only Rails/RakeEnvironment /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/rake_environment/offense.rb with ^ annotation
    • FP fix: add the false-positive pattern to tests/fixtures/cops/rails/rake_environment/no_offense.rb
  4. Verify test fails: cargo test --lib -- cop::rails::rake_environment
  5. Fix src/cop/rails/rake_environment.rs
  6. Verify test passes: cargo test --lib -- cop::rails::rake_environment
  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/RakeEnvironment: Trailing whitespace detected.

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

Mixed issues: some code bugs, some config issues

Pre-diagnostic shows SOME patterns are correctly detected in isolation (config issues)
and SOME are genuinely missed (code bugs). See the per-example diagnosis below.

  • For examples marked CODE BUG: follow the standard TDD workflow
  • For examples marked CONFIG/CONTEXT: investigate config resolution, not detection logic

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/RakeEnvironment /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/rake_environment.rs and tests/fixtures/cops/rails/rake_environment/
  • Run cargo test --lib -- cop::rails::rake_environment 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/RakeEnvironment --repos-only
  • python3 scripts/investigate-cop.py Rails/RakeEnvironment --context
  • python3 scripts/verify-cop-locations.py Rails/RakeEnvironment

Top FP repos:

  • rkh__big_band__1a4e50d (1 FP) — example Rakefile:65

Top FN repos:

  • cjstewart88__Tubalr__f6956c8 (3 FN) — example heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:325

Representative FP examples:

  • rkh__big_band__1a4e50d: Rakefile:65 — Add :environment dependency to the rake task.

Representative FN examples:

  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:325 — Include :environment task as a dependency for all Rake tasks.
  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:376 — Include :environment task as a dependency for all Rake tasks.
  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:403 — Include :environment task as a dependency for all Rake tasks.

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: 3 code bug(s), 0 config/context issue(s)
  • FP: 0 confirmed code bug(s), 1 context-dependent

FN #1: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:325

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

Message: Include :environment task as a dependency for all Rake tasks.

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

task :release, :rel, :reuse, :reltest,
^ Rails/RakeEnvironment: Include `:environment` task as a dependency for all Rake tasks.

Full source context:

  require "rake/plugins/#{plugin_name}"
end

task :noop
#plugin "release_manager"

desc "Make a new release"
task :release, :rel, :reuse, :reltest,
  :needs => [
    :prerelease,
    :clobber,
    :test_all,
    :update_version,
    :package,
    :tag

FN #2: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:376

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

Message: Include :environment task as a dependency for all Rake tasks.

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

task :update_version, :rel, :reuse, :reltest,
^ Rails/RakeEnvironment: Include `:environment` task as a dependency for all Rake tasks.

Full source context:

    unless data =~ /^$/
      abort "svn status is not clean ... do you have unchecked-in files?"
    end
    announce "No outstanding checkins found ... OK"
  end
end

task :update_version, :rel, :reuse, :reltest,
  :needs => [:prerelease] do |t, args|
  if args.rel == CURRENT_VERSION
    announce "No version change ... skipping version update"
  else
    announce "Updating Rake version to #{args.rel}"
    open("lib/rake.rb") do |rakein|
      open("lib/rake.rb.new", "w") do |rakeout|

FN #3: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rake-0.8.7/Rakefile:403

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

Message: Include :environment task as a dependency for all Rake tasks.

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

task :tag, :rel, :reuse, :reltest,
^ Rails/RakeEnvironment: Include `:environment` task as a dependency for all Rake tasks.

Full source context:

    else
      sh %{svn commit -m "Updated to version #{args.rel}" lib/rake.rb} # "
    end
  end
end

desc "Tag all the CVS files with the latest release number (REL=x.y.z)"
task :tag, :rel, :reuse, :reltest,
  :needs => [:prerelease] do |t, args|
  reltag = "REL_#{args.rel.gsub(/\./, '_')}"
  reltag << args.reuse.gsub(/\./, '_') if args.reuse
  announce "Tagging Repository with [#{reltag}]"
  if args.reltest
    announce "Release Task Testing, skipping CVS tagging"
  else

FP #1: rkh__big_band__1a4e50d: Rakefile:65

NOT REPRODUCED in isolation — CONTEXT-DEPENDENT
nitrocop does not flag this in isolation. The FP is triggered
by surrounding code context or file-level state.
Investigate what full-file context causes the false detection.

Source context:

  else
    insert_desc @project.name
  end
  if name.to_s == "default"
    @default_desc = Rake.application.last_comment
    desc nil
  end
  task({name => dependencies}, &subproject_block(@project, &block))
end

Dir.glob("tasks/*.task").sort.each do |file|
  eval File.read(file), binding, file, 1
end

Message: Add :environment dependency to the rake task.

Current Rust Implementation

src/cop/rails/rake_environment.rs

use crate::cop::node_type::CALL_NODE;
use crate::cop::{Cop, CopConfig};
use crate::diagnostic::{Diagnostic, Severity};
use crate::parse::source::SourceFile;

/// Rails/RakeEnvironment cop - checks that rake tasks depend on :environment.
///
/// ## Investigation findings (2026-03-08)
///
/// **30 FP:** The `:default` task was not excluded from the check. RuboCop skips
/// `task :default` / `task default: [...]` because the default task is just a
/// dispatcher and doesn't need `:environment`.
///
/// **63 FN:** The cop only handled `task :name` (SymbolNode/StringNode as first arg)
/// but not the hash-first-arg form `task name: :dep do ... end` where the first arg
/// is a KeywordHashNode. In this form, the key is the task name and the value is the
/// dependency list. Tasks like `task foo: :environment` were not recognized at all,
/// and tasks like `task foo: []` (hash form, no deps) were not flagged.
///
/// **Fix:** Extract task name from both symbol/string first-arg and hash-first-arg
/// forms. Skip if task name is "default". For hash-first-arg, check the value for
/// dependencies (symbol = has dep, non-empty array = has dep, empty array = no dep).
///
/// ## Investigation findings (2026-03-10)
///
/// **45 FP:** `has_dependencies()` was too restrictive — only recognized SymbolNode,
/// StringNode, and non-empty ArrayNode as dependencies. Method calls (`task foo: dep`),
/// constants, and variables were not recognized, causing false positives. RuboCop's
/// `with_hash_style_dependencies?` treats ANY non-array, non-nil value as having
/// dependencies (the `else true` branch).
///
/// **Fix:** Simplified `has_dependencies()` to return `true` for anything except an
/// empty array, matching RuboCop's logic exactly.
///
/// ## Investigation findings (2026-03-15)
///
/// **62 FN:** The cop's `else` branch returned early (skipping the offense) when the
/// first argument was not a SymbolNode, StringNode, or KeywordHashNode. This missed
/// task definitions where the task name is a local variable (`task name do`), method
/// call (`task(a.to_sym) {}`), or other expression. RuboCop's `task_name` returns
/// `nil` for these cases, which is != `:default`, so it proceeds to check
/// dependencies and flags the offense.
///
/// **Fix:** Changed the `else` branch to set `task_name_is_default = false` and
/// `hash_first_arg = false`, allowing the cop to continue checking for dependencies
/// and flag the offense when appropriate.
pub struct RakeEnvironment;

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

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

    fn interested_node_types(&self) -> &'static [u8] {
        &[CALL_NODE]
    }

    fn check_node(
        &self,
        source: &SourceFile,
        node: &ruby_prism::Node<'_>,
        _parse_result: &ruby_prism::ParseResult<'_>,
        _config: &CopConfig,
        diagnostics: &mut Vec<Diagnostic>,
        _corrections: Option<&mut Vec<crate::correction::Correction>>,
    ) {
        // Start from CallNode `task`, then check if it has a block
        let call = match node.as_call_node() {
            Some(c) => c,
            None => return,
        };

        // Must be receiverless `task` call
        if call.receiver().is_some() {
            return;
        }

        if call.name().as_slice() != b"task" {
            return;
        }

        // Must have a block
        if call.block().is_none() {
            return;
        }

        let args = match call.arguments() {
            Some(a) => a,
            None => return,
        };

        let arg_list: Vec<ruby_prism::Node<'_>> = args.arguments().iter().collect();
        if arg_list.is_empty() {
            return;
        }

        // Determine if this is a simple task definition (first arg is symbol/string)
        // or a hash-first-arg form (first arg is KeywordHashNode like `task foo: :dep`).
        let first = &arg_list[0];
        let task_name_is_default;
        let hash_first_arg;

        if let Some(sym) = first.as_symbol_node() {
            // task :foo do ... end
            task_name_is_default = sym.unescaped() == b"default";
            hash_first_arg = false;
        } else if let Some(s) = first.as_string_node() {
            // task 'foo' do ... end
            task_name_is_default = s.unescaped() == b"default";
            hash_first_arg = false;
        } else if let Some(kw) = first.as_keyword_hash_node() {
            // task foo: :dep do ... end  (hash-first-arg form)
            // Extract the key as the task name and check value for dependencies.
            let mut elements = kw.elements().iter();
            if let Some(first_elem) = elements.next() {
                if let Some(assoc) = first_elem.as_assoc_node() {
                    // Check task name from the key
                    let key = assoc.key();
                    task_name_is_default = is_name_default(&key);

                    // Check if value represents dependencies
                    let value = assoc.value();
                    if has_dependencies(&value) {
                        if task_name_is_default {
                            return;
                        }
                        // Has dependencies — not an offense (unless we also need to
                        // check remaining hash entries, but RuboCop doesn't).
                        return;
                    }
                    hash_first_arg = true;
                } else {
                    return;
                }
            } else {
                return;
            }
        } else {
            // Any other expression as task name (local variable, method call, constant, etc.)
            // — can't be :default, treat as non-default and check for dependencies.
            task_name_is_default = false;
            hash_first_arg = false;
        }

        // Skip :default task
        if task_name_is_default {
            return;
        }

        // For non-hash-first-arg form, check remaining args for dependency hashes.
        if !hash_first_arg {
            for arg in &arg_list[1..] {
                if let Some(kw) = arg.as_keyword_hash_node() {
                    for elem in kw.elements().iter() {
                        if let Some(assoc) = elem.as_assoc_node() {
                            let value = assoc.value();
                            if has_dependencies(&value) {
                                return;
                            }
                        }
                    }
                }
            }
        }

        let loc = node.location();
        let (line, column) = source.offset_to_line_col(loc.start_offset());
        diagnostics.push(self.diagnostic(
            source,
            line,
            column,
            "Add `:environment` dependency to the rake task.".to_string(),
        ));
    }
}

/// Check if a node represents the name "default" (as symbol or string).
fn is_name_default(node: &ruby_prism::Node<'_>) -> bool {
    if let Some(sym) = node.as_symbol_node() {
        return sym.unescaped() == b"default";
    }
    if let Some(s) = node.as_string_node() {
        return s.unescaped() == b"default";
    }
    false
}

/// Check if a node represents non-empty dependencies.
/// Matches RuboCop's logic: anything except an empty array counts as a dependency.
/// This includes symbols, strings, method calls, constants, variables, etc.
fn has_dependencies(node: &ruby_prism::Node<'_>) -> bool {
    if let Some(arr) = node.as_array_node() {
        // Empty array means no dependencies
        return arr.elements().iter().next().is_some();
    }
    // Any non-array value is treated as a dependency (symbol, string, method call, etc.)
    true
}

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

RuboCop Ruby Implementation (ground truth)

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

# frozen_string_literal: true

module RuboCop
  module Cop
    module Rails
      # Checks for Rake tasks without the `:environment` task
      # dependency. The `:environment` task loads application code for other
      # Rake tasks. Without it, tasks cannot make use of application code like
      # models.
      #
      # You can ignore the offense if the task satisfies at least one of the
      # following conditions:
      #
      # * The task does not need application code.
      # * The task invokes the `:environment` task.
      #
      # @safety
      #   Probably not a problem in most cases, but it is possible that calling `:environment` task
      #   will break a behavior. It's also slower. E.g. some task that only needs one gem to be
      #   loaded to run will run significantly faster without loading the whole application.
      #
      # @example
      #   # bad
      #   task :foo do
      #     do_something
      #   end
      #
      #   # good
      #   task foo: :environment do
      #     do_something
      #   end
      #
      class RakeEnvironment < Base
        extend AutoCorrector

        MSG = 'Include `:environment` task as a dependency for all Rake tasks.'

        def_node_matcher :task_definition?, <<~PATTERN
          (block $(send nil? :task ...) ...)
        PATTERN

        def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
          task_definition?(node) do |task_method|
            return if task_name(task_method) == :default
            return if with_dependencies?(task_method)

            add_offense(task_method) do |corrector|
              if with_arguments?(task_method)
                new_task_dependency = correct_task_arguments_dependency(task_method)
                corrector.replace(task_arguments(task_method), new_task_dependency)
              else
                task_name = task_method.first_argument
                new_task_dependency = correct_task_dependency(task_name)
                corrector.replace(task_name, new_task_dependency)
              end
            end
          end
        end

        private

        def correct_task_arguments_dependency(task_method)
          "#{task_arguments(task_method).source} => :environment"
        end

        def correct_task_dependency(task_name)
          if task_name.sym_type?
            "#{task_name.source.delete(':|\'|"')}: :environment"
          else
            "#{task_name.source} => :environment"
          end
        end

        def task_name(node)
          first_arg = node.first_argument
          case first_arg&.type
          when :sym, :str
            first_arg.value.to_sym
          when :hash
            return nil if first_arg.children.size != 1

            pair = first_arg.children.first
            key = pair.children.first
            case key.type
            when :sym, :str
              key.value.to_sym
            end
          end
        end

        def task_arguments(node)
          node.arguments[1]
        end

        def with_arguments?(node)
          node.arguments.size > 1 && node.arguments[1].array_type?
        end

        def with_dependencies?(node)
          first_arg = node.first_argument
          return false unless first_arg

          if first_arg.hash_type?
            with_hash_style_dependencies?(first_arg)
          else
            task_args = node.arguments[1]
            return false unless task_args
            return false unless task_args.hash_type?

            with_hash_style_dependencies?(task_args)
          end
        end

        def with_hash_style_dependencies?(hash_node)
          deps = hash_node.pairs.first&.value
          return false unless deps

          case deps.type
          when :array
            !deps.values.empty?
          else
            true
          end
        end
      end
    end
  end
end

RuboCop Test Excerpts

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

  it 'registers an offense to task without :environment' do

    expect_offense(<<~RUBY)
      task :foo do
      ^^^^^^^^^ Include `:environment` task as a dependency for all Rake tasks.
      end
    RUBY

  it 'registers an offense for string task name' do

    expect_offense(<<~RUBY)
      task 'bar' do
      ^^^^^^^^^^ Include `:environment` task as a dependency for all Rake tasks.
      end
    RUBY

  it 'registers an offense for namespaced task name' do

    expect_offense(<<~RUBY)
      task 'foo:bar:baz' do
      ^^^^^^^^^^^^^^^^^^ Include `:environment` task as a dependency for all Rake tasks.
      end
    RUBY

  it 'registers an offense for a task with arguments' do

    expect_offense(<<~RUBY)
      task :foo, [:arg] do
      ^^^^^^^^^^^^^^^^^ Include `:environment` task as a dependency for all Rake tasks.
      end
    RUBY

  it 'does not register an offense to task with :environment but it has other dependency before it' do

    expect_no_offenses(<<~RUBY)
      task foo: [:bar, `:environment`] do
      end
    RUBY

  it 'does not register an offense to task with an dependency' do

    expect_no_offenses(<<~RUBY)
      task foo: :bar do
      end
    RUBY

  it 'does not register an offense to task with dependencies' do

    expect_no_offenses(<<~RUBY)
      task foo: [:foo, :bar] do
      end
    RUBY

  it 'does not register an offense to task with a dependency as an array literal element method call' do

    expect_no_offenses(<<~RUBY)
      task foo: [:bar, dep]
    RUBY

  it 'does not register an offense to task with :environment' do

    expect_no_offenses(<<~RUBY)
      task foo: `:environment` do
      end
    RUBY

  it 'does not register an offense to task with :environment and other dependencies' do

    expect_no_offenses(<<~RUBY)
      task foo: [`:environment`, :bar] do
      end
    RUBY

Current Fixture: offense.rb

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

task :foo do
^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  puts "hello"
end

task :bar do
^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  User.all.each { |u| puts u.name }
end

task :cleanup do
^^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  OldRecord.delete_all
end

task 'generate_report' do
^^^^^^^^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  Report.generate
end

task('update_cache') { Cache.refresh }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.

task migrate: [] do
^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  ActiveRecord::Base.connection.migrate
end

task refresh: [] do
^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  Cache.clear
end

task name do
^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  puts "local variable task name"
end

task(a.to_sym) { puts "method call task name" }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.

task short_name do
^^^^^^^^^^^^^^^^^^ Rails/RakeEnvironment: Add `:environment` dependency to the rake task.
  run_command
end

Current Fixture: no_offense.rb

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

task foo: :environment do
  puts "hello"
end
task :bar => :environment do
  puts "world"
end
task :notices_delete, [:problem_id] => [:environment] do
  puts "delete"
end
task :baz, [:arg] => [:environment, :other] do
  puts "multi deps"
end
task :default do
  puts "default task"
end
task default: [:test] do
  puts "default with deps"
end
task setup: :database do
  puts "setup depends on database"
end
task foo: dep do
  puts "method call dependency"
end
task foo: [dep, :bar] do
  puts "method call in array dependency"
end
task :foo, [:arg] => dep do
  puts "args with method call dependency"
end
task :generate => [:environment] do
  puts "hash rocket with array"
end
task name => new_name do
  puts "local variable with hash rocket dependency"
end

@6 6 closed this Mar 24, 2026
@6 6 deleted the fix/rails-rake_environment-23511804121 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/RakeEnvironment

1 participant