Fix method definition cop #44

kuahyeow · 2023-02-17T08:36:42Z

Fixes #42

Fixes ClassDefinitionInTask, and MethodDefinitionInTask to allow classes, modules and methods to defined inside tasks

kuahyeow · 2023-02-19T22:57:52Z

@koic Is the configured Circle CI still working. Am getting the error below:

 CircleCI Pipeline — Could not find a usable config.yml, you may have revoked the CircleCI OAuth app.

pboling

This is correct. The difference between which definition of a method or class wins is according to lexical order. The only one that works properly, without polluting the global Ruby Object namespace is inside a task, (whether or not it is inside a namespace is irrelevant).

pboling · 2023-02-26T04:46:50Z

lib/rubocop/cop/rake/class_definition_in_task.rb

      # because it is defined to the top level.
-      # It is confusing because the scope looks in the task or namespace,
+      # It is confusing because the scope looks in the namespace,


Suggested change

# It is confusing because the scope looks in the namespace,

# It is confusing because the scope appears as if it is in the namespace,

lib/rubocop/cop/rake/class_definition_in_task.rb

lib/rubocop/cop/rake/helper/task_definition.rb

spec/rubocop/cop/rake/class_definition_in_task_spec.rb

pboling · 2023-11-09T09:52:33Z

Ping! Just ran into this false positive again, and came looking to see if any activity here.

pboling

LGTM! ❤️

q3aiml · 2023-12-22T17:54:55Z

Is this correctly handling when a task is nested inside a namespace? I believe that should be allowed, but the spec doesn't seem to cover it and it looks like it is disallowed.

For example this test fails:

  it 'does not register an offense to `def` in a task when wrapped in a namespace' do
    expect_no_offenses(<<~RUBY)
      namespace :bar do
        task :foo do
          def helper_method
            do_something
          end
        end
      end
    RUBY
  end

kuahyeow · 2024-01-23T03:56:35Z

That's true. I have now pushed a fix for this.

Is this correctly handling when a task is nested inside a namespace? I believe that should be allowed, but the spec doesn't seem to cover it and it looks like it is disallowed.

For example this test fails:
  it 'does not register an offense to `def` in a task when wrapped in a namespace' do
    expect_no_offenses(<<~RUBY)
      namespace :bar do
        task :foo do
          def helper_method
            do_something
          end
        end
      end
    RUBY
  end

pboling

LGTM

pboling · 2024-05-10T22:07:52Z

@kuahyeow The build passed before your last fix, but now it failing on Ruby >= 2.7. Any ideas?

kuahyeow · 2024-05-11T06:05:41Z

Looks like it's failing on unrelated rubocop errors....

rubocop
Inspecting 25 files
............C.......C....

Offenses:

lib/rubocop/cop/rake/helper/task_name.rb:11:30: C: [Correctable] InternalAffairs/NodeFirstOrLastArgument: Use #first_argument instead of #arguments[0].
            first_arg = node.arguments[0]
                             ^^^^^^^^^^^^
spec/rubocop/cop/rake/desc_spec.rb:4:6: C: [Correctable] InternalAffairs/ExampleDescription: Description does not match use of expect_offense.
  it 'register an offense for task on the top level' do
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/rubocop/cop/rake/desc_spec.rb:16:6: C: [Correctable] InternalAffairs/ExampleDescription: Description does not match use of expect_offense.
  it 'register an offense for task with block in a block' do
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/rubocop/cop/rake/desc_spec.rb:33:6: C: [Correctable] InternalAffairs/ExampleDescription: Description does not match use of expect_offense.
  it 'register an offense for task in kwbegin' do
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

25 files inspected, 4 offenses detected, 4 offenses autocorrectable
rake aborted!
Command failed with status (1): [rubocop...]
/home/circleci/project/Rakefile:36:in `block in <top (required)>'
/home/circleci/.rubygems/gems/rake-12.3.3/exe/rake:27:in `<top (required)>'
/usr/local/bin/bundle:25:in `load'
/usr/local/bin/bundle:25:in `<main>'
Tasks: TOP => default => rubocop
(See full trace by running task with --trace)

Exited with code exit status 1

ydakuka · 2025-02-23T09:13:35Z

@kuahyeow could you please rebase the branch? The CI specs should pass as expected now.

pboling · 2025-02-23T13:24:35Z

LGTM still!

As methods defined in task do not pollute top level For example, this rakefile below will show that `do_something` does not appear in top level methods ``` task :a do def do_something puts 'a' end do_something end task :b do def do_something puts 'b' end do_something end puts methods.include?(:do_something) ```

As classes defined in task do not pollute top level. For example, this Rakefile shows class C in task does not pollute top level ``` task :a do class C def do_something puts 'a' end end C.new.do_something end task :b do class C def do_something puts 'b' end end C.new.do_something end puts Object.const_defined?('C') ```

Since the cop is now fixed to warn against class in namespaces, update the cop name accordingly.

Since the cop is now fixed to warn against method in namespaces, update the cop name accordingly.

This is similar to definitions within a task (without a namespace).

kuahyeow · 2025-03-10T05:24:53Z

@ydakuka @pboling Rebased now ! The rspecs workflow now requires approval from a maintainer to run.